From 4cb5112c0b8dc076ba4d912b9bd8d5f893f19527 Mon Sep 17 00:00:00 2001 From: Maxi Wittich Date: Wed, 11 Nov 2026 13:00:03 +0100 Subject: [PATCH 1/4] feat: Make containerdebug configurable Adds the containerdebug config (enabled, interval) from stackable-operator 0.122.0 to the config common to all roles and starts containerdebug in the main container from the merged role group config. HDFS has no validated role group config layer, so the containerdebug config is validated while building each role group, which still fails the reconciliation before anything is applied. --- extra/crds.yaml | 120 +++++++++++++++ .../src/controller/build/container.rs | 6 +- .../src/controller/build/mod.rs | 141 +++++++++++++++++- .../controller/build/role_group/datanode.rs | 15 +- .../build/role_group/journalnode.rs | 15 +- .../src/controller/build/role_group/mod.rs | 11 ++ .../controller/build/role_group/namenode.rs | 15 +- rust/operator-binary/src/crd/mod.rs | 16 +- 8 files changed, 327 insertions(+), 12 deletions(-) diff --git a/extra/crds.yaml b/extra/crds.yaml index e497a204..539113b8 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -223,6 +223,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true @@ -790,6 +810,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true @@ -1451,6 +1491,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true @@ -1918,6 +1978,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true @@ -2380,6 +2460,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true @@ -3096,6 +3196,26 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: object + containerdebug: + default: + enabled: null + interval: null + description: |- + Configures the `containerdebug` tool, which periodically logs information about the container + environment (such as network interfaces, DNS lookups and disk usage) in the background of the + product container. + properties: + enabled: + description: Whether `containerdebug` runs in the background of the product container. Defaults to `true`. + nullable: true + type: boolean + interval: + description: |- + Time between two runs of `containerdebug`, e.g. `30m`, `1h` or `2d`. Must not be zero. + Defaults to `30m`. + nullable: true + type: string + type: object gracefulShutdownTimeout: description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. nullable: true diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 8777ec72..6d91dd48 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -574,7 +574,7 @@ impl ContainerConfig { {remove_vector_shutdown_file_command} prepare_signal_handlers mkdir --parents "${file_log_directory}" || true - containerdebug --output={STACKABLE_LOG_DIR}/containerdebug-state.json --loop & + {containerdebug_command} if [[ -d {LISTENER_VOLUME_DIR} ]]; then export POD_ADDRESS=$(cat {LISTENER_VOLUME_DIR}/default-address/address) for i in {LISTENER_VOLUME_DIR}/default-address/ports/*; do @@ -588,6 +588,10 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME, role = role.as_ref(), file_log_directory = ENV_FILE_LOG_DIRECTORY, + containerdebug_command = inputs + .containerdebug + .background_command(STACKABLE_LOG_DIR) + .unwrap_or_default(), remove_vector_shutdown_file_command = remove_vector_shutdown_file_command(STACKABLE_LOG_DIR), create_vector_shutdown_file_command = diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 3b9e7e3e..deda493d 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -493,12 +493,18 @@ pub(crate) fn role_group_selector( #[cfg(test)] mod tests { - use stackable_operator::{k8s_openapi::api::core::v1::ConfigMap, kube::Resource}; + use stackable_operator::{ + crd::containerdebug::ValidatedContainerdebugConfig, + k8s_openapi::api::{apps::v1::StatefulSet, core::v1::ConfigMap}, + kube::Resource, + shared::time::Duration, + v2::product_logging::framework::STACKABLE_LOG_DIR, + }; use super::build; use crate::{ controller::build::properties::test_support::{cluster_info, validated_cluster}, - test_support::namenode_listener, + test_support::{deserialize_and_validate_cluster, namenode_listener}, }; /// The sorted `metadata.name`s of a resource collection. @@ -781,4 +787,135 @@ mod tests { ); } } + + /// The `containerdebug` line of the main container's args in the `default` role group of + /// `role`, if any. The main container is named after its role. + fn containerdebug_line(stateful_sets: &[StatefulSet], role: &str) -> Option { + let name = format!("hdfs-{role}-default"); + stateful_sets + .iter() + .find(|stateful_set| stateful_set.meta().name.as_deref() == Some(name.as_str())) + .and_then(|stateful_set| stateful_set.spec.as_ref()) + .and_then(|spec| spec.template.spec.as_ref()) + .expect("the StatefulSet has a pod spec") + .containers + .iter() + .find(|container| container.name == role) + .and_then(|container| container.args.as_ref()) + .expect("the main container has args") + .join("\n") + .lines() + .map(str::trim) + .find(|line| line.starts_with("containerdebug ")) + .map(str::to_owned) + } + + /// Every role starts `containerdebug` in its main container from its own merged config. The + /// command itself is owned (and tested) by operator-rs, so only the wiring is checked here. + #[test] + fn containerdebug_is_started_per_role_from_the_merged_config() { + let cluster = deserialize_and_validate_cluster( + r#" +--- +apiVersion: hdfs.stackable.tech/v1alpha1 +kind: HdfsCluster +metadata: + name: hdfs + namespace: default + uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f +spec: + image: + productVersion: 3.4.0 + clusterConfig: + zookeeperConfigMapName: hdfs-zk + nameNodes: + config: + containerdebug: + interval: 10m + roleGroups: + default: + replicas: 1 + journalNodes: + roleGroups: + default: + replicas: 1 + dataNodes: + config: + containerdebug: + interval: 10m + roleGroups: + default: + replicas: 1 + config: + containerdebug: + enabled: false +"#, + ); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); + let enabled = |interval| { + ValidatedContainerdebugConfig::Enabled { interval } + .background_command(STACKABLE_LOG_DIR) + }; + + assert_eq!( + containerdebug_line(&resources.stateful_sets, "namenode"), + enabled(Duration::from_minutes_unchecked(10)) + ); + assert_eq!( + containerdebug_line(&resources.stateful_sets, "journalnode"), + enabled(Duration::from_minutes_unchecked(30)) + ); + assert_eq!( + containerdebug_line(&resources.stateful_sets, "datanode"), + None + ); + } + + /// HDFS validates the `containerdebug` config while building the role group, so an invalid + /// one fails the build (and with it the reconciliation, before anything is applied). + #[test] + fn zero_containerdebug_interval_fails_the_build() { + let cluster = deserialize_and_validate_cluster( + r#" +--- +apiVersion: hdfs.stackable.tech/v1alpha1 +kind: HdfsCluster +metadata: + name: hdfs + namespace: default + uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f +spec: + image: + productVersion: 3.4.0 + clusterConfig: + zookeeperConfigMapName: hdfs-zk + nameNodes: + roleGroups: + default: + replicas: 1 + journalNodes: + config: + containerdebug: + interval: 0s + roleGroups: + default: + replicas: 1 + dataNodes: + roleGroups: + default: + replicas: 1 +"#, + ); + + let Err(error) = build(&cluster, &cluster_info()) else { + panic!("the build should fail"); + }; + + assert!( + snafu::Report::from_error(&error) + .to_string() + .contains("invalid containerdebug config for role journalnode role group default"), + "{error:?}" + ); + } } diff --git a/rust/operator-binary/src/controller/build/role_group/datanode.rs b/rust/operator-binary/src/controller/build/role_group/datanode.rs index 2806eb03..34722092 100644 --- a/rust/operator-binary/src/controller/build/role_group/datanode.rs +++ b/rust/operator-binary/src/controller/build/role_group/datanode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, ExtraContainer, ListenerVolumeSnafu, RoleGroupBuilder, RoleGroupInputs, - RoleGroupSelectorLabelsSnafu, common_container_logging, + Error, ExtraContainer, InvalidContainerdebugConfigSnafu, ListenerVolumeSnafu, RoleGroupBuilder, + RoleGroupInputs, RoleGroupSelectorLabelsSnafu, common_container_logging, }; use crate::{ controller::{ @@ -54,6 +54,16 @@ pub(crate) fn build<'a>( DataNodeContainer::Vector, ); + let containerdebug = + config + .common + .containerdebug + .validate() + .context(InvalidContainerdebugConfigSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + let inputs = RoleGroupInputs { cluster, cluster_info, @@ -61,6 +71,7 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), + containerdebug, resources: config.resources.clone().into(), volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), // First, because the pod's `volumes` are an ordered list: moving the listener volume diff --git a/rust/operator-binary/src/controller/build/role_group/journalnode.rs b/rust/operator-binary/src/controller/build/role_group/journalnode.rs index 1739d320..ff31601d 100644 --- a/rust/operator-binary/src/controller/build/role_group/journalnode.rs +++ b/rust/operator-binary/src/controller/build/role_group/journalnode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, RoleGroupBuilder, RoleGroupInputs, RoleGroupSelectorLabelsSnafu, - common_container_logging, + Error, InvalidContainerdebugConfigSnafu, RoleGroupBuilder, RoleGroupInputs, + RoleGroupSelectorLabelsSnafu, common_container_logging, }; use crate::{ controller::{ @@ -44,6 +44,16 @@ pub(crate) fn build<'a>( JournalNodeContainer::Vector, ); + let containerdebug = + config + .common + .containerdebug + .validate() + .context(InvalidContainerdebugConfigSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + let inputs = RoleGroupInputs { cluster, cluster_info, @@ -51,6 +61,7 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), + containerdebug, resources: config.resources.clone().into(), volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), extra_pod_volumes: Vec::new(), diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index 3c75fed7..a0ca77a8 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -20,6 +20,7 @@ pub(crate) use journalnode::build as build_journalnode_role_group; pub(crate) use namenode::build as build_namenode_role_group; use snafu::{ResultExt, Snafu}; use stackable_operator::{ + crd::containerdebug::{self, ValidatedContainerdebugConfig}, k8s_openapi::api::{ apps::v1::StatefulSet, core::v1::{ @@ -96,6 +97,13 @@ pub enum Error { role: HdfsNodeRole, role_group: RoleGroupName, }, + + #[snafu(display("invalid containerdebug config for role {role} role group {role_group}", role = role.as_ref()))] + InvalidContainerdebugConfig { + source: containerdebug::Error, + role: HdfsNodeRole, + role_group: RoleGroupName, + }, } /// A container only one role runs, named by that role's module together with the log config that @@ -145,6 +153,9 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) selector_labels: Labels, /// The role group's merged config that is common to every role. pub(crate) common: CommonNodeConfig, + /// The role group's validated `containerdebug` config, which the main `hdfs` container starts + /// `containerdebug` from. + pub(crate) containerdebug: ValidatedContainerdebugConfig, /// The resource requirements of the role group's main and init containers; the ZKFC sidecar /// has fixed requirements of its own and ignores this. pub(crate) resources: ResourceRequirements, diff --git a/rust/operator-binary/src/controller/build/role_group/namenode.rs b/rust/operator-binary/src/controller/build/role_group/namenode.rs index cb61b2f7..a19bc2ec 100644 --- a/rust/operator-binary/src/controller/build/role_group/namenode.rs +++ b/rust/operator-binary/src/controller/build/role_group/namenode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, ExtraContainer, RoleGroupBuilder, RoleGroupInputs, RoleGroupSelectorLabelsSnafu, - VolumeClaimTemplatesSnafu, common_container_logging, + Error, ExtraContainer, InvalidContainerdebugConfigSnafu, RoleGroupBuilder, RoleGroupInputs, + RoleGroupSelectorLabelsSnafu, VolumeClaimTemplatesSnafu, common_container_logging, }; use crate::{ controller::{ @@ -57,6 +57,16 @@ pub(crate) fn build<'a>( NameNodeContainer::Vector, ); + let containerdebug = + config + .common + .containerdebug + .validate() + .context(InvalidContainerdebugConfigSnafu { + role: ROLE, + role_group: role_group_name.clone(), + })?; + let inputs = RoleGroupInputs { cluster, cluster_info, @@ -64,6 +74,7 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), + containerdebug, resources: config.resources.clone().into(), volume_claim_templates, extra_pod_volumes: Vec::new(), diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 9fcae60f..0ec16b5d 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -22,7 +22,10 @@ use stackable_operator::{ }, config::{fragment::Fragment, merge::Merge}, constant, - crd::listener, + crd::{ + containerdebug::{default_containerdebug_config, v1alpha1::ContainerdebugConfig}, + listener, + }, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::api::resource::Quantity, kube::CustomResource, @@ -328,7 +331,7 @@ pub enum DeprecatedClusterListenerClass { } /// Configuration options that are available for all roles. -#[derive(Clone, Debug, Default, Fragment, JsonSchema, PartialEq)] +#[derive(Clone, Debug, Fragment, JsonSchema, PartialEq)] #[fragment_attrs( derive( Clone, @@ -345,6 +348,10 @@ pub enum DeprecatedClusterListenerClass { pub struct CommonNodeConfig { #[fragment_attrs(serde(default))] pub affinity: StackableAffinity, + + #[fragment_attrs(serde(default))] + pub containerdebug: ContainerdebugConfig, + /// Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. #[fragment_attrs(serde(default))] pub graceful_shutdown_timeout: Option, @@ -668,6 +675,7 @@ impl NameNodeConfigFragment { listener_class: Some(DEFAULT_LISTENER_CLASS.clone()), common: CommonNodeConfigFragment { affinity: get_affinity(cluster_name, role, opa_config), + containerdebug: default_containerdebug_config(), graceful_shutdown_timeout: Some(DEFAULT_NAME_NODE_GRACEFUL_SHUTDOWN_TIMEOUT), requested_secret_lifetime: Some(Self::DEFAULT_NAME_NODE_SECRET_LIFETIME), }, @@ -758,6 +766,7 @@ impl DataNodeConfigFragment { // Only the NameNode is configured with the OPA authorizer, so this role gets no // affinity to the OPA Pods. affinity: get_affinity(cluster_name, role, None), + containerdebug: default_containerdebug_config(), graceful_shutdown_timeout: Some(DEFAULT_DATA_NODE_GRACEFUL_SHUTDOWN_TIMEOUT), requested_secret_lifetime: Some(Self::DEFAULT_DATA_NODE_SECRET_LIFETIME), }, @@ -786,7 +795,7 @@ pub enum JournalNodeContainer { Vector, } -#[derive(Clone, Debug, Default, Fragment, JsonSchema, PartialEq)] +#[derive(Clone, Debug, Fragment, JsonSchema, PartialEq)] #[fragment_attrs( derive( Clone, @@ -836,6 +845,7 @@ impl JournalNodeConfigFragment { // Only the NameNode is configured with the OPA authorizer, so this role gets no // affinity to the OPA Pods. affinity: get_affinity(cluster_name, role, None), + containerdebug: default_containerdebug_config(), graceful_shutdown_timeout: Some(DEFAULT_JOURNAL_NODE_GRACEFUL_SHUTDOWN_TIMEOUT), requested_secret_lifetime: Some(Self::DEFAULT_JOURNAL_NODE_SECRET_LIFETIME), }, From eaa3b2d86f80c0cc117f34d47ea95a94ee3917ad Mon Sep 17 00:00:00 2001 From: Maxi Wittich Date: Fri, 13 Nov 2026 09:35:48 +0100 Subject: [PATCH 2/4] Add containerdebug config and proper validation --- .../src/controller/build/container.rs | 19 +- .../src/controller/build/graceful_shutdown.rs | 4 +- .../src/controller/build/mod.rs | 48 ---- .../controller/build/role_group/datanode.rs | 15 +- .../build/role_group/journalnode.rs | 15 +- .../src/controller/build/role_group/mod.rs | 22 +- .../controller/build/role_group/namenode.rs | 15 +- rust/operator-binary/src/controller/mod.rs | 69 ++++- .../src/controller/validate.rs | 256 +++++++++++++++--- rust/operator-binary/src/crd/mod.rs | 4 +- rust/operator-binary/src/test_support.rs | 35 ++- 11 files changed, 337 insertions(+), 165 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 6d91dd48..11d672cd 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -60,7 +60,8 @@ use strum::{Display, EnumDiscriminants, IntoStaticStr}; use crate::{ controller::{ - ValidatedCluster, + ValidatedCluster, ValidatedDataNodeConfig, ValidatedJournalNodeConfig, + ValidatedNameNodeConfig, build::{ self, jvm::{self, construct_global_jvm_args, construct_role_specific_jvm_args}, @@ -74,7 +75,7 @@ use crate::{ }, }, crd::{ - DataNodeConfig, HdfsNodeRole, HdfsPodRef, JournalNodeConfig, NameNodeConfig, UpgradeState, + HdfsNodeRole, HdfsPodRef, UpgradeState, constants::{ DATANODE_ROOT_DATA_DIR_PREFIX, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, LIVENESS_PROBE_FAILURE_THRESHOLD, LIVENESS_PROBE_INITIAL_DELAY_SECONDS, @@ -337,7 +338,7 @@ impl ContainerConfig { /// The PVC templates for a namenode role group: one data PVC plus the listener PVC. pub fn namenode_volume_claim_templates( - config: &NameNodeConfig, + config: &ValidatedNameNodeConfig, labels: &Labels, ) -> Result> { let listener = ListenerOperatorVolumeSourceBuilder::new( @@ -369,7 +370,7 @@ impl ContainerConfig { /// The PVC template for a journalnode role group: one data PVC. pub fn journalnode_volume_claim_templates( - config: &JournalNodeConfig, + config: &ValidatedJournalNodeConfig, ) -> Vec { vec![config.resources.storage.data.build_pvc( ContainerConfig::DATA_VOLUME_MOUNT_NAME, @@ -378,7 +379,9 @@ impl ContainerConfig { } /// The PVC templates for a datanode role group, one per configured data volume. - pub fn datanode_volume_claim_templates(config: &DataNodeConfig) -> Vec { + pub fn datanode_volume_claim_templates( + config: &ValidatedDataNodeConfig, + ) -> Vec { DataNodeStorageConfig { pvcs: config.resources.storage.clone(), } @@ -390,7 +393,10 @@ impl ContainerConfig { /// Datanodes use an ephemeral listener volume, while namenodes use a persistent volume claim /// template for stable per-pod identity (see [`Self::namenode_volume_claim_templates`]) and /// journalnodes have no listener at all. - pub fn datanode_listener_volume(config: &DataNodeConfig, labels: &Labels) -> Result { + pub fn datanode_listener_volume( + config: &ValidatedDataNodeConfig, + labels: &Labels, + ) -> Result { Ok(VolumeBuilder::new(&*LISTENER_VOLUME_NAME) .ephemeral( ListenerOperatorVolumeSourceBuilder::new( @@ -589,6 +595,7 @@ impl ContainerConfig { role = role.as_ref(), file_log_directory = ENV_FILE_LOG_DIRECTORY, containerdebug_command = inputs + .common .containerdebug .background_command(STACKABLE_LOG_DIR) .unwrap_or_default(), diff --git a/rust/operator-binary/src/controller/build/graceful_shutdown.rs b/rust/operator-binary/src/controller/build/graceful_shutdown.rs index 819d6bde..e0cab1ab 100644 --- a/rust/operator-binary/src/controller/build/graceful_shutdown.rs +++ b/rust/operator-binary/src/controller/build/graceful_shutdown.rs @@ -1,7 +1,7 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::builder::pod::PodBuilder; -use crate::crd::CommonNodeConfig; +use crate::controller::ValidatedCommonNodeConfig; #[derive(Debug, Snafu)] pub enum Error { @@ -12,7 +12,7 @@ pub enum Error { } pub fn add_graceful_shutdown_config( - merged_config: &CommonNodeConfig, + merged_config: &ValidatedCommonNodeConfig, pod_builder: &mut PodBuilder, ) -> Result<(), Error> { // This must be always set by the merge mechanism, as we provide a default value, diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index deda493d..06f9a676 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -870,52 +870,4 @@ spec: None ); } - - /// HDFS validates the `containerdebug` config while building the role group, so an invalid - /// one fails the build (and with it the reconciliation, before anything is applied). - #[test] - fn zero_containerdebug_interval_fails_the_build() { - let cluster = deserialize_and_validate_cluster( - r#" ---- -apiVersion: hdfs.stackable.tech/v1alpha1 -kind: HdfsCluster -metadata: - name: hdfs - namespace: default - uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f -spec: - image: - productVersion: 3.4.0 - clusterConfig: - zookeeperConfigMapName: hdfs-zk - nameNodes: - roleGroups: - default: - replicas: 1 - journalNodes: - config: - containerdebug: - interval: 0s - roleGroups: - default: - replicas: 1 - dataNodes: - roleGroups: - default: - replicas: 1 -"#, - ); - - let Err(error) = build(&cluster, &cluster_info()) else { - panic!("the build should fail"); - }; - - assert!( - snafu::Report::from_error(&error) - .to_string() - .contains("invalid containerdebug config for role journalnode role group default"), - "{error:?}" - ); - } } diff --git a/rust/operator-binary/src/controller/build/role_group/datanode.rs b/rust/operator-binary/src/controller/build/role_group/datanode.rs index 34722092..2806eb03 100644 --- a/rust/operator-binary/src/controller/build/role_group/datanode.rs +++ b/rust/operator-binary/src/controller/build/role_group/datanode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, ExtraContainer, InvalidContainerdebugConfigSnafu, ListenerVolumeSnafu, RoleGroupBuilder, - RoleGroupInputs, RoleGroupSelectorLabelsSnafu, common_container_logging, + Error, ExtraContainer, ListenerVolumeSnafu, RoleGroupBuilder, RoleGroupInputs, + RoleGroupSelectorLabelsSnafu, common_container_logging, }; use crate::{ controller::{ @@ -54,16 +54,6 @@ pub(crate) fn build<'a>( DataNodeContainer::Vector, ); - let containerdebug = - config - .common - .containerdebug - .validate() - .context(InvalidContainerdebugConfigSnafu { - role: ROLE, - role_group: role_group_name.clone(), - })?; - let inputs = RoleGroupInputs { cluster, cluster_info, @@ -71,7 +61,6 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), - containerdebug, resources: config.resources.clone().into(), volume_claim_templates: ContainerConfig::datanode_volume_claim_templates(config), // First, because the pod's `volumes` are an ordered list: moving the listener volume diff --git a/rust/operator-binary/src/controller/build/role_group/journalnode.rs b/rust/operator-binary/src/controller/build/role_group/journalnode.rs index ff31601d..1739d320 100644 --- a/rust/operator-binary/src/controller/build/role_group/journalnode.rs +++ b/rust/operator-binary/src/controller/build/role_group/journalnode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, InvalidContainerdebugConfigSnafu, RoleGroupBuilder, RoleGroupInputs, - RoleGroupSelectorLabelsSnafu, common_container_logging, + Error, RoleGroupBuilder, RoleGroupInputs, RoleGroupSelectorLabelsSnafu, + common_container_logging, }; use crate::{ controller::{ @@ -44,16 +44,6 @@ pub(crate) fn build<'a>( JournalNodeContainer::Vector, ); - let containerdebug = - config - .common - .containerdebug - .validate() - .context(InvalidContainerdebugConfigSnafu { - role: ROLE, - role_group: role_group_name.clone(), - })?; - let inputs = RoleGroupInputs { cluster, cluster_info, @@ -61,7 +51,6 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), - containerdebug, resources: config.resources.clone().into(), volume_claim_templates: ContainerConfig::journalnode_volume_claim_templates(config), extra_pod_volumes: Vec::new(), diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index a0ca77a8..536753cb 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -20,7 +20,6 @@ pub(crate) use journalnode::build as build_journalnode_role_group; pub(crate) use namenode::build as build_namenode_role_group; use snafu::{ResultExt, Snafu}; use stackable_operator::{ - crd::containerdebug::{self, ValidatedContainerdebugConfig}, k8s_openapi::api::{ apps::v1::StatefulSet, core::v1::{ @@ -43,8 +42,8 @@ use super::{ resource, }; use crate::{ - controller::ValidatedCluster, - crd::{CommonNodeConfig, HdfsNodeRole, storage::DataNodeStorageConfigInnerType, v1alpha1}, + controller::{ValidatedCluster, ValidatedCommonNodeConfig}, + crd::{HdfsNodeRole, storage::DataNodeStorageConfigInnerType, v1alpha1}, }; #[derive(Snafu, Debug)] @@ -97,13 +96,6 @@ pub enum Error { role: HdfsNodeRole, role_group: RoleGroupName, }, - - #[snafu(display("invalid containerdebug config for role {role} role group {role_group}", role = role.as_ref()))] - InvalidContainerdebugConfig { - source: containerdebug::Error, - role: HdfsNodeRole, - role_group: RoleGroupName, - }, } /// A container only one role runs, named by that role's module together with the log config that @@ -151,11 +143,9 @@ pub(crate) struct RoleGroupInputs<'a> { /// cluster resources to "manage" listeners, which is wrong and leads to errors. The listeners /// are managed by the listener-operator. pub(crate) selector_labels: Labels, - /// The role group's merged config that is common to every role. - pub(crate) common: CommonNodeConfig, - /// The role group's validated `containerdebug` config, which the main `hdfs` container starts - /// `containerdebug` from. - pub(crate) containerdebug: ValidatedContainerdebugConfig, + /// The role group's validated config that is common to every role, including the + /// `containerdebug` config the main `hdfs` container starts `containerdebug` from. + pub(crate) common: ValidatedCommonNodeConfig, /// The resource requirements of the role group's main and init containers; the ZKFC sidecar /// has fixed requirements of its own and ignores this. pub(crate) resources: ResourceRequirements, @@ -202,7 +192,7 @@ pub(crate) struct RoleGroupBuilder<'a> { pub(crate) role_group_name: RoleGroupName, pub(crate) selector_labels: Labels, /// Carried for the pod's affinity and its graceful shutdown timeout. - pub(crate) common: CommonNodeConfig, + pub(crate) common: ValidatedCommonNodeConfig, pub(crate) replicas: Option, pub(crate) pod_overrides: PodTemplateSpec, pub(crate) volume_claim_templates: Vec, diff --git a/rust/operator-binary/src/controller/build/role_group/namenode.rs b/rust/operator-binary/src/controller/build/role_group/namenode.rs index a19bc2ec..cb61b2f7 100644 --- a/rust/operator-binary/src/controller/build/role_group/namenode.rs +++ b/rust/operator-binary/src/controller/build/role_group/namenode.rs @@ -6,8 +6,8 @@ use stackable_operator::{ }; use super::{ - Error, ExtraContainer, InvalidContainerdebugConfigSnafu, RoleGroupBuilder, RoleGroupInputs, - RoleGroupSelectorLabelsSnafu, VolumeClaimTemplatesSnafu, common_container_logging, + Error, ExtraContainer, RoleGroupBuilder, RoleGroupInputs, RoleGroupSelectorLabelsSnafu, + VolumeClaimTemplatesSnafu, common_container_logging, }; use crate::{ controller::{ @@ -57,16 +57,6 @@ pub(crate) fn build<'a>( NameNodeContainer::Vector, ); - let containerdebug = - config - .common - .containerdebug - .validate() - .context(InvalidContainerdebugConfigSnafu { - role: ROLE, - role_group: role_group_name.clone(), - })?; - let inputs = RoleGroupInputs { cluster, cluster_info, @@ -74,7 +64,6 @@ pub(crate) fn build<'a>( role_group_name: role_group_name.clone(), selector_labels, common: config.common.clone(), - containerdebug, resources: config.resources.clone().into(), volume_claim_templates, extra_pod_volumes: Vec::new(), diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 432a7c57..8e84c277 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -1,9 +1,13 @@ use std::{collections::BTreeMap, marker::PhantomData, str::FromStr}; use stackable_operator::{ - commons::product_image_selection::ResolvedProductImage, + commons::{ + affinity::StackableAffinity, + product_image_selection::ResolvedProductImage, + resources::{NoRuntimeLimits, Resources}, + }, constant, - crd::listener, + crd::{containerdebug::ValidatedContainerdebugConfig, listener}, k8s_openapi::api::{ apps::v1::StatefulSet, core::v1::{ConfigMap, Service, ServiceAccount}, @@ -11,12 +15,14 @@ use stackable_operator::{ rbac::v1::RoleBinding, }, kube::{Resource, api::ObjectMeta}, + product_logging::spec::Logging, + shared::time::Duration, v2::{ HasName, HasUid, NameIsValidLabelValue, role_group_utils::{QualifiedRoleGroupName, ResourceNames}, role_utils::{self, JavaCommonConfig, RoleGroupConfig}, types::{ - kubernetes::{ConfigMapName, NamespaceName, ServiceName, Uid}, + kubernetes::{ConfigMapName, ListenerClassName, NamespaceName, ServiceName, Uid}, operator::{ ClusterName, ControllerName, OperatorName, ProductName, ProductVersion, RoleGroupName, @@ -29,8 +35,11 @@ use crate::{ HDFS_OPERATOR_NAME, controller::build::opa::HdfsOpaConfig, crd::{ - DataNodeConfig, HdfsNodeRole, JournalNodeConfig, NameNodeConfig, UpgradeState, - constants::APP_NAME, security::AuthenticationConfig, v1alpha1, + DataNodeContainer, HdfsNodeRole, JournalNodeContainer, NameNodeContainer, UpgradeState, + constants::APP_NAME, + security::AuthenticationConfig, + storage::{DataNodeStorageConfigInnerType, HdfsStorageConfig}, + v1alpha1, }, hdfs_controller::HDFS_CONTROLLER_NAME, }; @@ -72,17 +81,61 @@ pub struct KubernetesResources { pub status: PhantomData, } +/// The settings every role shares, validated. +/// +/// This is the merged [`CommonNodeConfig`](crate::crd::CommonNodeConfig) with its `containerdebug` +/// config validated, so the build step cannot reach a `containerdebug` interval of zero. The two +/// [`Duration`]s stay optional, exactly as the merge leaves them. +#[derive(Clone, Debug)] +pub struct ValidatedCommonNodeConfig { + pub affinity: StackableAffinity, + /// The role group's validated `containerdebug` config, which the main `hdfs` container starts + /// `containerdebug` from. + pub containerdebug: ValidatedContainerdebugConfig, + pub graceful_shutdown_timeout: Option, + pub requested_secret_lifetime: Option, +} + +/// A namenode role group's validated config: the merged +/// [`NameNodeConfig`](crate::crd::NameNodeConfig) with its common settings validated. +#[derive(Clone, Debug)] +pub struct ValidatedNameNodeConfig { + pub resources: Resources, + pub logging: Logging, + pub listener_class: ListenerClassName, + pub common: ValidatedCommonNodeConfig, +} + +/// A datanode role group's validated config: the merged +/// [`DataNodeConfig`](crate::crd::DataNodeConfig) with its common settings validated. +#[derive(Clone, Debug)] +pub struct ValidatedDataNodeConfig { + pub resources: Resources, + pub logging: Logging, + pub listener_class: ListenerClassName, + pub common: ValidatedCommonNodeConfig, +} + +/// A journalnode role group's validated config: the merged +/// [`JournalNodeConfig`](crate::crd::JournalNodeConfig) with its common settings validated. +#[derive(Clone, Debug)] +pub struct ValidatedJournalNodeConfig { + pub resources: Resources, + pub logging: Logging, + pub common: ValidatedCommonNodeConfig, +} + /// The [`RoleGroupConfig`] of one namenode role group. pub type NameNodeRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; /// The [`RoleGroupConfig`] of one datanode role group. pub type DataNodeRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; /// The [`RoleGroupConfig`] of one journalnode role group. pub type JournalNodeRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; /// The validated cluster: proves that config merging and validation succeeded /// for every role and role group before any resources are created. Placed in the diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 00a2f863..e400dbc5 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -6,6 +6,7 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ commons::product_image_selection, config::{fragment::FromFragment, merge::Merge}, + crd::containerdebug, role_utils::GenericRoleConfig, v2::{ controller_utils::{get_cluster_name, get_namespace, get_uid}, @@ -16,12 +17,14 @@ use stackable_operator::{ use crate::{ controller::{ - ValidatedCluster, ValidatedClusterConfig, ValidatedClusterStatus, ValidatedRoleConfig, - dereference::DereferencedObjects, + ValidatedCluster, ValidatedClusterConfig, ValidatedClusterStatus, + ValidatedCommonNodeConfig, ValidatedDataNodeConfig, ValidatedJournalNodeConfig, + ValidatedNameNodeConfig, ValidatedRoleConfig, dereference::DereferencedObjects, }, crd::{ - DataNodeConfigFragment, HdfsNodeRole, JournalNodeConfigFragment, NameNodeConfigFragment, - UpgradeStateError, v1alpha1, + CommonNodeConfig, DataNodeConfig, DataNodeConfigFragment, HdfsNodeRole, JournalNodeConfig, + JournalNodeConfigFragment, NameNodeConfig, NameNodeConfigFragment, UpgradeStateError, + v1alpha1, }, }; @@ -62,6 +65,23 @@ pub enum Error { #[snafu(display("invalid upgrade state"))] UpgradeState { source: UpgradeStateError }, + + #[snafu(display("invalid config for role {role} role group {role_group}", role = role.as_ref()))] + InvalidConfig { + source: ConfigError, + role: HdfsNodeRole, + role_group: String, + }, +} + +/// Why a role group's merged config could not be turned into its validated config. +/// +/// Carries no role or role group: the caller adds them (see [`Error::InvalidConfig`]), as only it +/// knows which role group the config was merged for. +#[derive(Snafu, Debug)] +pub enum ConfigError { + #[snafu(display("invalid containerdebug config"))] + InvalidContainerdebugConfig { source: containerdebug::Error }, } pub fn validate_cluster( @@ -91,11 +111,13 @@ pub fn validate_cluster( // The first failure propagates, so this order decides which misconfiguration the user is told // about when more than one role is wrong. It is `HdfsNodeRole`'s variant order. - let journalnode_role_group_configs = validate_role_group_configs( + let journalnode_role_group_configs = validate_role_group_configs::<_, JournalNodeConfig>( + HdfsNodeRole::Journal, hdfs.spec.journal_nodes.as_ref(), JournalNodeConfigFragment::default_config(cluster_name.as_ref(), &HdfsNodeRole::Journal), )?; - let namenode_role_group_configs = validate_role_group_configs( + let namenode_role_group_configs = validate_role_group_configs::<_, NameNodeConfig>( + HdfsNodeRole::Name, hdfs.spec.name_nodes.as_ref(), NameNodeConfigFragment::default_config( cluster_name.as_ref(), @@ -107,7 +129,8 @@ pub fn validate_cluster( .map(|authorization| &authorization.opa), ), )?; - let datanode_role_group_configs = validate_role_group_configs( + let datanode_role_group_configs = validate_role_group_configs::<_, DataNodeConfig>( + HdfsNodeRole::Data, hdfs.spec.data_nodes.as_ref(), DataNodeConfigFragment::default_config(cluster_name.as_ref(), &HdfsNodeRole::Data), )?; @@ -165,28 +188,32 @@ fn validated_role_config( ) } +/// The validated role groups of one role, keyed by role group name. +type ValidatedRoleGroupConfigs = BTreeMap< + RoleGroupName, + RoleGroupConfig, +>; + /// Validates every role group of a role into a map keyed by role group name. /// /// Each role group is merged and validated via /// [`with_validated_config`], which folds the CRD config fragment (default <- /// role <- role group) plus the `configOverrides`, `envOverrides`, `cliOverrides` /// and `podOverrides` (role group wins) into a single -/// [`RoleGroupConfig`]. +/// [`RoleGroupConfig`]. Its merged config is then turned into the role's validated config by +/// [`IntoValidated::validate`], which is what the build step gets to see. /// /// Returns an empty map if the role is not configured. -fn validate_role_group_configs( - role: Option<&Role>, - default_config: Config, -) -> Result< - BTreeMap< - RoleGroupName, - RoleGroupConfig, +fn validate_role_group_configs( + hdfs_role: HdfsNodeRole, + role: Option< + &Role, >, - Error, -> + default_config: Fragment, +) -> Result, Error> where - Config: Clone + Merge, - ValidatedConfig: FromFragment, + Fragment: Clone + Merge, + Config: FromFragment + IntoValidated, { let Some(role) = role else { return Ok(BTreeMap::new()); @@ -195,25 +222,31 @@ where role.role_groups .iter() .map(|(role_group_name, role_group)| { - let validated = with_validated_config::< - ValidatedConfig, - JavaCommonConfig, + let merged = with_validated_config::< Config, + JavaCommonConfig, + Fragment, GenericRoleConfig, v1alpha1::HdfsConfigOverrides, >(role_group, role, &default_config) .context(ValidateRoleGroupConfigSnafu)?; // The overrides carry over unchanged. - let validated = RoleGroupConfig { - replicas: validated.replicas, - config: validated.config.config, - config_overrides: validated.config.config_overrides, - env_overrides: validated.config.env_overrides.into(), - cli_overrides: validated.config.cli_overrides, - pod_overrides: validated.config.pod_overrides, - product_specific_common_config: validated.config.product_specific_common_config, - }; + let validated = + RoleGroupConfig { + replicas: merged.replicas, + config: merged.config.config.validate().with_context(|_| { + InvalidConfigSnafu { + role: hdfs_role, + role_group: role_group_name.clone(), + } + })?, + config_overrides: merged.config.config_overrides, + env_overrides: merged.config.env_overrides.into(), + cli_overrides: merged.config.cli_overrides, + pod_overrides: merged.config.pod_overrides, + product_specific_common_config: merged.config.product_specific_common_config, + }; let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| { ParseRoleGroupNameSnafu { role_group: role_group_name.clone(), @@ -223,3 +256,164 @@ where }) .collect() } + +/// Turns a role group's merged config into the validated config the build step works with. +/// +/// Every impl destructures the merged config without `..`, so a field added to the CRD struct +/// fails to compile there instead of silently never reaching the build step. +// FIXME: `IntoValidated` should be a trait coming from operator-rs. +trait IntoValidated { + /// The validated config the build step gets to see. + type Validated; + + /// Consumes the merged config, as nothing needs it once it is validated. + fn validate(self) -> Result; +} + +impl IntoValidated for NameNodeConfig { + type Validated = ValidatedNameNodeConfig; + + fn validate(self) -> Result { + let NameNodeConfig { + resources, + logging, + listener_class, + common, + } = self; + + Ok(ValidatedNameNodeConfig { + resources, + logging, + listener_class, + common: common.validate()?, + }) + } +} + +impl IntoValidated for DataNodeConfig { + type Validated = ValidatedDataNodeConfig; + + fn validate(self) -> Result { + let DataNodeConfig { + resources, + logging, + listener_class, + common, + } = self; + + Ok(ValidatedDataNodeConfig { + resources, + logging, + listener_class, + common: common.validate()?, + }) + } +} + +impl IntoValidated for JournalNodeConfig { + type Validated = ValidatedJournalNodeConfig; + + fn validate(self) -> Result { + let JournalNodeConfig { + resources, + logging, + common, + } = self; + + Ok(ValidatedJournalNodeConfig { + resources, + logging, + common: common.validate()?, + }) + } +} + +/// The settings every role shares. +/// +/// The `containerdebug` config is the only part that can fail: an interval of zero would make +/// `containerdebug` collect in a tight loop. Rejecting it here, rather than in the build step, +/// means the build step can only ever be handed a validated `containerdebug` config. +impl IntoValidated for CommonNodeConfig { + type Validated = ValidatedCommonNodeConfig; + + fn validate(self) -> Result { + let CommonNodeConfig { + affinity, + containerdebug, + graceful_shutdown_timeout, + requested_secret_lifetime, + } = self; + + Ok(ValidatedCommonNodeConfig { + affinity, + containerdebug: containerdebug + .validate() + .context(InvalidContainerdebugConfigSnafu)?, + graceful_shutdown_timeout, + requested_secret_lifetime, + }) + } +} + +#[cfg(test)] +mod tests { + use crate::test_support::{deserialize_cluster, try_validate_cluster}; + + /// The `containerdebug` config is validated per role group, so an invalid one fails the + /// validate step and with it the reconciliation -- before any Kubernetes object is built, + /// let alone applied. The error names the role and role group it came from, which the + /// merged config alone no longer tells the user. + #[test] + fn zero_containerdebug_interval_fails_validation() { + let hdfs = deserialize_cluster( + r#" +--- +apiVersion: hdfs.stackable.tech/v1alpha1 +kind: HdfsCluster +metadata: + name: hdfs + namespace: default + uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f +spec: + image: + productVersion: 3.4.0 + clusterConfig: + zookeeperConfigMapName: hdfs-zk + nameNodes: + roleGroups: + default: + replicas: 1 + journalNodes: + config: + containerdebug: + interval: 0s + roleGroups: + default: + replicas: 1 + dataNodes: + roleGroups: + default: + replicas: 1 +"#, + ); + + let Err(error) = try_validate_cluster(&hdfs) else { + panic!("the validation should fail"); + }; + + let chain: Vec = + std::iter::successors(Some(&error as &dyn std::error::Error), |error| { + error.source() + }) + .map(ToString::to_string) + .collect(); + assert_eq!( + chain, + [ + "invalid config for role journalnode role group default", + "invalid containerdebug config", + "the containerdebug interval must not be zero", + ] + ); + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 0ec16b5d..8d197d68 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -879,13 +879,13 @@ mod test { use super::*; use crate::{ - controller::build, + controller::{ValidatedDataNodeConfig, build}, crd::storage::{DataNodePvc, HdfsStorageType}, test_support::{datanode_config, deserialize_and_validate_cluster, role_group_name}, }; fn datanode_pvc<'a>( - datanode_config: &'a DataNodeConfig, + datanode_config: &'a ValidatedDataNodeConfig, storage_name: &str, ) -> &'a DataNodePvc { datanode_config diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs index 31f93879..f1596cc5 100644 --- a/rust/operator-binary/src/test_support.rs +++ b/rust/operator-binary/src/test_support.rs @@ -5,11 +5,10 @@ use stackable_operator::v2::types::operator::RoleGroupName; use crate::{ controller::{ DataNodeRoleGroupConfig, JournalNodeRoleGroupConfig, NameNodeRoleGroupConfig, - ValidatedCluster, validate, - }, - crd::{ - CommonNodeConfig, DataNodeConfig, HdfsNodeRole, JournalNodeConfig, NameNodeConfig, v1alpha1, + ValidatedCluster, ValidatedCommonNodeConfig, ValidatedDataNodeConfig, + ValidatedJournalNodeConfig, ValidatedNameNodeConfig, validate, }, + crd::{HdfsNodeRole, v1alpha1}, }; /// The expected `app.kubernetes.io/version` label value for the given product version. @@ -24,7 +23,10 @@ pub fn app_version_label(product_version: &str) -> String { ) } -pub fn validate_cluster(hdfs: &v1alpha1::HdfsCluster) -> ValidatedCluster { +/// Runs the validate step, returning its result so tests can assert on a rejected spec. +pub fn try_validate_cluster( + hdfs: &v1alpha1::HdfsCluster, +) -> Result { validate::validate_cluster( hdfs, "oci.example.org", @@ -34,11 +36,18 @@ pub fn validate_cluster(hdfs: &v1alpha1::HdfsCluster) -> ValidatedCluster { discovery_config_map: None, }, ) - .expect("cluster spec should be valid") +} + +pub fn validate_cluster(hdfs: &v1alpha1::HdfsCluster) -> ValidatedCluster { + try_validate_cluster(hdfs).expect("cluster spec should be valid") +} + +pub fn deserialize_cluster(spec: &str) -> v1alpha1::HdfsCluster { + stackable_operator::utils::yaml_from_str_singleton_map(spec).expect("") } pub fn deserialize_and_validate_cluster(spec: &str) -> ValidatedCluster { - validate_cluster(&stackable_operator::utils::yaml_from_str_singleton_map(spec).expect("")) + validate_cluster(&deserialize_cluster(spec)) } /// Parses a role group name for use in tests, panicking if it is invalid. @@ -79,31 +88,31 @@ pub fn journalnode_role_group_config<'a>( pub fn namenode_config<'a>( validated_cluster: &'a ValidatedCluster, role_group_name: &RoleGroupName, -) -> &'a NameNodeConfig { +) -> &'a ValidatedNameNodeConfig { &namenode_role_group_config(validated_cluster, role_group_name).config } pub fn datanode_config<'a>( validated_cluster: &'a ValidatedCluster, role_group_name: &RoleGroupName, -) -> &'a DataNodeConfig { +) -> &'a ValidatedDataNodeConfig { &datanode_role_group_config(validated_cluster, role_group_name).config } pub fn journalnode_config<'a>( validated_cluster: &'a ValidatedCluster, role_group_name: &RoleGroupName, -) -> &'a JournalNodeConfig { +) -> &'a ValidatedJournalNodeConfig { &journalnode_role_group_config(validated_cluster, role_group_name).config } -/// The merged [`CommonNodeConfig`] of one role group, for tests that assert on the settings every -/// role shares. +/// The [`ValidatedCommonNodeConfig`] of one role group, for tests that assert on the settings +/// every role shares. pub fn common_config<'a>( validated_cluster: &'a ValidatedCluster, role: &HdfsNodeRole, role_group_name: &RoleGroupName, -) -> &'a CommonNodeConfig { +) -> &'a ValidatedCommonNodeConfig { match role { HdfsNodeRole::Name => &namenode_config(validated_cluster, role_group_name).common, HdfsNodeRole::Data => &datanode_config(validated_cluster, role_group_name).common, From 25ef329f9acf2b75a003cc2515817e48d8a73b70 Mon Sep 17 00:00:00 2001 From: Maxi Wittich Date: Fri, 13 Nov 2026 10:01:22 +0100 Subject: [PATCH 3/4] update trait IntoValidated for operator-rs port --- rust/operator-binary/src/controller/mod.rs | 3 +-- rust/operator-binary/src/controller/validate.rs | 7 ++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 8e84c277..c2a07acd 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -84,8 +84,7 @@ pub struct KubernetesResources { /// The settings every role shares, validated. /// /// This is the merged [`CommonNodeConfig`](crate::crd::CommonNodeConfig) with its `containerdebug` -/// config validated, so the build step cannot reach a `containerdebug` interval of zero. The two -/// [`Duration`]s stay optional, exactly as the merge leaves them. +/// config validated. Further validations can be implemented later. #[derive(Clone, Debug)] pub struct ValidatedCommonNodeConfig { pub affinity: StackableAffinity, diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index e400dbc5..a2b58798 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -265,13 +265,15 @@ where trait IntoValidated { /// The validated config the build step gets to see. type Validated; + type ConfigError; /// Consumes the merged config, as nothing needs it once it is validated. - fn validate(self) -> Result; + fn validate(self) -> Result; } impl IntoValidated for NameNodeConfig { type Validated = ValidatedNameNodeConfig; + type ConfigError = ConfigError; fn validate(self) -> Result { let NameNodeConfig { @@ -292,6 +294,7 @@ impl IntoValidated for NameNodeConfig { impl IntoValidated for DataNodeConfig { type Validated = ValidatedDataNodeConfig; + type ConfigError = ConfigError; fn validate(self) -> Result { let DataNodeConfig { @@ -312,6 +315,7 @@ impl IntoValidated for DataNodeConfig { impl IntoValidated for JournalNodeConfig { type Validated = ValidatedJournalNodeConfig; + type ConfigError = ConfigError; fn validate(self) -> Result { let JournalNodeConfig { @@ -335,6 +339,7 @@ impl IntoValidated for JournalNodeConfig { /// means the build step can only ever be handed a validated `containerdebug` config. impl IntoValidated for CommonNodeConfig { type Validated = ValidatedCommonNodeConfig; + type ConfigError = ConfigError; fn validate(self) -> Result { let CommonNodeConfig { From 0bb60704b89f9de36f9fc7bad154fe054801ca4b Mon Sep 17 00:00:00 2001 From: Maxi Wittich Date: Fri, 13 Nov 2026 11:00:15 +0100 Subject: [PATCH 4/4] change error type to Error --- rust/operator-binary/src/controller/validate.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index a2b58798..0f73cad9 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -265,15 +265,15 @@ where trait IntoValidated { /// The validated config the build step gets to see. type Validated; - type ConfigError; + type Error; /// Consumes the merged config, as nothing needs it once it is validated. - fn validate(self) -> Result; + fn validate(self) -> Result; } impl IntoValidated for NameNodeConfig { type Validated = ValidatedNameNodeConfig; - type ConfigError = ConfigError; + type Error = ConfigError; fn validate(self) -> Result { let NameNodeConfig { @@ -294,7 +294,7 @@ impl IntoValidated for NameNodeConfig { impl IntoValidated for DataNodeConfig { type Validated = ValidatedDataNodeConfig; - type ConfigError = ConfigError; + type Error = ConfigError; fn validate(self) -> Result { let DataNodeConfig { @@ -315,7 +315,7 @@ impl IntoValidated for DataNodeConfig { impl IntoValidated for JournalNodeConfig { type Validated = ValidatedJournalNodeConfig; - type ConfigError = ConfigError; + type Error = ConfigError; fn validate(self) -> Result { let JournalNodeConfig { @@ -339,7 +339,7 @@ impl IntoValidated for JournalNodeConfig { /// means the build step can only ever be handed a validated `containerdebug` config. impl IntoValidated for CommonNodeConfig { type Validated = ValidatedCommonNodeConfig; - type ConfigError = ConfigError; + type Error = ConfigError; fn validate(self) -> Result { let CommonNodeConfig {