feat: Support podOverrides (#352)

# Description Part of stackabletech/issues#346 ## Definition of Done Checklist - Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant - Please make sure all these things are done and tick the boxes ```[tasklist] # Author - [ ] Changes are OpenShift compatible - [ ] CRD changes approved - [ ] Helm chart can be installed and deployed operator works - [ ] Integration tests passed (for non trivial changes) ``` ```[tasklist] # Reviewer - [ ] Code contains useful comments - [ ] (Integration-)Test cases added - [ ] Documentation added or updated - [ ] Changelog updated - [ ] Cargo.toml only contains references to git tags (not specific commits or branches) ``` ```[tasklist] # Acceptance - [ ] Feature Tracker has been updated - [ ] Proper release label has been added ``` Once the review is done, comment `bors r+` (or `bors merge`) to merge. [Further information](https://bors.tech/documentation/getting-started/#reviewing-pull-requests) Co-authored-by: Sebastian Bernauer <[email protected]>
stackabletech · Jul 5, 2023 · 1afd8ee · 1afd8ee
1 parent f8bdb81
commit 1afd8ee
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 - Generate OLM bundle for Release 23.4.0 ([#338]).
 - Missing CRD defaults for `status.conditions` field ([#340]).
 - Set explicit resources on all container ([#345], [#347])
+- Support podOverrides ([#352])
 
 ### Changed
 
@@ -24,6 +25,7 @@ All notable changes to this project will be documented in this file.
 [#345]: https://github.com/stackabletech/hive-operator/pull/345
 [#347]: https://github.com/stackabletech/hive-operator/pull/347
 [#348]: https://github.com/stackabletech/hive-operator/pull/348
+[#352]: https://github.com/stackabletech/hive-operator/pull/352
 
 ## [23.4.0] - 2023-04-17
 

diff --git a/rust/crd/src/affinity.rs b/rust/crd/src/affinity.rs
@@ -23,6 +23,7 @@ pub fn get_affinity(cluster_name: &str, role: &HiveRole) -> StackableAffinityFra
 mod tests {
     use super::*;
 
+    use rstest::rstest;
     use std::collections::BTreeMap;
 
     use crate::HiveCluster;
@@ -37,8 +38,9 @@ mod tests {
         },
     };
 
-    #[test]
-    fn test_affinity_defaults() {
+    #[rstest]
+    #[case(HiveRole::MetaStore)]
+    fn test_affinity_defaults(#[case] role: HiveRole) {
         let input = r#"
         apiVersion: hive.stackable.tech/v1alpha1
         kind: HiveCluster
@@ -60,7 +62,9 @@ mod tests {
                 replicas: 1
         "#;
         let hive: HiveCluster = serde_yaml::from_str(input).expect("illegal test input");
-        let merged_config = hive.merged_config(&HiveRole::MetaStore, "default").unwrap();
+        let merged_config = hive
+            .merged_config(&role, &role.rolegroup_ref(&hive, "default"))
+            .unwrap();
 
         assert_eq!(
             merged_config.affinity,
@@ -99,8 +103,9 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_affinity_legacy_node_selector() {
+    #[rstest]
+    #[case(HiveRole::MetaStore)]
+    fn test_affinity_legacy_node_selector(#[case] role: HiveRole) {
         let input = r#"
         apiVersion: hive.stackable.tech/v1alpha1
         kind: HiveCluster
@@ -131,7 +136,9 @@ mod tests {
                         - antarctica-west1
         "#;
         let hive: HiveCluster = serde_yaml::from_str(input).expect("illegal test input");
-        let merged_config = hive.merged_config(&HiveRole::MetaStore, "default").unwrap();
+        let merged_config = hive
+            .merged_config(&role, &role.rolegroup_ref(&hive, "default"))
+            .unwrap();
 
         assert_eq!(
             merged_config.affinity,

diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs
@@ -24,8 +24,8 @@ use stackable_operator::{
     schemars::{self, JsonSchema},
     status::condition::{ClusterCondition, HasStatusCondition},
 };
-use std::collections::BTreeMap;
-use strum::{Display, EnumIter, EnumString};
+use std::{collections::BTreeMap, str::FromStr};
+use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
 
 pub const APP_NAME: &str = "hive";
 // directories
@@ -62,8 +62,22 @@ pub const JVM_HEAP_FACTOR: f32 = 0.8;
 pub enum Error {
     #[snafu(display("no metastore role configuration provided"))]
     MissingMetaStoreRole,
+
     #[snafu(display("fragment validation failure"))]
     FragmentValidationFailure { source: ValidationError },
+
+    #[snafu(display("the role {role} is not defined"))]
+    CannotRetrieveHiveRole { role: String },
+
+    #[snafu(display("the role group {role_group} is not defined"))]
+    CannotRetrieveHiveRoleGroup { role_group: String },
+
+    #[snafu(display("unknown role {role}. Should be one of {roles:?}"))]
+    UnknownHiveRole {
+        source: strum::ParseError,
+        role: String,
+        roles: Vec<String>,
+    },
 }
 
 #[derive(Clone, CustomResource, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
@@ -154,7 +168,7 @@ pub struct HdfsConnection {
     pub config_map: String,
 }
 
-#[derive(Display)]
+#[derive(Display, EnumString, EnumIter)]
 #[strum(serialize_all = "camelCase")]
 pub enum HiveRole {
     #[strum(serialize = "metastore")]
@@ -184,6 +198,27 @@ impl HiveRole {
             ]
         }
     }
+
+    /// Metadata about a rolegroup
+    pub fn rolegroup_ref(
+        &self,
+        hive: &HiveCluster,
+        group_name: impl Into<String>,
+    ) -> RoleGroupRef<HiveCluster> {
+        RoleGroupRef {
+            cluster: ObjectRef::from_obj(hive),
+            role: self.to_string(),
+            role_group: group_name.into(),
+        }
+    }
+
+    pub fn roles() -> Vec<String> {
+        let mut roles = vec![];
+        for role in Self::iter() {
+            roles.push(role.to_string())
+        }
+        roles
+    }
 }
 
 #[derive(
@@ -516,42 +551,60 @@ impl HiveCluster {
             }))
     }
 
-    pub fn get_role(&self, role: &HiveRole) -> Option<&Role<MetaStoreConfigFragment>> {
-        match role {
+    pub fn role(&self, role_variant: &HiveRole) -> Result<&Role<MetaStoreConfigFragment>, Error> {
+        match role_variant {
             HiveRole::MetaStore => self.spec.metastore.as_ref(),
         }
+        .with_context(|| CannotRetrieveHiveRoleSnafu {
+            role: role_variant.to_string(),
+        })
+    }
+
+    pub fn rolegroup(
+        &self,
+        rolegroup_ref: &RoleGroupRef<HiveCluster>,
+    ) -> Result<RoleGroup<MetaStoreConfigFragment>, Error> {
+        let role_variant =
+            HiveRole::from_str(&rolegroup_ref.role).with_context(|_| UnknownHiveRoleSnafu {
+                role: rolegroup_ref.role.to_owned(),
+                roles: HiveRole::roles(),
+            })?;
+
+        let role = self.role(&role_variant)?;
+        role.role_groups
+            .get(&rolegroup_ref.role_group)
+            .with_context(|| CannotRetrieveHiveRoleGroupSnafu {
+                role_group: rolegroup_ref.role_group.to_owned(),
+            })
+            .cloned()
     }
 
     /// Retrieve and merge resource configs for role and role groups
     pub fn merged_config(
         &self,
         role: &HiveRole,
-        role_group: &str,
+        rolegroup_ref: &RoleGroupRef<Self>,
     ) -> Result<MetaStoreConfig, Error> {
         // Initialize the result with all default values as baseline
         let conf_defaults = MetaStoreConfig::default_config(&self.name_any(), role);
 
-        let role = self.get_role(role).context(MissingMetaStoreRoleSnafu)?;
-
         // Retrieve role resource config
+        let role = self.role(role)?;
         let mut conf_role = role.config.config.to_owned();
 
         // Retrieve rolegroup specific resource config
-        let mut conf_rolegroup = role
-            .role_groups
-            .get(role_group)
-            .map(|rg| rg.config.config.clone())
-            .unwrap_or_default();
+        let role_group = self.rolegroup(rolegroup_ref)?;
+        let mut conf_role_group = role_group.config.config;
 
         if let Some(RoleGroup {
             selector: Some(selector),
             ..
-        }) = role.role_groups.get(role_group)
+        }) = role.role_groups.get(&rolegroup_ref.role_group)
         {
             // Migrate old `selector` attribute, see ADR 26 affinities.
             // TODO Can be removed after support for the old `selector` field is dropped.
             #[allow(deprecated)]
-            conf_rolegroup.affinity.add_legacy_selector(selector);
+            conf_role_group.affinity.add_legacy_selector(selector);
         }
 
         // Merge more specific configs into default config
@@ -560,10 +613,10 @@ impl HiveCluster {
         // 2. Role
         // 3. Default
         conf_role.merge(&conf_defaults);
-        conf_rolegroup.merge(&conf_role);
+        conf_role_group.merge(&conf_role);
 
-        tracing::debug!("Merged config: {:?}", conf_rolegroup);
-        fragment::validate(conf_rolegroup).context(FragmentValidationFailureSnafu)
+        tracing::debug!("Merged config: {:?}", conf_role_group);
+        fragment::validate(conf_role_group).context(FragmentValidationFailureSnafu)
     }
 }
 

diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs
@@ -13,6 +13,7 @@ use stackable_hive_crd::{
     STACKABLE_LOG_CONFIG_MOUNT_DIR, STACKABLE_LOG_CONFIG_MOUNT_DIR_NAME, STACKABLE_LOG_DIR,
     STACKABLE_LOG_DIR_NAME,
 };
+use stackable_operator::k8s_openapi::DeepMerge;
 use stackable_operator::memory::MemoryQuantity;
 use stackable_operator::{
     builder::{
@@ -201,6 +202,9 @@ pub enum Error {
     BuildRbacResources {
         source: stackable_operator::error::Error,
     },
+
+    #[snafu(display("internal operator failure"))]
+    InternalOperatorError { source: stackable_hive_crd::Error },
 }
 type Result<T, E = Error> = std::result::Result<T, E>;
 
@@ -215,6 +219,7 @@ pub async fn reconcile_hive(hive: Arc<HiveCluster>, ctx: Arc<Ctx>) -> Result<Act
     let client = &ctx.client;
     let resolved_product_image: ResolvedProductImage =
         hive.spec.image.resolve(DOCKER_IMAGE_BASE_NAME);
+    let hive_role = HiveRole::MetaStore;
 
     let s3_connection_spec: Option<S3ConnectionSpec> =
         if let Some(s3) = &hive.spec.cluster_config.s3 {
@@ -303,7 +308,7 @@ pub async fn reconcile_hive(hive: Arc<HiveCluster>, ctx: Arc<Ctx>) -> Result<Act
         let rolegroup = hive.metastore_rolegroup_ref(rolegroup_name);
 
         let config = hive
-            .merged_config(&HiveRole::MetaStore, &rolegroup.role_group)
+            .merged_config(&HiveRole::MetaStore, &rolegroup)
             .context(FailedToResolveResourceConfigSnafu)?;
 
         let rg_service = build_rolegroup_service(&hive, &resolved_product_image, &rolegroup)?;
@@ -318,6 +323,7 @@ pub async fn reconcile_hive(hive: Arc<HiveCluster>, ctx: Arc<Ctx>) -> Result<Act
         )?;
         let rg_statefulset = build_metastore_rolegroup_statefulset(
             &hive,
+            &hive_role,
             &resolved_product_image,
             &rolegroup,
             rolegroup_config,
@@ -621,20 +627,19 @@ fn build_rolegroup_service(
 /// corresponding [`Service`] (from [`build_rolegroup_service`]).
 fn build_metastore_rolegroup_statefulset(
     hive: &HiveCluster,
+    hive_role: &HiveRole,
     resolved_product_image: &ResolvedProductImage,
     rolegroup_ref: &RoleGroupRef<HiveCluster>,
     metastore_config: &HashMap<PropertyNameKind, BTreeMap<String, String>>,
     s3_connection: Option<&S3ConnectionSpec>,
     merged_config: &MetaStoreConfig,
     sa_name: &str,
 ) -> Result<StatefulSet> {
+    let role = hive.role(hive_role).context(InternalOperatorSnafu)?;
     let rolegroup = hive
-        .spec
-        .metastore
-        .as_ref()
-        .context(NoMetaStoreRoleSnafu)?
-        .role_groups
-        .get(&rolegroup_ref.role_group);
+        .rolegroup(rolegroup_ref)
+        .context(InternalOperatorSnafu)?;
+
     let mut db_type: Option<DbType> = None;
     let mut container_builder =
         ContainerBuilder::new(APP_NAME).context(FailedToCreateHiveContainerSnafu {
@@ -857,6 +862,10 @@ fn build_metastore_rolegroup_statefulset(
         ));
     }
 
+    let mut pod_template = pod_builder.build_template();
+    pod_template.merge_from(role.config.pod_overrides.clone());
+    pod_template.merge_from(rolegroup.config.pod_overrides.clone());
+
     Ok(StatefulSet {
         metadata: ObjectMetaBuilder::new()
             .name_and_namespace(hive)
@@ -872,7 +881,7 @@ fn build_metastore_rolegroup_statefulset(
             .build(),
         spec: Some(StatefulSetSpec {
             pod_management_policy: Some("Parallel".to_string()),
-            replicas: rolegroup.and_then(|rg| rg.replicas).map(i32::from),
+            replicas: rolegroup.replicas.map(i32::from),
             selector: LabelSelector {
                 match_labels: Some(role_group_selector_labels(
                     hive,
@@ -883,7 +892,7 @@ fn build_metastore_rolegroup_statefulset(
                 ..LabelSelector::default()
             },
             service_name: rolegroup_ref.object_name(),
-            template: pod_builder.build_template(),
+            template: pod_template,
             ..StatefulSetSpec::default()
         }),
         status: None,

diff --git a/tests/templates/kuttl/resources/10-assert.yaml.j2 b/tests/templates/kuttl/resources/10-assert.yaml.j2
@@ -48,3 +48,26 @@ spec:
 status:
   readyReplicas: 1
   replicas: 1
+---
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: hive-metastore-resources-from-pod-overrides
+spec:
+  template:
+    spec:
+      containers:
+        - name: hive
+          resources:
+            requests:
+              cpu: 500m
+              memory: 4Gi
+            limits:
+              cpu: 3100m
+              memory: 4Gi
+{% if lookup('env', 'VECTOR_AGGREGATOR') %}
+        - name: vector
+{% endif %}
+status:
+  readyReplicas: 1
+  replicas: 1
diff --git a/tests/templates/kuttl/resources/10-install-hive.yaml.j2 b/tests/templates/kuttl/resources/10-install-hive.yaml.j2
@@ -38,3 +38,13 @@ spec:
               max: "3"
             memory:
               limit: 3Gi
+      resources-from-pod-overrides:
+        podOverrides:
+          spec:
+            containers:
+              - name: hive
+                resources:
+                  requests:
+                    cpu: 500m
+                  limits:
+                    cpu: 3100m
diff --git a/tests/templates/kuttl/smoke/60-assert.yaml b/tests/templates/kuttl/smoke/60-assert.yaml