From a4834a39dbdbd24fe01291f004fd2b2323ca1d80 Mon Sep 17 00:00:00 2001 From: Guillaume Leroy Date: Wed, 7 Aug 2024 18:41:47 +0200 Subject: [PATCH] refactor: replace service by containers (#32) --- .../templates/{ => container}/deployment.yaml | 2 +- .../templates/{ => container}/service.yaml | 2 +- charts/simpaas-app/templates/ingress.yaml | 2 +- charts/simpaas-app/values.yaml | 2 +- charts/simpaas/crds/app.yaml | 18 ++++---- charts/simpaas/templates/op/configmap.yaml | 4 +- charts/simpaas/templates/op/deployment.yaml | 4 +- charts/simpaas/values.yaml | 2 +- get-simpaas | 2 +- src/api.rs | 29 +++++++------ src/deploy/helm.rs | 41 +++++++++++++----- src/deploy/mod.rs | 4 +- src/domain.rs | 6 +-- src/helm/default.rs | 42 +++++++------------ src/helm/mod.rs | 9 ++-- src/kube/default.rs | 10 +++-- src/kube/mod.rs | 4 +- src/op.rs | 33 +++++---------- 18 files changed, 112 insertions(+), 104 deletions(-) rename charts/simpaas-app/templates/{ => container}/deployment.yaml (98%) rename charts/simpaas-app/templates/{ => container}/service.yaml (93%) diff --git a/charts/simpaas-app/templates/deployment.yaml b/charts/simpaas-app/templates/container/deployment.yaml similarity index 98% rename from charts/simpaas-app/templates/deployment.yaml rename to charts/simpaas-app/templates/container/deployment.yaml index 8c2c9a5..989964e 100644 --- a/charts/simpaas-app/templates/deployment.yaml +++ b/charts/simpaas-app/templates/container/deployment.yaml @@ -1,4 +1,4 @@ -{{- range $service := .Values.services }} +{{- range $service := .Values.containers }} {{- $podAnnotations := merge $.Values.common.podAnnotations (default dict $service.values.podAnnotations) -}} {{- $podLabels := merge $.Values.common.podLabels (default dict $service.values.podLabels) -}} {{- $imagePullSecrets := concat $.Values.common.imagePullSecrets (default list $service.values.imagePullSecrets) -}} diff --git a/charts/simpaas-app/templates/service.yaml b/charts/simpaas-app/templates/container/service.yaml similarity index 93% rename from charts/simpaas-app/templates/service.yaml rename to charts/simpaas-app/templates/container/service.yaml index 1e2a03c..60e6d39 100644 --- a/charts/simpaas-app/templates/service.yaml +++ b/charts/simpaas-app/templates/container/service.yaml @@ -1,4 +1,4 @@ -{{- range $service := .Values.services }} +{{- range $service := .Values.containers }} {{- if gt (len (default list $service.expose)) 0 }} {{- $vars := dict "Chart" $.Chart "Release" $.Release "Service" $service -}} apiVersion: v1 diff --git a/charts/simpaas-app/templates/ingress.yaml b/charts/simpaas-app/templates/ingress.yaml index e74c442..e2f4a7b 100644 --- a/charts/simpaas-app/templates/ingress.yaml +++ b/charts/simpaas-app/templates/ingress.yaml @@ -1,4 +1,4 @@ -{{- range $service := .Values.services }} +{{- range $service := .Values.containers }} {{- range $expose := default list $service.expose }} {{- if $expose.ingress }} {{- $vars := dict "Chart" $.Chart "Expose" $expose "Release" $.Release "Service" $service -}} diff --git a/charts/simpaas-app/values.yaml b/charts/simpaas-app/values.yaml index 987c21c..db05910 100644 --- a/charts/simpaas-app/values.yaml +++ b/charts/simpaas-app/values.yaml @@ -28,4 +28,4 @@ common: tolerations: [] -services: [] +containers: [] diff --git a/charts/simpaas/crds/app.yaml b/charts/simpaas/crds/app.yaml index 104f389..9b3fec8 100644 --- a/charts/simpaas/crds/app.yaml +++ b/charts/simpaas/crds/app.yaml @@ -20,14 +20,8 @@ spec: properties: spec: properties: - namespace: - description: Namespace. - type: string - owner: - description: Owner of the app. - type: string - services: - description: List of app services. + containers: + description: List of container services. items: properties: expose: @@ -93,15 +87,21 @@ spec: - name type: object type: array + namespace: + description: Namespace. + type: string + owner: + description: Owner of the app. + type: string values: additionalProperties: true default: {} description: Helm chart values. type: object required: + - containers - namespace - owner - - services type: object status: nullable: true diff --git a/charts/simpaas/templates/op/configmap.yaml b/charts/simpaas/templates/op/configmap.yaml index a725b82..653417a 100644 --- a/charts/simpaas/templates/op/configmap.yaml +++ b/charts/simpaas/templates/op/configmap.yaml @@ -5,5 +5,5 @@ metadata: labels: {{- include "simpaas.op.labels" . | nindent 4 }} data: - values.yaml: | - {{ toYaml .Values.op.chartValues | nindent 4 }} + app.yaml: | + {{ toYaml .Values.op.appChartValues | nindent 4 }} diff --git a/charts/simpaas/templates/op/deployment.yaml b/charts/simpaas/templates/op/deployment.yaml index 7663302..cd72a9a 100644 --- a/charts/simpaas/templates/op/deployment.yaml +++ b/charts/simpaas/templates/op/deployment.yaml @@ -56,8 +56,8 @@ spec: {{ toYaml . | nindent 10 }} {{- end }} env: - - name: CHART_VALUES - value: {{ printf "%s/values.yaml" $chartValuesDir }} + - name: APP_CHART_VALUES + value: {{ printf "%s/app.yaml" $chartValuesDir }} - name: SMTP_HOST value: {{ default (printf "%s-smtp" .Release.Name) .Values.op.smtp.host }} - name: SMTP_PORT diff --git a/charts/simpaas/values.yaml b/charts/simpaas/values.yaml index 8f8f193..d83e18b 100644 --- a/charts/simpaas/values.yaml +++ b/charts/simpaas/values.yaml @@ -155,7 +155,7 @@ op: livenessProbe: {} readinessProbe: {} - chartValues: {} + appChartValues: {} logFilter: "" diff --git a/get-simpaas b/get-simpaas index 4400145..5088951 100755 --- a/get-simpaas +++ b/get-simpaas @@ -134,7 +134,7 @@ install_chart $ns_simpaas simpaas $repo_simpaas/simpaas-stack \ --set-json "simpaas.ingress.annotations={\"cert-manager.io/cluster-issuer\":\"$issuer\"}" \ --set simpaas.ingress.create=$ingress_create \ --set "simpaas.ingress.domain=$SIMPAAS_DOMAIN" \ - --set-json "simpaas.op.chartValues={\"ingress\":{\"annotations\":{\"cert-manager.io/cluster-issuer\":\"$issuer\"}}}" \ + --set-json "simpaas.op.appChartValues={\"ingress\":{\"annotations\":{\"cert-manager.io/cluster-issuer\":\"$issuer\"}}}" \ "${set_smtp_env[@]}" \ "${set_values[@]}" diff --git a/src/api.rs b/src/api.rs index 0e96cd7..5b45fec 100644 --- a/src/api.rs +++ b/src/api.rs @@ -36,7 +36,8 @@ use validator::{Validate, ValidationError, ValidationErrors}; use crate::{ domain::{ - Action, App, AppSpec, AppStatus, Invitation, InvitationSpec, Service, User, UserSpec, + Action, App, AppSpec, AppStatus, ContainerService, Invitation, InvitationSpec, User, + UserSpec, }, jwt::JwtEncoder, kube::{AppFilter, KubeClient, FINALIZER}, @@ -164,6 +165,9 @@ struct AppFilterQuery { #[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize, Validate)] #[serde(rename_all = "camelCase")] struct CreateAppRequest { + /// List of container services. + #[validate(nested)] + containers: Vec, /// Name. #[serde(deserialize_with = "string_trim")] #[validate(length(min = 1))] @@ -172,9 +176,6 @@ struct CreateAppRequest { #[serde(default, deserialize_with = "option_string_trim")] #[validate(length(min = 1))] namespace: Option, - /// List of app services. - #[validate(nested)] - services: Vec, /// Helm chart values. #[serde(default)] values: Map, @@ -195,11 +196,11 @@ struct SendInvitationRequest { #[derive(Clone, Debug, Deserialize, PartialEq, JsonSchema, Serialize, Validate)] #[serde(rename_all = "camelCase")] struct UpdateAppRequest { + /// List of container services. + #[validate(nested)] + containers: Vec, /// Owner of the app. owner: String, - /// List of app services. - #[validate(nested)] - services: Vec, /// Helm chart values. #[serde(default)] values: Map, @@ -376,7 +377,11 @@ async fn check_permission(user: &User, action: Action<'_>, kube: } } -async fn ensure_domains_are_free(name: &str, svcs: &[Service], kube: &K) -> Result { +async fn ensure_domains_are_free( + name: &str, + svcs: &[ContainerService], + kube: &K, +) -> Result { let usages = kube.domain_usages(name, svcs).await?; if usages.is_empty() { Ok(()) @@ -508,7 +513,7 @@ async fn create_app( async { check_permission(&user, Action::CreateApp, &ctx.kube).await?; req.validate()?; - ensure_domains_are_free(&req.name, &req.services, &ctx.kube).await?; + ensure_domains_are_free(&req.name, &req.containers, &ctx.kube).await?; if ctx.kube.get_app(&req.name).await?.is_some() { return Err(Error::ResourceAlreadyExists(vec![ ResourceAlreadyExistsItem { @@ -522,7 +527,7 @@ async fn create_app( let spec = AppSpec { namespace, owner: username, - services: req.services, + containers: req.containers, values: req.values, }; let app = App { @@ -788,7 +793,7 @@ async fn update_app( check_permission(&user, Action::UpdateApp(&name), &ctx.kube).await?; } req.validate()?; - ensure_domains_are_free(&name, &req.services, &ctx.kube).await?; + ensure_domains_are_free(&name, &req.containers, &ctx.kube).await?; if ctx.kube.get_user(&req.owner).await?.is_none() { return Err(Error::PreconditionFailed(PreconditionFailedResponse { field: "owner".into(), @@ -802,7 +807,7 @@ async fn update_app( }, spec: AppSpec { owner: req.owner, - services: req.services, + containers: req.containers, values: req.values, ..app.spec }, diff --git a/src/deploy/helm.rs b/src/deploy/helm.rs index 999a2fd..1967841 100644 --- a/src/deploy/helm.rs +++ b/src/deploy/helm.rs @@ -9,6 +9,10 @@ use crate::{domain::App, helm::HelmClient, kube::KubeClient}; use super::{Deployer, Result}; +// Defaults + +const DEFAULT_CHART_PATH: &str = "charts/simpaas-app"; + // Errors #[derive(Debug, thiserror::Error)] @@ -41,15 +45,30 @@ pub enum Error { // Data structs -#[derive(clap::Args, Clone, Debug, Default, Eq, PartialEq)] +#[derive(clap::Args, Clone, Debug, Eq, PartialEq)] pub struct HelmDeployerArgs { #[arg( - long = "CHART_VALUES", - env = "CHART_VALUES", - name = "CHART_VALUES", + long, + env, + default_value = DEFAULT_CHART_PATH, + long_help = "Path to built-in simpaas-app chart" + )] + pub app_chart: String, + #[arg( + long, + env, long_help = "Path to YAML file of default values of simpaas-app chart" )] - pub values_filepath: Option, + pub app_chart_values: Option, +} + +impl Default for HelmDeployerArgs { + fn default() -> Self { + Self { + app_chart: DEFAULT_CHART_PATH.into(), + app_chart_values: None, + } + } } // HelmDeployer @@ -75,25 +94,27 @@ impl HelmDeployer { impl Deployer for HelmDeployer { #[instrument(skip(self, app, _kube), fields(app.name = name))] - async fn deploy(&self, name: &str, app: &App, _kube: &K) -> Result { + async fn deploy_app(&self, name: &str, app: &App, _kube: &K) -> Result { info!("deploying app"); debug!("creating temporary directory"); let dir = TempDir::new(name)?; let app_filepath = Self::dump_yaml(&dir, &app.spec)?; let mut filepaths = vec![]; - if let Some(path) = &self.args.values_filepath { + if let Some(path) = &self.args.app_chart_values { filepaths.push(path.clone()); }; filepaths.push(app_filepath); - self.helm.upgrade(name, app, &filepaths).await?; + self.helm + .upgrade(&self.args.app_chart, name, &app.spec.namespace, &filepaths) + .await?; info!("app deployed"); Ok(()) } #[instrument(skip(self, app, kube), fields(app.name = name))] - async fn undeploy(&self, name: &str, app: &App, kube: &K) -> Result { + async fn undeploy_app(&self, name: &str, app: &App, kube: &K) -> Result { info!("undeploying app"); - self.helm.uninstall(name, app).await?; + self.helm.uninstall(name, &app.spec.namespace).await?; kube.delete_namespace(&app.spec.namespace).await?; info!("app undeployed"); Ok(()) diff --git a/src/deploy/mod.rs b/src/deploy/mod.rs index 42cd520..fba505a 100644 --- a/src/deploy/mod.rs +++ b/src/deploy/mod.rs @@ -19,14 +19,14 @@ pub struct Error(#[source] pub Box); // Traits pub trait Deployer: Send + Sync { - fn deploy( + fn deploy_app( &self, name: &str, app: &App, kube: &K, ) -> impl Future + Send; - fn undeploy( + fn undeploy_app( &self, name: &str, app: &App, diff --git a/src/domain.rs b/src/domain.rs index d88bc74..e212cb5 100644 --- a/src/domain.rs +++ b/src/domain.rs @@ -41,12 +41,12 @@ pub enum Action<'a> { )] #[serde(rename_all = "camelCase")] pub struct AppSpec { + /// List of container services. + pub containers: Vec, /// Namespace. pub namespace: String, /// Owner of the app. pub owner: String, - /// List of app services. - pub services: Vec, /// Helm chart values. #[serde(default)] pub values: Map, @@ -192,7 +192,7 @@ pub struct RoleSpec { #[derive(Clone, Debug, Deserialize, Eq, PartialEq, JsonSchema, Serialize, Validate)] #[serde(rename_all = "camelCase")] -pub struct Service { +pub struct ContainerService { /// List of ports to expose. #[serde(default)] #[validate(nested)] diff --git a/src/helm/default.rs b/src/helm/default.rs index 10b426a..d6e1b9a 100644 --- a/src/helm/default.rs +++ b/src/helm/default.rs @@ -2,14 +2,13 @@ use std::path::PathBuf; use tracing::{debug, error, instrument}; -use crate::{cmd::CommandRunner, domain::App}; +use crate::cmd::CommandRunner; use super::{HelmClient, Result}; // Defaults const DEFAULT_BIN: &str = "helm"; -const DEFAULT_CHART_PATH: &str = "charts/simpaas-app"; // Errors @@ -37,20 +36,12 @@ pub struct DefaultHelmClientArgs { long_help = "Helm binary to use" )] pub bin: String, - #[arg( - long, - env, - default_value = DEFAULT_CHART_PATH, - long_help = "Path to built-in simpaas-app chart" - )] - pub chart_path: PathBuf, } impl Default for DefaultHelmClientArgs { fn default() -> Self { Self { bin: DEFAULT_BIN.into(), - chart_path: DEFAULT_CHART_PATH.into(), } } } @@ -69,31 +60,30 @@ impl DefaultHelmClient { } impl HelmClient for DefaultHelmClient { - #[instrument("helm_uninstall", skip(self, name, app), fields(app.name = name, app.namespace = app.spec.namespace))] - async fn uninstall(&self, name: &str, app: &App) -> Result { + #[instrument("helm_uninstall", skip(self))] + async fn uninstall(&self, release: &str, namespace: &str) -> Result { debug!("running helm uninstall"); self.runner .run( - "helm", - &[ - "uninstall", - "-n", - &app.spec.namespace, - "--ignore-not-found", - name, - ], + &self.args.bin, + &["uninstall", "-n", namespace, "--ignore-not-found", release], ) .await?; Ok(()) } - #[instrument("helm_upgrade", skip(self, app, filepaths), fields(app.name = name, app.namespace = app.spec.namespace))] - async fn upgrade(&self, name: &str, app: &App, filepaths: &[PathBuf]) -> Result { - let chart = self.args.chart_path.to_str().ok_or(Error::InvalidUnicode)?; + #[instrument("helm_upgrade", skip(self, filepaths))] + async fn upgrade( + &self, + chart: &str, + release: &str, + namespace: &str, + filepaths: &[PathBuf], + ) -> Result { let mut args = vec![ "upgrade", "-n", - &app.spec.namespace, + namespace, "--create-namespace", "--install", ]; @@ -103,9 +93,9 @@ impl HelmClient for DefaultHelmClient { args.push(path); } debug!("running helm upgrade"); - args.push(name); + args.push(release); args.push(chart); - self.runner.run("helm", &args).await?; + self.runner.run(&self.args.bin, &args).await?; Ok(()) } } diff --git a/src/helm/mod.rs b/src/helm/mod.rs index b3aaf81..5fe5c25 100644 --- a/src/helm/mod.rs +++ b/src/helm/mod.rs @@ -2,8 +2,6 @@ use std::path::PathBuf; use futures::Future; -use crate::domain::App; - // Mods pub mod default; @@ -21,12 +19,13 @@ pub struct Error(#[source] pub Box); // Traits pub trait HelmClient: Send + Sync { - fn uninstall(&self, name: &str, app: &App) -> impl Future + Send; + fn uninstall(&self, release: &str, namespace: &str) -> impl Future + Send; fn upgrade( &self, - name: &str, - app: &App, + chart: &str, + release: &str, + namespace: &str, filepaths: &[PathBuf], ) -> impl Future + Send; } diff --git a/src/kube/default.rs b/src/kube/default.rs index 4d6f297..91c878b 100644 --- a/src/kube/default.rs +++ b/src/kube/default.rs @@ -15,8 +15,8 @@ use tracing::{debug, instrument, warn}; use crate::{ domain::{ - Action, App, AppStatus, Invitation, InvitationStatus, Permission, PermissionError, Role, - Service, User, + Action, App, AppStatus, ContainerService, Invitation, InvitationStatus, Permission, + PermissionError, Role, User, }, CARGO_PKG_NAME, }; @@ -95,7 +95,11 @@ impl KubeClient for DefaultKubeClient { } #[instrument(skip(self, name, svcs), fields(app.name = name))] - async fn domain_usages(&self, name: &str, svcs: &[Service]) -> Result> { + async fn domain_usages( + &self, + name: &str, + svcs: &[ContainerService], + ) -> Result> { let domains: Vec<&String> = svcs .iter() .flat_map(|svc| { diff --git a/src/kube/mod.rs b/src/kube/mod.rs index e56cee9..83a315c 100644 --- a/src/kube/mod.rs +++ b/src/kube/mod.rs @@ -4,7 +4,7 @@ use futures::Future; use regex::Regex; use crate::domain::{ - Action, App, AppStatus, Invitation, InvitationStatus, Permission, Role, Service, User, + Action, App, AppStatus, ContainerService, Invitation, InvitationStatus, Permission, Role, User, }; // Mods @@ -87,7 +87,7 @@ pub trait KubeClient: Send + Sync { fn domain_usages( &self, name: &str, - svcs: &[Service], + svcs: &[ContainerService], ) -> impl Future>> + Send; fn get_app(&self, name: &str) -> impl Future>> + Send; diff --git a/src/op.rs b/src/op.rs index d8d94a2..28d7ebe 100644 --- a/src/op.rs +++ b/src/op.rs @@ -10,7 +10,7 @@ use kube::{ Api, }; use tokio::{sync::broadcast::channel, task::JoinSet}; -use tracing::{debug, error, info, info_span, warn, Instrument}; +use tracing::{debug, error, info, info_span, instrument, warn, Instrument}; use crate::{ deploy::Deployer, @@ -103,14 +103,9 @@ pub async fn start_op< }) .run(reconcile_app, on_error, ctx.clone()) .for_each(|res| async move { - match res { - Ok(_) => { - debug!("app reconcilation succeeded"); - } - Err(err) => { - error!("app reconcilation failed: {err}"); - log_controller_error(&err); - } + if let Err(err) = res { + error!("app reconcilation failed: {err}"); + log_controller_error(&err); } }); let invit_api = Api::default_namespaced(kube); @@ -120,14 +115,9 @@ pub async fn start_op< }) .run(reconcile_invitation, on_error, ctx) .for_each(|res| async move { - match res { - Ok(_) => { - debug!("invitation reconcilation succeeded"); - } - Err(err) => { - error!("invitation reconcilation failed: {err}"); - log_controller_error(&err); - } + if let Err(err) = res { + error!("invitation reconcilation failed: {err}"); + log_controller_error(&err); } }); tasks.spawn(app_ctrl); @@ -177,12 +167,11 @@ async fn reconcile_app { ctx.publisher .publish_app_event(&app, AppEvent::Deployed) @@ -240,7 +229,6 @@ async fn reconcile_invitation( Ok(()) } +#[instrument(skip(name, app, kube, publisher), fields(app.name = name))] async fn update_service_statuses( name: &str, app: &App, @@ -289,7 +278,7 @@ async fn update_service_statuses( ) -> Result { let monitor = async { let mut statuses = BTreeMap::new(); - for service in &app.spec.services { + for service in &app.spec.containers { let pods = kube.list_service_pods(name, &service.name).await?; let mut stopped = 0; for pod in &pods {