WIP: Add support for profiles #25

Closed
letian wants to merge 1 commits from runtime-profiles into master
7 changed files with 55 additions and 9 deletions

View File

@@ -2,9 +2,10 @@ use harmony::{
data::Version,
inventory::Inventory,
maestro::Maestro,
modules::lamp::{LAMPConfig, LAMPScore},
modules::lamp::{LAMPConfig, LAMPProfile, LAMPScore},
topology::{K8sAnywhereTopology, Url},
};
use std::collections::HashMap;
#[tokio::main]
async fn main() {
@@ -17,6 +18,10 @@ async fn main() {
project_root: "./php".into(),
..Default::default()
},
profiles: HashMap::from([
Review

On ne veut pas donner la tache de setup du profil a l'utilisateur du Score.

L'utilisateur du Score ne devrait pas etre au courant que les profils existent a ce niveau. Lorsqu'il ouvre l'implementation d'un score il peut voir quelles differences sont faites d'un profil a l'autre, mais comme c'est suggéré ici ça ne correspond pas à l'objectif initial :

Comme le propose le projet score-spec, les configurations doivent être exactement la même pour le développeur (ici, celui qui écrit le code du fichier examples/lamp/src/main.rs ) . La plateforme ensuite va déployer la configuration demandée automatiquement.

Il faut bien garder en tête que c'est le rôle de la plateforme elle-même d'offrir une fonctionnalité de déploiement standard et d'implémenter les fonctionnalités qui correspondent à son profil à elle.

Donc, l'implémentation interne du Score/Interpret peut fournir de l'information sur comment déployer/orchestrer son workload selon différents profils standards, et la plateforme elle même ira consulter le score pour voir quelle configuration doit être appliquée selon son profil à elle.

Aussi, la plateforme fournira une implémentation par défaut et de l'instrumentation automatique selon son implémentation spécifique pour les différents profils. Par exemple, une plateforme de profil PROD pourrait être en single-availability-zone et une autre en mode "warm-failover" et une autre en "decentralized", mais la configuration haut niveau du Score ne doit pas changer d'une implémentation à l'autre.

Donc, fournir un nouveau champ profiles: HashMap ne me semble pas convenir. Je ne suis pas totalement certain par contre.

On ne veut pas donner la tache de setup du profil a l'utilisateur du Score. L'utilisateur du Score ne devrait pas etre au courant que les profils existent a ce niveau. Lorsqu'il ouvre l'implementation d'un score il peut voir quelles differences sont faites d'un profil a l'autre, mais comme c'est suggéré ici ça ne correspond pas à l'objectif initial : Comme le propose le projet score-spec, les configurations doivent être exactement la même pour le développeur (ici, celui qui écrit le code du fichier `examples/lamp/src/main.rs` ) . La plateforme ensuite va déployer la configuration demandée automatiquement. Il faut bien garder en tête que c'est le rôle de la plateforme elle-même d'offrir une fonctionnalité de déploiement standard et d'implémenter les fonctionnalités qui correspondent à son profil à elle. Donc, l'implémentation interne du Score/Interpret peut fournir de l'information sur comment déployer/orchestrer son workload selon différents profils standards, et la plateforme elle même ira consulter le score pour voir quelle configuration doit être appliquée selon son profil à elle. Aussi, la plateforme fournira une implémentation par défaut et de l'instrumentation automatique selon son implémentation spécifique pour les différents profils. Par exemple, une plateforme de profil PROD pourrait être en single-availability-zone et une autre en mode "warm-failover" et une autre en "decentralized", mais la configuration haut niveau du Score ne doit pas changer d'une implémentation à l'autre. Donc, fournir un nouveau champ `profiles: HashMap` ne me semble pas convenir. Je ne suis pas totalement certain par contre.
Review

Dans ce cas, ca semble toucher 2 aspects differents en fait:

  1. Le developpeur final devrait pouvoir avoir le controle de configurer certains aspects d'un score en fonction du profil (ce que cette PR introduit). Le score va donc exposer un API public (via sa config) de ce que le developpeur a le controle dessus.
  2. Le maintainer d'un Score devrait "imposer" des facons de faire differentes selon le profil pour s'assurer que son workload est deploye/orchestre comme il le faudrait.

Sauf qu'en ce moment, pour le point 2, ca se ferait directement dans l'implementation Interpret::execute du Score. Ce qui, par extension, force dans cette PR a pousser le profile a la fois dans le apply_profile (ou create_interpret si on veut regler le probleme de temporal coupling) et dans le Interpret::execute. Ce qui est en fait un beau gros smell d'un plus grand probleme de design.

Actuellement, les Score cachent a l'interieur de l'execute le fait qu'ils vont utiliser d'autres Score et que ces Score la pourraient avoir besoin du resultat d'eux meme ou d'un autre score.

Exemple:

+-----------+
|LampScore  |
+-----------+
     |
     V
+------------+
|Build Docker|
|  (action)  |
+------------+
     |
     V
+-----------+
|Image Name |
|  (output) |
+-----------+
     | 
     |
     |
     V
+-----------+
|K8sDeploy  |
|  Score    |
+-----------+
     |
     V
+-----------+
|Use Image  |
|  (action) |
+-----------+
     |
     V
+-----------+
|  Result   |
+-----------+

En ce moment, cela fonctionne car c'est a petite echelle et on a peu de Score qui se reutilisent les uns et les autres. Mais, ca va vite devenir un gros probleme.

Il faudrait donc etre capable de vraiment separer le process en deux etapes:

  1. Prepare l'arbre de tout ce qui va etre fait en liant les dependances entre les extrants <-> intrants (e.g. LampScore -> Build Docker Image -> K8sDeploymentScore)
  2. Execute l'arbre et chainant les extrants avec les intrants correctement (comme une Pipeline, mais possiblement plus complexe)

Tout ceci aurait l'avantage de pouvoir faire comme Terraform avec des dry-run et exprimer un "plan de match" de comment le workload va etre deploye/orchestre.

Il y a encore de la reflexion a avoir sur comment tout ca peut etre design, mais ca serait a faire plus tot que plus tard puisque les smells commencent a sortir de plus en plus.

Dans ce cas, ca semble toucher 2 aspects differents en fait: 1. Le developpeur final devrait pouvoir avoir le controle de configurer certains aspects d'un score en fonction du profil (ce que cette PR introduit). Le score va donc exposer un API _public_ (via sa config) de ce que le developpeur a le controle dessus. 2. Le maintainer d'un Score devrait "imposer" des facons de faire differentes selon le profil pour s'assurer que son workload est deploye/orchestre comme il le faudrait. Sauf qu'en ce moment, pour le point 2, ca se ferait directement dans l'implementation `Interpret::execute` du Score. Ce qui, par extension, force dans cette PR a pousser le `profile` a la fois dans le `apply_profile` (ou `create_interpret` si on veut regler le probleme de temporal coupling) et dans le `Interpret::execute`. Ce qui est en fait un beau gros smell d'un plus grand probleme de design. Actuellement, les Score cachent a l'interieur de l'execute le fait qu'ils vont utiliser d'autres Score et que ces Score la pourraient avoir besoin du resultat d'eux meme ou d'un autre score. Exemple: ``` +-----------+ |LampScore | +-----------+ | V +------------+ |Build Docker| | (action) | +------------+ | V +-----------+ |Image Name | | (output) | +-----------+ | | | V +-----------+ |K8sDeploy | | Score | +-----------+ | V +-----------+ |Use Image | | (action) | +-----------+ | V +-----------+ | Result | +-----------+ ``` En ce moment, cela fonctionne car c'est a petite echelle et on a peu de Score qui se reutilisent les uns et les autres. Mais, ca va vite devenir un gros probleme. Il faudrait donc etre capable de vraiment separer le process en deux etapes: 1. Prepare l'arbre de tout ce qui va etre fait en liant les dependances entre les extrants <-> intrants (e.g. LampScore -> Build Docker Image -> K8sDeploymentScore) 2. Execute l'arbre et chainant les extrants avec les intrants correctement (comme une Pipeline, mais possiblement plus complexe) Tout ceci aurait l'avantage de pouvoir faire comme Terraform avec des dry-run et exprimer un "plan de match" de comment le workload va etre deploye/orchestre. Il y a encore de la reflexion a avoir sur comment tout ca peut etre design, mais ca serait a faire plus tot que plus tard puisque les smells commencent a sortir de plus en plus.
("dev", LAMPProfile { ssl_enabled: false }),
Review

Pourrait etre un enum :

pub enum DEPLOYMENT_PROFILE {
  DEV,
  STAGING,
  PROD,
  CUSTOM(String),
}
Pourrait etre un enum : ```rust pub enum DEPLOYMENT_PROFILE { DEV, STAGING, PROD, CUSTOM(String), } ```
("prod", LAMPProfile { ssl_enabled: true }),
]),
};
let mut maestro = Maestro::<K8sAnywhereTopology>::initialize(

View File

@@ -39,8 +39,12 @@ impl std::fmt::Display for InterpretName {
#[async_trait]
pub trait Interpret<T>: std::fmt::Debug + Send {
async fn execute(&self, inventory: &Inventory, topology: &T)
-> Result<Outcome, InterpretError>;
async fn execute(
&self,
inventory: &Inventory,
topology: &T,
profile: &String,
) -> Result<Outcome, InterpretError>;
fn get_name(&self) -> InterpretName;
fn get_version(&self) -> Version;
fn get_status(&self) -> InterpretStatus;

View File

@@ -16,20 +16,23 @@ pub struct Maestro<T: Topology> {
topology: T,
scores: Arc<RwLock<ScoreVec<T>>>,
topology_preparation_result: Mutex<Option<Outcome>>,
profile: String,
}
impl<T: Topology> Maestro<T> {
pub fn new(inventory: Inventory, topology: T) -> Self {
pub fn new(inventory: Inventory, topology: T, profile: String) -> Self {
Self {
inventory,
topology,
scores: Arc::new(RwLock::new(Vec::new())),
topology_preparation_result: None.into(),
profile,
}
}
pub async fn initialize(inventory: Inventory, topology: T) -> Result<Self, InterpretError> {
let instance = Self::new(inventory, topology);
let profile = "dev".to_string(); // TODO: retrieve from env?
let instance = Self::new(inventory, topology, profile);
instance.prepare_topology().await?;
Ok(instance)
}
@@ -78,9 +81,11 @@ impl<T: Topology> Maestro<T> {
);
}
info!("Running score {score:?}");
let interpret = score.create_interpret();
let interpret = score.apply_profile(&self.profile).create_interpret();
info!("Launching interpret {interpret:?}");
let result = interpret.execute(&self.inventory, &self.topology).await;
let result = interpret
.execute(&self.inventory, &self.topology, &self.profile)
.await;
info!("Got result {result:?}");
result
}

View File

@@ -8,6 +8,9 @@ use super::{interpret::Interpret, topology::Topology};
pub trait Score<T: Topology>:
std::fmt::Debug + ScoreToString<T> + Send + Sync + CloneBoxScore<T> + SerializeScore<T>
{
fn apply_profile(&self, profile: &String) -> Box<dyn Score<T>> {
Box::new(self.clone())
}
fn create_interpret(&self) -> Box<dyn Interpret<T>>;
fn name(&self) -> String;
}

View File

@@ -75,6 +75,7 @@ impl<T: Topology> Interpret<T> for DummyInterpret {
&self,
_inventory: &Inventory,
_topology: &T,
_profile: &String,
) -> Result<Outcome, InterpretError> {
self.result.clone()
}
@@ -121,6 +122,7 @@ impl<T: Topology> Interpret<T> for PanicInterpret {
&self,
_inventory: &Inventory,
_topology: &T,
_profile: &String,
) -> Result<Outcome, InterpretError> {
panic!("Panic interpret always panics when executed")
}

View File

@@ -1,3 +1,5 @@
use std::collections::HashMap;
use std::fmt::Debug;
use std::path::{Path, PathBuf};
use async_trait::async_trait;
@@ -19,6 +21,7 @@ pub struct LAMPScore {
pub domain: Url,
pub config: LAMPConfig,
pub php_version: Version,
pub profiles: HashMap<&'static str, LAMPProfile>,
}
#[derive(Debug, Clone, Serialize)]
@@ -27,6 +30,11 @@ pub struct LAMPConfig {
pub ssl_enabled: bool,
}
#[derive(Debug, Clone, Serialize)]
pub struct LAMPProfile {
pub ssl_enabled: bool,
}
impl Default for LAMPConfig {
fn default() -> Self {
LAMPConfig {
@@ -37,6 +45,23 @@ impl Default for LAMPConfig {
}
impl<T: Topology + K8sclient> Score<T> for LAMPScore {
fn apply_profile(&self, profile: &String) -> Box<dyn Score<T>> {
Review

Je n'ai pas le temps de le faire maintenant, mais pour regler le probleme de temporal coupling que cette methode introduit, il faudrait simplement a la supprimer et reutiliser create_interpret pour y mettre la logique de merge la config avec le profile avant de creer l'interpret.

Je n'ai pas le temps de le faire maintenant, mais pour regler le probleme de temporal coupling que cette methode introduit, il faudrait simplement a la supprimer et reutiliser `create_interpret` pour y mettre la logique de merge la config avec le profile avant de creer l'interpret.
let profile = match self.profiles.get(profile.as_str()) {
Some(profile) => profile,
None => panic!("Not good"), // TODO: better handling
Review

instead of panic!() // TODO just use todo!("comment here");

instead of panic!() // TODO just use `todo!("comment here");`
};
let config = LAMPConfig {
ssl_enabled: profile.ssl_enabled,
..self.config.clone()
};
Box::new(LAMPScore {
config,
..self.clone()
})
}
fn create_interpret(&self) -> Box<dyn Interpret<T>> {
Box::new(LAMPInterpret {
score: self.clone(),
@@ -59,6 +84,7 @@ impl<T: Topology + K8sclient> Interpret<T> for LAMPInterpret {
&self,
inventory: &Inventory,
topology: &T,
profile: &String,
) -> Result<Outcome, InterpretError> {
let image_name = match self.build_docker_image() {
Ok(name) => name,
@@ -78,8 +104,9 @@ impl<T: Topology + K8sclient> Interpret<T> for LAMPInterpret {
info!("LAMP deployment_score {deployment_score:?}");
todo!();
deployment_score
.apply_profile(profile)
.create_interpret()
.execute(inventory, topology)
.execute(inventory, topology, profile)
.await?;
todo!()
}

View File

@@ -41,7 +41,7 @@ pub mod tui {
/// async fn main() {
/// let inventory = Inventory::autoload();
/// let topology = HAClusterTopology::autoload();
/// let mut maestro = Maestro::new(inventory, topology);
/// let mut maestro = Maestro::new(inventory, topology, "local");
///
/// maestro.register_all(vec![
/// Box::new(SuccessScore {}),