diff --git a/Cargo.lock b/Cargo.lock index df205d79..0e9837a4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1514,7 +1514,7 @@ dependencies = [ [[package]] name = "k8s-version" version = "0.1.3" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "darling", "regex", @@ -2893,7 +2893,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-certs" version = "0.4.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "const-oid", "ecdsa", @@ -2939,7 +2939,7 @@ dependencies = [ [[package]] name = "stackable-operator" version = "0.111.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "base64", "clap", @@ -2983,7 +2983,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "darling", "proc-macro2", @@ -2994,7 +2994,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.1.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "jiff", "k8s-openapi", @@ -3011,7 +3011,7 @@ dependencies = [ [[package]] name = "stackable-telemetry" version = "0.6.4" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "axum", "clap", @@ -3035,7 +3035,7 @@ dependencies = [ [[package]] name = "stackable-versioned" version = "0.10.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "kube", "schemars", @@ -3049,7 +3049,7 @@ dependencies = [ [[package]] name = "stackable-versioned-macros" version = "0.10.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "convert_case", "convert_case_extras", @@ -3067,7 +3067,7 @@ dependencies = [ [[package]] name = "stackable-webhook" version = "0.9.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#451088f77acee6c3d296754698260256c250ecb2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#893cd4b0c82547e7f93764feb3c6dcc04877858a" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.nix b/Cargo.nix index a479f544..3922edb1 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -4826,8 +4826,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "k8s_version"; authors = [ @@ -9509,8 +9509,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_certs"; authors = [ @@ -9706,8 +9706,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_operator"; authors = [ @@ -9900,8 +9900,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -9935,8 +9935,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_shared"; authors = [ @@ -10016,8 +10016,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_telemetry"; authors = [ @@ -10126,8 +10126,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_versioned"; authors = [ @@ -10176,8 +10176,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; procMacro = true; libName = "stackable_versioned_macros"; @@ -10244,8 +10244,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "451088f77acee6c3d296754698260256c250ecb2"; - sha256 = "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b"; + rev = "893cd4b0c82547e7f93764feb3c6dcc04877858a"; + sha256 = "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3"; }; libName = "stackable_webhook"; authors = [ diff --git a/Cargo.toml b/Cargo.toml index 71492a17..056d9df5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,6 @@ tracing = "0.1" tracing-futures = { version = "0.2", features = ["futures-03"] } [patch."https://github.com/stackabletech/operator-rs.git"] -stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "smooth-operator" } +stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "jvm-argument-overrides" } # stackable-operator = { path = "../operator-rs/crates/stackable-operator" } # stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } diff --git a/crate-hashes.json b/crate-hashes.json index c9a6e6a9..3f4e3f75 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,12 +1,12 @@ { - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-shared@0.1.1": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.4": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", - "git+https://github.com/stackabletech//operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "1ifdpn0jvrf3xbgqldqxrq9ig1dc34d4fip7qxn38526k8004p4b", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#k8s-version@0.1.3": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-certs@0.4.0": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-operator-derive@0.3.1": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-operator@0.111.1": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-shared@0.1.1": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-telemetry@0.6.4": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-versioned-macros@0.10.0": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-versioned@0.10.0": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", + "git+https://github.com/stackabletech//operator-rs.git?branch=jvm-argument-overrides#stackable-webhook@0.9.1": "0s5giiz1zhz644s2fy9bfvd1hybm4smz75ak33fkvh2flvjfqba3", "git+https://github.com/stackabletech/product-config.git?tag=0.8.0#product-config@0.8.0": "1dz70kapm2wdqcr7ndyjji0lhsl98bsq95gnb2lw487wf6yr7987" } \ No newline at end of file diff --git a/rust/operator-binary/src/config/jvm.rs b/rust/operator-binary/src/config/jvm.rs index 95ebcd22..58ab6565 100644 --- a/rust/operator-binary/src/config/jvm.rs +++ b/rust/operator-binary/src/config/jvm.rs @@ -2,12 +2,11 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ k8s_openapi::api::core::v1::ResourceRequirements, memory::{BinaryMultiple, MemoryQuantity}, - role_utils::JvmArgumentOverrides, + v2::jvm_argument_overrides::JvmArgumentOverrides, }; use crate::{ - controller::build::properties::ConfigFileName, - crd::{HdfsNodeRole, v1alpha1}, + controller::build::properties::ConfigFileName, crd::HdfsNodeRole, security::kerberos::KERBEROS_CONTAINER_PATH, }; @@ -20,9 +19,6 @@ pub enum Error { source: stackable_operator::memory::Error, role: String, }, - - #[snafu(display("failed to merge jvm argument overrides"))] - MergeJvmArgumentOverrides { source: crate::crd::Error }, } // All init or sidecar containers must have access to the following settings. @@ -51,9 +47,8 @@ pub fn construct_global_jvm_args(kerberos_enabled: bool) -> String { } pub fn construct_role_specific_jvm_args( - hdfs: &v1alpha1::HdfsCluster, hdfs_role: &HdfsNodeRole, - role_group: &str, + jvm_argument_overrides: &JvmArgumentOverrides, kerberos_enabled: bool, resources: Option<&ResourceRequirements>, config_dir: &str, @@ -91,21 +86,20 @@ pub fn construct_role_specific_jvm_args( )); } - let operator_generated = JvmArgumentOverrides::new_with_only_additions(jvm_args); - let merged_jvm_args = hdfs - .get_merged_jvm_argument_overrides(hdfs_role, role_group, &operator_generated) - .context(MergeJvmArgumentOverridesSnafu)?; + let merged_jvm_args = jvm_argument_overrides.apply_to(jvm_args); - Ok(merged_jvm_args - .effective_jvm_config_after_merging() - .join(" ")) + Ok(merged_jvm_args.join(" ")) } #[cfg(test)] mod tests { use super::*; - use crate::{container::ContainerConfig, crd::constants::DEFAULT_NAME_NODE_METRICS_PORT}; + use crate::{ + container::ContainerConfig, + crd::constants::DEFAULT_NAME_NODE_METRICS_PORT, + test_support::{deserialize_and_validate_cluster, role_group_config}, + }; #[test] fn test_global_jvm_args() { @@ -123,6 +117,8 @@ mod tests { kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -151,6 +147,8 @@ mod tests { kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -196,18 +194,18 @@ mod tests { } fn construct_test_role_specific_jvm_args(hdfs_cluster: &str, kerberos_enabled: bool) -> String { - let hdfs: v1alpha1::HdfsCluster = - serde_yaml::from_str(hdfs_cluster).expect("illegal test input"); - let role = HdfsNodeRole::Name; - let merged_config = role.merged_config(&hdfs, "default").unwrap(); - let container_config = ContainerConfig::from(role); - let resources = container_config.resources(&merged_config); + + let validated_cluster = deserialize_and_validate_cluster(hdfs_cluster); + let role_group_config = role_group_config(&validated_cluster, &role, "default"); + + let resources = ContainerConfig::from(role).resources(&role_group_config.config); construct_role_specific_jvm_args( - &hdfs, &role, - "default", + &role_group_config + .product_specific_common_config + .jvm_argument_overrides, kerberos_enabled, resources.as_ref(), "/stackable/config", diff --git a/rust/operator-binary/src/container.rs b/rust/operator-binary/src/container.rs index 9e2e57ea..8aef0891 100644 --- a/rust/operator-binary/src/container.rs +++ b/rust/operator-binary/src/container.rs @@ -53,7 +53,6 @@ use stackable_operator::{ }, role_utils::RoleGroupRef, utils::{COMMON_BASH_TRAP_FUNCTIONS, cluster_info::KubernetesClusterInfo}, - v2::builder::pod::container::EnvVarSet, }; use strum::{Display, EnumDiscriminants, IntoStaticStr}; @@ -62,12 +61,15 @@ use crate::{ self, jvm::{construct_global_jvm_args, construct_role_specific_jvm_args}, }, - controller::build::properties::logging::{ - FORMAT_NAMENODES_LOG4J_CONFIG_FILE, FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, - HDFS_LOG4J_CONFIG_FILE, MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, - MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, MAX_HDFS_LOG_FILE_SIZE, - MAX_WAIT_NAMENODES_LOG_FILE_SIZE, MAX_ZKFC_LOG_FILE_SIZE, STACKABLE_LOG_DIR, - WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, ZKFC_LOG4J_CONFIG_FILE, + controller::{ + ValidatedRoleGroupConfig, + build::properties::logging::{ + FORMAT_NAMENODES_LOG4J_CONFIG_FILE, FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, + HDFS_LOG4J_CONFIG_FILE, MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, + MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, MAX_HDFS_LOG_FILE_SIZE, + MAX_WAIT_NAMENODES_LOG_FILE_SIZE, MAX_ZKFC_LOG_FILE_SIZE, STACKABLE_LOG_DIR, + WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, ZKFC_LOG4J_CONFIG_FILE, + }, }, crd::{ AnyNodeConfig, DataNodeContainer, HdfsNodeRole, HdfsPodRef, NameNodeContainer, @@ -218,8 +220,7 @@ impl ContainerConfig { role: &HdfsNodeRole, rolegroup_ref: &RoleGroupRef, resolved_product_image: &ResolvedProductImage, - merged_config: &AnyNodeConfig, - env_overrides: &EnvVarSet, + rolegroup_config: &ValidatedRoleGroupConfig, zk_config_map_name: &str, namenode_podrefs: &[HdfsPodRef], labels: &Labels, @@ -227,6 +228,7 @@ impl ContainerConfig { // HDFS main container let main_container_config = Self::from(*role); let object_name = rolegroup_ref.object_name(); + let merged_config = &rolegroup_config.config; pb.add_volumes(main_container_config.volumes(merged_config, &object_name, labels)?) .context(AddVolumeSnafu)?; @@ -234,10 +236,9 @@ impl ContainerConfig { hdfs, cluster_info, role, - rolegroup_ref, resolved_product_image, zk_config_map_name, - env_overrides, + rolegroup_config, merged_config, labels, )?); @@ -335,10 +336,9 @@ impl ContainerConfig { hdfs, cluster_info, role, - rolegroup_ref, resolved_product_image, zk_config_map_name, - env_overrides, + rolegroup_config, merged_config, labels, )?); @@ -356,10 +356,9 @@ impl ContainerConfig { hdfs, cluster_info, role, - &rolegroup_ref.role_group, resolved_product_image, zk_config_map_name, - env_overrides, + rolegroup_config, namenode_podrefs, merged_config, labels, @@ -378,10 +377,9 @@ impl ContainerConfig { hdfs, cluster_info, role, - &rolegroup_ref.role_group, resolved_product_image, zk_config_map_name, - env_overrides, + rolegroup_config, namenode_podrefs, merged_config, labels, @@ -401,10 +399,9 @@ impl ContainerConfig { hdfs, cluster_info, role, - &rolegroup_ref.role_group, resolved_product_image, zk_config_map_name, - env_overrides, + rolegroup_config, namenode_podrefs, merged_config, labels, @@ -470,10 +467,9 @@ impl ContainerConfig { hdfs: &v1alpha1::HdfsCluster, cluster_info: &KubernetesClusterInfo, role: &HdfsNodeRole, - rolegroup_ref: &RoleGroupRef, resolved_product_image: &ResolvedProductImage, zookeeper_config_map_name: &str, - env_overrides: &EnvVarSet, + rolegroup_config: &ValidatedRoleGroupConfig, merged_config: &AnyNodeConfig, labels: &Labels, ) -> Result { @@ -490,9 +486,8 @@ impl ContainerConfig { .add_env_vars(self.env( hdfs, role, - &rolegroup_ref.role_group, zookeeper_config_map_name, - env_overrides, + rolegroup_config, resources.as_ref(), )?) .add_volume_mounts(self.volume_mounts(hdfs, merged_config, labels)?) @@ -532,10 +527,9 @@ impl ContainerConfig { hdfs: &v1alpha1::HdfsCluster, cluster_info: &KubernetesClusterInfo, role: &HdfsNodeRole, - role_group: &str, resolved_product_image: &ResolvedProductImage, zookeeper_config_map_name: &str, - env_overrides: &EnvVarSet, + rolegroup_config: &ValidatedRoleGroupConfig, namenode_podrefs: &[HdfsPodRef], merged_config: &AnyNodeConfig, labels: &Labels, @@ -549,9 +543,8 @@ impl ContainerConfig { .add_env_vars(self.env( hdfs, role, - role_group, zookeeper_config_map_name, - env_overrides, + rolegroup_config, None, )?) .add_volume_mounts(self.volume_mounts(hdfs, merged_config, labels)?) @@ -873,9 +866,8 @@ impl ContainerConfig { &self, hdfs: &v1alpha1::HdfsCluster, role: &HdfsNodeRole, - role_group: &str, zookeeper_config_map_name: &str, - env_overrides: &EnvVarSet, + rolegroup_config: &ValidatedRoleGroupConfig, resources: Option<&ResourceRequirements>, ) -> Result, Error> { // Maps env var name to env var object. This allows env_overrides to work @@ -906,7 +898,7 @@ impl ContainerConfig { role_opts_name.clone(), EnvVar { name: role_opts_name, - value: Some(self.build_hadoop_opts(hdfs, role_group, resources)?), + value: Some(self.build_hadoop_opts(hdfs, resources, rolegroup_config)?), ..EnvVar::default() }, ); @@ -970,7 +962,8 @@ impl ContainerConfig { ); // Overrides need to come last - let mut env_override_vars: BTreeMap = env_overrides + let mut env_override_vars: BTreeMap = rolegroup_config + .env_overrides .clone() .into_iter() .map(|env_var| (env_var.name.clone(), env_var)) @@ -1263,8 +1256,8 @@ impl ContainerConfig { fn build_hadoop_opts( &self, hdfs: &v1alpha1::HdfsCluster, - role_group: &str, resources: Option<&ResourceRequirements>, + rolegroup_config: &ValidatedRoleGroupConfig, ) -> Result { match self { ContainerConfig::Hdfs { @@ -1273,9 +1266,10 @@ impl ContainerConfig { let cvd = ContainerVolumeDirs::from(role); let config_dir = cvd.final_config(); construct_role_specific_jvm_args( - hdfs, role, - role_group, + &rolegroup_config + .product_specific_common_config + .jvm_argument_overrides, hdfs.has_kerberos_enabled(), resources, config_dir, diff --git a/rust/operator-binary/src/controller/build/properties/hdfs_site.rs b/rust/operator-binary/src/controller/build/properties/hdfs_site.rs index cc196c5a..cffa8ff2 100644 --- a/rust/operator-binary/src/controller/build/properties/hdfs_site.rs +++ b/rust/operator-binary/src/controller/build/properties/hdfs_site.rs @@ -114,25 +114,23 @@ pub fn build( mod tests { use super::*; use crate::{ - controller::build::properties::test_support::{ - cluster_info, minimal_hdfs, validated_cluster, - }, - crd::{HdfsNodeRole, v1alpha1}, + controller::build::properties::test_support::{cluster_info, validated_cluster}, + crd::HdfsNodeRole, + test_support::anynode_config, }; - fn namenode_merged_config(hdfs: &v1alpha1::HdfsCluster) -> AnyNodeConfig { - HdfsNodeRole::Name - .merged_config(hdfs, "default") - .expect("merged config for the minimal namenode group") + fn namenode_merged_config(validated_cluster: &ValidatedCluster) -> &AnyNodeConfig { + anynode_config(validated_cluster, &HdfsNodeRole::Name, "default") } #[test] fn renders_operator_defaults() { - let merged = namenode_merged_config(&minimal_hdfs()); + let validated_cluster = validated_cluster(); + let merged = namenode_merged_config(&validated_cluster); let xml = build( - &validated_cluster(), + &validated_cluster, &cluster_info(), - &merged, + merged, KeyValueConfigOverrides::default(), ); assert!( @@ -147,11 +145,12 @@ mod tests { #[test] fn user_overrides_win_over_defaults() { - let merged = namenode_merged_config(&minimal_hdfs()); + let validated_cluster = validated_cluster(); + let merged = namenode_merged_config(&validated_cluster); let xml = build( - &validated_cluster(), + &validated_cluster, &cluster_info(), - &merged, + merged, [("dfs.replication", "5")].into(), ); assert!( diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 317206a2..19863499 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -70,10 +70,7 @@ pub(crate) mod test_support { commons::networking::DomainName, utils::cluster_info::KubernetesClusterInfo, }; - use crate::{ - controller::{ValidatedCluster, validate::validate_cluster}, - crd::v1alpha1, - }; + use crate::{controller::ValidatedCluster, test_support::deserialize_and_validate_cluster}; /// The rendered output of an empty Hadoop-XML configuration (no entries). pub const EMPTY_HADOOP_XML: &str = concat!( @@ -110,10 +107,6 @@ spec: replicas: 1 "#; - pub fn minimal_hdfs() -> v1alpha1::HdfsCluster { - serde_yaml::from_str(MINIMAL_HDFS_YAML).expect("invalid test HdfsCluster YAML") - } - pub fn cluster_info() -> KubernetesClusterInfo { KubernetesClusterInfo { cluster_domain: DomainName::try_from("cluster.local").unwrap(), @@ -121,13 +114,6 @@ spec: } pub fn validated_cluster() -> ValidatedCluster { - validate_cluster( - &minimal_hdfs(), - "oci.example.org", - crate::controller::dereference::DereferencedObjects { - hdfs_opa_config: None, - }, - ) - .expect("validate should succeed for the minimal fixture") + deserialize_and_validate_cluster(MINIMAL_HDFS_YAML) } } diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index e7e16a27..5e67077a 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -3,11 +3,15 @@ use std::collections::{BTreeMap, HashMap}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, commons::product_image_selection::ResolvedProductImage, + k8s_openapi::api::core::v1::PodTemplateSpec, kube::{Resource, api::ObjectMeta}, role_utils::RoleGroupRef, - v2::types::{ - kubernetes::{NamespaceName, Uid}, - operator::ClusterName, + v2::{ + builder::pod::container::EnvVarSet, + types::{ + kubernetes::{NamespaceName, Uid}, + operator::ClusterName, + }, }, }; @@ -27,12 +31,24 @@ pub mod validate; /// the product-specific common config is [`JavaCommonConfig`] (whose JVM-argument /// merge is fallible, hence the vendored framework variant), and the config /// overrides are [`v1alpha1::HdfsConfigOverrides`]. -pub type ValidatedRoleGroupConfig = crate::framework::role_utils::RoleGroupConfig< +pub type ValidatedRoleGroupConfig = RoleGroupConfig< AnyNodeConfig, - stackable_operator::role_utils::JavaCommonConfig, + stackable_operator::v2::role_utils::JavaCommonConfig, v1alpha1::HdfsConfigOverrides, >; +/// HDFS-friendly view of a validated, merged `RoleGroup`. +#[derive(Clone, Debug, PartialEq)] +pub struct RoleGroupConfig { + pub replicas: u16, + pub config: Config, + pub config_overrides: ConfigOverrides, + pub env_overrides: EnvVarSet, + pub cli_overrides: BTreeMap, + pub pod_overrides: PodTemplateSpec, + pub product_specific_common_config: CommonConfig, +} + /// The validated cluster: proves that config merging and validation succeeded /// for every role and role group before any resources are created. Placed in the /// controller so that subsequent steps that reference this struct only depend on diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 6600f257..73070727 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -7,14 +7,18 @@ //! `cliOverrides` and `podOverrides` (role group wins) into a single //! [`RoleGroupConfig`](crate::framework::role_utils::RoleGroupConfig). -use std::collections::BTreeMap; +use std::{collections::BTreeMap, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ commons::product_image_selection, config::{fragment::FromFragment, merge::Merge}, - role_utils::{GenericRoleConfig, JavaCommonConfig, Role}, - v2::controller_utils::{get_cluster_name, get_namespace, get_uid}, + role_utils::{GenericRoleConfig, Role}, + v2::{ + builder::pod::container::{EnvVarName, EnvVarSet}, + controller_utils::{get_cluster_name, get_namespace, get_uid}, + role_utils::{JavaCommonConfig, with_validated_config}, + }, }; use strum::IntoEnumIterator; @@ -27,7 +31,6 @@ use crate::{ AnyNodeConfig, DataNodeConfigFragment, HdfsNodeRole, JournalNodeConfigFragment, NameNodeConfigFragment, v1alpha1, }, - framework::role_utils::with_validated_config, }; const CONTAINER_IMAGE_BASE_NAME: &str = "hadoop"; @@ -54,9 +57,14 @@ pub enum Error { source: stackable_operator::v2::controller_utils::Error, }, + #[snafu(display("invalid environment variable override name"))] + ParseEnvVarName { + source: stackable_operator::v2::builder::pod::container::Error, + }, + #[snafu(display("failed to merge and validate the role group config"))] ValidateRoleGroupConfig { - source: crate::framework::role_utils::Error, + source: stackable_operator::config::fragment::ValidationError, }, } @@ -157,16 +165,24 @@ where >(role_group, role, &default_config) .context(ValidateRoleGroupConfigSnafu)?; + let mut env_overrides = EnvVarSet::new(); + for (env_var_name, env_var_value) in validated.config.env_overrides { + env_overrides = env_overrides.with_value( + &EnvVarName::from_str(&env_var_name).context(ParseEnvVarNameSnafu)?, + env_var_value, + ); + } + // Re-wrap the per-role validated config into the role-agnostic // `AnyNodeConfig`; the merged overrides carry over unchanged. let validated = ValidatedRoleGroupConfig { - replicas: validated.replicas, - config: wrap(validated.config), - config_overrides: validated.config_overrides, - env_overrides: validated.env_overrides, - cli_overrides: validated.cli_overrides, - pod_overrides: validated.pod_overrides, - product_specific_common_config: validated.product_specific_common_config, + replicas: validated.replicas.unwrap_or(1), + config: wrap(validated.config.config), + config_overrides: validated.config.config_overrides, + env_overrides, + cli_overrides: validated.config.cli_overrides, + pod_overrides: validated.config.pod_overrides, + product_specific_common_config: validated.config.product_specific_common_config, }; Ok((role_group_name.clone(), validated)) }) diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 8e14ecb3..ed34dd35 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -41,7 +41,10 @@ mod test { }, }; - use crate::crd::{HdfsNodeRole, v1alpha1}; + use crate::{ + crd::HdfsNodeRole, + test_support::{anynode_config, deserialize_and_validate_cluster}, + }; #[rstest] #[case(HdfsNodeRole::Journal)] @@ -53,6 +56,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: simple-hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -71,8 +76,9 @@ spec: default: replicas: 1 "#; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(input).unwrap(); - let merged_config = role.merged_config(&hdfs, "default").unwrap(); + + let validated_cluster = deserialize_and_validate_cluster(input); + let merged_config = anynode_config(&validated_cluster, &role, "default"); assert_eq!( merged_config.affinity, diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 37314a5c..1949050b 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -20,27 +20,24 @@ use stackable_operator::{ }, }, config::{ - fragment::{self, Fragment, ValidationError}, + fragment::{Fragment, ValidationError}, merge::Merge, }, crd::listener, deep_merger::ObjectOverrides, k8s_openapi::{api::core::v1::Pod, apimachinery::pkg::api::resource::Quantity}, - kube::{CustomResource, ResourceExt, runtime::reflector::ObjectRef}, + kube::{CustomResource, runtime::reflector::ObjectRef}, kvp::{LabelError, Labels}, product_logging::{ self, spec::{ContainerLogConfig, Logging}, }, - role_utils::{ - self, GenericRoleConfig, JavaCommonConfig, JvmArgumentOverrides, Role, RoleGroup, - RoleGroupRef, - }, + role_utils::{self, GenericRoleConfig, Role, RoleGroup, RoleGroupRef}, schemars::{self, JsonSchema}, shared::time::Duration, status::condition::{ClusterCondition, HasStatusCondition}, utils::cluster_info::KubernetesClusterInfo, - v2::config_overrides::KeyValueConfigOverrides, + v2::{config_overrides::KeyValueConfigOverrides, role_utils::JavaCommonConfig}, versioned::versioned, }; use strum::{Display, EnumIter, EnumString, IntoStaticStr}; @@ -334,41 +331,6 @@ impl v1alpha1::HdfsCluster { } } - pub fn get_merged_jvm_argument_overrides( - &self, - hdfs_role: &HdfsNodeRole, - role_group: &str, - operator_generated: &JvmArgumentOverrides, - ) -> Result { - match hdfs_role { - HdfsNodeRole::Journal => self - .spec - .journal_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Journal.to_string(), - })? - .get_merged_jvm_argument_overrides(role_group, operator_generated), - HdfsNodeRole::Name => self - .spec - .name_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Name.to_string(), - })? - .get_merged_jvm_argument_overrides(role_group, operator_generated), - HdfsNodeRole::Data => self - .spec - .data_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Data.to_string(), - })? - .get_merged_jvm_argument_overrides(role_group, operator_generated), - } - .context(MergeJvmArgumentOverridesSnafu) - } - pub fn rolegroup_ref( &self, role_name: impl Into, @@ -820,6 +782,7 @@ impl AnyNodeConfig { } } + #[allow(unused)] pub fn as_journalnode(&self) -> Option<&JournalNodeConfig> { if let Self::Journal(node) = self { Some(node) @@ -919,102 +882,6 @@ impl HdfsNodeRole { } } - /// Merge the [Name|Data|Journal]NodeConfigFragment defaults, role and role group settings. - /// The priority is: default < role config < role_group config - pub fn merged_config( - &self, - hdfs: &v1alpha1::HdfsCluster, - role_group: &str, - ) -> Result { - match self { - HdfsNodeRole::Name => { - let default_config = NameNodeConfigFragment::default_config(&hdfs.name_any(), self); - let role = hdfs - .spec - .name_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Name.to_string(), - })?; - - let mut role_config = role.config.config.clone(); - let mut role_group_config = hdfs - .namenode_rolegroup(role_group) - .with_context(|| MissingRoleGroupSnafu { - role: HdfsNodeRole::Name.to_string(), - role_group: role_group.to_string(), - })? - .config - .config - .clone(); - - role_config.merge(&default_config); - role_group_config.merge(&role_config); - Ok(AnyNodeConfig::Name( - fragment::validate::(role_group_config) - .context(FragmentValidationFailureSnafu)?, - )) - } - HdfsNodeRole::Data => { - let default_config = DataNodeConfigFragment::default_config(&hdfs.name_any(), self); - let role = hdfs - .spec - .data_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Data.to_string(), - })?; - - let mut role_config = role.config.config.clone(); - let mut role_group_config = hdfs - .datanode_rolegroup(role_group) - .with_context(|| MissingRoleGroupSnafu { - role: HdfsNodeRole::Data.to_string(), - role_group: role_group.to_string(), - })? - .config - .config - .clone(); - - role_config.merge(&default_config); - role_group_config.merge(&role_config); - Ok(AnyNodeConfig::Data( - fragment::validate::(role_group_config) - .context(FragmentValidationFailureSnafu)?, - )) - } - HdfsNodeRole::Journal => { - let default_config = - JournalNodeConfigFragment::default_config(&hdfs.name_any(), self); - let role = hdfs - .spec - .journal_nodes - .as_ref() - .with_context(|| MissingRoleSnafu { - role: HdfsNodeRole::Journal.to_string(), - })?; - - let mut role_config = role.config.config.clone(); - let mut role_group_config = hdfs - .journalnode_rolegroup(role_group) - .with_context(|| MissingRoleGroupSnafu { - role: HdfsNodeRole::Journal.to_string(), - role_group: role_group.to_string(), - })? - .config - .config - .clone(); - - role_config.merge(&default_config); - role_group_config.merge(&role_config); - Ok(AnyNodeConfig::Journal( - fragment::validate::(role_group_config) - .context(FragmentValidationFailureSnafu)?, - )) - } - } - } - /// Name of the Hadoop process HADOOP_OPTS. pub fn hadoop_opts_env_var_for_role(&self) -> &'static str { match self { @@ -1441,7 +1308,21 @@ mod test { }; use super::*; - use crate::crd::storage::HdfsStorageType; + use crate::{ + crd::storage::{DataNodePvc, HdfsStorageType}, + test_support::{datanode_config, deserialize_and_validate_cluster}, + }; + + fn datanode_pvc<'a>( + datanode_config: &'a DataNodeConfig, + storage_name: &str, + ) -> &'a DataNodePvc { + datanode_config + .resources + .storage + .get(storage_name) + .expect("storage should be defined") + } #[test] pub fn test_pvc_rolegroup_from_yaml() { @@ -1451,6 +1332,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1467,11 +1350,9 @@ spec: replicas: 1 "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - let role = HdfsNodeRole::Data; - let config = &role.merged_config(&hdfs, "default").unwrap(); - let resources = &config.as_datanode().unwrap().resources; - let pvc = resources.storage.get("data").unwrap(); + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); + let pvc = datanode_pvc(datanode_config, "data"); assert_eq!(pvc.count, 1); assert_eq!(pvc.hdfs_storage_type, HdfsStorageType::Disk); @@ -1486,6 +1367,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1502,11 +1385,9 @@ spec: replicas: 1 "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - let role = HdfsNodeRole::Data; - let config = &role.merged_config(&hdfs, "default").unwrap(); - let resources = &config.as_datanode().unwrap().resources; - let pvc = resources.storage.get("data").unwrap(); + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); + let pvc = datanode_pvc(datanode_config, "data"); assert_eq!(pvc.count, 1); assert_eq!(pvc.hdfs_storage_type, HdfsStorageType::Disk); @@ -1521,6 +1402,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1532,11 +1415,9 @@ spec: replicas: 1 "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - let role = HdfsNodeRole::Data; - let config = role.merged_config(&hdfs, "default").unwrap(); - let resources = &config.as_datanode().unwrap().resources; - let pvc = resources.storage.get("data").unwrap(); + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); + let pvc = datanode_pvc(datanode_config, "data"); assert_eq!(pvc.count, 1); assert_eq!(pvc.hdfs_storage_type, HdfsStorageType::Disk); @@ -1551,6 +1432,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1585,23 +1468,19 @@ spec: default: replicas: 1"; - let deserializer = serde_yaml::Deserializer::from_str(cr); - let hdfs: v1alpha1::HdfsCluster = - serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); - let role = HdfsNodeRole::Data; - let config = &role.merged_config(&hdfs, "default").unwrap(); - let resources = &config.as_datanode().unwrap().resources; + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); - let pvc = resources.storage.get("data").unwrap(); + let pvc = datanode_pvc(datanode_config, "data"); assert_eq!(pvc.count, 0); - let pvc = resources.storage.get("my-disks").unwrap(); + let pvc = datanode_pvc(datanode_config, "my-disks"); assert_eq!(pvc.count, 5); assert_eq!(pvc.hdfs_storage_type, HdfsStorageType::Disk); assert_eq!(pvc.pvc.capacity, Some(Quantity("100Gi".to_string()))); assert_eq!(pvc.pvc.storage_class, None); - let pvc = resources.storage.get("my-ssds").unwrap(); + let pvc = datanode_pvc(datanode_config, "my-ssds"); assert_eq!(pvc.count, 3); assert_eq!(pvc.hdfs_storage_type, HdfsStorageType::Ssd); assert_eq!(pvc.pvc.capacity, Some(Quantity("10Gi".to_string()))); @@ -1616,6 +1495,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1634,16 +1515,9 @@ spec: replicas: 1 "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - let role = HdfsNodeRole::Data; - let rr: ResourceRequirements = role - .merged_config(&hdfs, "default") - .unwrap() - .as_datanode() - .unwrap() - .resources - .clone() - .into(); + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); + let rr = datanode_config.resources.clone().into(); let expected = ResourceRequirements { requests: Some( @@ -1672,6 +1546,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1689,16 +1565,9 @@ spec: min: '250m' "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - let role = HdfsNodeRole::Data; - let rr: ResourceRequirements = role - .merged_config(&hdfs, "default") - .unwrap() - .as_datanode() - .unwrap() - .resources - .clone() - .into(); + let validated_cluster = deserialize_and_validate_cluster(cr); + let datanode_config = datanode_config(&validated_cluster, "default"); + let rr = datanode_config.resources.clone().into(); let expected = ResourceRequirements { requests: Some( @@ -1728,6 +1597,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 @@ -1755,6 +1626,8 @@ apiVersion: hdfs.stackable.tech/v1alpha1 kind: HdfsCluster metadata: name: hdfs + namespace: test + uid: 8047b73b-db0f-4281-811f-de59105ae6bf spec: image: productVersion: 3.4.2 diff --git a/rust/operator-binary/src/framework.rs b/rust/operator-binary/src/framework.rs deleted file mode 100644 index d9f0b69a..00000000 --- a/rust/operator-binary/src/framework.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! Local framework helpers that mirror the work-in-progress upstream -//! `stackable_operator::v2::*` modules. -//! -//! We vendor `role_utils` because the upstream `v2::role_utils` requires -//! `CommonConfig: Merge`. HDFS (like hive and trino) uses `JavaCommonConfig`, -//! whose JVM-argument merge is fallible and so does not implement `Merge`. -//! -//! Follow-up: replace with `stackable_operator::v2::role_utils::*` once upstream -//! relaxes the `Merge` bound. - -pub mod role_utils; diff --git a/rust/operator-binary/src/framework/role_utils.rs b/rust/operator-binary/src/framework/role_utils.rs deleted file mode 100644 index 19a51bdb..00000000 --- a/rust/operator-binary/src/framework/role_utils.rs +++ /dev/null @@ -1,152 +0,0 @@ -//! Vendored variant of `stackable_operator::v2::role_utils` from the -//! `smooth-operator` branch, with simplifications appropriate for hdfs-operator. -//! -//! Differences from upstream: -//! - No `cli_overrides_to_vec` helper, `ResourceNames`, or service-account helpers. -//! - The `CommonConfig` (a.k.a. `product_specific_common_config`) does NOT need to -//! implement `Merge`. HDFS uses `JavaCommonConfig`, which intentionally does not -//! implement `Merge` because its inner `JvmArgumentOverrides::try_merge` is -//! fallible (regex validation). The `RoleGroupConfig::product_specific_common_config` -//! field here simply carries the role-group level value through. -//! -//! Replace with `stackable_operator::v2::role_utils::*` once upstream relaxes the -//! `Merge` bound. - -use std::{ - collections::{BTreeMap, HashMap}, - str::FromStr, -}; - -use serde::Serialize; -use snafu::{ResultExt, Snafu}; -use stackable_operator::{ - config::{ - fragment::{self, FromFragment}, - merge::{Merge, merge}, - }, - k8s_openapi::{DeepMerge, api::core::v1::PodTemplateSpec}, - role_utils::{Role, RoleGroup}, - schemars::JsonSchema, - v2::builder::pod::container::{self, EnvVarName, EnvVarSet}, -}; - -#[derive(Snafu, Debug)] -pub enum Error { - #[snafu(display("failed to validate the role group config"))] - ValidateConfig { source: fragment::ValidationError }, - - #[snafu(display("invalid environment variable override name"))] - ParseEnvVarName { source: container::Error }, -} - -/// HDFS-friendly view of a validated, merged `RoleGroup`. -#[derive(Clone, Debug, PartialEq)] -pub struct RoleGroupConfig { - pub replicas: u16, - pub config: Config, - pub config_overrides: ConfigOverrides, - pub env_overrides: EnvVarSet, - pub cli_overrides: BTreeMap, - pub pod_overrides: PodTemplateSpec, - pub product_specific_common_config: CommonConfig, -} - -/// Merges and validates the `RoleGroup` with the given `role` and `default_config`. -pub fn with_validated_config( - role_group: &RoleGroup, - role: &Role, - default_config: &Config, -) -> Result, Error> -where - ValidatedConfig: FromFragment, - CommonConfig: Clone + Default + JsonSchema + Serialize, - Config: Clone + Merge, - RoleConfig: Default + JsonSchema + Serialize, - ConfigOverrides: Clone + Default + JsonSchema + Merge + Serialize, -{ - let validated_config = - validate_config(role_group, role, default_config).context(ValidateConfigSnafu)?; - Ok(RoleGroupConfig { - replicas: role_group.replicas.unwrap_or(1), - config: validated_config, - config_overrides: merged_config_overrides( - &role.config.config_overrides, - role_group.config.config_overrides.clone(), - ), - env_overrides: merged_env_overrides( - &role.config.env_overrides, - &role_group.config.env_overrides, - )?, - cli_overrides: merged_cli_overrides( - role.config.cli_overrides.clone(), - role_group.config.cli_overrides.clone(), - ), - pod_overrides: merged_pod_overrides( - role.config.pod_overrides.clone(), - role_group.config.pod_overrides.clone(), - ), - product_specific_common_config: role_group.config.product_specific_common_config.clone(), - }) -} - -fn validate_config( - role_group: &RoleGroup, - role: &Role, - default_config: &Config, -) -> Result -where - ValidatedConfig: FromFragment, - CommonConfig: Default + JsonSchema + Serialize, - Config: Clone + Merge, - RoleConfig: Default + JsonSchema + Serialize, - ConfigOverrides: Default + JsonSchema + Serialize, -{ - role_group.validate_config(role, default_config) -} - -fn merged_config_overrides( - role_config_overrides: &ConfigOverrides, - role_group_config_overrides: ConfigOverrides, -) -> ConfigOverrides -where - ConfigOverrides: Merge, -{ - merge(role_group_config_overrides, role_config_overrides) -} - -fn merged_env_overrides( - role_env_overrides: &HashMap, - role_group_env_overrides: &HashMap, -) -> Result { - // Process the role first, then the role group, so that role-group overrides win on key - // collisions (`EnvVarSet::with_value` overrides earlier entries with the same name). - let mut env_overrides = EnvVarSet::new(); - for (name, value) in role_env_overrides - .iter() - .chain(role_group_env_overrides.iter()) - { - env_overrides = env_overrides.with_value( - &EnvVarName::from_str(name).context(ParseEnvVarNameSnafu)?, - value.clone(), - ); - } - Ok(env_overrides) -} - -fn merged_cli_overrides( - role_cli_overrides: BTreeMap, - role_group_cli_overrides: BTreeMap, -) -> BTreeMap { - let mut merged = role_cli_overrides; - merged.extend(role_group_cli_overrides); - merged -} - -fn merged_pod_overrides( - role_pod_overrides: PodTemplateSpec, - role_group_pod_overrides: PodTemplateSpec, -) -> PodTemplateSpec { - let mut merged = role_pod_overrides; - merged.merge_from(role_group_pod_overrides); - merged -} diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index b4e6d6f5..01b7a3a3 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -455,7 +455,6 @@ fn rolegroup_statefulset( let image = &validated.image; let merged_config = &rolegroup_config.config; - let env_overrides = &rolegroup_config.env_overrides; // Pod references for all namenodes across all role groups, needed to wire up the // init containers of this role group. @@ -492,8 +491,7 @@ fn rolegroup_statefulset( role, rolegroup_ref, image, - merged_config, - env_overrides, + rolegroup_config, &hdfs.spec.cluster_config.zookeeper_config_map_name, &namenode_podrefs, &rolegroup_selector_labels, @@ -556,7 +554,7 @@ mod test { use stackable_operator::commons::networking::DomainName; use super::*; - use crate::controller::validate::validate_cluster; + use crate::test_support::{deserialize_cluster, role_group_config, validate_cluster}; #[test] pub fn test_env_overrides() { @@ -592,28 +590,14 @@ spec: GROUP_VAR: group-value # only defined here at group level replicas: 1 "; - let hdfs: v1alpha1::HdfsCluster = serde_yaml::from_str(cr).unwrap(); - - let validated = validate_cluster( - &hdfs, - "oci.example.org", - crate::controller::dereference::DereferencedObjects { - hdfs_opa_config: None, - }, - ) - .unwrap(); let role = HdfsNodeRole::Data; - let validated_rg_config = validated - .role_groups - .get(&role) - .unwrap() - .get("default") - .unwrap(); + let hdfs = deserialize_cluster(cr); + let validated_cluster = validate_cluster(&hdfs); + let role_group_config = role_group_config(&validated_cluster, &role, "default"); + let rolegroup_ref = hdfs.rolegroup_ref(role.to_string(), "default"); - let env_overrides = &validated_rg_config.env_overrides; - let merged_config = &validated_rg_config.config; - let resolved_product_image = &validated.image; + let resolved_product_image = &validated_cluster.image; let mut pb = PodBuilder::new(); pb.metadata(ObjectMeta::default()); @@ -626,8 +610,7 @@ spec: &role, &rolegroup_ref, resolved_product_image, - merged_config, - env_overrides, + role_group_config, &hdfs.spec.cluster_config.zookeeper_config_map_name, &[], &Labels::new(), diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index ba87daa5..c0da7b3b 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -47,7 +47,6 @@ mod container; mod controller; mod crd; mod event; -mod framework; mod hdfs_clusterrolebinding_nodes_controller; mod hdfs_controller; mod operations; @@ -267,3 +266,6 @@ fn references_config_map( None => false, } } + +#[cfg(test)] +pub(crate) mod test_support; diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs new file mode 100644 index 00000000..e5a088e6 --- /dev/null +++ b/rust/operator-binary/src/test_support.rs @@ -0,0 +1,54 @@ +use crate::{ + controller::{ValidatedCluster, ValidatedRoleGroupConfig, validate}, + crd::{AnyNodeConfig, DataNodeConfig, HdfsNodeRole, v1alpha1}, +}; + +pub fn deserialize_cluster(spec: &str) -> v1alpha1::HdfsCluster { + let deserializer = serde_yaml::Deserializer::from_str(spec); + serde_yaml::with::singleton_map_recursive::deserialize(deserializer).expect("") +} + +pub fn validate_cluster(hdfs: &v1alpha1::HdfsCluster) -> ValidatedCluster { + validate::validate_cluster( + hdfs, + "oci.example.org", + crate::controller::dereference::DereferencedObjects { + hdfs_opa_config: None, + }, + ) + .expect("cluster spec should be valid") +} + +pub fn deserialize_and_validate_cluster(spec: &str) -> ValidatedCluster { + validate_cluster(&deserialize_cluster(spec)) +} + +pub fn role_group_config<'a>( + validated_cluster: &'a ValidatedCluster, + role: &HdfsNodeRole, + role_group_name: &str, +) -> &'a ValidatedRoleGroupConfig { + validated_cluster + .role_groups + .get(role) + .expect("role should be defined") + .get(role_group_name) + .expect("role group should be defined") +} + +pub fn anynode_config<'a>( + validated_cluster: &'a ValidatedCluster, + role: &HdfsNodeRole, + role_group_name: &str, +) -> &'a AnyNodeConfig { + &role_group_config(validated_cluster, role, role_group_name).config +} + +pub fn datanode_config<'a>( + validated_cluster: &'a ValidatedCluster, + role_group_name: &str, +) -> &'a DataNodeConfig { + anynode_config(validated_cluster, &HdfsNodeRole::Data, role_group_name) + .as_datanode() + .expect("should be a DataNode") +} diff --git a/tests/templates/kuttl/smoke/30-assert.yaml.j2 b/tests/templates/kuttl/smoke/30-assert.yaml.j2 index 3d71a8ea..5267d762 100644 --- a/tests/templates/kuttl/smoke/30-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/30-assert.yaml.j2 @@ -348,8 +348,9 @@ spec: value: /stackable/hadoop - name: HADOOP_OPTS - name: HDFS_NAMENODE_OPTS - value: -Xms819m -Xmx819m -Djava.security.properties=/stackable/config/namenode/security.properties + value: -Djava.security.properties=/stackable/config/namenode/security.properties -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=8183:/stackable/jmx/namenode.yaml + -Xms820m -Xmx820m - name: POD_NAME valueFrom: fieldRef: diff --git a/tests/templates/kuttl/smoke/30-install-hdfs.yaml.j2 b/tests/templates/kuttl/smoke/30-install-hdfs.yaml.j2 index 94295f94..0bd4ba9e 100644 --- a/tests/templates/kuttl/smoke/30-install-hdfs.yaml.j2 +++ b/tests/templates/kuttl/smoke/30-install-hdfs.yaml.j2 @@ -32,12 +32,24 @@ spec: listenerClass: {{ test_scenario['values']['listener-class'] }} logging: enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} + jvmArgumentOverrides: + remove: + - -Xms819m + removeRegex: + - -Xmx.* + # The regex only matches the entire string and therefore `-javaagent:...` should not be + # removed. + - -javaagent roleGroups: default: replicas: 2 envOverrides: COMMON_VAR: group-value # overrides role value GROUP_VAR: group-value # only defined here at group level + jvmArgumentOverrides: + add: + - -Xms820m + - -Xmx820m dataNodes: envOverrides: COMMON_VAR: role-value # overridden by role group below