From d4d3faeaac37dcbc58c8f62391ba8aeb257f2f98 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 11 Sep 2026 11:38:51 +0200 Subject: [PATCH 1/4] derive container names from typed constants instead of Display --- .../src/controller/build/container.rs | 128 +++++++++++------- .../build/properties/product_logging/mod.rs | 22 +-- 2 files changed, 94 insertions(+), 56 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 5b50f552..2c8286cd 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -101,6 +101,18 @@ pub(crate) const TLS_STORE_PASSWORD: &str = "changeit"; constant!(pub(crate) KERBEROS_VOLUME_NAME: VolumeName = "kerberos"); constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector"); +// The main container of each role is named after the role (not after its logging key `hdfs`); +// a unit test pins these names to the role names. +constant!(NAMENODE_CONTAINER_NAME: ContainerName = "namenode"); +constant!(DATANODE_CONTAINER_NAME: ContainerName = "datanode"); +constant!(JOURNALNODE_CONTAINER_NAME: ContainerName = "journalnode"); +// The side and init containers are named like the strum `Display` of the corresponding +// `NameNodeContainer`/`DataNodeContainer` variants, which operator-rs's `Logging` still +// requires; a unit test pins the two spellings together. +constant!(pub(crate) ZKFC_CONTAINER_NAME: ContainerName = "zkfc"); +constant!(pub(crate) FORMAT_NAMENODES_CONTAINER_NAME: ContainerName = "format-namenodes"); +constant!(pub(crate) FORMAT_ZOOKEEPER_CONTAINER_NAME: ContainerName = "format-zookeeper"); +constant!(pub(crate) WAIT_FOR_NAMENODES_CONTAINER_NAME: ContainerName = "wait-for-namenodes"); // The volume the rolegroup ConfigMap (including the static `vector.yaml`) is mounted into. constant!(VECTOR_CONFIG_VOLUME_NAME: VolumeName = "hdfs-config"); // The volume holding the product logs that Vector tails. @@ -161,10 +173,8 @@ pub enum Error { #[derive(Display)] pub enum ContainerConfig { Hdfs { - /// HDFS role (name-, data-, journal-node) which will be the container_name. + /// HDFS role (name-, data-, journal-node) which determines the container name. role: HdfsNodeRole, - /// The container name derived from the provided role. - container_name: String, /// Volume mounts for config and logging. volume_mounts: ContainerVolumeDirs, /// Port name of the IPC/RPC port, used for the readiness probe. @@ -177,26 +187,18 @@ pub enum ContainerConfig { metrics_port: Port, }, Zkfc { - /// The provided custom container name. - container_name: String, /// Volume mounts for config and logging. volume_mounts: ContainerVolumeDirs, }, FormatNameNodes { - /// The provided custom container name. - container_name: String, /// Volume mounts for config and logging. volume_mounts: ContainerVolumeDirs, }, FormatZooKeeper { - /// The provided custom container name. - container_name: String, /// Volume mounts for config and logging. volume_mounts: ContainerVolumeDirs, }, WaitForNameNodes { - /// The provided custom container name. - container_name: String, /// Volume mounts for config and logging. volume_mounts: ContainerVolumeDirs, }, @@ -474,7 +476,7 @@ impl ContainerConfig { labels: &Labels, ) -> Result { let merged_config = &rolegroup_config.config; - let mut cb = new_container_builder(&self.container_name()); + let mut cb = new_container_builder(self.name()); let resources = self.resources(merged_config); @@ -523,7 +525,7 @@ impl ContainerConfig { labels: &Labels, ) -> Result { let merged_config = &rolegroup_config.config; - let mut cb = new_container_builder(&self.container_name()); + let mut cb = new_container_builder(self.name()); cb.image_from_product_image(&cluster.image) .command(Self::command()) @@ -542,23 +544,21 @@ impl ContainerConfig { Ok(cb.build()) } - /// Return the container name. - fn name(&self) -> &str { - match &self { - ContainerConfig::Hdfs { container_name, .. } => container_name.as_str(), - ContainerConfig::Zkfc { container_name, .. } => container_name.as_str(), - ContainerConfig::FormatNameNodes { container_name, .. } => container_name.as_str(), - ContainerConfig::FormatZooKeeper { container_name, .. } => container_name.as_str(), - ContainerConfig::WaitForNameNodes { container_name, .. } => container_name.as_str(), + /// Return the typed container name. + fn name(&self) -> &'static ContainerName { + match self { + ContainerConfig::Hdfs { role, .. } => match role { + HdfsNodeRole::Name => &NAMENODE_CONTAINER_NAME, + HdfsNodeRole::Data => &DATANODE_CONTAINER_NAME, + HdfsNodeRole::Journal => &JOURNALNODE_CONTAINER_NAME, + }, + ContainerConfig::Zkfc { .. } => &ZKFC_CONTAINER_NAME, + ContainerConfig::FormatNameNodes { .. } => &FORMAT_NAMENODES_CONTAINER_NAME, + ContainerConfig::FormatZooKeeper { .. } => &FORMAT_ZOOKEEPER_CONTAINER_NAME, + ContainerConfig::WaitForNameNodes { .. } => &WAIT_FOR_NAMENODES_CONTAINER_NAME, } } - /// Return the type-safe container name. - fn container_name(&self) -> ContainerName { - ContainerName::from_str(self.name()) - .expect("a ContainerConfig name is a valid container name") - } - /// Return volume mount directories depending on the container. fn volume_mount_dirs(&self) -> &ContainerVolumeDirs { match &self { @@ -652,8 +652,8 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME )); } - ContainerConfig::FormatNameNodes { container_name, .. } => { - args.push_str(&bash_capture_shell_helper(container_name)); + ContainerConfig::FormatNameNodes { .. } => { + args.push_str(&bash_capture_shell_helper(self.name().as_ref())); if let Some(container_config) = merged_config.as_namenode().map(|node| { node.logging @@ -728,8 +728,8 @@ impl ContainerConfig { .join(" "), )); } - ContainerConfig::FormatZooKeeper { container_name, .. } => { - args.push_str(&bash_capture_shell_helper(container_name)); + ContainerConfig::FormatZooKeeper { .. } => { + args.push_str(&bash_capture_shell_helper(self.name().as_ref())); if let Some(container_config) = merged_config.as_namenode().map(|node| { node.logging @@ -760,8 +760,8 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME, )); } - ContainerConfig::WaitForNameNodes { container_name, .. } => { - args.push_str(&bash_capture_shell_helper(container_name)); + ContainerConfig::WaitForNameNodes { .. } => { + args.push_str(&bash_capture_shell_helper(self.name().as_ref())); if let Some(container_config) = merged_config.as_datanode().map(|node| { node.logging @@ -1385,7 +1385,6 @@ impl From for ContainerConfig { match role { HdfsNodeRole::Name => Self::Hdfs { role, - container_name: role.to_string(), volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, @@ -1394,7 +1393,6 @@ impl From for ContainerConfig { }, HdfsNodeRole::Data => Self::Hdfs { role, - container_name: role.to_string(), volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_IPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, @@ -1403,7 +1401,6 @@ impl From for ContainerConfig { }, HdfsNodeRole::Journal => Self::Hdfs { role, - container_name: role.to_string(), volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, @@ -1417,53 +1414,45 @@ impl From for ContainerConfig { impl ContainerConfig { /// The ZooKeeper fail-over controller side container of the namenodes. fn zkfc() -> Self { - let container_name = NameNodeContainer::Zkfc.to_string(); Self::Zkfc { volume_mounts: ContainerVolumeDirs::for_container( - &container_name, + ZKFC_CONTAINER_NAME.as_ref(), Self::ZKFC_CONFIG_VOLUME_MOUNT_NAME, Self::ZKFC_LOG_VOLUME_MOUNT_NAME, ), - container_name, } } /// The init container formatting the namenodes. fn format_namenodes() -> Self { - let container_name = NameNodeContainer::FormatNameNodes.to_string(); Self::FormatNameNodes { volume_mounts: ContainerVolumeDirs::for_container( - &container_name, + FORMAT_NAMENODES_CONTAINER_NAME.as_ref(), Self::FORMAT_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, Self::FORMAT_NAMENODES_LOG_VOLUME_MOUNT_NAME, ), - container_name, } } /// The init container formatting ZooKeeper for the namenodes. fn format_zookeeper() -> Self { - let container_name = NameNodeContainer::FormatZooKeeper.to_string(); Self::FormatZooKeeper { volume_mounts: ContainerVolumeDirs::for_container( - &container_name, + FORMAT_ZOOKEEPER_CONTAINER_NAME.as_ref(), Self::FORMAT_ZOOKEEPER_CONFIG_VOLUME_MOUNT_NAME, Self::FORMAT_ZOOKEEPER_LOG_VOLUME_MOUNT_NAME, ), - container_name, } } /// The init container of the datanodes waiting for the namenodes. fn wait_for_namenodes() -> Self { - let container_name = DataNodeContainer::WaitForNameNodes.to_string(); Self::WaitForNameNodes { volume_mounts: ContainerVolumeDirs::for_container( - &container_name, + WAIT_FOR_NAMENODES_CONTAINER_NAME.as_ref(), Self::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, Self::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME, ), - container_name, } } } @@ -1614,6 +1603,8 @@ fn bash_capture_shell_helper(container_name: &str) -> String { #[cfg(test)] mod tests { + use strum::IntoEnumIterator; + use super::*; #[test] @@ -1622,7 +1613,48 @@ mod tests { let _ = *TLS_STORE_VOLUME_NAME; let _ = *KERBEROS_VOLUME_NAME; let _ = *VECTOR_CONTAINER_NAME; + let _ = *NAMENODE_CONTAINER_NAME; + let _ = *DATANODE_CONTAINER_NAME; + let _ = *JOURNALNODE_CONTAINER_NAME; + let _ = *ZKFC_CONTAINER_NAME; + let _ = *FORMAT_NAMENODES_CONTAINER_NAME; + let _ = *FORMAT_ZOOKEEPER_CONTAINER_NAME; + let _ = *WAIT_FOR_NAMENODES_CONTAINER_NAME; let _ = *VECTOR_CONFIG_VOLUME_NAME; let _ = *VECTOR_LOG_VOLUME_NAME; } + + /// The main container of every role is named after the role. + #[test] + fn main_container_names_match_role_names() { + for role in HdfsNodeRole::iter() { + assert_eq!( + ContainerConfig::from(role).name().to_string(), + role.to_string() + ); + } + } + + /// The typed side and init container names must agree with the strum `Display` of the + /// `NameNodeContainer`/`DataNodeContainer` variants, which operator-rs's `Logging` still + /// requires. + #[test] + fn side_container_names_match_display() { + assert_eq!( + ContainerConfig::zkfc().name().to_string(), + NameNodeContainer::Zkfc.to_string() + ); + assert_eq!( + ContainerConfig::format_namenodes().name().to_string(), + NameNodeContainer::FormatNameNodes.to_string() + ); + assert_eq!( + ContainerConfig::format_zookeeper().name().to_string(), + NameNodeContainer::FormatZooKeeper.to_string() + ); + assert_eq!( + ContainerConfig::wait_for_namenodes().name().to_string(), + DataNodeContainer::WaitForNameNodes.to_string() + ); + } } diff --git a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs index 23409962..6b956cd1 100644 --- a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs @@ -1,7 +1,7 @@ //! Builders for the logging-related files in the rolegroup `ConfigMap`: the per-container //! `*.log4j.properties` configs and the (static) Vector agent config (`vector.yaml`). -use std::{borrow::Cow, fmt::Display}; +use std::borrow::Cow; use stackable_operator::{ memory::{BinaryMultiple, MemoryQuantity}, @@ -12,7 +12,13 @@ use stackable_operator::{ v2::product_logging::framework::STACKABLE_LOG_DIR, }; -use crate::crd::{AnyNodeConfig, DataNodeContainer, NameNodeContainer}; +use crate::{ + controller::build::container::{ + FORMAT_NAMENODES_CONTAINER_NAME, FORMAT_ZOOKEEPER_CONTAINER_NAME, + WAIT_FOR_NAMENODES_CONTAINER_NAME, ZKFC_CONTAINER_NAME, + }, + crd::{AnyNodeConfig, DataNodeContainer, NameNodeContainer}, +}; // We have a maximum of 4 continuous logging files for Namenodes. Datanodes and Journalnodes // require less. @@ -90,7 +96,7 @@ pub fn build_log4j_configs(merged_config: &AnyNodeConfig) -> Vec<(&'static str, .as_namenode() .map(|nn| nn.logging.for_container(&NameNodeContainer::Zkfc)), ZKFC_LOG4J_CONFIG_FILE, - &NameNodeContainer::Zkfc, + ZKFC_CONTAINER_NAME.as_ref(), ZKFC_LOG_FILE, MAX_ZKFC_LOG_FILE_SIZE, ); @@ -101,7 +107,7 @@ pub fn build_log4j_configs(merged_config: &AnyNodeConfig) -> Vec<(&'static str, .for_container(&NameNodeContainer::FormatNameNodes) }), FORMAT_NAMENODES_LOG4J_CONFIG_FILE, - &NameNodeContainer::FormatNameNodes, + FORMAT_NAMENODES_CONTAINER_NAME.as_ref(), FORMAT_NAMENODES_LOG_FILE, MAX_FORMAT_NAMENODE_LOG_FILE_SIZE, ); @@ -112,7 +118,7 @@ pub fn build_log4j_configs(merged_config: &AnyNodeConfig) -> Vec<(&'static str, .for_container(&NameNodeContainer::FormatZooKeeper) }), FORMAT_ZOOKEEPER_LOG4J_CONFIG_FILE, - &NameNodeContainer::FormatZooKeeper, + FORMAT_ZOOKEEPER_CONTAINER_NAME.as_ref(), FORMAT_ZOOKEEPER_LOG_FILE, MAX_FORMAT_ZOOKEEPER_LOG_FILE_SIZE, ); @@ -123,7 +129,7 @@ pub fn build_log4j_configs(merged_config: &AnyNodeConfig) -> Vec<(&'static str, .for_container(&DataNodeContainer::WaitForNameNodes) }), WAIT_FOR_NAMENODES_LOG4J_CONFIG_FILE, - &DataNodeContainer::WaitForNameNodes, + WAIT_FOR_NAMENODES_CONTAINER_NAME.as_ref(), WAIT_FOR_NAMENODES_LOG_FILE, MAX_WAIT_NAMENODES_LOG_FILE_SIZE, ); @@ -135,7 +141,7 @@ fn add_log4j_config_if_automatic( configs: &mut Vec<(&'static str, String)>, log_config: Option>, log_config_file: &'static str, - container_name: impl Display, + log_dir_name: &str, log_file: &str, max_log_file_size: MemoryQuantity, ) { @@ -146,7 +152,7 @@ fn add_log4j_config_if_automatic( configs.push(( log_config_file, product_logging::framework::create_log4j_config( - &format!("{STACKABLE_LOG_DIR}/{container_name}"), + &format!("{STACKABLE_LOG_DIR}/{log_dir_name}"), log_file, max_log_file_size .scale_to(BinaryMultiple::Mebi) From dfcdb769f2414c22c13ae7bf0aefda14817c49f7 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 11 Sep 2026 11:45:49 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b04ec2d..08f4c974 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ All notable changes to this project will be documented in this file. are no longer created with the placeholder `app.kubernetes.io/component: none` and `app.kubernetes.io/role-group: none` labels. StatefulSet selectors and volume claim templates are unchanged, so upgrading is non-breaking. -- Make operations infallible where dependent on static inputs ([#824]). +- Make operations infallible where dependent on static inputs ([#824], [#829]). ### Fixed @@ -41,6 +41,7 @@ All notable changes to this project will be documented in this file. [#819]: https://github.com/stackabletech/hdfs-operator/pull/819 [#821]: https://github.com/stackabletech/hdfs-operator/pull/821 [#824]: https://github.com/stackabletech/hdfs-operator/pull/824 +[#829]: https://github.com/stackabletech/hdfs-operator/pull/824 ## [26.7.0] - 2026-07-21 From 29bfdd2d0940a73962a5c88548b5888509f0d368 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:30:09 +0200 Subject: [PATCH 3/4] Update CHANGELOG.md Co-authored-by: Siegfried Weber --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08f4c974..43228e48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ All notable changes to this project will be documented in this file. [#819]: https://github.com/stackabletech/hdfs-operator/pull/819 [#821]: https://github.com/stackabletech/hdfs-operator/pull/821 [#824]: https://github.com/stackabletech/hdfs-operator/pull/824 -[#829]: https://github.com/stackabletech/hdfs-operator/pull/824 +[#829]: https://github.com/stackabletech/hdfs-operator/pull/829 ## [26.7.0] - 2026-07-21 From 9c9ae3b8a6a922cdc84d3907443da14eb4b73c17 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 11 Sep 2026 15:30:28 +0200 Subject: [PATCH 4/4] changes following review --- .../src/controller/build/container.rs | 328 +++++++++--------- 1 file changed, 157 insertions(+), 171 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 2c8286cd..c3a5e298 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -175,8 +175,6 @@ pub enum ContainerConfig { Hdfs { /// HDFS role (name-, data-, journal-node) which determines the container name. role: HdfsNodeRole, - /// Volume mounts for config and logging. - volume_mounts: ContainerVolumeDirs, /// Port name of the IPC/RPC port, used for the readiness probe. ipc_port_name: &'static str, /// Port name of the web UI HTTP port, used for the liveness probe. @@ -186,22 +184,14 @@ pub enum ContainerConfig { /// The JMX Exporter metrics port. metrics_port: Port, }, - Zkfc { - /// Volume mounts for config and logging. - volume_mounts: ContainerVolumeDirs, - }, - FormatNameNodes { - /// Volume mounts for config and logging. - volume_mounts: ContainerVolumeDirs, - }, - FormatZooKeeper { - /// Volume mounts for config and logging. - volume_mounts: ContainerVolumeDirs, - }, - WaitForNameNodes { - /// Volume mounts for config and logging. - volume_mounts: ContainerVolumeDirs, - }, + /// The ZooKeeper fail-over controller side container of the namenodes. + Zkfc, + /// The init container formatting the namenodes. + FormatNameNodes, + /// The init container formatting ZooKeeper for the namenodes. + FormatZooKeeper, + /// The init container of the datanodes waiting for the namenodes. + WaitForNameNodes, } impl ContainerConfig { @@ -344,7 +334,7 @@ impl ContainerConfig { match role { HdfsNodeRole::Name => { // Zookeeper fail over container - let zkfc_container_config = Self::zkfc(); + let zkfc_container_config = Self::Zkfc; pb.add_volumes(zkfc_container_config.volumes( merged_config, &object_name, @@ -360,7 +350,7 @@ impl ContainerConfig { )?); // Format namenode init container - let format_namenodes_container_config = Self::format_namenodes(); + let format_namenodes_container_config = Self::FormatNameNodes; pb.add_volumes(format_namenodes_container_config.volumes( merged_config, &object_name, @@ -377,7 +367,7 @@ impl ContainerConfig { )?); // Format ZooKeeper init container - let format_zookeeper_container_config = Self::format_zookeeper(); + let format_zookeeper_container_config = Self::FormatZooKeeper; pb.add_volumes(format_zookeeper_container_config.volumes( merged_config, &object_name, @@ -395,7 +385,7 @@ impl ContainerConfig { } HdfsNodeRole::Data => { // Wait for namenode init container - let wait_for_namenodes_container_config = Self::wait_for_namenodes(); + let wait_for_namenodes_container_config = Self::WaitForNameNodes; pb.add_volumes(wait_for_namenodes_container_config.volumes( merged_config, &object_name, @@ -476,7 +466,7 @@ impl ContainerConfig { labels: &Labels, ) -> Result { let merged_config = &rolegroup_config.config; - let mut cb = new_container_builder(self.name()); + let mut cb = new_container_builder(self.container_name()); let resources = self.resources(merged_config); @@ -525,7 +515,7 @@ impl ContainerConfig { labels: &Labels, ) -> Result { let merged_config = &rolegroup_config.config; - let mut cb = new_container_builder(self.name()); + let mut cb = new_container_builder(self.container_name()); cb.image_from_product_image(&cluster.image) .command(Self::command()) @@ -545,28 +535,49 @@ impl ContainerConfig { } /// Return the typed container name. - fn name(&self) -> &'static ContainerName { + fn container_name(&self) -> &'static ContainerName { match self { ContainerConfig::Hdfs { role, .. } => match role { HdfsNodeRole::Name => &NAMENODE_CONTAINER_NAME, HdfsNodeRole::Data => &DATANODE_CONTAINER_NAME, HdfsNodeRole::Journal => &JOURNALNODE_CONTAINER_NAME, }, - ContainerConfig::Zkfc { .. } => &ZKFC_CONTAINER_NAME, - ContainerConfig::FormatNameNodes { .. } => &FORMAT_NAMENODES_CONTAINER_NAME, - ContainerConfig::FormatZooKeeper { .. } => &FORMAT_ZOOKEEPER_CONTAINER_NAME, - ContainerConfig::WaitForNameNodes { .. } => &WAIT_FOR_NAMENODES_CONTAINER_NAME, + ContainerConfig::Zkfc => &ZKFC_CONTAINER_NAME, + ContainerConfig::FormatNameNodes => &FORMAT_NAMENODES_CONTAINER_NAME, + ContainerConfig::FormatZooKeeper => &FORMAT_ZOOKEEPER_CONTAINER_NAME, + ContainerConfig::WaitForNameNodes => &WAIT_FOR_NAMENODES_CONTAINER_NAME, } } /// Return volume mount directories depending on the container. - fn volume_mount_dirs(&self) -> &ContainerVolumeDirs { - match &self { - ContainerConfig::Hdfs { volume_mounts, .. } => volume_mounts, - ContainerConfig::Zkfc { volume_mounts, .. } => volume_mounts, - ContainerConfig::FormatNameNodes { volume_mounts, .. } => volume_mounts, - ContainerConfig::FormatZooKeeper { volume_mounts, .. } => volume_mounts, - ContainerConfig::WaitForNameNodes { volume_mounts, .. } => volume_mounts, + fn volume_mount_dirs(&self) -> ContainerVolumeDirs { + let container_name = self.container_name().as_ref(); + match self { + ContainerConfig::Hdfs { .. } => ContainerVolumeDirs::for_container( + container_name, + Self::HDFS_CONFIG_VOLUME_MOUNT_NAME, + Self::HDFS_LOG_VOLUME_MOUNT_NAME, + ), + ContainerConfig::Zkfc => ContainerVolumeDirs::for_container( + container_name, + Self::ZKFC_CONFIG_VOLUME_MOUNT_NAME, + Self::ZKFC_LOG_VOLUME_MOUNT_NAME, + ), + ContainerConfig::FormatNameNodes => ContainerVolumeDirs::for_container( + container_name, + Self::FORMAT_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, + Self::FORMAT_NAMENODES_LOG_VOLUME_MOUNT_NAME, + ), + ContainerConfig::FormatZooKeeper => ContainerVolumeDirs::for_container( + container_name, + Self::FORMAT_ZOOKEEPER_CONFIG_VOLUME_MOUNT_NAME, + Self::FORMAT_ZOOKEEPER_LOG_VOLUME_MOUNT_NAME, + ), + ContainerConfig::WaitForNameNodes => ContainerVolumeDirs::for_container( + container_name, + Self::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, + Self::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME, + ), } } @@ -638,7 +649,7 @@ impl ContainerConfig { create_vector_shutdown_file_command(STACKABLE_LOG_DIR), )); } - ContainerConfig::Zkfc { .. } => { + ContainerConfig::Zkfc => { if let Some(container_config) = merged_config .as_namenode() .map(|node| node.logging.for_container(&NameNodeContainer::Zkfc)) @@ -652,8 +663,8 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME )); } - ContainerConfig::FormatNameNodes { .. } => { - args.push_str(&bash_capture_shell_helper(self.name().as_ref())); + ContainerConfig::FormatNameNodes => { + args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); if let Some(container_config) = merged_config.as_namenode().map(|node| { node.logging @@ -728,8 +739,8 @@ impl ContainerConfig { .join(" "), )); } - ContainerConfig::FormatZooKeeper { .. } => { - args.push_str(&bash_capture_shell_helper(self.name().as_ref())); + ContainerConfig::FormatZooKeeper => { + args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); if let Some(container_config) = merged_config.as_namenode().map(|node| { node.logging @@ -760,8 +771,8 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME, )); } - ContainerConfig::WaitForNameNodes { .. } => { - args.push_str(&bash_capture_shell_helper(self.name().as_ref())); + ContainerConfig::WaitForNameNodes => { + args.push_str(&bash_capture_shell_helper(self.container_name().as_ref())); if let Some(container_config) = merged_config.as_datanode().map(|node| { node.logging @@ -966,7 +977,7 @@ impl ContainerConfig { pub fn resources(&self, merged_config: &AnyNodeConfig) -> Option { match self { // Namenode sidecar containers - ContainerConfig::Zkfc { .. } => Some( + ContainerConfig::Zkfc => Some( ResourceRequirementsBuilder::new() .with_cpu_request("100m") .with_cpu_limit("400m") @@ -976,9 +987,9 @@ impl ContainerConfig { ), // Main container and init containers ContainerConfig::Hdfs { .. } - | ContainerConfig::FormatNameNodes { .. } - | ContainerConfig::FormatZooKeeper { .. } - | ContainerConfig::WaitForNameNodes { .. } => match merged_config { + | ContainerConfig::FormatNameNodes + | ContainerConfig::FormatZooKeeper + | ContainerConfig::WaitForNameNodes => match merged_config { AnyNodeConfig::Name(node) => Some(node.resources.clone().into()), AnyNodeConfig::Data(node) => Some(node.resources.clone().into()), AnyNodeConfig::Journal(node) => Some(node.resources.clone().into()), @@ -1090,27 +1101,28 @@ impl ContainerConfig { let container_log_config = match self { ContainerConfig::Hdfs { .. } => Some(merged_config.hdfs_logging()), - ContainerConfig::Zkfc { .. } => merged_config + ContainerConfig::Zkfc => merged_config .as_namenode() .map(|node| node.logging.for_container(&NameNodeContainer::Zkfc)), - ContainerConfig::FormatNameNodes { .. } => merged_config.as_namenode().map(|node| { + ContainerConfig::FormatNameNodes => merged_config.as_namenode().map(|node| { node.logging .for_container(&NameNodeContainer::FormatNameNodes) }), - ContainerConfig::FormatZooKeeper { .. } => merged_config.as_namenode().map(|node| { + ContainerConfig::FormatZooKeeper => merged_config.as_namenode().map(|node| { node.logging .for_container(&NameNodeContainer::FormatZooKeeper) }), - ContainerConfig::WaitForNameNodes { .. } => merged_config.as_datanode().map(|node| { + ContainerConfig::WaitForNameNodes => merged_config.as_datanode().map(|node| { node.logging .for_container(&DataNodeContainer::WaitForNameNodes) }), }; + let volume_mount_dirs = self.volume_mount_dirs(); volumes.extend(Self::common_container_volumes( container_log_config.as_deref(), object_name, - self.volume_mount_dirs().config_mount_name(), - self.volume_mount_dirs().log_mount_name(), + volume_mount_dirs.config_mount_name(), + volume_mount_dirs.log_mount_name(), )); Ok(volumes) @@ -1123,17 +1135,18 @@ impl ContainerConfig { merged_config: &AnyNodeConfig, labels: &Labels, ) -> Result> { + let volume_mount_dirs = self.volume_mount_dirs(); let mut volume_mounts = vec![ VolumeMountBuilder::new(Self::STACKABLE_LOG_VOLUME_MOUNT_NAME, STACKABLE_LOG_DIR) .build(), VolumeMountBuilder::new( - self.volume_mount_dirs().config_mount_name(), - self.volume_mount_dirs().config_mount(), + volume_mount_dirs.config_mount_name(), + volume_mount_dirs.config_mount(), ) .build(), VolumeMountBuilder::new( - self.volume_mount_dirs().log_mount_name(), - self.volume_mount_dirs().log_mount(), + volume_mount_dirs.log_mount_name(), + volume_mount_dirs.log_mount(), ) .build(), ]; @@ -1151,7 +1164,7 @@ impl ContainerConfig { } match self { - ContainerConfig::FormatNameNodes { .. } => { + ContainerConfig::FormatNameNodes => { // As FormatNameNodes only runs on the Namenodes we can safely assume the only pvc is called "data". volume_mounts.push( VolumeMountBuilder::new(Self::DATA_VOLUME_MOUNT_NAME, STACKABLE_ROOT_DATA_DIR) @@ -1191,9 +1204,9 @@ impl ContainerConfig { } } // The other containers don't need any data pvcs to be mounted - ContainerConfig::Zkfc { .. } - | ContainerConfig::WaitForNameNodes { .. } - | ContainerConfig::FormatZooKeeper { .. } => {} + ContainerConfig::Zkfc + | ContainerConfig::WaitForNameNodes + | ContainerConfig::FormatZooKeeper => {} } Ok(volume_mounts) @@ -1209,10 +1222,11 @@ impl ContainerConfig { /// Copy all the configuration files to the respective container config dir. fn copy_config_xml_cmd(&self) -> String { + let volume_mount_dirs = self.volume_mount_dirs(); format!( "cp {config_dir_mount}/*.xml {config_dir_name}\n", - config_dir_mount = self.volume_mount_dirs().config_mount(), - config_dir_name = self.volume_mount_dirs().final_config() + config_dir_mount = volume_mount_dirs.config_mount(), + config_dir_name = volume_mount_dirs.final_config() ) } @@ -1225,20 +1239,21 @@ impl ContainerConfig { log4j_config_file: &str, container_log_config: &ContainerLogConfig, ) -> String { + let volume_mount_dirs = self.volume_mount_dirs(); let source_log4j_properties_dir = if let ContainerLogConfig { choice: Some(ContainerLogConfigChoice::Custom(_)), } = container_log_config { - self.volume_mount_dirs().log_mount() + volume_mount_dirs.log_mount() } else { - self.volume_mount_dirs().config_mount() + volume_mount_dirs.config_mount() }; format!( "cp {log4j_properties_dir}/{file_name} {config_dir}/{LOG4J_PROPERTIES}\n", log4j_properties_dir = source_log4j_properties_dir, file_name = log4j_config_file, - config_dir = self.volume_mount_dirs().final_config() + config_dir = volume_mount_dirs.final_config() ) } @@ -1253,8 +1268,8 @@ impl ContainerConfig { ContainerConfig::Hdfs { role, metrics_port, .. } => { - let cvd = ContainerVolumeDirs::from(role); - let config_dir = cvd.final_config(); + let volume_mount_dirs = self.volume_mount_dirs(); + let config_dir = volume_mount_dirs.final_config(); construct_role_specific_jvm_args( role, &rolegroup_config @@ -1385,7 +1400,6 @@ impl From for ContainerConfig { match role { HdfsNodeRole::Name => Self::Hdfs { role, - volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, @@ -1393,7 +1407,6 @@ impl From for ContainerConfig { }, HdfsNodeRole::Data => Self::Hdfs { role, - volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_IPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, @@ -1401,7 +1414,6 @@ impl From for ContainerConfig { }, HdfsNodeRole::Journal => Self::Hdfs { role, - volume_mounts: ContainerVolumeDirs::from(role), ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, @@ -1411,52 +1423,6 @@ impl From for ContainerConfig { } } -impl ContainerConfig { - /// The ZooKeeper fail-over controller side container of the namenodes. - fn zkfc() -> Self { - Self::Zkfc { - volume_mounts: ContainerVolumeDirs::for_container( - ZKFC_CONTAINER_NAME.as_ref(), - Self::ZKFC_CONFIG_VOLUME_MOUNT_NAME, - Self::ZKFC_LOG_VOLUME_MOUNT_NAME, - ), - } - } - - /// The init container formatting the namenodes. - fn format_namenodes() -> Self { - Self::FormatNameNodes { - volume_mounts: ContainerVolumeDirs::for_container( - FORMAT_NAMENODES_CONTAINER_NAME.as_ref(), - Self::FORMAT_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, - Self::FORMAT_NAMENODES_LOG_VOLUME_MOUNT_NAME, - ), - } - } - - /// The init container formatting ZooKeeper for the namenodes. - fn format_zookeeper() -> Self { - Self::FormatZooKeeper { - volume_mounts: ContainerVolumeDirs::for_container( - FORMAT_ZOOKEEPER_CONTAINER_NAME.as_ref(), - Self::FORMAT_ZOOKEEPER_CONFIG_VOLUME_MOUNT_NAME, - Self::FORMAT_ZOOKEEPER_LOG_VOLUME_MOUNT_NAME, - ), - } - } - - /// The init container of the datanodes waiting for the namenodes. - fn wait_for_namenodes() -> Self { - Self::WaitForNameNodes { - volume_mounts: ContainerVolumeDirs::for_container( - WAIT_FOR_NAMENODES_CONTAINER_NAME.as_ref(), - Self::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, - Self::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME, - ), - } - } -} - /// Helper struct to collect required config and logging dirs. pub struct ContainerVolumeDirs { /// The final config dir where to store core-site.xml, hdfs-size.xml and logging configs. @@ -1497,56 +1463,8 @@ impl ContainerVolumeDirs { } } -impl From for ContainerVolumeDirs { - fn from(role: HdfsNodeRole) -> Self { - ContainerVolumeDirs { - final_config_dir: format!( - "{base}/{role}", - base = Self::NODE_BASE_CONFIG_DIR, - role = role.as_ref() - ), - config_mount: format!( - "{base}/{role}", - base = Self::NODE_BASE_CONFIG_DIR_MOUNT, - role = role.as_ref() - ), - config_mount_name: ContainerConfig::HDFS_CONFIG_VOLUME_MOUNT_NAME.to_string(), - log_mount: format!( - "{base}/{role}", - base = Self::NODE_BASE_LOG_DIR_MOUNT, - role = role.as_ref() - ), - log_mount_name: ContainerConfig::HDFS_LOG_VOLUME_MOUNT_NAME.to_string(), - } - } -} - -impl From<&HdfsNodeRole> for ContainerVolumeDirs { - fn from(role: &HdfsNodeRole) -> Self { - ContainerVolumeDirs { - final_config_dir: format!( - "{base}/{role}", - base = Self::NODE_BASE_CONFIG_DIR, - role = role.as_ref() - ), - config_mount: format!( - "{base}/{role}", - base = Self::NODE_BASE_CONFIG_DIR_MOUNT, - role = role.as_ref() - ), - config_mount_name: ContainerConfig::HDFS_CONFIG_VOLUME_MOUNT_NAME.to_string(), - log_mount: format!( - "{base}/{role}", - base = Self::NODE_BASE_LOG_DIR_MOUNT, - role = role.as_ref() - ), - log_mount_name: ContainerConfig::HDFS_LOG_VOLUME_MOUNT_NAME.to_string(), - } - } -} - impl ContainerVolumeDirs { - /// The volume dirs of a side or init container with the given fixed name and mount names. + /// The volume dirs of the container with the given name and config/log volume mount names. fn for_container(container_name: &str, config_mount_name: &str, log_mount_name: &str) -> Self { ContainerVolumeDirs { final_config_dir: format!("{base}/{container_name}", base = Self::NODE_BASE_CONFIG_DIR), @@ -1629,7 +1547,7 @@ mod tests { fn main_container_names_match_role_names() { for role in HdfsNodeRole::iter() { assert_eq!( - ContainerConfig::from(role).name().to_string(), + ContainerConfig::from(role).container_name().to_string(), role.to_string() ); } @@ -1641,20 +1559,88 @@ mod tests { #[test] fn side_container_names_match_display() { assert_eq!( - ContainerConfig::zkfc().name().to_string(), + ContainerConfig::Zkfc.container_name().to_string(), NameNodeContainer::Zkfc.to_string() ); assert_eq!( - ContainerConfig::format_namenodes().name().to_string(), + ContainerConfig::FormatNameNodes + .container_name() + .to_string(), NameNodeContainer::FormatNameNodes.to_string() ); assert_eq!( - ContainerConfig::format_zookeeper().name().to_string(), + ContainerConfig::FormatZooKeeper + .container_name() + .to_string(), NameNodeContainer::FormatZooKeeper.to_string() ); assert_eq!( - ContainerConfig::wait_for_namenodes().name().to_string(), + ContainerConfig::WaitForNameNodes + .container_name() + .to_string(), DataNodeContainer::WaitForNameNodes.to_string() ); } + + /// The volume dirs of every container are derived from its container name and its + /// config/log volume mount names. + #[test] + fn volume_mount_dirs_follow_container_name() { + let cases: [(ContainerConfig, &str, &str, &str); 7] = [ + ( + ContainerConfig::from(HdfsNodeRole::Name), + "namenode", + ContainerConfig::HDFS_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::HDFS_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::from(HdfsNodeRole::Data), + "datanode", + ContainerConfig::HDFS_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::HDFS_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::from(HdfsNodeRole::Journal), + "journalnode", + ContainerConfig::HDFS_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::HDFS_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::Zkfc, + "zkfc", + ContainerConfig::ZKFC_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::ZKFC_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::FormatNameNodes, + "format-namenodes", + ContainerConfig::FORMAT_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::FORMAT_NAMENODES_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::FormatZooKeeper, + "format-zookeeper", + ContainerConfig::FORMAT_ZOOKEEPER_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::FORMAT_ZOOKEEPER_LOG_VOLUME_MOUNT_NAME, + ), + ( + ContainerConfig::WaitForNameNodes, + "wait-for-namenodes", + ContainerConfig::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, + ContainerConfig::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME, + ), + ]; + + for (config, name, config_mount_name, log_mount_name) in cases { + let dirs = config.volume_mount_dirs(); + assert_eq!(dirs.final_config(), format!("/stackable/config/{name}")); + assert_eq!( + dirs.config_mount(), + format!("/stackable/mount/config/{name}") + ); + assert_eq!(dirs.config_mount_name(), config_mount_name); + assert_eq!(dirs.log_mount(), format!("/stackable/mount/log/{name}")); + assert_eq!(dirs.log_mount_name(), log_mount_name); + } + } }