diff --git a/CHANGELOG.md b/CHANGELOG.md index a39fdf1d..3dbeab4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +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 appropriate ([#824]). ### Fixed @@ -39,6 +40,7 @@ All notable changes to this project will be documented in this file. [#814]: https://github.com/stackabletech/hdfs-operator/pull/814 [#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 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index c756ed0d..fd286142 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -14,16 +14,12 @@ use std::{collections::BTreeMap, str::FromStr}; use indoc::formatdoc; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ - builder::{ - self, - pod::{ - PodBuilder, - resources::ResourceRequirementsBuilder, - volume::{ - ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError, - ListenerReference, SecretFormat, SecretOperatorVolumeSourceBuilder, - SecretOperatorVolumeSourceBuilderError, VolumeBuilder, VolumeMountBuilder, - }, + builder::pod::{ + PodBuilder, + resources::ResourceRequirementsBuilder, + volume::{ + ListenerOperatorVolumeSourceBuilder, ListenerReference, SecretFormat, + SecretOperatorVolumeSourceBuilder, VolumeBuilder, VolumeMountBuilder, }, }, commons::secret_class::SecretClassVolumeProvisionParts, @@ -128,30 +124,6 @@ pub enum Error { #[snafu(display("failed to construct JVM arguments fro role {role:?}"))] ConstructJvmArguments { source: jvm::Error, role: String }, - #[snafu(display( - "could not determine any ContainerConfig actions for {container_name:?}. Container not recognized." - ))] - UnrecognizedContainerName { container_name: String }, - - #[snafu(display("failed to build secret volume for {volume_name:?}"))] - BuildSecretVolume { - source: SecretOperatorVolumeSourceBuilderError, - volume_name: String, - }, - - #[snafu(display("failed to build listener volume"))] - BuildListenerVolume { - source: ListenerOperatorVolumeSourceBuilderError, - }, - - #[snafu(display("failed to add needed volume"))] - AddVolume { source: builder::pod::Error }, - - #[snafu(display("failed to add needed volumeMount"))] - AddVolumeMount { - source: builder::pod::container::Error, - }, - #[snafu(display("vector agent is enabled but vector aggregator ConfigMap is missing"))] VectorAggregatorConfigMapMissing, @@ -241,8 +213,8 @@ impl ContainerConfig { let object_name = resource_names.qualified_role_group_name().to_string(); let merged_config = &rolegroup_config.config; - pb.add_volumes(main_container_config.volumes(merged_config, &object_name, labels)?) - .context(AddVolumeSnafu)?; + pb.add_volumes(main_container_config.volumes(merged_config, &object_name, labels)) + .expect("The volume names are statically defined and there should be no duplicates."); pb.add_container(main_container_config.main_container( cluster, cluster_info, @@ -314,13 +286,11 @@ impl ContainerConfig { .context(MissingSecretLifetimeSnafu)?, ) .build() - .context(BuildSecretVolumeSnafu { - volume_name: &*TLS_STORE_VOLUME_NAME, - })?, + .expect("The annotation keys are static and annotation values cannot be invalid."), ) .build(), ) - .context(AddVolumeSnafu)?; + .expect("The volume names are statically defined and there should be no duplicates."); pb.add_volume( VolumeBuilder::new(&*KERBEROS_VOLUME_NAME) @@ -334,26 +304,24 @@ impl ContainerConfig { .with_kerberos_service_name(role.kerberos_service_name()) .with_kerberos_service_name("HTTP") .build() - .context(BuildSecretVolumeSnafu { - volume_name: &*KERBEROS_VOLUME_NAME, - })?, + .expect("The annotation keys are static and annotation values cannot be invalid."), ) .build(), ) - .context(AddVolumeSnafu)?; + .expect("The volume names are statically defined and there should be no duplicates."); } // role specific pod settings configured here match role { HdfsNodeRole::Name => { // Zookeeper fail over container - let zkfc_container_config = Self::try_from(NameNodeContainer::Zkfc.to_string())?; + let zkfc_container_config = Self::zkfc(); pb.add_volumes(zkfc_container_config.volumes( merged_config, &object_name, labels, - )?) - .context(AddVolumeSnafu)?; + )) + .expect("The volume names are statically defined and there should be no duplicates."); pb.add_container(zkfc_container_config.main_container( cluster, cluster_info, @@ -363,14 +331,15 @@ impl ContainerConfig { )?); // Format namenode init container - let format_namenodes_container_config = - Self::try_from(NameNodeContainer::FormatNameNodes.to_string())?; + let format_namenodes_container_config = Self::format_namenodes(); pb.add_volumes(format_namenodes_container_config.volumes( merged_config, &object_name, labels, - )?) - .context(AddVolumeSnafu)?; + )) + .expect( + "The volume names are statically defined and there should be no duplicates.", + ); pb.add_init_container(format_namenodes_container_config.init_container( cluster, cluster_info, @@ -381,14 +350,15 @@ impl ContainerConfig { )?); // Format ZooKeeper init container - let format_zookeeper_container_config = - Self::try_from(NameNodeContainer::FormatZooKeeper.to_string())?; + let format_zookeeper_container_config = Self::format_zookeeper(); pb.add_volumes(format_zookeeper_container_config.volumes( merged_config, &object_name, labels, - )?) - .context(AddVolumeSnafu)?; + )) + .expect( + "The volume names are statically defined and there should be no duplicates.", + ); pb.add_init_container(format_zookeeper_container_config.init_container( cluster, cluster_info, @@ -400,14 +370,15 @@ impl ContainerConfig { } HdfsNodeRole::Data => { // Wait for namenode init container - let wait_for_namenodes_container_config = - Self::try_from(DataNodeContainer::WaitForNameNodes.to_string())?; + let wait_for_namenodes_container_config = Self::wait_for_namenodes(); pb.add_volumes(wait_for_namenodes_container_config.volumes( merged_config, &object_name, labels, - )?) - .context(AddVolumeSnafu)?; + )) + .expect( + "The volume names are statically defined and there should be no duplicates.", + ); pb.add_init_container(wait_for_namenodes_container_config.init_container( cluster, cluster_info, @@ -426,7 +397,7 @@ impl ContainerConfig { pub fn volume_claim_templates( merged_config: &AnyNodeConfig, labels: &Labels, - ) -> Result> { + ) -> Vec { match merged_config { AnyNodeConfig::Name(node) => { let listener = ListenerOperatorVolumeSourceBuilder::new( @@ -434,9 +405,9 @@ impl ContainerConfig { labels, ) .build_ephemeral() - .context(BuildListenerVolumeSnafu)? + .expect("The annotation keys are static and annotation values cannot be invalid.") .volume_claim_template - .unwrap(); + .expect("The listener volume source builder always sets a volume claim template."); let pvcs = vec![ node.resources.storage.data.build_pvc( @@ -446,23 +417,25 @@ impl ContainerConfig { PersistentVolumeClaim { metadata: ObjectMeta { name: Some(LISTENER_VOLUME_NAME.to_string()), - ..listener.metadata.unwrap() + ..listener.metadata.expect( + "The listener volume claim template always carries metadata.", + ) }, spec: Some(listener.spec), ..Default::default() }, ]; - Ok(pvcs) + pvcs } - AnyNodeConfig::Journal(node) => Ok(vec![node.resources.storage.data.build_pvc( + AnyNodeConfig::Journal(node) => vec![node.resources.storage.data.build_pvc( ContainerConfig::DATA_VOLUME_MOUNT_NAME, Some(vec!["ReadWriteOnce"]), - )]), - AnyNodeConfig::Data(node) => Ok(DataNodeStorageConfig { + )], + AnyNodeConfig::Data(node) => DataNodeStorageConfig { pvcs: node.resources.storage.clone(), } - .build_pvcs()), + .build_pvcs(), } } @@ -488,8 +461,11 @@ impl ContainerConfig { .command(Self::command()) .args(self.args(cluster, cluster_info, role, merged_config, &[])?) .add_env_vars(self.env(cluster, role, rolegroup_config, resources.as_ref())?) - .add_volume_mounts(self.volume_mounts(cluster, merged_config, labels)?) - .context(AddVolumeMountSnafu)? + .add_volume_mounts(self.volume_mounts(cluster, merged_config, labels)) + .expect( + "The mount paths are either statically defined or derived from the unique PVC \ + names, so there are no duplicates.", + ) .add_container_ports(self.container_ports(cluster)); if let Some(resources) = resources { @@ -535,8 +511,11 @@ impl ContainerConfig { .command(Self::command()) .args(self.args(cluster, cluster_info, role, merged_config, namenode_podrefs)?) .add_env_vars(self.env(cluster, role, rolegroup_config, None)?) - .add_volume_mounts(self.volume_mounts(cluster, merged_config, labels)?) - .context(AddVolumeMountSnafu)?; + .add_volume_mounts(self.volume_mounts(cluster, merged_config, labels)) + .expect( + "The mount paths are either statically defined or derived from the unique PVC \ + names, so there are no duplicates.", + ); // We use the main app container resources here in contrast to several operators (which use // hardcoded resources) due to the different code structure. @@ -1057,7 +1036,7 @@ impl ContainerConfig { merged_config: &AnyNodeConfig, object_name: &str, labels: &Labels, - ) -> Result> { + ) -> Vec { let mut volumes = vec![]; if let ContainerConfig::Hdfs { .. } = self { @@ -1070,7 +1049,7 @@ impl ContainerConfig { labels, ) .build_ephemeral() - .context(BuildListenerVolumeSnafu)?, + .expect("The annotation keys are static and annotation values cannot be invalid."), ) .build(), ); @@ -1119,7 +1098,7 @@ impl ContainerConfig { self.volume_mount_dirs().log_mount_name(), )); - Ok(volumes) + volumes } /// Returns the container volume mounts. @@ -1128,7 +1107,7 @@ impl ContainerConfig { cluster: &ValidatedCluster, merged_config: &AnyNodeConfig, labels: &Labels, - ) -> Result> { + ) -> Vec { let mut volume_mounts = vec![ VolumeMountBuilder::new(Self::STACKABLE_LOG_VOLUME_MOUNT_NAME, STACKABLE_LOG_DIR) .build(), @@ -1146,8 +1125,9 @@ impl ContainerConfig { // Adding this for all containers, as not only the main container needs Kerberos or TLS if cluster.has_kerberos_enabled() { - volume_mounts - .push(VolumeMountBuilder::new("kerberos", KERBEROS_CONTAINER_PATH).build()); + volume_mounts.push( + VolumeMountBuilder::new(&*KERBEROS_VOLUME_NAME, KERBEROS_CONTAINER_PATH).build(), + ); } if cluster.has_https_enabled() { // This volume will be propagated by the create-tls-cert-bundle container @@ -1184,7 +1164,7 @@ impl ContainerConfig { ); } HdfsNodeRole::Data => { - for pvc in Self::volume_claim_templates(merged_config, labels)? { + for pvc in Self::volume_claim_templates(merged_config, labels) { let pvc_name = pvc.name_any(); volume_mounts.push(VolumeMount { mount_path: format!("{DATANODE_ROOT_DATA_DIR_PREFIX}{pvc_name}"), @@ -1201,7 +1181,7 @@ impl ContainerConfig { | ContainerConfig::FormatZooKeeper { .. } => {} } - Ok(volume_mounts) + volume_mounts } /// Create a config directory for the respective container. @@ -1419,41 +1399,56 @@ impl From for ContainerConfig { } } -impl TryFrom for ContainerConfig { - type Error = Error; - - fn try_from(container_name: String) -> Result { - match HdfsNodeRole::from_str(container_name.as_str()) { - Ok(role) => Ok(ContainerConfig::from(role)), - // No hadoop main process container - Err(_) => match container_name { - // namenode side container - name if name == NameNodeContainer::Zkfc.to_string() => Ok(Self::Zkfc { - volume_mounts: ContainerVolumeDirs::try_from(name.as_str())?, - container_name: name, - }), - // namenode init containers - name if name == NameNodeContainer::FormatNameNodes.to_string() => { - Ok(Self::FormatNameNodes { - volume_mounts: ContainerVolumeDirs::try_from(name.as_str())?, - container_name: name, - }) - } - name if name == NameNodeContainer::FormatZooKeeper.to_string() => { - Ok(Self::FormatZooKeeper { - volume_mounts: ContainerVolumeDirs::try_from(name.as_str())?, - container_name: name, - }) - } - // datanode init containers - name if name == DataNodeContainer::WaitForNameNodes.to_string() => { - Ok(Self::WaitForNameNodes { - volume_mounts: ContainerVolumeDirs::try_from(name.as_str())?, - container_name: name, - }) - } - _ => Err(Error::UnrecognizedContainerName { container_name }), - }, +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, + 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, + 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, + 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, + Self::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME, + Self::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME, + ), + container_name, } } } @@ -1546,59 +1541,22 @@ impl From<&HdfsNodeRole> for ContainerVolumeDirs { } } -impl TryFrom<&str> for ContainerVolumeDirs { - type Error = Error; - - fn try_from(container_name: &str) -> Result { - if let Ok(role) = HdfsNodeRole::from_str(container_name) { - return Ok(ContainerVolumeDirs::from(role)); - } - - let (config_mount_name, log_mount_name) = match container_name { - // namenode side container - name if name == NameNodeContainer::Zkfc.to_string() => ( - ContainerConfig::ZKFC_CONFIG_VOLUME_MOUNT_NAME.to_string(), - ContainerConfig::ZKFC_LOG_VOLUME_MOUNT_NAME.to_string(), - ), - // namenode init containers - name if name == NameNodeContainer::FormatNameNodes.to_string() => ( - ContainerConfig::FORMAT_NAMENODES_CONFIG_VOLUME_MOUNT_NAME.to_string(), - ContainerConfig::FORMAT_NAMENODES_LOG_VOLUME_MOUNT_NAME.to_string(), - ), - name if name == NameNodeContainer::FormatZooKeeper.to_string() => ( - ContainerConfig::FORMAT_ZOOKEEPER_CONFIG_VOLUME_MOUNT_NAME.to_string(), - ContainerConfig::FORMAT_ZOOKEEPER_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. + 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), + config_mount: format!( + "{base}/{container_name}", + base = Self::NODE_BASE_CONFIG_DIR_MOUNT ), - // datanode init containers - name if name == DataNodeContainer::WaitForNameNodes.to_string() => ( - ContainerConfig::WAIT_FOR_NAMENODES_CONFIG_VOLUME_MOUNT_NAME.to_string(), - ContainerConfig::WAIT_FOR_NAMENODES_LOG_VOLUME_MOUNT_NAME.to_string(), + config_mount_name: config_mount_name.to_owned(), + log_mount: format!( + "{base}/{container_name}", + base = Self::NODE_BASE_LOG_DIR_MOUNT ), - _ => { - return Err(Error::UnrecognizedContainerName { - container_name: container_name.to_string(), - }); - } - }; - - let final_config_dir = - format!("{base}/{container_name}", base = Self::NODE_BASE_CONFIG_DIR); - let config_mount = format!( - "{base}/{container_name}", - base = Self::NODE_BASE_CONFIG_DIR_MOUNT - ); - let log_mount = format!( - "{base}/{container_name}", - base = Self::NODE_BASE_LOG_DIR_MOUNT - ); - - Ok(ContainerVolumeDirs { - final_config_dir, - config_mount, - config_mount_name, - log_mount, - log_mount_name, - }) + log_mount_name: log_mount_name.to_owned(), + } } } @@ -1638,3 +1596,18 @@ fn bash_capture_shell_helper(container_name: &str) -> String { "### } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *TLS_STORE_VOLUME_NAME; + let _ = *KERBEROS_VOLUME_NAME; + let _ = *VECTOR_CONTAINER_NAME; + let _ = *VECTOR_CONFIG_VOLUME_NAME; + let _ = *VECTOR_LOG_VOLUME_NAME; + } +} diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index 154373cb..8428ed10 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -33,13 +33,6 @@ pub enum Error { source: PropertiesWriterError, rolegroup: String, }, - - #[snafu(display("cannot build config map for role {role:?} and role group {role_group:?}"))] - Assemble { - source: stackable_operator::builder::configmap::Error, - role: String, - role_group: String, - }, } type Result = std::result::Result; @@ -118,8 +111,7 @@ pub fn build_rolegroup_config_map( ); } - builder.build().with_context(|_| AssembleSnafu { - role: role.to_string(), - role_group: role_group_name.to_string(), - }) + Ok(builder + .build() + .expect("The ConfigMap metadata is set in this function.")) } diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 337b9027..175de4ab 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -27,11 +27,6 @@ type Result = std::result::Result; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build ConfigMap"))] - BuildConfigMap { - source: stackable_operator::builder::configmap::Error, - }, - #[snafu(display("failed to collect the namenode listener refs"))] CollectListenerRefs { source: crate::crd::Error }, } @@ -88,7 +83,7 @@ pub fn build_discovery_config_map( build_discovery_core_site_xml(cluster, cluster_info), ) .build() - .context(BuildConfigMapSnafu)?; + .expect("The ConfigMap metadata is set in this function."); Ok(Some(config_map)) } diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 8d31c139..57dd3a34 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -21,11 +21,6 @@ use crate::{ #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build object meta data"))] - ObjectMeta { - source: stackable_operator::builder::meta::Error, - }, - #[snafu(display("failed to build roleGroup selector labels"))] RoleGroupSelectorLabels { source: LabelError }, } diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index dfc8e54d..9af18f9e 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -36,9 +36,6 @@ pub enum Error { #[snafu(display("failed to configure graceful shutdown"))] GracefulShutdown { source: graceful_shutdown::Error }, - - #[snafu(display("failed to build role-group volume claim templates from config"))] - BuildRoleGroupVolumeClaimTemplates { source: container::Error }, } pub(crate) fn build_rolegroup_statefulset( @@ -108,8 +105,7 @@ pub(crate) fn build_rolegroup_statefulset( pod_template.merge_from(rolegroup_config.pod_overrides.clone()); // The same comment regarding labels is valid here as it is for the ContainerConfig::add_containers_and_volumes() call above. - let pvcs = ContainerConfig::volume_claim_templates(merged_config, &rolegroup_selector_labels) - .context(BuildRoleGroupVolumeClaimTemplatesSnafu)?; + let pvcs = ContainerConfig::volume_claim_templates(merged_config, &rolegroup_selector_labels); let statefulset_spec = StatefulSetSpec { pod_management_policy: Some("OrderedReady".to_string()), diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 4db91552..afbbfedc 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -13,7 +13,7 @@ use stackable_operator::{ kube::{Resource, api::ObjectMeta}, v2::{ HasName, HasUid, NameIsValidLabelValue, - role_group_utils::ResourceNames, + role_group_utils::{QualifiedRoleGroupName, ResourceNames}, role_utils::{self, RoleGroupConfig}, types::{ kubernetes::{ConfigMapName, NamespaceName, ServiceName, Uid}, @@ -211,6 +211,13 @@ impl ValidatedCluster { role: &HdfsNodeRole, role_group_name: &RoleGroupName, ) -> ServiceName { + const _: () = assert!( + QualifiedRoleGroupName::MAX_LENGTH <= ServiceName::MAX_LENGTH, + "The string `` must not exceed the limit of Service names." + ); + let _ = QualifiedRoleGroupName::IS_RFC_1035_LABEL_NAME; + let _ = QualifiedRoleGroupName::IS_VALID_LABEL_VALUE; + ServiceName::from_str( self.role_group_resource_names(role, role_group_name) .qualified_role_group_name() @@ -337,3 +344,16 @@ impl ValidatedClusterConfig { pub struct ValidatedRoleConfig { pub pdb: stackable_operator::commons::pdb::PdbConfig, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *PRODUCT_NAME; + let _ = *OPERATOR_NAME; + let _ = *CONTROLLER_NAME; + } +} diff --git a/rust/operator-binary/src/crd/constants.rs b/rust/operator-binary/src/crd/constants.rs index 7afe1bf0..23d6850e 100644 --- a/rust/operator-binary/src/crd/constants.rs +++ b/rust/operator-binary/src/crd/constants.rs @@ -3,7 +3,10 @@ use std::str::FromStr; use stackable_operator::{ constant, shared::time::Duration, - v2::types::{common::Port, kubernetes::VolumeName}, + v2::types::{ + common::Port, + kubernetes::{ListenerClassName, VolumeName}, + }, }; pub const DEFAULT_DFS_REPLICATION_FACTOR: u8 = 3; @@ -23,7 +26,7 @@ pub const SERVICE_PORT_NAME_DATA: &str = "data"; pub const SERVICE_PORT_NAME_METRICS: &str = "metrics"; pub const SERVICE_PORT_NAME_JMX_METRICS: &str = "jmx-metrics"; -pub const DEFAULT_LISTENER_CLASS: &str = "cluster-internal"; +constant!(pub DEFAULT_LISTENER_CLASS: ListenerClassName = "cluster-internal"); pub const DEFAULT_NAME_NODE_METRICS_PORT: Port = Port(8183); pub const DEFAULT_NAME_NODE_NATIVE_METRICS_HTTP_PORT: Port = Port(9870); @@ -91,3 +94,15 @@ pub const DATANODE_ROOT_DATA_DIR_SUFFIX: &str = "/datanode"; constant!(pub LISTENER_VOLUME_NAME: VolumeName = "listener"); pub const LISTENER_VOLUME_DIR: &str = "/stackable/listener"; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *LISTENER_VOLUME_NAME; + let _ = *DEFAULT_LISTENER_CLASS; + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 8a6404b3..8c254786 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -19,10 +19,7 @@ use stackable_operator::{ PvcConfigFragment, Resources, ResourcesFragment, }, }, - config::{ - fragment::{Fragment, ValidationError}, - merge::Merge, - }, + config::{fragment::Fragment, merge::Merge}, constant, crd::listener, deep_merger::ObjectOverrides, @@ -32,7 +29,7 @@ use stackable_operator::{ self, spec::{ContainerLogConfig, Logging}, }, - role_utils::{self, GenericRoleConfig}, + role_utils::GenericRoleConfig, schemars::{self, JsonSchema}, shared::time::Duration, status::condition::{ClusterCondition, HasStatusCondition}, @@ -94,27 +91,15 @@ type Result = std::result::Result; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("object has no associated namespace"))] - NoNamespace, - - #[snafu(display("missing role {role:?}"))] - MissingRole { role: String }, - #[snafu(display("missing role group {role_group:?} for role {role:?}"))] MissingRoleGroup { role: String, role_group: String }, - #[snafu(display("fragment validation failure"))] - FragmentValidationFailure { source: ValidationError }, - #[snafu(display("port {port} ({port_name:?}) is out of bounds, must be within {range:?}", range = 0..=u16::MAX))] PortOutOfBounds { source: TryFromIntError, port_name: String, port: i32, }, - - #[snafu(display("failed to merge jvm argument overrides"))] - MergeJvmArgumentOverrides { source: role_utils::Error }, } #[versioned( @@ -701,12 +686,6 @@ pub enum NameNodeContainer { FormatZooKeeper, } -/// The default [`ListenerClassName`] used to expose a role group. -pub fn default_listener_class() -> ListenerClassName { - ListenerClassName::from_str(DEFAULT_LISTENER_CLASS) - .expect("the default listener class is a valid ListenerClassName") -} - #[derive(Clone, Debug, Fragment, JsonSchema, PartialEq)] #[fragment_attrs( derive( @@ -757,7 +736,7 @@ impl NameNodeConfigFragment { }, }, logging: product_logging::spec::default_logging(), - listener_class: Some(default_listener_class()), + listener_class: Some(DEFAULT_LISTENER_CLASS.clone()), common: CommonNodeConfigFragment { affinity: get_affinity(cluster_name, role), graceful_shutdown_timeout: Some(DEFAULT_NAME_NODE_GRACEFUL_SHUTDOWN_TIMEOUT), @@ -845,7 +824,7 @@ impl DataNodeConfigFragment { )]), }, logging: product_logging::spec::default_logging(), - listener_class: Some(default_listener_class()), + listener_class: Some(DEFAULT_LISTENER_CLASS.clone()), common: CommonNodeConfigFragment { affinity: get_affinity(cluster_name, role), graceful_shutdown_timeout: Some(DEFAULT_DATA_NODE_GRACEFUL_SHUTDOWN_TIMEOUT), @@ -973,6 +952,14 @@ mod test { .expect("storage should be defined") } + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *NAMENODE_ROLE_NAME; + let _ = *DATANODE_ROLE_NAME; + let _ = *JOURNALNODE_ROLE_NAME; + } + #[test] pub fn test_pvc_rolegroup_from_yaml() { let cr = " diff --git a/rust/operator-binary/src/crd/security.rs b/rust/operator-binary/src/crd/security.rs index cc111361..e2625920 100644 --- a/rust/operator-binary/src/crd/security.rs +++ b/rust/operator-binary/src/crd/security.rs @@ -3,6 +3,7 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use stackable_operator::{ commons::opa::OpaConfig, + constant, schemars::{self, JsonSchema}, v2::types::kubernetes::SecretClassName, }; @@ -17,8 +18,12 @@ pub struct AuthenticationConfig { pub kerberos: KerberosConfig, } +constant!(DEFAULT_TLS_SECRET_CLASS: SecretClassName = "tls"); + +/// Serde default for `tlsSecretClass`. Kept as a function because `#[serde(default = "...")]` +/// requires a function path. fn default_tls_secret_class() -> SecretClassName { - SecretClassName::from_str("tls").expect("\"tls\" should be a valid SecretClassName") + DEFAULT_TLS_SECRET_CLASS.clone() } #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] @@ -34,3 +39,14 @@ pub struct AuthorizationConfig { // No doc - it's in the struct. pub opa: OpaConfig, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *DEFAULT_TLS_SECRET_CLASS; + } +} diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index 0f358590..99a2eb07 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -10,7 +10,6 @@ use stackable_operator::{ core::{DeserializeGuard, error_boundary}, runtime::{controller::Action, events::Recorder}, }, - kvp::LabelError, logging::controller::ReconcilerError, shared::time::Duration, }; @@ -55,9 +54,6 @@ pub enum Error { #[snafu(display("failed to create cluster event"))] FailedToCreateClusterEvent { source: crate::event::Error }, - #[snafu(display("failed to build cluster resources label"))] - BuildClusterResourcesLabel { source: LabelError }, - #[snafu(display("HdfsCluster object is invalid"))] InvalidHdfsCluster { source: error_boundary::InvalidObject,