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..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( @@ -574,7 +580,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 +594,11 @@ impl ContainerConfig { hadoop_home = Self::HADOOP_HOME, role = role.as_ref(), file_log_directory = ENV_FILE_LOG_DIRECTORY, + containerdebug_command = inputs + .common + .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/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 3b9e7e3e..06f9a676 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,87 @@ 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 + ); + } } 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..536753cb 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -42,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)] @@ -143,8 +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 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, @@ -191,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/mod.rs b/rust/operator-binary/src/controller/mod.rs index 432a7c57..c2a07acd 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,60 @@ 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. Further validations can be implemented later. +#[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..0f73cad9 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,169 @@ 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; + type Error; + + /// Consumes the merged config, as nothing needs it once it is validated. + fn validate(self) -> Result; +} + +impl IntoValidated for NameNodeConfig { + type Validated = ValidatedNameNodeConfig; + type Error = ConfigError; + + 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; + type Error = ConfigError; + + 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; + type Error = ConfigError; + + 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; + type Error = ConfigError; + + 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 9fcae60f..8d197d68 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), }, @@ -869,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,