diff --git a/CHANGELOG.md b/CHANGELOG.md index c7794a53..8fdb622c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ All notable changes to this project will be documented in this file. environment variables by name, so an override replaces the operator's value instead of producing a duplicated entry whose precedence depended on Kubernetes' duplicate-name handling ([#1077]). +- Make operations infallible where dependent on static inputs ([#1084]). ### Fixed @@ -48,6 +49,7 @@ All notable changes to this project will be documented in this file. [#1070]: https://github.com/stackabletech/zookeeper-operator/pull/1070 [#1077]: https://github.com/stackabletech/zookeeper-operator/pull/1077 [#1079]: https://github.com/stackabletech/zookeeper-operator/pull/1079 +[#1084]: https://github.com/stackabletech/zookeeper-operator/pull/1084 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 8f1c18de..2973b491 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -1,13 +1,14 @@ use stackable_operator::{ commons::affinity::{StackableAffinityFragment, affinity_between_role_pods}, k8s_openapi::api::core::v1::PodAntiAffinity, + v2::types::operator::ClusterName, }; use crate::crd::{APP_NAME, ZookeeperRole}; -pub fn get_affinity(cluster_name: &str, role: &ZookeeperRole) -> StackableAffinityFragment { +pub fn get_affinity(cluster_name: &ClusterName, role: &ZookeeperRole) -> StackableAffinityFragment { let affinity_between_role_pods = - affinity_between_role_pods(APP_NAME, cluster_name, role.as_ref(), 70); + affinity_between_role_pods(APP_NAME, cluster_name.as_ref(), role.as_ref(), 70); StackableAffinityFragment { pod_affinity: None, diff --git a/rust/operator-binary/src/crd/authentication.rs b/rust/operator-binary/src/crd/authentication.rs index 93b0a31c..30a087c8 100644 --- a/rust/operator-binary/src/crd/authentication.rs +++ b/rust/operator-binary/src/crd/authentication.rs @@ -125,7 +125,8 @@ impl DereferencedAuthenticationClasses { Ok(self.clone()) } - /// USE ONLY IN TESTS! We can not put it behind `#[cfg(test)]` because of + /// Test fixture without any AuthenticationClasses. + #[cfg(test)] pub fn new_for_tests() -> Self { DereferencedAuthenticationClasses { dereferenced_authentication_classes: vec![], diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 8eac203d..fc81e092 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -16,7 +16,7 @@ use stackable_operator::{ crd::ClusterRef, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::api::resource::Quantity, - kube::{CustomResource, ResourceExt}, + kube::CustomResource, product_logging::{self, spec::Logging}, role_utils::GenericRoleConfig, schemars::{self, JsonSchema}, @@ -31,7 +31,7 @@ use stackable_operator::{ kubernetes::{ ConfigMapName, ListenerClassName, ListenerName, NamespaceName, ServiceName, }, - operator::{OperatorName, ProductName, RoleName}, + operator::{ClusterName, OperatorName, ProductName, RoleName}, }, }, versioned::versioned, @@ -49,10 +49,37 @@ pub mod tls; /// exposing the given `zk_role`, `-`. /// /// Lives in the `crd` module (rather than the controller build tree) because it is shared by both -/// controllers and by [`v1alpha1::ZookeeperCluster::server_role_listener_fqdn`]. -pub fn role_listener_name(cluster_name: &str, zk_role: &ZookeeperRole) -> ListenerName { - ListenerName::from_str(&format!("{cluster_name}-{role}", role = zk_role.as_ref())) - .expect("the role listener name should be a valid Listener name") +/// controllers and by [`role_listener_fqdn`]. +/// +/// The returned ListenerName is a lowercase RFC 1035 label name (checked by a unit test). +pub fn role_listener_name(cluster_name: &ClusterName, zk_role: &ZookeeperRole) -> ListenerName { + const _: () = assert!( + ClusterName::MAX_LENGTH + 1 /* dash */ + RoleName::MAX_LENGTH <= ListenerName::MAX_LENGTH, + "The string `-` must not exceed the limit of Listener names." + ); + // Both halves are RFC 1123 labels joined by a dash, which is a valid RFC 1123 subdomain. + let _ = ClusterName::IS_RFC_1123_SUBDOMAIN_NAME; + let _ = RoleName::IS_RFC_1123_LABEL_NAME; + + let role_name: &RoleName = zk_role; + ListenerName::from_str(&format!("{cluster_name}-{role_name}")) + .expect("is a valid Listener name") +} + +/// The fully-qualified domain name of the role-level +/// [`Listener`](stackable_operator::crd::listener::v1alpha1::Listener) exposing the given +/// `zk_role`, `-..svc.`. +pub fn role_listener_fqdn( + cluster_name: &ClusterName, + namespace: &NamespaceName, + zk_role: &ZookeeperRole, + cluster_info: &KubernetesClusterInfo, +) -> String { + format!( + "{role_listener_name}.{namespace}.svc.{cluster_domain}", + role_listener_name = role_listener_name(cluster_name, zk_role), + cluster_domain = cluster_info.cluster_domain + ) } pub const APP_NAME: &str = "zookeeper"; @@ -87,7 +114,7 @@ pub const STACKABLE_RW_CONFIG_DIR: &str = "/stackable/rwconfig"; pub const CONTAINER_IMAGE_BASE_NAME: &str = "zookeeper"; const DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(2); -pub const DEFAULT_LISTENER_CLASS: &str = "cluster-internal"; +constant!(pub DEFAULT_LISTENER_CLASS: ListenerClassName = "cluster-internal"); pub type ZookeeperServerRoleType = Role< v1alpha1::ZookeeperConfigFragment, @@ -315,7 +342,7 @@ pub mod versioned { } } -#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Debug, Eq, EnumIter, Hash, Ord, PartialEq, PartialOrd)] pub enum ZookeeperRole { Server, } @@ -358,8 +385,7 @@ fn cluster_config_default() -> v1alpha1::ZookeeperClusterConfig { } pub(crate) fn default_listener_class() -> ListenerClassName { - ListenerClassName::from_str(DEFAULT_LISTENER_CLASS) - .expect("the default listener class should be a valid ListenerClass name") + DEFAULT_LISTENER_CLASS.clone() } impl Default for ZookeeperServerRoleConfig { @@ -380,7 +406,7 @@ impl v1alpha1::ZookeeperConfig { pub const TICK_TIME: &'static str = "tickTime"; pub(crate) fn default_server_config( - cluster_name: &str, + cluster_name: &ClusterName, role: &ZookeeperRole, ) -> v1alpha1::ZookeeperConfigFragment { v1alpha1::ZookeeperConfigFragment { @@ -435,21 +461,6 @@ impl ZookeeperPodRef { } impl v1alpha1::ZookeeperCluster { - /// The fully-qualified domain name of the role-level [Listener] - /// - /// [Listener]: stackable_operator::crd::listener::v1alpha1::Listener - pub fn server_role_listener_fqdn( - &self, - cluster_info: &KubernetesClusterInfo, - ) -> Option { - Some(format!( - "{role_listener_name}.{namespace}.svc.{cluster_domain}", - role_listener_name = role_listener_name(&self.name_any(), &ZookeeperRole::Server), - namespace = self.metadata.namespace.as_ref()?, - cluster_domain = cluster_info.cluster_domain - )) - } - /// Returns the given role (the `servers` role is required by the CRD). pub fn role(&self, role_variant: &ZookeeperRole) -> &ZookeeperServerRoleType { match role_variant { @@ -466,7 +477,10 @@ impl v1alpha1::ZookeeperCluster { #[cfg(test)] mod tests { - use stackable_operator::versioned::test_utils::RoundtripTestData; + use stackable_operator::{ + commons::networking::DomainName, versioned::test_utils::RoundtripTestData, + }; + use strum::IntoEnumIterator; use super::*; @@ -476,6 +490,26 @@ mod tests { let _ = *PRODUCT_NAME; let _ = *OPERATOR_NAME; let _ = *SERVER_ROLE_NAME; + let _ = *DEFAULT_LISTENER_CLASS; + } + + #[test] + fn role_listener_fqdn_joins_name_namespace_and_cluster_domain() { + let cluster_name = ClusterName::from_str("simple-zookeeper").expect("valid cluster name"); + let namespace = NamespaceName::from_str("default").expect("valid namespace"); + let cluster_info = KubernetesClusterInfo { + cluster_domain: DomainName::from_str("cluster.local").expect("valid domain"), + }; + + assert_eq!( + role_listener_fqdn( + &cluster_name, + &namespace, + &ZookeeperRole::Server, + &cluster_info + ), + "simple-zookeeper-server.default.svc.cluster.local" + ); } fn get_server_secret_class(zk: &v1alpha1::ZookeeperCluster) -> Option<&str> { @@ -743,4 +777,22 @@ mod tests { .expect("Failed to parse ZookeeperZnodeSpec YAML") } } + + #[test] + fn role_listener_name_is_rfc_1035_label_name() { + // Every ClusterName is a valid RFC 1035 label name, so we use just some string with maximum + // length. + let _ = ClusterName::IS_RFC_1035_LABEL_NAME; + let cluster_name = ClusterName::from_str_unsafe(&"a".repeat(ClusterName::MAX_LENGTH)); + + for role in ZookeeperRole::iter() { + let role_listener_name = role_listener_name(&cluster_name, &role); + assert!( + stackable_operator::validation::is_lowercase_rfc_1035_label( + role_listener_name.as_ref() + ) + .is_ok() + ); + } + } } diff --git a/rust/operator-binary/src/crd/security.rs b/rust/operator-binary/src/crd/security.rs index 5ed28ce2..4211a2e7 100644 --- a/rust/operator-binary/src/crd/security.rs +++ b/rust/operator-binary/src/crd/security.rs @@ -51,11 +51,6 @@ pub enum Error { #[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, - }, } /// Helper struct combining TLS settings for server and quorum with the resolved AuthenticationClasses @@ -151,6 +146,11 @@ impl ZookeeperSecurity { /// Adds required volumes and volume mounts to the pod and container builders /// depending on the tls and authentication settings. + /// + /// # Panics + /// + /// Panics if the volume mounts cannot be added to the container builder. Only call this on a + /// container builder whose mount paths are still distinct from the ones added here. pub fn add_volume_mounts( &self, pod_builder: &mut PodBuilder, @@ -162,7 +162,9 @@ impl ZookeeperSecurity { if let Some(secret_class) = tls_secret_class { cb_zookeeper .add_volume_mount(&*SERVER_TLS_VOLUME_NAME, Self::SERVER_TLS_DIR) - .context(AddVolumeMountSnafu)?; + .expect( + "The mount paths are statically defined and there should be no duplicates.", + ); pod_builder .add_volume(Self::create_server_tls_volume( &SERVER_TLS_VOLUME_NAME, @@ -175,7 +177,7 @@ impl ZookeeperSecurity { // quorum cb_zookeeper .add_volume_mount(&*QUORUM_TLS_VOLUME_NAME, Self::QUORUM_TLS_DIR) - .context(AddVolumeMountSnafu)?; + .expect("The mount paths are statically defined and there should be no duplicates."); pod_builder .add_volume(Self::create_quorum_tls_volume( &QUORUM_TLS_VOLUME_NAME, @@ -376,18 +378,6 @@ impl ZookeeperSecurity { Ok(volume) } - - /// USE ONLY IN TESTS! We can not put it behind `#[cfg(test)]` because of - pub fn new_for_tests() -> Self { - ZookeeperSecurity { - resolved_authentication_classes: DereferencedAuthenticationClasses::new_for_tests(), - server_secret_class: Some( - SecretClassName::from_str("tls").expect("'tls' is a valid SecretClass name"), - ), - quorum_secret_class: SecretClassName::from_str("tls") - .expect("'tls' is a valid SecretClass name"), - } - } } #[cfg(test)] diff --git a/rust/operator-binary/src/crd/tls.rs b/rust/operator-binary/src/crd/tls.rs index 64c24b5d..c2e8a65b 100644 --- a/rust/operator-binary/src/crd/tls.rs +++ b/rust/operator-binary/src/crd/tls.rs @@ -2,12 +2,13 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use stackable_operator::{ + constant, schemars::{self, JsonSchema}, v2::types::kubernetes::SecretClassName, versioned::versioned, }; -const TLS_DEFAULT_SECRET_CLASS: &str = "tls"; +constant!(TLS_DEFAULT_SECRET_CLASS: SecretClassName = "tls"); #[versioned(version(name = "v1alpha1"))] pub mod versioned { @@ -53,6 +54,16 @@ pub fn server_tls_default() -> Option { /// Helper methods to provide defaults in the CRDs and tests pub fn quorum_tls_default() -> SecretClassName { - SecretClassName::from_str(TLS_DEFAULT_SECRET_CLASS) - .expect("the default TLS secret class should be a valid SecretClass name") + TLS_DEFAULT_SECRET_CLASS.clone() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constant does not panic. + let _ = *TLS_DEFAULT_SECRET_CLASS; + } } diff --git a/rust/operator-binary/src/zk_controller/build/resource/listener.rs b/rust/operator-binary/src/zk_controller/build/resource/listener.rs index 70b546d9..95d70049 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/listener.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/listener.rs @@ -24,7 +24,7 @@ pub fn build_role_listener( listener::v1alpha1::Listener { metadata: object_meta( cluster, - role_listener_name(cluster.name.as_ref(), zk_role), + role_listener_name(&cluster.name, zk_role), recommended_labels_for_role_resources(cluster, zk_role), ) .build(), diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index a2f9c693..57cff615 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -121,7 +121,6 @@ fn container_command() -> Vec { } #[derive(Snafu, Debug)] -#[allow(clippy::enum_variant_names)] pub enum Error { #[snafu(display("missing secret lifetime"))] MissingSecretLifetime, @@ -132,11 +131,6 @@ pub enum Error { #[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("failed to construct JVM arguments"))] ConstructJvmArguments { source: crate::zk_controller::build::jvm::Error, @@ -217,16 +211,19 @@ pub fn build_server_rolegroup_statefulset( recommended_labels_for_unversioned_role_group_resources(cluster, zk_role, role_group_name); let listener_pvc = build_role_listener_pvc( - role_listener_name(cluster.name.as_ref(), zk_role), + role_listener_name(&cluster.name, zk_role), &unversioned_recommended_labels, ); let mut pvcs = original_pvcs; pvcs.extend([listener_pvc]); + // Every mount path below is an operator-defined constant, so the mounts cannot collide with + // each other and adding them is infallible. Adding the volumes stays fallible, because the + // volumes are built from computed arguments (ConfigMap names, log volume size). cb_zookeeper .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR) - .context(AddVolumeMountSnafu)?; + .expect("The mount paths are statically defined and there should be no duplicates."); let requested_secret_lifetime = merged_config .requested_secret_lifetime @@ -259,13 +256,13 @@ pub fn build_server_rolegroup_statefulset( .args(vec![args.join("\n")]) .add_env_vars(prepare_env_vars) .add_volume_mount(&*DATA_VOLUME_NAME, STACKABLE_DATA_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*CONFIG_VOLUME_NAME, STACKABLE_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*RW_CONFIG_VOLUME_NAME, STACKABLE_RW_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*LOG_VOLUME_NAME, STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources( ResourceRequirementsBuilder::new() .with_cpu_request("200m") @@ -325,15 +322,15 @@ pub fn build_server_rolegroup_statefulset( .add_container_port(JMX_METRICS_PORT_NAME, i32::from(JMX_METRICS_PORT)) .add_container_port(METRICS_PROVIDER_HTTP_PORT_NAME, metrics_port.into()) .add_volume_mount(&*DATA_VOLUME_NAME, STACKABLE_DATA_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*CONFIG_VOLUME_NAME, STACKABLE_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*LOG_CONFIG_VOLUME_NAME, STACKABLE_LOG_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*RW_CONFIG_VOLUME_NAME, STACKABLE_RW_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(&*LOG_VOLUME_NAME, STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(resources) .build(); diff --git a/rust/operator-binary/src/zk_controller/dereference.rs b/rust/operator-binary/src/zk_controller/dereference.rs index ce80dee7..c4384a90 100644 --- a/rust/operator-binary/src/zk_controller/dereference.rs +++ b/rust/operator-binary/src/zk_controller/dereference.rs @@ -85,7 +85,7 @@ async fn fetch_role_listener( cluster_name: &ClusterName, namespace: &NamespaceName, ) -> Result> { - let listener_name = role_listener_name(cluster_name.as_ref(), &ZookeeperRole::Server); + let listener_name = role_listener_name(cluster_name, &ZookeeperRole::Server); client .get_opt(listener_name.as_ref(), namespace.as_ref()) diff --git a/rust/operator-binary/src/zk_controller/validate.rs b/rust/operator-binary/src/zk_controller/validate.rs index 8d80cbea..aad9c318 100644 --- a/rust/operator-binary/src/zk_controller/validate.rs +++ b/rust/operator-binary/src/zk_controller/validate.rs @@ -22,7 +22,7 @@ use stackable_operator::{ config::fragment, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta, - kube::{Resource, ResourceExt}, + kube::Resource, product_logging::spec::Logging, shared::time::Duration, v2::{ @@ -395,9 +395,13 @@ pub fn validate( .vector_aggregator_config_map_name .clone(); + let name = get_cluster_name(zk).context(GetClusterNameSnafu)?; + let namespace = get_namespace(zk).context(GetNamespaceSnafu)?; + let uid = get_uid(zk).context(GetUidSnafu)?; + let zk_role = ZookeeperRole::Server; let role = zk.role(&zk_role); - let default_config = ZookeeperConfig::default_server_config(&zk.name_any(), &zk_role); + let default_config = ZookeeperConfig::default_server_config(&name, &zk_role); let mut groups = BTreeMap::new(); for (rg_name, rg) in &role.role_groups { @@ -416,10 +420,6 @@ pub fn validate( } let role_group_configs = BTreeMap::from([(zk_role, groups)]); - let name = get_cluster_name(zk).context(GetClusterNameSnafu)?; - let namespace = get_namespace(zk).context(GetNamespaceSnafu)?; - let uid = get_uid(zk).context(GetUidSnafu)?; - let product_version = ProductVersion::from_str(&image.app_version_label_value).with_context(|_| { ParseProductVersionSnafu { diff --git a/rust/operator-binary/src/znode_controller.rs b/rust/operator-binary/src/znode_controller.rs index 190be624..301905df 100644 --- a/rust/operator-binary/src/znode_controller.rs +++ b/rust/operator-binary/src/znode_controller.rs @@ -9,7 +9,7 @@ use std::{borrow::Cow, convert::Infallible, str::FromStr, sync::Arc}; use const_format::concatcp; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ cli::OperatorEnvironmentOptions, cluster_resources::ClusterResourceApplyStrategy, @@ -23,14 +23,17 @@ use stackable_operator::{ logging::controller::ReconcilerError, shared::time::Duration, utils::cluster_info::KubernetesClusterInfo, - v2::types::operator::ControllerName, + v2::types::{ + kubernetes::NamespaceName, + operator::{ClusterName, ControllerName}, + }, }; use strum::{EnumDiscriminants, IntoStaticStr}; use tracing::{debug, info}; use crate::{ ZOOKEEPER_OPERATOR_NAME, - crd::{security::ZookeeperSecurity, v1alpha1}, + crd::{ZookeeperRole, role_listener_fqdn, security::ZookeeperSecurity, v1alpha1}, znode_controller::apply::{Applier, ensure_znode_exists}, }; @@ -70,11 +73,6 @@ pub enum Error { ))] ObjectMissingMetadata, - #[snafu(display("failed to calculate FQDN for {zk:?}"))] - NoZkFqdn { - zk: ObjectRef, - }, - #[snafu(display("failed to ensure that ZNode {znode_path:?} is missing from {zk:?}"))] EnsureZnodeMissing { source: znode_mgmt::Error, @@ -132,7 +130,6 @@ impl ReconcilerError for Error { Error::Dereference { .. } => None, Error::ValidateCluster { .. } => None, Error::ObjectMissingMetadata => None, - Error::NoZkFqdn { zk } => Some(zk.clone().erase()), Error::EnsureZnodeMissing { zk, .. } => Some(zk.clone().erase()), Error::BuildResources { .. } => None, Error::ApplyResources { .. } => None, @@ -216,7 +213,7 @@ pub async fn reconcile_znode( let validated_znode = validate::validate(&znode, &dereferenced, &ctx.operator_environment) .context(ValidateClusterSnafu)?; - reconcile_apply(client, &validated_znode, dereferenced.zk, &znode_path).await + reconcile_apply(client, &validated_znode, &dereferenced, &znode_path).await } finalizer::Event::Cleanup(_znode) => { let dereferenced = match dereferenced_objects { @@ -234,7 +231,7 @@ pub async fn reconcile_znode( &dereferenced.zk, dereferenced.authentication_classes.clone(), ); - reconcile_cleanup(client, dereferenced.zk, &zookeeper_security, &znode_path) + reconcile_cleanup(client, &dereferenced, &zookeeper_security, &znode_path) .await } } @@ -247,16 +244,17 @@ pub async fn reconcile_znode( async fn reconcile_apply( client: &stackable_operator::client::Client, validated_znode: &validate::ValidatedZnode, - zk: v1alpha1::ZookeeperCluster, + dereferenced: &dereference::DereferencedObjects, znode_path: &str, ) -> Result { // The znode must exist in the ZooKeeper ensemble before the discovery ConfigMap advertises it. ensure_znode_exists( &zk_mgmt_addr( - &zk, + &dereferenced.zk_name, + &dereferenced.zk_namespace, &validated_znode.zookeeper_security, &client.kubernetes_cluster_info, - )?, + ), znode_path, ) .await @@ -281,18 +279,23 @@ async fn reconcile_apply( async fn reconcile_cleanup( client: &stackable_operator::client::Client, - zk: v1alpha1::ZookeeperCluster, + dereferenced: &dereference::DereferencedObjects, zookeeper_security: &ZookeeperSecurity, znode_path: &str, ) -> Result { // Clean up znode from the ZooKeeper cluster before letting Kubernetes delete the object znode_mgmt::ensure_znode_missing( - &zk_mgmt_addr(&zk, zookeeper_security, &client.kubernetes_cluster_info)?, + &zk_mgmt_addr( + &dereferenced.zk_name, + &dereferenced.zk_namespace, + zookeeper_security, + &client.kubernetes_cluster_info, + ), znode_path, ) .await .with_context(|_| EnsureZnodeMissingSnafu { - zk: ObjectRef::from_obj(&zk), + zk: ObjectRef::from_obj(&dereferenced.zk), znode_path, })?; // No need to delete the ConfigMap, since that has an OwnerReference on the ZookeeperZnode object @@ -310,21 +313,18 @@ async fn reconcile_cleanup( // NOTE (@NickLarsenNZ): If we want to keep this traffic internal, we would need to choose one of // the RoleGroups headless services - or make a dedicated ClusterIP service for the operator to use. fn zk_mgmt_addr( - zk: &v1alpha1::ZookeeperCluster, + zk_name: &ClusterName, + zk_namespace: &NamespaceName, zookeeper_security: &ZookeeperSecurity, cluster_info: &KubernetesClusterInfo, -) -> Result { +) -> String { // Rust ZooKeeper client does not support client-side load-balancing, so use // (load-balanced) global service instead. - Ok(format!( + format!( "{hostname}:{port}", - hostname = zk - .server_role_listener_fqdn(cluster_info) - .with_context(|| NoZkFqdnSnafu { - zk: ObjectRef::from_obj(zk), - })?, + hostname = role_listener_fqdn(zk_name, zk_namespace, &ZookeeperRole::Server, cluster_info), port = zookeeper_security.client_port(), - )) + ) } pub fn error_policy( @@ -338,7 +338,10 @@ pub fn error_policy( /// Shared helpers for building validated test znodes from minimal YAML fixtures. #[cfg(test)] pub(crate) mod test_support { - use stackable_operator::crd::listener; + use stackable_operator::{ + crd::listener, + v2::controller_utils::{get_cluster_name, get_namespace}, + }; use crate::{ crd::{authentication::DereferencedAuthenticationClasses, v1alpha1}, @@ -391,10 +394,13 @@ pub(crate) mod test_support { znode: &v1alpha1::ZookeeperZnode, maybe_role_listener: Option, ) -> Result { + let zk = referenced_zk(); validate( znode, &DereferencedObjects { - zk: referenced_zk(), + zk_name: get_cluster_name(&zk).expect("the fixture has a valid cluster name"), + zk_namespace: get_namespace(&zk).expect("the fixture has a namespace"), + zk, authentication_classes: DereferencedAuthenticationClasses::new_for_tests(), maybe_role_listener, }, diff --git a/rust/operator-binary/src/znode_controller/dereference.rs b/rust/operator-binary/src/znode_controller/dereference.rs index 1d0a4055..35ccef83 100644 --- a/rust/operator-binary/src/znode_controller/dereference.rs +++ b/rust/operator-binary/src/znode_controller/dereference.rs @@ -5,11 +5,15 @@ //! cluster. Both Apply and Cleanup paths in `reconcile_znode` share this output. Synchronous //! validation of the fetched objects happens in the validate step. -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ client::Client, crd::listener, - kube::{self, ResourceExt, runtime::reflector::ObjectRef}, + kube::{self, runtime::reflector::ObjectRef}, + v2::{ + controller_utils::{get_cluster_name, get_namespace}, + types::{kubernetes::NamespaceName, operator::ClusterName}, + }, }; use crate::crd::{ @@ -38,8 +42,15 @@ pub enum Error { #[snafu(display("failed to fetch authentication classes"))] FetchAuthenticationClasses { source: authentication::Error }, - #[snafu(display("{zk} has no namespace"))] - ZkHasNoNamespace { + #[snafu(display("failed to get the cluster name of {zk}"))] + GetClusterName { + source: stackable_operator::v2::controller_utils::Error, + zk: ObjectRef, + }, + + #[snafu(display("failed to get the namespace of {zk}"))] + GetNamespace { + source: stackable_operator::v2::controller_utils::Error, zk: ObjectRef, }, @@ -55,6 +66,10 @@ type Result = std::result::Result; /// Kubernetes objects referenced from the [`v1alpha1::ZookeeperZnode`] spec, already fetched. pub struct DereferencedObjects { pub zk: v1alpha1::ZookeeperCluster, + /// The referenced cluster's name and namespace as typed values, from which the role Listener + /// name and the management address are derived. + pub zk_name: ClusterName, + pub zk_namespace: NamespaceName, pub authentication_classes: DereferencedAuthenticationClasses, /// The role Listener of the referenced cluster, if it exists already. @@ -71,6 +86,11 @@ pub async fn dereference( znode: &v1alpha1::ZookeeperZnode, ) -> Result { let zk = find_zk_of_znode(client, znode).await?; + let zk_ref = ObjectRef::from_obj(&zk); + let zk_name = + get_cluster_name(&zk).with_context(|_| GetClusterNameSnafu { zk: zk_ref.clone() })?; + let zk_namespace = + get_namespace(&zk).with_context(|_| GetNamespaceSnafu { zk: zk_ref.clone() })?; let authentication_classes = DereferencedAuthenticationClasses::fetch_references( client, @@ -79,10 +99,12 @@ pub async fn dereference( .await .context(FetchAuthenticationClassesSnafu)?; - let maybe_role_listener = fetch_role_listener(client, &zk).await?; + let maybe_role_listener = fetch_role_listener(client, &zk_name, &zk_namespace, zk_ref).await?; Ok(DereferencedObjects { zk, + zk_name, + zk_namespace, authentication_classes, maybe_role_listener, }) @@ -90,18 +112,14 @@ pub async fn dereference( async fn fetch_role_listener( client: &Client, - zk: &v1alpha1::ZookeeperCluster, + zk_name: &ClusterName, + zk_namespace: &NamespaceName, + zk_ref: ObjectRef, ) -> Result> { - let zk_ref = ObjectRef::from_obj(zk); - let namespace = zk - .metadata - .namespace - .as_deref() - .with_context(|| ZkHasNoNamespaceSnafu { zk: zk_ref.clone() })?; - let listener_name = role_listener_name(&zk.name_any(), &ZookeeperRole::Server); + let listener_name = role_listener_name(zk_name, &ZookeeperRole::Server); client - .get_opt(listener_name.as_ref(), namespace) + .get_opt(listener_name.as_ref(), zk_namespace.as_ref()) .await .with_context(|_| FetchRoleListenerSnafu { zk: zk_ref }) }