diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f9f1abd..c485c156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,13 @@ All notable changes to this project will be documented in this file. - Internal operator refactoring: introduce a build() step in the reconciler that assembles all relevant Kubernetes resources before anything is applied ([#801]). +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#806]). + - Bump stackable-operator to 0.114.0 ([#810]). [#801]: https://github.com/stackabletech/hdfs-operator/pull/801 +[#806]: https://github.com/stackabletech/hdfs-operator/pull/806 [#810]: https://github.com/stackabletech/hdfs-operator/pull/810 ## [26.7.0] - 2026-07-21 diff --git a/Cargo.toml b/Cargo.toml index 1859a75b..ceba5671 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,5 +30,5 @@ tracing = "0.1" tracing-futures = { version = "0.2", features = ["futures-03"] } [patch."https://github.com/stackabletech/operator-rs.git"] -#stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } -#stackable-operator = { path = "../operator-rs/crates/stackable-operator" } +# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } +# stackable-operator = { path = "../operator-rs/crates/stackable-operator" } diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 14bf10a6..be4ca4ad 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -237,7 +237,7 @@ impl ContainerConfig { // HDFS main container let main_container_config = Self::from(*role); - let resource_names = cluster.resource_names(role, role_group_name); + let resource_names = cluster.role_group_resource_names(role, role_group_name); let object_name = resource_names.qualified_role_group_name().to_string(); let merged_config = &rolegroup_config.config; @@ -285,7 +285,7 @@ impl ContainerConfig { log_config, vector_aggregator_config_map_name, }, - &cluster.resource_names(role, role_group_name), + &cluster.role_group_resource_names(role, role_group_name), &VECTOR_CONFIG_VOLUME_NAME, &VECTOR_LOG_VOLUME_NAME, EnvVarSet::new(), diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 9b18e795..a19c50d0 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, str::FromStr}; +use std::collections::HashMap; use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -7,13 +7,15 @@ use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::{ builder::meta::ownerreference_from_resource, - types::{common::Port, kubernetes::ServiceName, operator::RoleGroupName}, + types::{common::Port, operator::RoleGroupName}, }, }; use crate::{ - build_recommended_labels, - controller::{KubernetesResources, ValidatedCluster}, + controller::{ + KubernetesResources, ValidatedCluster, + build::resource::rbac::{build_role_binding, build_service_account}, + }, crd::{ HdfsNodeRole, HdfsPodRef, constants::{ @@ -32,7 +34,6 @@ use crate::{ SERVICE_PORT_NAME_RPC, }, }, - hdfs_controller::RESOURCE_MANAGER_HDFS_CONTROLLER, }; pub mod container; @@ -74,9 +75,6 @@ pub enum Error { /// `cluster_info` carries static cluster information resolved at operator startup (e.g. the /// cluster domain used to build Kerberos principals), not a live client. /// -/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under. -/// The RBAC resources are built and applied separately in the reconcile step. -/// /// The resources are returned as flat, unordered collections. The reconcile step re-groups the /// StatefulSets by role to preserve HDFS's ordered, rollout-gated deployment during upgrades. The /// discovery `ConfigMap` is deliberately not built here: it needs a live client to resolve @@ -84,7 +82,6 @@ pub enum Error { pub fn build( cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, - service_account_name: &str, ) -> Result { let mut services = vec![]; let mut config_maps = vec![]; @@ -126,7 +123,6 @@ pub fn build( role, role_group_name, rg_config, - service_account_name, ) .context(StatefulSetSnafu { role: *role, @@ -145,6 +141,8 @@ pub fn build( config_maps, pod_disruption_budgets, stateful_sets, + service_accounts: vec![build_service_account(cluster)], + role_bindings: vec![build_role_binding(cluster)], }) } @@ -168,15 +166,7 @@ pub(crate) fn pod_refs(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Vec Vec, + labels: Labels, +) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(cluster) + .name(name) + .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) + .with_labels(labels); + builder +} + /// Builds the common [`ObjectMetaBuilder`] shared by a role group's owned resources /// (the ConfigMap and the StatefulSet): name, namespace, owner reference and the /// recommended labels, all derived from the validated cluster. @@ -203,28 +209,14 @@ pub(crate) fn rolegroup_metadata( role: &HdfsNodeRole, role_group_name: &RoleGroupName, ) -> ObjectMetaBuilder { - let role_name = role.to_string(); - let mut metadata = ObjectMetaBuilder::new(); - metadata - .name_and_namespace(cluster) - .name( - cluster - .resource_names(role, role_group_name) - .qualified_role_group_name(), - ) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .expect( - "the recommended labels are valid because the ValidatedCluster uses \ - fail-safe typed values", - ); - metadata + object_meta( + cluster, + cluster + .role_group_resource_names(role, role_group_name) + .qualified_role_group_name() + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) } /// The rolegroup selector labels (also used as `Service`/`StatefulSet` selectors) for @@ -394,8 +386,7 @@ mod tests { #[test] fn build_produces_expected_resource_names() { let cluster = validated_cluster(); - let resources = - build(&cluster, &cluster_info(), "hdfs-serviceaccount").expect("build succeeds"); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); assert_eq!( sorted_names(&resources.stateful_sets), @@ -405,8 +396,19 @@ mod tests { "hdfs-namenode-default", ] ); - // One headless and one metrics Service per role group. - assert_eq!(resources.services.len(), 6); + // One headless (un-suffixed, see `ValidatedCluster::governing_service_name`) and one + // metrics Service per role group. + assert_eq!( + sorted_names(&resources.services), + [ + "hdfs-datanode-default", + "hdfs-datanode-default-metrics", + "hdfs-journalnode-default", + "hdfs-journalnode-default-metrics", + "hdfs-namenode-default", + "hdfs-namenode-default-metrics", + ] + ); assert_eq!( sorted_names(&resources.config_maps), [ @@ -420,5 +422,34 @@ mod tests { sorted_names(&resources.pod_disruption_budgets), ["hdfs-datanode", "hdfs-journalnode", "hdfs-namenode"] ); + // The cluster-shared RBAC pair. + assert_eq!( + sorted_names(&resources.service_accounts), + ["hdfs-serviceaccount"] + ); + assert_eq!(sorted_names(&resources.role_bindings), ["hdfs-rolebinding"]); + } + + /// Every StatefulSet's (immutable) `serviceName` must reference a headless Service that the + /// build step actually produces — the pods' DNS names depend on the pair agreeing. Guards the + /// coupling that `ValidatedCluster::governing_service_name` centralises. + #[test] + fn statefulset_service_name_references_built_service() { + let cluster = validated_cluster(); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); + + let service_names = sorted_names(&resources.services); + for stateful_set in &resources.stateful_sets { + let service_name = stateful_set + .spec + .as_ref() + .and_then(|spec| spec.service_name.as_deref()) + .expect("every StatefulSet sets serviceName"); + assert!( + service_names.iter().any(|name| name == service_name), + "StatefulSet references headless Service {service_name:?}, which is not built \ + (built Services: {service_names:?})" + ); + } } } diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 5db023af..44b52cb8 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -1,23 +1,29 @@ //! Build the discovery `ConfigMap` for the HdfsCluster. +use std::str::FromStr; + use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder}, + builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap, utils::cluster_info::KubernetesClusterInfo, - v2::builder::meta::ownerreference_from_resource, + v2::{ + kvp::label::recommended_labels, + types::operator::{ControllerName, RoleGroupName}, + }, }; use crate::{ - build_recommended_labels, controller::{ ValidatedCluster, build::{ kerberos::KerberosConfig, + object_meta, properties::{ ConfigFileName, core_site::CoreSiteConfigBuilder, hdfs_site::HdfsSiteConfigBuilder, }, }, + operator_name, product_name, }, crd::{HdfsNodeRole, HdfsPodRef}, hdfs_controller::HDFS_CONTROLLER_NAME, @@ -25,6 +31,8 @@ use crate::{ type Result = std::result::Result; +stackable_operator::constant!(DISCOVERY_ROLE_GROUP: RoleGroupName = "discovery"); + #[derive(Snafu, Debug)] #[allow(clippy::enum_variant_names)] pub enum Error { @@ -32,11 +40,6 @@ pub enum Error { BuildConfigMap { source: stackable_operator::builder::configmap::Error, }, - - #[snafu(display("failed to build object meta data"))] - ObjectMeta { - source: stackable_operator::builder::meta::Error, - }, } /// Creates a discovery config map containing the `hdfs-site.xml` and `core-site.xml` @@ -50,18 +53,22 @@ pub fn build_discovery_config_map( cluster_info: &KubernetesClusterInfo, namenode_podrefs: &[HdfsPodRef], ) -> Result { - let metadata = ObjectMetaBuilder::new() - .name_and_namespace(cluster) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - HDFS_CONTROLLER_NAME, - &cluster.image.app_version_label_value, - &HdfsNodeRole::Name.to_string(), - "discovery", - )) - .context(ObjectMetaSnafu)? - .build(); + // The discovery ConfigMap deliberately deviates from the standard resource identity: it is + // labelled with the `hdfs-controller` controller name (NOT `controller_name()`, i.e. + // `hdfs-operator-hdfs-controller` like the role-group resources), which keeps it outside their + // `ClusterResources` orphan-matching, and with the namenode role plus a `discovery` role-group. + let labels = recommended_labels( + cluster, + &product_name(), + &cluster.product_version, + &operator_name(), + &ControllerName::from_str(HDFS_CONTROLLER_NAME) + .expect("the hdfs controller name is a valid label value"), + &HdfsNodeRole::Name.into(), + &DISCOVERY_ROLE_GROUP, + ); + + let metadata = object_meta(cluster, cluster.name.clone(), labels).build(); ConfigMapBuilder::new() .metadata(metadata) diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 9c3b1b5a..9788477b 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -1,5 +1,6 @@ pub mod config_map; pub mod discovery; pub mod pdb; +pub mod rbac; pub mod service; pub mod statefulset; diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index de088c61..1251c729 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -1,7 +1,4 @@ -use std::{ - cmp::{max, min}, - str::FromStr, -}; +use std::cmp::{max, min}; use stackable_operator::{ k8s_openapi::api::policy::v1::PodDisruptionBudget, @@ -28,8 +25,7 @@ pub fn build_pdb(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Option max_unavailable_journal_nodes(), }); - let role_name = - RoleName::from_str(&role.to_string()).expect("a HdfsNodeRole is a valid role name"); + let role_name: RoleName = role.into(); let pdb = pod_disruption_budget_builder_with_role( cluster, &product_name(), diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs new file mode 100644 index 00000000..2e05dc7e --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -0,0 +1,142 @@ +//! Builds the RBAC resources (ServiceAccount + RoleBinding) shared by all role groups. + +use std::str::FromStr; + +use stackable_operator::{ + k8s_openapi::api::{core::v1::ServiceAccount, rbac::v1::RoleBinding}, + kvp::Labels, + v2::{ + rbac, + types::operator::{RoleGroupName, RoleName}, + }, +}; + +use crate::controller::ValidatedCluster; + +stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none"); +stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none"); + +/// Builds the [`ServiceAccount`] that the role-group Pods run under. +pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount { + rbac::build_service_account( + cluster, + &cluster.cluster_resource_names(), + rbac_labels(cluster), + ) +} + +/// Builds the [`RoleBinding`] that binds the [`ServiceAccount`] from [`build_service_account`] to +/// the operator-deployed ClusterRole. +pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding { + rbac::build_role_binding( + cluster, + &cluster.cluster_resource_names(), + rbac_labels(cluster), + ) +} + +/// Both resources are shared by the whole cluster rather than tied to a role or role group, so +/// the recommended labels carry `none` for both values. +fn rbac_labels(cluster: &ValidatedCluster) -> Labels { + cluster.recommended_labels_for(&NONE_ROLE_NAME, &NONE_ROLE_GROUP_NAME) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + use crate::{ + controller::build::properties::test_support::MINIMAL_HDFS_YAML, + test_support::{app_version_label, deserialize_and_validate_cluster}, + }; + + /// The cluster name is deliberately different from the product name (`hdfs`) so that + /// swapped `name`/`instance` label values cannot pass unnoticed. + fn swap_guard_cluster() -> crate::controller::ValidatedCluster { + deserialize_and_validate_cluster(&MINIMAL_HDFS_YAML.replace("name: hdfs", "name: my-hdfs")) + } + + #[test] + fn test_service_account() { + let service_account = build_service_account(&swap_guard_cluster()); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + // The RBAC resources are cluster-shared, so role and role group are `none`. + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "my-hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("3.4.0"), + "stackable.tech/vendor": "Stackable" + }, + "name": "my-hdfs-serviceaccount", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "my-hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + } + }), + serde_json::to_value(service_account).expect("must be serializable") + ); + } + + #[test] + fn test_role_binding() { + let role_binding = build_role_binding(&swap_guard_cluster()); + + assert_eq!( + json!({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "my-hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("3.4.0"), + "stackable.tech/vendor": "Stackable" + }, + "name": "my-hdfs-rolebinding", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "my-hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "hdfs-clusterrole" + }, + "subjects": [ + { + "kind": "ServiceAccount", + "name": "my-hdfs-serviceaccount", + "namespace": "default" + } + ] + }), + serde_json::to_value(role_binding).expect("must be serializable") + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 64662cc8..61a5863d 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -3,23 +3,17 @@ //! Prometheus scrape labels/annotations. use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::meta::ObjectMetaBuilder, k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}, kvp::LabelError, v2::{ - builder::{ - meta::ownerreference_from_resource, - service::{Scheme, Scraping, prometheus_annotations, prometheus_labels}, - }, + builder::service::{Scheme, Scraping, prometheus_annotations, prometheus_labels}, types::operator::RoleGroupName, }, }; use crate::{ - build_recommended_labels, controller::{ValidatedCluster, build}, crd::HdfsNodeRole, - hdfs_controller::RESOURCE_MANAGER_HDFS_CONTROLLER, }; #[derive(Snafu, Debug)] @@ -42,26 +36,6 @@ pub(crate) fn rolegroup_headless_service( ) -> Result { tracing::info!("Setting up headless Service for role {role} role group {role_group_name}"); - let resource_names = cluster.resource_names(role, role_group_name); - let role_name = role.to_string(); - // TODO: The v2 `ResourceNames::headless_service_name()` would add a `-headless` suffix, but - // we deliberately keep the un-suffixed name here so the StatefulSet's (immutable) `serviceName` - // and the pod DNS names stay unchanged for existing clusters. A decision is needed on whether - // to adopt the suffixed name (requires StatefulSet recreation on upgrade). - let mut metadata_builder = ObjectMetaBuilder::new(); - metadata_builder - .name_and_namespace(cluster) - .name(resource_names.qualified_role_group_name()) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .context(ObjectMetaSnafu)?; - let service_spec = ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -87,7 +61,14 @@ pub(crate) fn rolegroup_headless_service( }; Ok(Service { - metadata: metadata_builder.build(), + metadata: build::object_meta( + cluster, + cluster + .governing_service_name(role, role_group_name) + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) + .build(), spec: Some(service_spec), status: None, }) @@ -100,9 +81,6 @@ pub(crate) fn rolegroup_metrics_service( ) -> Result { tracing::info!("Setting up metrics Service for role {role} role group {role_group_name}"); - let resource_names = cluster.resource_names(role, role_group_name); - let role_name = role.to_string(); - let service_spec = ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -128,31 +106,114 @@ pub(crate) fn rolegroup_metrics_service( }; Ok(Service { - metadata: ObjectMetaBuilder::new() - .name_and_namespace(cluster) - .name(resource_names.metrics_service_name()) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .context(ObjectMetaSnafu)? - .with_labels(prometheus_labels(&Scraping::Enabled)) - .with_annotations(prometheus_annotations( - &Scraping::Enabled, - if cluster.has_https_enabled() { - &Scheme::Https - } else { - &Scheme::Http - }, - "/prom", - &build::native_metrics_port(cluster, role), - )) - .build(), + metadata: build::object_meta( + cluster, + cluster + .role_group_resource_names(role, role_group_name) + .metrics_service_name() + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) + .with_labels(prometheus_labels(&Scraping::Enabled)) + .with_annotations(prometheus_annotations( + &Scraping::Enabled, + if cluster.has_https_enabled() { + &Scheme::Https + } else { + &Scheme::Http + }, + "/prom", + &build::native_metrics_port(cluster, role), + )) + .build(), spec: Some(service_spec), status: None, }) } + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::v2::types::operator::RoleGroupName; + + use super::*; + use crate::{ + controller::build::properties::test_support::validated_cluster, crd::HdfsNodeRole, + test_support::app_version_label, + }; + + #[test] + fn test_rolegroup_metrics_service() { + let cluster = validated_cluster(); + let role = &HdfsNodeRole::Name; + let role_group_name: RoleGroupName = "default".parse().expect("valid role group name"); + + let service = + rolegroup_metrics_service(&cluster, role, &role_group_name).expect("should not fail"); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + // Every metrics Service must carry the Prometheus scrape label and the + // `prometheus.io/path|port|scheme|scrape` annotations, or Prometheus stops + // discovering the endpoints. + "annotations": { + "prometheus.io/path": "/prom", + "prometheus.io/port": "9870", + "prometheus.io/scheme": "http", + "prometheus.io/scrape": "true" + }, + "labels": { + "app.kubernetes.io/component": "namenode", + "app.kubernetes.io/instance": "hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "default", + "app.kubernetes.io/version": app_version_label("3.4.0"), + "prometheus.io/scrape": "true", + "stackable.tech/vendor": "Stackable" + }, + "name": "hdfs-namenode-default-metrics", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + }, + "spec": { + "clusterIP": "None", + "ports": [ + { + "name": "metrics", + "port": 9870, + "protocol": "TCP" + }, + { + "name": "jmx-metrics", + "port": 8183, + "protocol": "TCP" + } + ], + "publishNotReadyAddresses": true, + "selector": { + "app.kubernetes.io/component": "namenode", + "app.kubernetes.io/instance": "hdfs", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "default", + "group": "default", + "role": "namenode" + }, + "type": "ClusterIP" + } + }), + serde_json::to_value(service).expect("must be serializable") + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 6b7b32f2..590b5b7a 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -47,7 +47,6 @@ pub(crate) fn build_rolegroup_statefulset( role: &HdfsNodeRole, role_group_name: &RoleGroupName, rolegroup_config: &ValidatedRoleGroupConfig, - service_account_name: &str, ) -> Result { tracing::info!("Setting up StatefulSet for role {role} role group {role_group_name}"); @@ -69,7 +68,12 @@ pub(crate) fn build_rolegroup_statefulset( pb.metadata(pb_metadata) .image_pull_secrets_from_product_image(image) .affinity(&merged_config.affinity) - .service_account_name(service_account_name) + .service_account_name( + validated + .cluster_resource_names() + .service_account_name() + .to_string(), + ) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); // Adds all containers and volumes to the pod builder @@ -107,13 +111,9 @@ pub(crate) fn build_rolegroup_statefulset( match_labels: Some(rolegroup_selector_labels.into()), ..LabelSelector::default() }, - // Must match the headless Service name so the StatefulSet's pods get stable DNS names. - // See the TODO in `build::resource::service::rolegroup_headless_service` about the - // un-suffixed name. service_name: Some( validated - .resource_names(role, role_group_name) - .qualified_role_group_name() + .governing_service_name(role, role_group_name) .to_string(), ), template: pod_template, diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 800d6157..2f351d7a 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -4,18 +4,22 @@ use stackable_operator::{ commons::product_image_selection::ResolvedProductImage, k8s_openapi::api::{ apps::v1::StatefulSet, - core::v1::{ConfigMap, Service}, + core::v1::{ConfigMap, Service, ServiceAccount}, policy::v1::PodDisruptionBudget, + rbac::v1::RoleBinding, }, kube::{Resource, api::ObjectMeta}, + kvp::Labels, v2::{ HasName, HasUid, NameIsValidLabelValue, + kvp::label::recommended_labels, role_group_utils::ResourceNames, - role_utils::RoleGroupConfig, + role_utils::{self, RoleGroupConfig}, types::{ - kubernetes::{ConfigMapName, NamespaceName, Uid}, + kubernetes::{ConfigMapName, NamespaceName, ServiceName, Uid}, operator::{ - ClusterName, ControllerName, OperatorName, ProductName, RoleGroupName, RoleName, + ClusterName, ControllerName, OperatorName, ProductName, ProductVersion, + RoleGroupName, RoleName, }, }, }, @@ -47,6 +51,8 @@ pub struct KubernetesResources { pub config_maps: Vec, pub pod_disruption_budgets: Vec, pub stateful_sets: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// The [`RoleGroupConfig`] specialised for HDFS: the validated config is the @@ -73,6 +79,7 @@ pub struct ValidatedCluster { pub namespace: NamespaceName, /// The cluster's Kubernetes UID, used to build owner references. pub uid: Uid, + pub product_version: ProductVersion, pub image: ResolvedProductImage, pub cluster_config: ValidatedClusterConfig, pub role_groups: BTreeMap>, @@ -94,6 +101,10 @@ impl ValidatedCluster { role_configs: BTreeMap, status: ValidatedClusterStatus, ) -> Self { + // `app_version_label_value` is constructed to be a valid label value, so it is also a valid + // `ProductVersion`. + let product_version = ProductVersion::from_str(&image.app_version_label_value) + .expect("the app version label value is a valid product version"); Self { metadata: ObjectMeta { name: Some(name.to_string()), @@ -107,6 +118,7 @@ impl ValidatedCluster { namespace, uid, image, + product_version, cluster_config, role_groups, role_configs, @@ -135,22 +147,85 @@ impl ValidatedCluster { } /// The type-safe role name for an HDFS role (`namenode`/`datanode`/`journalnode`). - pub(crate) fn role_name(role: &HdfsNodeRole) -> RoleName { - RoleName::from_str(&role.to_string()).expect("a HdfsNodeRole is a valid role name") + /// Type-safe names for the per-cluster RBAC resources: the ServiceAccount shared by all + /// Pods, its (namespaced) RoleBinding, and the operator-deployed ClusterRole it binds. + pub fn cluster_resource_names(&self) -> role_utils::ResourceNames { + role_utils::ResourceNames { + cluster_name: self.name.clone(), + product_name: product_name(), + } } /// Type-safe names for the resources of the given role group. - pub(crate) fn resource_names( + pub(crate) fn role_group_resource_names( &self, role: &HdfsNodeRole, role_group_name: &RoleGroupName, ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: Self::role_name(role), + role_name: role.into(), role_group_name: role_group_name.clone(), } } + + /// Recommended labels for a resource that is not tied to a concrete [`HdfsNodeRole`], using a free-form role/role-group label value. + pub fn recommended_labels_for( + &self, + role_name: &RoleName, + role_group_name: &RoleGroupName, + ) -> Labels { + self.recommended_labels_with(&self.product_version, role_name, role_group_name) + } + + fn recommended_labels_with( + &self, + product_version: &ProductVersion, + role_name: &RoleName, + role_group_name: &RoleGroupName, + ) -> Labels { + recommended_labels( + self, + &product_name(), + product_version, + &operator_name(), + &controller_name(), + role_name, + role_group_name, + ) + } + + /// Recommended labels for a role-group resource. + pub fn recommended_labels( + &self, + role: &HdfsNodeRole, + role_group_name: &RoleGroupName, + ) -> Labels { + self.recommended_labels_for(&role.into(), role_group_name) + } + + /// The name of a role group's governing headless Service. + /// + /// Used as the headless Service's own name, as the StatefulSet's (immutable) `serviceName`, + /// and for the pod DNS references derived from them; the three reference each other, so they + /// must be derived here and nowhere else. + // + // TODO: The v2 `ResourceNames::headless_service_name()` would add a `-headless` suffix, but we + // deliberately keep the un-suffixed name so the StatefulSet's (immutable) `serviceName` and the + // pod DNS names stay unchanged for existing clusters. A decision is needed on whether to adopt + // the suffixed name (requires StatefulSet recreation on upgrade). + pub(crate) fn governing_service_name( + &self, + role: &HdfsNodeRole, + role_group_name: &RoleGroupName, + ) -> ServiceName { + ServiceName::from_str( + self.role_group_resource_names(role, role_group_name) + .qualified_role_group_name() + .as_ref(), + ) + .expect("a qualified role group name is a valid Service name") + } } /// The product name (`hdfs`) as a type-safe label value. @@ -169,8 +244,7 @@ pub(crate) fn controller_name() -> ControllerName { .expect("the controller name is a valid label value") } -/// Lets [`ValidatedCluster`] be used as the owner [`Resource`] (e.g. in -/// [`ObjectMetaBuilder::ownerreference_from_resource`]). The kind/group/version/plural +/// Lets [`ValidatedCluster`] be used as the owner [`Resource`]. The kind/group/version/plural /// are delegated to [`v1alpha1::HdfsCluster`] so the generated owner references are /// identical to the ones built from the raw cluster object. impl Resource for ValidatedCluster { @@ -287,3 +361,21 @@ impl ValidatedClusterConfig { pub struct ValidatedRoleConfig { pub pdb: stackable_operator::commons::pdb::PdbConfig, } + +#[cfg(test)] +mod tests { + use stackable_operator::v2::types::operator::RoleName; + use strum::IntoEnumIterator; + + use crate::crd::HdfsNodeRole; + + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `HdfsNodeRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_hdfs_node_role_serialises_to_a_valid_role_name() { + for role in HdfsNodeRole::iter() { + let _: RoleName = (&role).into(); + let _: RoleName = role.into(); + } + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 5747df6e..75fe5c84 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -43,6 +43,7 @@ use stackable_operator::{ types::{ common::Port, kubernetes::{ConfigMapName, ListenerClassName, NamespaceName, ServiceName}, + operator::RoleName, }, }, versioned::versioned, @@ -497,6 +498,18 @@ pub enum HdfsNodeRole { Data, } +impl From for RoleName { + fn from(value: HdfsNodeRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a HdfsNodeRole is a valid role name") + } +} + +impl From<&HdfsNodeRole> for RoleName { + fn from(value: &HdfsNodeRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a HdfsNodeRole is a valid role name") + } +} + impl HdfsNodeRole { pub fn min_replicas(&self) -> u16 { match self { diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index 2df9ac2e..5b94a220 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -5,7 +5,6 @@ use stackable_operator::{ cli::OperatorEnvironmentOptions, client::Client, cluster_resources::ClusterResourceApplyStrategy, - commons::rbac::build_rbac_resources, iter::reverse_if, kube::{ Resource, ResourceExt, @@ -45,6 +44,11 @@ pub const HDFS_CONTROLLER_NAME: &str = "hdfs-controller"; #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(IntoStaticStr))] pub enum Error { + #[snafu(display("failed to apply Kubernetes resource"))] + ApplyResource { + source: stackable_operator::cluster_resources::Error, + }, + #[snafu(display("failed to dereference cluster resources"))] Dereference { source: crate::controller::dereference::Error, @@ -55,18 +59,6 @@ pub enum Error { source: crate::controller::validate::Error, }, - #[snafu(display("cannot create rolegroup service {name:?}"))] - ApplyRoleGroupService { - source: stackable_operator::cluster_resources::Error, - name: String, - }, - - #[snafu(display("cannot create role group config map {name:?}"))] - ApplyRoleGroupConfigMap { - source: stackable_operator::cluster_resources::Error, - name: String, - }, - #[snafu(display("cannot create role group stateful set {name:?}"))] ApplyRoleGroupStatefulSet { source: stackable_operator::cluster_resources::Error, @@ -90,16 +82,6 @@ pub enum Error { #[snafu(display("cannot build config discovery config map"))] BuildDiscoveryConfigMap { source: discovery::Error }, - #[snafu(display("failed to patch service account"))] - ApplyServiceAccount { - source: stackable_operator::cluster_resources::Error, - }, - - #[snafu(display("failed to patch role binding"))] - ApplyRoleBinding { - source: stackable_operator::cluster_resources::Error, - }, - #[snafu(display("failed to delete orphaned resources"))] DeleteOrphanedResources { source: stackable_operator::cluster_resources::Error, @@ -118,11 +100,6 @@ pub enum Error { source: stackable_operator::client::Error, }, - #[snafu(display("failed to build RBAC resources"))] - BuildRbacResources { - source: stackable_operator::commons::rbac::Error, - }, - #[snafu(display("failed to build cluster resources label"))] BuildClusterResourcesLabel { source: LabelError }, @@ -181,57 +158,44 @@ pub async fn reconcile_hdfs( &hdfs.spec.object_overrides, ); - // The service account and rolebinding will be created per cluster - let (rbac_sa, rbac_rolebinding) = build_rbac_resources( - hdfs, - APP_NAME, - cluster_resources - .get_required_labels() - .context(BuildClusterResourcesLabelSnafu)?, - ) - .context(BuildRbacResourcesSnafu)?; - - cluster_resources - .add(client, rbac_sa.clone()) - .await - .context(ApplyServiceAccountSnafu)?; - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - // Build every (non-discovery) Kubernetes resource up front. This step needs no client: all // external references are already dereferenced and validated. The ServiceAccount name is // deterministic on the built RBAC object, so the build does not depend on the applied one. - let resources = build::build( - &validated_cluster, - &client.kubernetes_cluster_info, - &rbac_sa.name_any(), - ) - .context(BuildResourcesSnafu)?; + let resources = build::build(&validated_cluster, &client.kubernetes_cluster_info) + .context(BuildResourcesSnafu)?; // Apply Services, ConfigMaps and PodDisruptionBudgets first. The StatefulSets are applied // afterwards so that every ConfigMap a Pod mounts already exists, which prevents unnecessary // Pod restarts. See https://github.com/stackabletech/commons-operator/issues/111 for details. + for service_account in resources.service_accounts { + cluster_resources + .add(client, service_account) + .await + .context(ApplyResourceSnafu)?; + } + for role_binding in resources.role_bindings { + cluster_resources + .add(client, role_binding) + .await + .context(ApplyResourceSnafu)?; + } for service in resources.services { - let name = service.name_any(); cluster_resources .add(client, service) .await - .with_context(|_| ApplyRoleGroupServiceSnafu { name })?; + .context(ApplyResourceSnafu)?; } for config_map in resources.config_maps { - let name = config_map.name_any(); cluster_resources .add(client, config_map) .await - .with_context(|_| ApplyRoleGroupConfigMapSnafu { name })?; + .context(ApplyResourceSnafu)?; } for pdb in resources.pod_disruption_budgets { cluster_resources .add(client, pdb) .await - .context(ApplyPdbSnafu)?; + .context(ApplyResourceSnafu)?; } let upgrade_state = validated_cluster.status.upgrade_state; diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index a09ca370..fe6d1c25 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -28,7 +28,6 @@ use stackable_operator::{ reflector, watcher, }, }, - kvp::ObjectLabels, logging::controller::report_controller_reconciled, shared::yaml::SerializeOptions, telemetry::Tracing, @@ -38,7 +37,7 @@ use tracing::info_span; use tracing_futures::Instrument; use crate::{ - crd::{HdfsCluster, HdfsClusterVersion, constants::APP_NAME, v1alpha1}, + crd::{HdfsCluster, HdfsClusterVersion, v1alpha1}, webhooks::conversion::create_webhook_server, }; @@ -223,25 +222,6 @@ async fn main() -> anyhow::Result<()> { Ok(()) } -/// Creates recommended `ObjectLabels` to be used in deployed resources -pub fn build_recommended_labels<'a, T>( - owner: &'a T, - controller_name: &'a str, - app_version: &'a str, - role: &'a str, - role_group: &'a str, -) -> ObjectLabels<'a, T> { - ObjectLabels { - owner, - app_name: APP_NAME, - app_version, - operator_name: OPERATOR_NAME, - controller_name, - role, - role_group, - } -} - fn references_config_map( hdfs: &DeserializeGuard, config_map: &DeserializeGuard, diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs index 61a02307..4f47b3b9 100644 --- a/rust/operator-binary/src/test_support.rs +++ b/rust/operator-binary/src/test_support.rs @@ -7,6 +7,18 @@ use crate::{ crd::{AnyNodeConfig, DataNodeConfig, HdfsNodeRole, v1alpha1}, }; +/// The expected `app.kubernetes.io/version` label value for the given product version. +/// +/// The `-stackable` suffix carries the operator's own version, which is `0.0.0-dev` on main +/// but rewritten by the release process — so tests must derive it rather than hardcode it, +/// or they fail on release branches. +pub fn app_version_label(product_version: &str) -> String { + format!( + "{product_version}-stackable{}", + crate::built_info::PKG_VERSION + ) +} + pub fn deserialize_cluster(spec: &str) -> v1alpha1::HdfsCluster { let deserializer = serde_yaml::Deserializer::from_str(spec); serde_yaml::with::singleton_map_recursive::deserialize(deserializer).expect("")