From 80b8c79298ab91617ddb558312e87323eea86afb Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 16 Oct 2026 11:21:25 +0200 Subject: [PATCH 01/13] temporary diff-harness test --- .../src/controller/build/kerberos.rs | 2 +- .../src/controller/build/mod.rs | 350 +++++++++++++++++- .../controller/build/properties/hbase_env.rs | 12 +- .../controller/build/properties/hbase_site.rs | 6 +- .../src/controller/build/resource/listener.rs | 2 +- .../controller/build/resource/statefulset.rs | 14 +- rust/operator-binary/src/test_utils.rs | 14 +- 7 files changed, 379 insertions(+), 21 deletions(-) diff --git a/rust/operator-binary/src/controller/build/kerberos.rs b/rust/operator-binary/src/controller/build/kerberos.rs index 17f9ca7c..fc72e8d2 100644 --- a/rust/operator-binary/src/controller/build/kerberos.rs +++ b/rust/operator-binary/src/controller/build/kerberos.rs @@ -31,7 +31,7 @@ pub const STACKABLE_KERBEROS_DIR: &str = "/stackable/kerberos"; /// `KRB5_CONFIG` env var) and by the JVM args builder. pub const KRB5_CONFIG_PATH: &str = const_format::concatcp!(STACKABLE_KERBEROS_DIR, "/krb5.conf"); // Name of the Kerberos secret volume. -constant!(KERBEROS_VOLUME_NAME: VolumeName = "kerberos"); +constant!(pub(crate) KERBEROS_VOLUME_NAME: VolumeName = "kerberos"); // The env var pointing the Kerberos libraries at the rendered `krb5.conf`. constant!(KRB5_CONFIG_ENV: EnvVarName = "KRB5_CONFIG"); /// The RPC/data-transfer quality-of-protection level used when Kerberos is enabled. diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 396a38d9..77cded2b 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -211,10 +211,354 @@ pub mod role; mod tests { use std::collections::BTreeMap; - use stackable_operator::kube::Resource; + use stackable_operator::{ + k8s_openapi::api::{apps::v1::StatefulSetSpec, core::v1::PodSpec}, + kube::Resource, + }; + use strum::IntoEnumIterator; - use super::build; - use crate::test_utils; + use super::{ + build, + kerberos::KERBEROS_VOLUME_NAME, + opa::HbaseOpaConfig, + properties::{ + ConfigFileName, + hbase_env::{HBASE_MASTER_OPTS, HBASE_REGIONSERVER_OPTS, HBASE_REST_OPTS}, + hbase_site::{ + HBASE_MASTER_IPC_ADDRESS, HBASE_REGIONSERVER_IPC_ADDRESS, HBASE_REST_ENDPOINT, + }, + }, + resource::{ + listener::LISTENER_PVC_NAME, + statefulset::{ + HBASE_CONFIG_VOLUME_NAME, HBASE_CONTAINER_NAME, HDFS_DISCOVERY_VOLUME_NAME, + LOG_CONFIG_VOLUME_NAME, LOG_VOLUME_NAME, REGION_MOVER_OPTS_ENV, + RUN_REGION_MOVER_ENV, + }, + }, + }; + use crate::{ + controller::{ + KubernetesResources, Prepared, ValidatedCluster, dereference::DereferencedObjects, + zookeeper::ZookeeperConnectionInformation, + }, + crd::{HbaseRole, LISTENER_VOLUME_NAME, TLS_STORE_VOLUME_NAME}, + test_utils, + }; + + /// A cluster that takes every role-specific build path at once: Kerberos with TLS (the + /// `kerberos` and `tls` pod volumes), OPA authorization, the Vector agent on every role, a + /// custom log ConfigMap on one master role group, overrides for the three rendered config + /// files, the environment, the pod template and the JVM arguments, the region mover enabled + /// on the region servers, rest servers, and two master role groups (so + /// `hbase.client.bootstrap.servers` lists more than one group). + const RICH_HBASE_YAML: &str = r#" +--- +apiVersion: hbase.stackable.tech/v1alpha1 +kind: HbaseCluster +metadata: + name: hbase + namespace: default + uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f +spec: + image: + productVersion: 2.6.3 + clusterConfig: + hdfsConfigMapName: simple-hdfs + zookeeperConfigMapName: simple-znode + vectorAggregatorConfigMapName: vector-aggregator-discovery + authentication: + tlsSecretClass: tls + kerberos: + secretClass: kerberos + authorization: + opa: + configMapName: opa + package: hbase + masters: + config: + logging: + enableVectorAgent: true + configOverrides: + hbase-site.xml: + hbase.master.custom.setting: from-role-override + roleGroups: + default: + replicas: 2 + configOverrides: + hbase-env.sh: + HBASE_CUSTOM_ENV: from-role-group-override + security.properties: + networkaddress.cache.negative.ttl: "1" + envOverrides: + CUSTOM_ENV: custom-value + podOverrides: + metadata: + labels: + custom-label: custom-value + jvmArgumentOverrides: + add: + - -Dcustom.jvm.argument=true + other: + replicas: 1 + config: + logging: + enableVectorAgent: true + containers: + hbase: + custom: + configMap: hbase-log-config + regionServers: + config: + logging: + enableVectorAgent: true + regionMover: + runBeforeShutdown: true + roleGroups: + default: + replicas: 2 + restServers: + config: + logging: + enableVectorAgent: true + roleGroups: + default: + replicas: 1 +"#; + + /// [`RICH_HBASE_YAML`], validated with a dereferenced ZooKeeper connection and OPA endpoint. + fn rich_validated_cluster() -> ValidatedCluster { + test_utils::validated_cluster_with( + &test_utils::hbase_from_yaml(RICH_HBASE_YAML), + DereferencedObjects { + zookeeper_connection_information: ZookeeperConnectionInformation::for_tests(), + hbase_opa_config: Some(HbaseOpaConfig::new( + "http://opa.default.svc.cluster.local:8081/v1/data/hbase/allow", + )), + }, + ) + } + + /// Not a test: dumps every built resource as JSON to `$RENDER_DUMP`, so the refactoring can be + /// diffed against the baseline. Run with `cargo test dump_rendered_resources -- --ignored`. + #[test] + #[ignore = "not a test: writes the rendered resources to $RENDER_DUMP for the render diff"] + fn dump_rendered_resources() { + let path = std::env::var("RENDER_DUMP").expect("RENDER_DUMP should be set"); + let resources = + build(&rich_validated_cluster(), &test_utils::cluster_info()).expect("build succeeds"); + let dump = serde_json::json!({ + "services": resources.services, + "config_maps": resources.config_maps, + "pod_disruption_budgets": resources.pod_disruption_budgets, + "stateful_sets": resources.stateful_sets, + "service_accounts": resources.service_accounts, + "role_bindings": resources.role_bindings, + }); + std::fs::write( + path, + serde_json::to_string_pretty(&dump).expect("the resources should serialise"), + ) + .expect("the dump should be written"); + } + + /// The `default` role group's StatefulSet spec for `role`, found by the name the builder + /// derives from the cluster. + fn stateful_set_spec<'a>( + resources: &'a KubernetesResources, + cluster: &ValidatedCluster, + role: &HbaseRole, + ) -> &'a StatefulSetSpec { + let name = cluster + .role_group_resource_names(role, &test_utils::role_group_name("default")) + .stateful_set_name() + .to_string(); + resources + .stateful_sets + .iter() + .find(|stateful_set| stateful_set.metadata.name.as_deref() == Some(name.as_str())) + .and_then(|stateful_set| stateful_set.spec.as_ref()) + .expect("the role group's StatefulSet should be built with a spec") + } + + fn pod_spec(spec: &StatefulSetSpec) -> &PodSpec { + spec.template + .spec + .as_ref() + .expect("the pod template should have a spec") + } + + /// The `hbase` container's environment variables, by name. + fn hbase_container_env(spec: &StatefulSetSpec) -> BTreeMap { + pod_spec(spec) + .containers + .iter() + .find(|container| container.name == HBASE_CONTAINER_NAME.to_string()) + .expect("the hbase container should exist") + .env + .iter() + .flatten() + .map(|env_var| { + ( + env_var.name.clone(), + env_var.value.clone().unwrap_or_default(), + ) + }) + .collect() + } + + /// The pod volume names, in the order the pod template lists them. + fn pod_volume_names(spec: &StatefulSetSpec) -> Vec { + pod_spec(spec) + .volumes + .iter() + .flatten() + .map(|volume| volume.name.clone()) + .collect() + } + + fn claim_template_names(spec: &StatefulSetSpec) -> Vec { + spec.volume_claim_templates + .iter() + .flatten() + .filter_map(|claim| claim.metadata.name.clone()) + .collect() + } + + /// The `default` role group's ConfigMap data for `role`, found by the name the builder + /// derives from the cluster. + fn config_map_data<'a>( + resources: &'a KubernetesResources, + cluster: &ValidatedCluster, + role: &HbaseRole, + ) -> &'a BTreeMap { + let name = cluster + .role_group_resource_names(role, &test_utils::role_group_name("default")) + .role_group_config_map() + .to_string(); + resources + .config_maps + .iter() + .find(|config_map| config_map.metadata.name.as_deref() == Some(name.as_str())) + .and_then(|config_map| config_map.data.as_ref()) + .expect("the role group's ConfigMap should be built with data") + } + + /// RestServers get their listener from a volume claim template; Masters and RegionServers + /// from an ephemeral pod volume. Both at once for one role would collide by name at apply + /// time. + #[test] + fn the_listener_is_a_claim_template_only_for_rest_servers() { + let cluster = rich_validated_cluster(); + let resources = build(&cluster, &test_utils::cluster_info()).expect("build succeeds"); + + for role in HbaseRole::iter() { + let claim_template = matches!(role, HbaseRole::RestServer); + let spec = stateful_set_spec(&resources, &cluster, &role); + assert_eq!( + pod_volume_names(spec).contains(&LISTENER_VOLUME_NAME.to_string()), + !claim_template, + "{role:?} listener pod volume" + ); + assert_eq!( + claim_template_names(spec).contains(&LISTENER_PVC_NAME.to_string()), + claim_template, + "{role:?} listener claim template" + ); + } + } + + /// The entrypoint runs under `set -u`, so every role carries both region-mover variables; + /// only RegionServers get values that run the mover (the fixture enables it for them). + #[test] + fn every_role_sets_the_region_mover_variables() { + let cluster = rich_validated_cluster(); + let resources = build(&cluster, &test_utils::cluster_info()).expect("build succeeds"); + + for role in HbaseRole::iter() { + let runs_the_mover = matches!(role, HbaseRole::RegionServer); + let env = hbase_container_env(stateful_set_spec(&resources, &cluster, &role)); + assert_eq!( + env.get(®ION_MOVER_OPTS_ENV.to_string()) + .map(|options| !options.is_empty()), + Some(runs_the_mover), + "{role:?} {} is set, and non-empty only for region servers", + *REGION_MOVER_OPTS_ENV + ); + assert_eq!( + env.get(&RUN_REGION_MOVER_ENV.to_string()) + .map(String::as_str), + Some(if runs_the_mover { "true" } else { "false" }), + "{role:?} {}", + *RUN_REGION_MOVER_ENV + ); + } + } + + /// Each role's `hbase-site.xml` carries only its own bind settings, and its `hbase-env.sh` + /// its own `*_OPTS` variable. + #[test] + fn each_role_gets_only_its_own_site_keys_and_opts_variable() { + let cluster = rich_validated_cluster(); + let resources = build(&cluster, &test_utils::cluster_info()).expect("build succeeds"); + + // The hbase-site key that identifies each role's own section, and its OPTS variable. + let own = |role: &HbaseRole| match role { + HbaseRole::Master => (HBASE_MASTER_IPC_ADDRESS, HBASE_MASTER_OPTS), + HbaseRole::RegionServer => (HBASE_REGIONSERVER_IPC_ADDRESS, HBASE_REGIONSERVER_OPTS), + HbaseRole::RestServer => (HBASE_REST_ENDPOINT, HBASE_REST_OPTS), + }; + for role in HbaseRole::iter() { + let data = config_map_data(&resources, &cluster, &role); + let hbase_site = &data[&ConfigFileName::HbaseSite.to_string()]; + let hbase_env = &data[&ConfigFileName::HbaseEnv.to_string()]; + let (own_key, own_opts) = own(&role); + + assert!( + hbase_site.contains(own_key), + "{role:?} hbase-site has {own_key}" + ); + for other in HbaseRole::iter().filter(|other| *other != role) { + let (foreign_key, _) = own(&other); + assert!( + !hbase_site.contains(foreign_key), + "{role:?} hbase-site must not have {foreign_key}" + ); + } + assert!( + hbase_env.contains(&format!("export {own_opts}=")), + "{role:?} hbase-env sets {own_opts}" + ); + } + } + + /// The pod volume order is part of the pod template: changing it restarts every pod. The + /// operator volumes come first, then the Kerberos and TLS volumes, then the ephemeral + /// listener volume of Masters and RegionServers (RestServers have a claim template instead). + #[test] + fn pod_volumes_keep_their_order() { + let cluster = rich_validated_cluster(); + let resources = build(&cluster, &test_utils::cluster_info()).expect("build succeeds"); + + for role in HbaseRole::iter() { + let mut expected: Vec = vec![ + HBASE_CONFIG_VOLUME_NAME.to_string(), + HDFS_DISCOVERY_VOLUME_NAME.to_string(), + LOG_VOLUME_NAME.to_string(), + LOG_CONFIG_VOLUME_NAME.to_string(), + KERBEROS_VOLUME_NAME.to_string(), + TLS_STORE_VOLUME_NAME.to_string(), + ]; + if matches!(role, HbaseRole::Master | HbaseRole::RegionServer) { + expected.push(LISTENER_VOLUME_NAME.to_string()); + } + assert_eq!( + pod_volume_names(stateful_set_spec(&resources, &cluster, &role)), + expected, + "{role:?} pod volumes" + ); + } + } /// The expected `app.kubernetes.io/version` label value for the given product version. /// diff --git a/rust/operator-binary/src/controller/build/properties/hbase_env.rs b/rust/operator-binary/src/controller/build/properties/hbase_env.rs index 8c0af6c5..41756b21 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_env.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_env.rs @@ -10,6 +10,12 @@ use crate::{ crd::{AnyServiceConfig, HbaseRole}, }; +/// The `hbase-env.sh` variable that carries a role's own JVM options; `bin/hbase` reads the one +/// matching the process it starts. +pub(crate) const HBASE_MASTER_OPTS: &str = "HBASE_MASTER_OPTS"; +pub(crate) const HBASE_REGIONSERVER_OPTS: &str = "HBASE_REGIONSERVER_OPTS"; +pub(crate) const HBASE_REST_OPTS: &str = "HBASE_REST_OPTS"; + #[derive(Snafu, Debug)] pub enum Error { #[snafu(display("failed to construct the HBASE_HEAPSIZE env variable"))] @@ -38,13 +44,13 @@ pub fn build( match role { HbaseRole::Master => { - env.insert("HBASE_MASTER_OPTS".to_string(), non_heap_jvm_args); + env.insert(HBASE_MASTER_OPTS.to_string(), non_heap_jvm_args); } HbaseRole::RegionServer => { - env.insert("HBASE_REGIONSERVER_OPTS".to_string(), non_heap_jvm_args); + env.insert(HBASE_REGIONSERVER_OPTS.to_string(), non_heap_jvm_args); } HbaseRole::RestServer => { - env.insert("HBASE_REST_OPTS".to_string(), non_heap_jvm_args); + env.insert(HBASE_REST_OPTS.to_string(), non_heap_jvm_args); } } diff --git a/rust/operator-binary/src/controller/build/properties/hbase_site.rs b/rust/operator-binary/src/controller/build/properties/hbase_site.rs index bb33ab64..d39388ce 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_site.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_site.rs @@ -22,19 +22,19 @@ use crate::{ // `hbase-site.xml` property keys (the `port` keys carry a `_KEY` suffix to avoid clashing with the // `Port`-typed `HBASE_*_PORT` constants imported from the `crd` module). const HBASE_CLIENT_RPC_BIND_ADDRESS: &str = "hbase.client.rpc.bind.address"; -const HBASE_MASTER_IPC_ADDRESS: &str = "hbase.master.ipc.address"; +pub(crate) const HBASE_MASTER_IPC_ADDRESS: &str = "hbase.master.ipc.address"; const HBASE_MASTER_IPC_PORT: &str = "hbase.master.ipc.port"; const HBASE_MASTER_HOSTNAME: &str = "hbase.master.hostname"; const HBASE_MASTER_PORT_KEY: &str = "hbase.master.port"; const HBASE_MASTER_INFO_PORT: &str = "hbase.master.info.port"; const HBASE_MASTER_BOUND_INFO_PORT: &str = "hbase.master.bound.info.port"; -const HBASE_REGIONSERVER_IPC_ADDRESS: &str = "hbase.regionserver.ipc.address"; +pub(crate) const HBASE_REGIONSERVER_IPC_ADDRESS: &str = "hbase.regionserver.ipc.address"; const HBASE_REGIONSERVER_IPC_PORT: &str = "hbase.regionserver.ipc.port"; const HBASE_UNSAFE_REGIONSERVER_HOSTNAME: &str = "hbase.unsafe.regionserver.hostname"; const HBASE_REGIONSERVER_PORT_KEY: &str = "hbase.regionserver.port"; const HBASE_REGIONSERVER_INFO_PORT: &str = "hbase.regionserver.info.port"; const HBASE_REGIONSERVER_BOUND_INFO_PORT: &str = "hbase.regionserver.bound.info.port"; -const HBASE_REST_ENDPOINT: &str = "hbase.rest.endpoint"; +pub(crate) const HBASE_REST_ENDPOINT: &str = "hbase.rest.endpoint"; // `hbase-site.xml` property values that recur across roles. The `${env:...}` placeholders are // resolved by HBase at runtime from the Pod's environment. diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index deb9c6fa..71d40c65 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -22,7 +22,7 @@ use stackable_operator::{ use crate::crd::{AnyServiceConfig, HbaseRole, LISTENER_VOLUME_NAME}; -constant!(LISTENER_PVC_NAME: PersistentVolumeClaimName = LISTENER_VOLUME_NAME); +constant!(pub(crate) LISTENER_PVC_NAME: PersistentVolumeClaimName = LISTENER_VOLUME_NAME); #[derive(Snafu, Debug)] pub enum Error { diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 3d50b39c..7709b1bc 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -48,23 +48,23 @@ use crate::{ crd::{CONFIG_DIR_NAME, HbaseRole, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME}, }; -constant!(HBASE_CONTAINER_NAME: ContainerName = "hbase"); +constant!(pub(crate) HBASE_CONTAINER_NAME: ContainerName = "hbase"); constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector"); // Pod volume names. The Vector container reuses the `hbase-config` (rolegroup ConfigMap, which // carries `vector.yaml`) and `log` volumes, so the produced volume mounts match the rest of the // Pod. -constant!(HBASE_CONFIG_VOLUME_NAME: VolumeName = "hbase-config"); -constant!(HDFS_DISCOVERY_VOLUME_NAME: VolumeName = "hdfs-discovery"); -constant!(LOG_CONFIG_VOLUME_NAME: VolumeName = "log-config"); -constant!(LOG_VOLUME_NAME: VolumeName = "log"); +constant!(pub(crate) HBASE_CONFIG_VOLUME_NAME: VolumeName = "hbase-config"); +constant!(pub(crate) HDFS_DISCOVERY_VOLUME_NAME: VolumeName = "hdfs-discovery"); +constant!(pub(crate) LOG_CONFIG_VOLUME_NAME: VolumeName = "log-config"); +constant!(pub(crate) LOG_VOLUME_NAME: VolumeName = "log"); // Environment variable names set on the HBase container. Declared as typed constants (instead of // `EnvVarName::from_str_unsafe` at the use site) and validated by `test_constants`. constant!(HBASE_CONF_DIR_ENV: EnvVarName = "HBASE_CONF_DIR"); constant!(HADOOP_CONF_DIR_ENV: EnvVarName = "HADOOP_CONF_DIR"); -constant!(REGION_MOVER_OPTS_ENV: EnvVarName = "REGION_MOVER_OPTS"); -constant!(RUN_REGION_MOVER_ENV: EnvVarName = "RUN_REGION_MOVER"); +constant!(pub(crate) REGION_MOVER_OPTS_ENV: EnvVarName = "REGION_MOVER_OPTS"); +constant!(pub(crate) RUN_REGION_MOVER_ENV: EnvVarName = "RUN_REGION_MOVER"); constant!(STACKABLE_LOG_DIR_ENV: EnvVarName = "STACKABLE_LOG_DIR"); constant!(CONTAINERDEBUG_LOG_DIRECTORY_ENV: EnvVarName = "CONTAINERDEBUG_LOG_DIRECTORY"); diff --git a/rust/operator-binary/src/test_utils.rs b/rust/operator-binary/src/test_utils.rs index 998e4eb3..5b327bfa 100644 --- a/rust/operator-binary/src/test_utils.rs +++ b/rust/operator-binary/src/test_utils.rs @@ -62,15 +62,23 @@ pub fn minimal_hbase() -> v1alpha1::HbaseCluster { /// Runs the real validation pipeline over `hbase`, with a fixed dereferenced ZooKeeper connection /// and no OPA. pub fn validated_cluster_from(hbase: &v1alpha1::HbaseCluster) -> ValidatedCluster { - validate_cluster( + validated_cluster_with( hbase, - "oci.example.org", DereferencedObjects { zookeeper_connection_information: ZookeeperConnectionInformation::for_tests(), hbase_opa_config: None, }, ) - .expect("validate should succeed for the test fixture") +} + +/// Runs the real validation pipeline over `hbase` with the given dereferenced objects, for +/// fixtures that need OPA. +pub fn validated_cluster_with( + hbase: &v1alpha1::HbaseCluster, + dereferenced_objects: DereferencedObjects, +) -> ValidatedCluster { + validate_cluster(hbase, "oci.example.org", dereferenced_objects) + .expect("validate should succeed for the test fixture") } /// Runs the real validation pipeline over [`minimal_hbase`]. From 4bbc28d73467eb8846b0a0126b9ba9debee15239 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 16 Oct 2026 18:08:35 +0200 Subject: [PATCH 02/13] refactor: Replace AnyServiceConfig with master, region server and rest server fields on ValidatedCluster --- .../src/controller/build/graceful_shutdown.rs | 10 +- .../src/controller/build/jvm.rs | 52 ++- .../src/controller/build/mod.rs | 153 +++++-- .../controller/build/properties/hbase_env.rs | 28 +- .../controller/build/properties/hbase_site.rs | 53 ++- .../src/controller/build/region_mover.rs | 110 ++--- .../controller/build/resource/config_map.rs | 82 ++-- .../src/controller/build/resource/listener.rs | 16 +- .../controller/build/resource/statefulset.rs | 102 ++--- .../src/controller/build/role_group/master.rs | 53 +++ .../src/controller/build/role_group/mod.rs | 49 +++ .../build/role_group/region_server.rs | 57 +++ .../build/role_group/rest_server.rs | 53 +++ rust/operator-binary/src/controller/mod.rs | 114 ++--- .../src/controller/validate.rs | 394 ++++++++++++------ rust/operator-binary/src/crd/affinity.rs | 27 +- rust/operator-binary/src/crd/mod.rs | 114 ++--- rust/operator-binary/src/test_utils.rs | 33 +- 18 files changed, 943 insertions(+), 557 deletions(-) create mode 100644 rust/operator-binary/src/controller/build/role_group/master.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/mod.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/region_server.rs create mode 100644 rust/operator-binary/src/controller/build/role_group/rest_server.rs diff --git a/rust/operator-binary/src/controller/build/graceful_shutdown.rs b/rust/operator-binary/src/controller/build/graceful_shutdown.rs index 62cdadcf..9dc27a5f 100644 --- a/rust/operator-binary/src/controller/build/graceful_shutdown.rs +++ b/rust/operator-binary/src/controller/build/graceful_shutdown.rs @@ -1,7 +1,5 @@ use snafu::{ResultExt, Snafu}; -use stackable_operator::builder::pod::PodBuilder; - -use crate::crd::AnyServiceConfig; +use stackable_operator::{builder::pod::PodBuilder, shared::time::Duration}; #[derive(Debug, Snafu)] pub enum Error { @@ -12,14 +10,14 @@ pub enum Error { } pub fn add_graceful_shutdown_config( - merged_config: &AnyServiceConfig, + graceful_shutdown_timeout: Option, pod_builder: &mut PodBuilder, ) -> Result<(), Error> { // This must be always set by the merge mechanism, as we provide a default value, // users can not disable graceful shutdown. - if let Some(graceful_shutdown_timeout) = merged_config.graceful_shutdown_timeout() { + if let Some(graceful_shutdown_timeout) = graceful_shutdown_timeout { pod_builder - .termination_grace_period(graceful_shutdown_timeout) + .termination_grace_period(&graceful_shutdown_timeout) .context(SetTerminationGracePeriodSnafu)?; } diff --git a/rust/operator-binary/src/controller/build/jvm.rs b/rust/operator-binary/src/controller/build/jvm.rs index a0ca96be..f37b613b 100644 --- a/rust/operator-binary/src/controller/build/jvm.rs +++ b/rust/operator-binary/src/controller/build/jvm.rs @@ -1,9 +1,13 @@ use snafu::{OptionExt, ResultExt, Snafu}; -use stackable_operator::memory::{BinaryMultiple, MemoryQuantity}; +use stackable_operator::{ + commons::resources::{NoRuntimeLimits, Resources}, + memory::{BinaryMultiple, MemoryQuantity}, + v2::jvm_argument_overrides::JvmArgumentOverrides, +}; use crate::{ - controller::{HbaseRoleGroupConfig, ValidatedCluster, build::kerberos::KRB5_CONFIG_PATH}, - crd::{AnyServiceConfig, CONFIG_DIR_NAME, JVM_SECURITY_PROPERTIES_FILE}, + controller::{ValidatedCluster, build::kerberos::KRB5_CONFIG_PATH}, + crd::{CONFIG_DIR_NAME, HbaseStorageConfig, JVM_SECURITY_PROPERTIES_FILE}, }; const JAVA_HEAP_FACTOR: f32 = 0.8; @@ -45,7 +49,7 @@ pub fn construct_global_jvm_args(kerberos_enabled: bool) -> String { /// are applied on top of the operator-generated base arguments below. pub fn construct_role_specific_non_heap_jvm_args( cluster: &ValidatedCluster, - rg: &HbaseRoleGroupConfig, + jvm_argument_overrides: &JvmArgumentOverrides, ) -> String { let mut operator_generated = vec![format!( "-Djava.security.properties={CONFIG_DIR_NAME}/{JVM_SECURITY_PROPERTIES_FILE}" @@ -55,10 +59,7 @@ pub fn construct_role_specific_non_heap_jvm_args( operator_generated.push(format!("-Djava.security.krb5.conf={KRB5_CONFIG_PATH}")); } - let mut jvm_args = rg - .product_specific_common_config - .jvm_argument_overrides - .apply_to(operator_generated); + let mut jvm_args = jvm_argument_overrides.apply_to(operator_generated); jvm_args.retain(|arg| !is_heap_jvm_argument(arg)); jvm_args.join(" ") @@ -71,10 +72,11 @@ pub fn construct_role_specific_non_heap_jvm_args( /// Looking at `bin/hbase`, you can actually add the `m` suffix to make the unit more clear, the /// script will detect this [here](https://github.com/apache/hbase/blob/777010361abb203b8b17673d84acf4f7f1d0283a/bin/hbase#L165) /// and work correctly. -pub fn construct_hbase_heapsize_env(merged_config: &AnyServiceConfig) -> Result { +pub fn construct_hbase_heapsize_env( + resources: &Resources, +) -> Result { let heap_size = MemoryQuantity::try_from( - merged_config - .resources() + resources .memory .limit .as_ref() @@ -98,7 +100,7 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::{crd::HbaseRole, test_utils}; + use crate::test_utils; #[test] fn test_construct_jvm_arguments_defaults() { @@ -126,16 +128,21 @@ mod tests { "#; let hbase = test_utils::hbase_from_yaml(input); let validated_cluster = test_utils::validated_cluster_from(&hbase); - let region_server = &validated_cluster.role_group_configs[&HbaseRole::RegionServer] - [&test_utils::role_group_name("default")]; let global_jvm_args = construct_global_jvm_args(validated_cluster.has_kerberos_enabled()); + let region_server = + test_utils::region_server_role_group_config(&validated_cluster, "default"); let hbase_heapsize_env = - construct_hbase_heapsize_env(®ion_server.config.config).unwrap(); + construct_hbase_heapsize_env(®ion_server.config.config.resources).unwrap(); assert_eq!(global_jvm_args, ""); assert_eq!( - construct_role_specific_non_heap_jvm_args(&validated_cluster, region_server), + construct_role_specific_non_heap_jvm_args( + &validated_cluster, + ®ion_server + .product_specific_common_config + .jvm_argument_overrides + ), "-Djava.security.properties=/stackable/conf/security.properties" ); assert_eq!(hbase_heapsize_env, "819m"); @@ -186,19 +193,24 @@ mod tests { "#; let hbase = test_utils::hbase_from_yaml(input); let validated_cluster = test_utils::validated_cluster_from(&hbase); - let region_server = &validated_cluster.role_group_configs[&HbaseRole::RegionServer] - [&test_utils::role_group_name("default")]; + let region_server = + test_utils::region_server_role_group_config(&validated_cluster, "default"); let global_jvm_args = construct_global_jvm_args(validated_cluster.has_kerberos_enabled()); let hbase_heapsize_env = - construct_hbase_heapsize_env(®ion_server.config.config).unwrap(); + construct_hbase_heapsize_env(®ion_server.config.config.resources).unwrap(); assert_eq!( global_jvm_args, "-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf" ); assert_eq!( - construct_role_specific_non_heap_jvm_args(&validated_cluster, region_server), + construct_role_specific_non_heap_jvm_args( + &validated_cluster, + ®ion_server + .product_specific_common_config + .jvm_argument_overrides + ), "-Djava.security.properties=/stackable/conf/security.properties \ -Djava.security.krb5.conf=/stackable/kerberos/krb5.conf \ -Dhttps.proxyHost=proxy.my.corp \ diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 77cded2b..0be2ae69 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -1,7 +1,7 @@ //! Builders that turn a [`ValidatedCluster`] into //! Kubernetes resources. -use std::{marker::PhantomData, ops::Deref}; +use std::marker::PhantomData; use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -19,13 +19,16 @@ use crate::{ controller::{ CONTROLLER_NAME, KubernetesResources, OPERATOR_NAME, PRODUCT_NAME, Prepared, ValidatedCluster, - build::resource::{ - config_map::{self, build_rolegroup_config_map}, - discovery::{self, build_discovery_config_map}, - pdb::build_pdb, - rbac::{build_role_binding, build_service_account}, - service::{build_rolegroup_metrics_service, build_rolegroup_service}, - statefulset::{self, build_rolegroup_statefulset}, + build::{ + resource::{ + config_map::{self, build_rolegroup_config_map}, + discovery::{self, build_discovery_config_map}, + pdb::build_pdb, + rbac::{build_role_binding, build_service_account}, + service::{build_rolegroup_metrics_service, build_rolegroup_service}, + statefulset::{self, build_rolegroup_statefulset}, + }, + role_group::{master, region_server, rest_server}, }, }, crd::HbaseRole, @@ -52,17 +55,17 @@ pub(crate) fn object_meta( #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build ConfigMap for role {role} role group {role_group}", role = hbase_role.deref()))] + #[snafu(display("failed to build ConfigMap for role {role} role group {role_group}"))] ConfigMap { source: config_map::Error, - hbase_role: HbaseRole, + role: HbaseRole, role_group: RoleGroupName, }, - #[snafu(display("failed to build StatefulSet for role {role} role group {role_group}", role = hbase_role.deref()))] + #[snafu(display("failed to build StatefulSet for role {role} role group {role_group}"))] StatefulSet { source: statefulset::Error, - hbase_role: HbaseRole, + role: HbaseRole, role_group: RoleGroupName, }, @@ -84,39 +87,100 @@ pub fn build( let mut config_maps = vec![]; let mut pod_disruption_budgets = vec![]; - for (hbase_role, role_group_configs) in &cluster.role_group_configs { - for (role_group_name, rg_config) in role_group_configs { - services.push(build_rolegroup_service( - cluster, - hbase_role, - role_group_name, - )); - services.push(build_rolegroup_metrics_service( - cluster, - hbase_role, - role_group_name, - )); - config_maps.push( - build_rolegroup_config_map(cluster, cluster_info, hbase_role, role_group_name) - .with_context(|_| ConfigMapSnafu { - hbase_role: hbase_role.clone(), - role_group: role_group_name.clone(), - })?, - ); - stateful_sets.push( - build_rolegroup_statefulset(cluster, hbase_role, role_group_name, rg_config) - .with_context(|_| StatefulSetSnafu { - hbase_role: hbase_role.clone(), - role_group: role_group_name.clone(), - })?, - ); - } + for (role_group_name, rg_config) in &cluster.master_role_group_configs { + let inputs = master::inputs(cluster, cluster_info, role_group_name, rg_config); + services.push(build_rolegroup_service( + cluster, + &master::ROLE, + role_group_name, + )); + services.push(build_rolegroup_metrics_service( + cluster, + &master::ROLE, + role_group_name, + )); + config_maps.push( + build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { + role: master::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { + StatefulSetSnafu { + role: master::ROLE, + role_group: role_group_name.clone(), + } + })?); + } - if let Some(role_config) = cluster.role_configs.get(hbase_role) - && let Some(pdb) = build_pdb(&role_config.pdb, cluster, hbase_role) - { - pod_disruption_budgets.push(pdb); - } + if let Some(pdb) = build_pdb(&cluster.master_config.pdb, cluster, &master::ROLE) { + pod_disruption_budgets.push(pdb); + } + + for (role_group_name, rg_config) in &cluster.region_server_role_group_configs { + let inputs = region_server::inputs(cluster, cluster_info, role_group_name, rg_config); + services.push(build_rolegroup_service( + cluster, + ®ion_server::ROLE, + role_group_name, + )); + services.push(build_rolegroup_metrics_service( + cluster, + ®ion_server::ROLE, + role_group_name, + )); + config_maps.push( + build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { + role: region_server::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { + StatefulSetSnafu { + role: region_server::ROLE, + role_group: role_group_name.clone(), + } + })?); + } + + if let Some(pdb) = build_pdb( + &cluster.region_server_config.pdb, + cluster, + ®ion_server::ROLE, + ) { + pod_disruption_budgets.push(pdb); + } + + for (role_group_name, rg_config) in &cluster.rest_server_role_group_configs { + let inputs = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config); + services.push(build_rolegroup_service( + cluster, + &rest_server::ROLE, + role_group_name, + )); + services.push(build_rolegroup_metrics_service( + cluster, + &rest_server::ROLE, + role_group_name, + )); + config_maps.push( + build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { + role: rest_server::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { + StatefulSetSnafu { + role: rest_server::ROLE, + role_group: role_group_name.clone(), + } + })?); + } + + if let Some(rest_server_config) = &cluster.rest_server_config + && let Some(pdb) = build_pdb(&rest_server_config.pdb, cluster, &rest_server::ROLE) + { + pod_disruption_budgets.push(pdb); } // The role-level discovery ConfigMap advertises the cluster's connection information; it is @@ -206,6 +270,7 @@ pub mod properties; pub mod region_mover; pub mod resource; pub mod role; +pub mod role_group; #[cfg(test)] mod tests { diff --git a/rust/operator-binary/src/controller/build/properties/hbase_env.rs b/rust/operator-binary/src/controller/build/properties/hbase_env.rs index 41756b21..f816657d 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_env.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_env.rs @@ -3,11 +3,14 @@ use std::{collections::BTreeMap, fmt::Write}; use snafu::{ResultExt, Snafu}; -use stackable_operator::v2::config_overrides::KeyValueConfigOverrides; +use stackable_operator::{ + commons::resources::{NoRuntimeLimits, Resources}, + v2::config_overrides::KeyValueConfigOverrides, +}; use crate::{ controller::build::jvm::{self, construct_global_jvm_args, construct_hbase_heapsize_env}, - crd::{AnyServiceConfig, HbaseRole}, + crd::{HbaseRole, HbaseStorageConfig}, }; /// The `hbase-env.sh` variable that carries a role's own JVM options; `bin/hbase` reads the one @@ -24,7 +27,7 @@ pub enum Error { /// Renders `hbase-env.sh` as `export VAR="VALUE"` lines. pub fn build( - merged_config: &AnyServiceConfig, + resources: &Resources, role: &HbaseRole, kerberos_enabled: bool, non_heap_jvm_args: String, @@ -35,7 +38,7 @@ pub fn build( env.insert("HBASE_MANAGES_ZK".to_string(), "false".to_string()); env.insert( "HBASE_HEAPSIZE".to_string(), - construct_hbase_heapsize_env(merged_config).context(ConstructHbaseHeapsizeEnvSnafu)?, + construct_hbase_heapsize_env(resources).context(ConstructHbaseHeapsizeEnvSnafu)?, ); env.insert( "HBASE_OPTS".to_string(), @@ -68,12 +71,15 @@ pub fn build( #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{merged_config, validated_cluster}; + use crate::test_utils::{self, validated_cluster}; #[test] fn renders_operator_defaults() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::Master); + let merged = &test_utils::master_role_group_config(&validated_cluster, "default") + .config + .config + .resources; let env = build( merged, &HbaseRole::Master, @@ -89,7 +95,10 @@ mod tests { #[test] fn renders_region_server_opts() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::RegionServer); + let merged = &test_utils::region_server_role_group_config(&validated_cluster, "default") + .config + .config + .resources; let env = build( merged, &HbaseRole::RegionServer, @@ -104,7 +113,10 @@ mod tests { #[test] fn user_override_appears() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::Master); + let merged = &test_utils::master_role_group_config(&validated_cluster, "default") + .config + .config + .resources; let env = build( merged, &HbaseRole::Master, diff --git a/rust/operator-binary/src/controller/build/properties/hbase_site.rs b/rust/operator-binary/src/controller/build/properties/hbase_site.rs index d39388ce..a982c41f 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_site.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_site.rs @@ -12,9 +12,10 @@ use crate::{ controller::{ ValidatedCluster, build::{opa::HbaseOpaConfig, properties::build_xml_config}, + replicas_or_default, }, crd::{ - AnyServiceConfig, HBASE_CLUSTER_DISTRIBUTED, HBASE_MASTER_PORT, HBASE_MASTER_UI_PORT, + HBASE_CLUSTER_DISTRIBUTED, HBASE_MASTER_PORT, HBASE_MASTER_UI_PORT, HBASE_REGIONSERVER_PORT, HBASE_REGIONSERVER_UI_PORT, HBASE_ROOTDIR, HbaseRole, }, }; @@ -59,17 +60,13 @@ pub fn client_bootstrap_servers( let namespace = cluster.namespace.as_ref(); let cluster_domain = &cluster_info.cluster_domain; cluster - .role_group_configs - .get(&HbaseRole::Master) - .into_iter() - .flatten() + .master_role_group_configs.iter() .flat_map(|(role_group_name, role_group)| { let resource_names = cluster.role_group_resource_names(&HbaseRole::Master, role_group_name); let stateful_set = resource_names.stateful_set_name(); let headless_service = resource_names.headless_service_name(); - // `None` leaves `replicas` unset on the StatefulSet, which Kubernetes defaults to 1 - (0..role_group.replicas.unwrap_or(1)).map(move |ordinal| { + (0..replicas_or_default(role_group.replicas)).map(move |ordinal| { format!( "{stateful_set}-{ordinal}.{headless_service}.{namespace}.svc.{cluster_domain}:{HBASE_MASTER_PORT}" ) @@ -82,7 +79,7 @@ pub fn client_bootstrap_servers( /// Renders `hbase-site.xml`. pub fn build( role: &HbaseRole, - merged_config: &AnyServiceConfig, + hbase_rootdir: &str, zookeeper_config: BTreeMap, kerberos_config: BTreeMap, opa_config: Option<&HbaseOpaConfig>, @@ -93,7 +90,7 @@ pub fn build( // Defaults config.insert(HBASE_CLUSTER_DISTRIBUTED.to_string(), "true".to_string()); - config.insert(HBASE_ROOTDIR.to_string(), merged_config.hbase_rootdir()); + config.insert(HBASE_ROOTDIR.to_string(), hbase_rootdir.to_owned()); config.insert( HBASE_CLIENT_BOOTSTRAP_SERVERS.to_string(), client_bootstrap_servers, @@ -187,7 +184,7 @@ mod tests { use super::*; use crate::test_utils::{ - cluster_info, hbase_from_yaml, merged_config, validated_cluster, validated_cluster_from, + self, cluster_info, hbase_from_yaml, validated_cluster, validated_cluster_from, }; fn bootstrap_servers_for(masters_yaml: &str) -> String { @@ -255,10 +252,12 @@ spec: #[test] fn renders_operator_defaults() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::Master); + let config = &test_utils::master_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::Master, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, @@ -282,10 +281,12 @@ spec: #[test] fn renders_region_server_bind_settings() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::RegionServer); + let config = &test_utils::region_server_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::RegionServer, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, @@ -309,10 +310,12 @@ spec: #[test] fn renders_rest_server_endpoint() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::RestServer); + let config = &test_utils::rest_server_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::RestServer, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, @@ -330,10 +333,12 @@ spec: #[test] fn renders_client_bootstrap_servers() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::RestServer); + let config = &test_utils::rest_server_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::RestServer, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, @@ -351,10 +356,12 @@ spec: #[test] fn user_override_wins_for_client_bootstrap_servers() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::RestServer); + let config = &test_utils::rest_server_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::RestServer, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, @@ -372,10 +379,12 @@ spec: #[test] fn user_override_wins() { let validated_cluster = validated_cluster(); - let merged = merged_config(&validated_cluster, &HbaseRole::Master); + let config = &test_utils::rest_server_role_group_config(&validated_cluster, "default") + .config + .config; let xml = build( &HbaseRole::Master, - merged, + &config.hbase_rootdir, BTreeMap::new(), BTreeMap::new(), None, diff --git a/rust/operator-binary/src/controller/build/region_mover.rs b/rust/operator-binary/src/controller/build/region_mover.rs index 9d2e8ed2..d716b4eb 100644 --- a/rust/operator-binary/src/controller/build/region_mover.rs +++ b/rust/operator-binary/src/controller/build/region_mover.rs @@ -1,11 +1,11 @@ -//! Build-side region-mover logic for [`AnyServiceConfig`]: derives the `bin/hbase` region-mover +//! Build-side region-mover logic for the config: derives the `bin/hbase` region-mover //! CLI arguments from the merged config. Kept out of the `crd` module so it stays a pure API //! definition. use shell_escape::escape; use stackable_operator::shared::time::Duration; -use crate::crd::AnyServiceConfig; +use crate::crd::RegionServerConfig; /// Fallback region-mover timeout, used only when no graceful shutdown timeout is set. In practice /// the merged config always defaults `graceful_shutdown_timeout` (60m for region servers), so this @@ -15,60 +15,46 @@ const DEFAULT_REGION_MOVER_TIMEOUT: Duration = Duration::from_minutes_unchecked( /// the region move finishes before the pod is terminated. const DEFAULT_REGION_MOVER_DELTA_TO_SHUTDOWN: Duration = Duration::from_minutes_unchecked(1); -impl AnyServiceConfig { - /// Returns command line arguments to pass on to the region mover tool. - /// The following arguments are excluded because they are already part of the - /// hbase-entrypoint.sh script. - /// The most important argument, '--regionserverhost' can only be computed on the Pod - /// because it contains the pod's hostname. - /// - /// Returns an empty string if the region mover is disabled or any other role is "self". - pub fn region_mover_args(&self) -> String { - match self { - AnyServiceConfig::RegionServer(config) => { - if config.region_mover.run_before_shutdown { - let timeout = config - .graceful_shutdown_timeout - .map(|d| { - if d.as_secs() <= DEFAULT_REGION_MOVER_DELTA_TO_SHUTDOWN.as_secs() { - d.as_secs() - } else { - d.as_secs() - DEFAULT_REGION_MOVER_DELTA_TO_SHUTDOWN.as_secs() - } - }) - .unwrap_or(DEFAULT_REGION_MOVER_TIMEOUT.as_secs()); - let mut command = vec![ - "--maxthreads".to_string(), - config.region_mover.max_threads.to_string(), - "--timeout".to_string(), - timeout.to_string(), - ]; - if !config.region_mover.ack { - command.push("--noack".to_string()); - } - - command.extend( - config - .region_mover - .cli_opts - .iter() - .flat_map(|o| o.additional_mover_options.clone()) - .map(|s| escape(std::borrow::Cow::Borrowed(&s)).to_string()), - ); - command.join(" ") +/// Returns command line arguments to pass on to the region mover tool. +/// The following arguments are excluded because they are already part of the +/// hbase-entrypoint.sh script. +/// The most important argument, '--regionserverhost' can only be computed on the Pod +/// because it contains the pod's hostname. +/// +/// Returns an empty string if the region mover is disabled or any other role is "self". +pub fn region_mover_args(config: &RegionServerConfig) -> String { + if config.region_mover.run_before_shutdown { + let timeout = config + .graceful_shutdown_timeout + .map(|d| { + if d.as_secs() <= DEFAULT_REGION_MOVER_DELTA_TO_SHUTDOWN.as_secs() { + d.as_secs() } else { - "".to_string() + d.as_secs() - DEFAULT_REGION_MOVER_DELTA_TO_SHUTDOWN.as_secs() } - } - _ => "".to_string(), + }) + .unwrap_or(DEFAULT_REGION_MOVER_TIMEOUT.as_secs()); + let mut command = vec![ + "--maxthreads".to_string(), + config.region_mover.max_threads.to_string(), + "--timeout".to_string(), + timeout.to_string(), + ]; + if !config.region_mover.ack { + command.push("--noack".to_string()); } - } - pub fn run_region_mover(&self) -> bool { - match self { - AnyServiceConfig::RegionServer(config) => config.region_mover.run_before_shutdown, - _ => false, - } + command.extend( + config + .region_mover + .cli_opts + .iter() + .flat_map(|o| o.additional_mover_options.clone()) + .map(|s| escape(std::borrow::Cow::Borrowed(&s)).to_string()), + ); + command.join(" ") + } else { + "".to_string() } } @@ -76,7 +62,7 @@ impl AnyServiceConfig { mod tests { use indoc::indoc; - use crate::{crd::HbaseRole, test_utils}; + use crate::{controller::build::region_mover::region_mover_args, test_utils}; /// Renders the region-mover args for the default region-server role group of a cluster whose /// `regionServers.config` is built from `config_lines`. Each entry is one YAML line nested @@ -122,8 +108,12 @@ mod tests { let hbase = test_utils::hbase_from_yaml(&yaml); let validated = test_utils::validated_cluster_from(&hbase); - test_utils::merged_config_for(&validated, &HbaseRole::RegionServer, "default") - .region_mover_args() + + region_mover_args( + &test_utils::region_server_role_group_config(&validated, "default") + .config + .config, + ) } #[test] @@ -134,16 +124,6 @@ mod tests { ); } - #[test] - fn empty_for_non_region_server_role() { - // The region mover only applies to region servers; every other role returns no args. - let validated = test_utils::validated_cluster(); - assert_eq!( - test_utils::merged_config(&validated, &HbaseRole::Master).region_mover_args(), - "" - ); - } - #[test] fn uses_default_graceful_shutdown_timeout_minus_delta() { // Default region-server graceful shutdown timeout is 60m (3600s); the region mover reserves 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 ff3dec26..29218ffc 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -1,28 +1,20 @@ //! Build the per-rolegroup `ConfigMap` for the HbaseCluster. -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::configmap::ConfigMapBuilder, - k8s_openapi::api::core::v1::ConfigMap, - product_logging::framework::VECTOR_CONFIG_FILE, - utils::cluster_info::KubernetesClusterInfo, - v2::{config_file_writer::PropertiesWriterError, types::operator::RoleGroupName}, + builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap, + product_logging::framework::VECTOR_CONFIG_FILE, v2::config_file_writer::PropertiesWriterError, }; -use crate::{ - controller::{ - ValidatedCluster, - build::{ - jvm::construct_role_specific_non_heap_jvm_args, - kerberos, object_meta, - properties::{ - ConfigFileName, hbase_env, hbase_site, product_logging, security_properties, - ssl_client, ssl_server, - }, - recommended_labels_for_role_group_resources, - }, +use crate::controller::build::{ + jvm::construct_role_specific_non_heap_jvm_args, + kerberos, object_meta, + properties::{ + ConfigFileName, hbase_env, hbase_site, product_logging, security_properties, ssl_client, + ssl_server, }, - crd::HbaseRole, + recommended_labels_for_role_group_resources, + role_group::RoleGroupInputs, }; #[derive(Snafu, Debug)] @@ -49,66 +41,53 @@ pub enum Error { type Result = std::result::Result; -pub fn build_rolegroup_config_map( - cluster: &ValidatedCluster, - cluster_info: &KubernetesClusterInfo, - role: &HbaseRole, - role_group_name: &RoleGroupName, -) -> Result { +pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result { + let cluster = inputs.cluster; + let role = &inputs.role; + let role_group_name = &inputs.role_group_name; + tracing::info!( "Setting up ConfigMap for {role}/{role_group_name}", role = role.to_string() ); - let rg = cluster - .role_group_configs - .get(role) - .and_then(|groups| groups.get(role_group_name)) - .with_context(|| MissingRoleGroupSnafu { - role: role.to_string(), - role_group: role_group_name.to_string(), - })?; - let cluster_config = &cluster.cluster_config; - let merged_config = &rg.config.config; - let overrides = &rg.config_overrides; let hbase_site_xml = hbase_site::build( role, - merged_config, + &inputs.hbase_rootdir, cluster_config .zookeeper_connection_information .as_hbase_settings(), - kerberos::hbase_site_kerberos_config(cluster, cluster_info), + kerberos::hbase_site_kerberos_config(cluster, inputs.cluster_info), cluster_config.hbase_opa_config.as_ref(), - hbase_site::client_bootstrap_servers(cluster, cluster_info), - overrides.hbase_site_xml.clone(), + hbase_site::client_bootstrap_servers(cluster, inputs.cluster_info), + inputs.config_overrides.hbase_site_xml.clone(), ); let hbase_env_sh = hbase_env::build( - merged_config, + &inputs.resources, role, cluster.has_kerberos_enabled(), - construct_role_specific_non_heap_jvm_args(cluster, rg), - overrides.hbase_env_sh.clone(), + construct_role_specific_non_heap_jvm_args(cluster, &inputs.jvm_argument_overrides), + inputs.config_overrides.hbase_env_sh.clone(), ) .context(BuildHbaseEnvSnafu)?; let ssl_server_xml = ssl_server::build( kerberos::ssl_server_settings(cluster), - overrides.ssl_server_xml.clone(), + inputs.config_overrides.ssl_server_xml.clone(), ); let ssl_client_xml = ssl_client::build( kerberos::ssl_client_settings(cluster), - overrides.ssl_client_xml.clone(), + inputs.config_overrides.ssl_client_xml.clone(), ); let security_properties = - security_properties::build(role, overrides.security_properties.clone()).with_context( - |_| JvmSecurityPropertiesSnafu { + security_properties::build(role, inputs.config_overrides.security_properties.clone()) + .with_context(|_| JvmSecurityPropertiesSnafu { role_group: role_group_name.to_string(), - }, - )?; + })?; let cm_metadata = object_meta( cluster, @@ -137,12 +116,11 @@ pub fn build_rolegroup_config_map( builder.add_data(ConfigFileName::SslClient.to_string(), ssl_client_xml); } - if let Some(log4j2_properties) = - product_logging::build_log4j2(&rg.config.logging.hbase_container) + if let Some(log4j2_properties) = product_logging::build_log4j2(&inputs.logging.hbase_container) { builder.add_data(ConfigFileName::Log4j2.to_string(), log4j2_properties); } - if rg.config.logging.enable_vector_agent { + if inputs.logging.enable_vector_agent { builder.add_data( VECTOR_CONFIG_FILE, product_logging::vector_config_file_content(), diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index 71d40c65..a04d3cfa 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -16,11 +16,11 @@ use stackable_operator::{ ListenerReference as TypedListenerReference, listener_operator_volume_source_builder_build_pvc, }, - types::kubernetes::PersistentVolumeClaimName, + types::kubernetes::{ListenerClassName, PersistentVolumeClaimName}, }, }; -use crate::crd::{AnyServiceConfig, HbaseRole, LISTENER_VOLUME_NAME}; +use crate::crd::{HbaseRole, LISTENER_VOLUME_NAME}; constant!(pub(crate) LISTENER_PVC_NAME: PersistentVolumeClaimName = LISTENER_VOLUME_NAME); @@ -38,7 +38,7 @@ type Result = std::result::Result; /// servers (which use a [`PersistentVolumeClaim`] instead, see [`build_listener_pvc`]). pub fn build_listener_volume( role: &HbaseRole, - merged_config: &AnyServiceConfig, + listener_class: &ListenerClassName, recommended_labels: &Labels, ) -> Result> { let volume = match role { @@ -51,12 +51,10 @@ pub fn build_listener_volume( // (`listener_operator_volume_source_builder_build_pvc`, used by // `build_listener_pvc`), so the ephemeral case still uses the legacy // `ListenerOperatorVolumeSourceBuilder` and its stringly-typed - // `ListenerReference`. We keep the typed `listener_class()` and convert to a + // `ListenerReference`. We keep the typed `listener_class` and convert to a // `String` only at this boundary; switch to a v2 helper once one exists. ListenerOperatorVolumeSourceBuilder::new( - &ListenerReference::ListenerClass( - merged_config.listener_class().to_string(), - ), + &ListenerReference::ListenerClass(listener_class.to_string()), recommended_labels, ) .build_ephemeral() @@ -73,13 +71,13 @@ pub fn build_listener_volume( /// and region servers (which use an ephemeral [`Volume`] instead, see [`build_listener_volume`]). pub fn build_listener_pvc( role: &HbaseRole, - merged_config: &AnyServiceConfig, + listener_class: &ListenerClassName, recommended_labels: &Labels, ) -> Option> { match role { HbaseRole::Master | HbaseRole::RegionServer => None, HbaseRole::RestServer => Some(vec![listener_operator_volume_source_builder_build_pvc( - &TypedListenerReference::ListenerClass(merged_config.listener_class()), + &TypedListenerReference::ListenerClass(listener_class.clone()), recommended_labels, &LISTENER_PVC_NAME, )]), diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 7709b1bc..b0935e39 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -26,24 +26,20 @@ use stackable_operator::{ product_logging::framework::{ STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container, }, - types::{ - kubernetes::{ContainerName, VolumeName}, - operator::RoleGroupName, - }, + types::kubernetes::{ContainerName, VolumeName}, }, }; use crate::{ - controller::{ - HbaseRoleGroupConfig, ValidatedCluster, - build::{ - graceful_shutdown::{self, add_graceful_shutdown_config}, - kerberos::{self, add_kerberos_pod_config}, - object_meta, - properties::product_logging::MAX_HBASE_LOG_FILES_SIZE, - recommended_labels_for_role_group_resources, - recommended_labels_for_unversioned_role_group_resources, role_group_selector, - }, + controller::build::{ + graceful_shutdown::{self, add_graceful_shutdown_config}, + kerberos::{self, add_kerberos_pod_config}, + object_meta, + properties::product_logging::MAX_HBASE_LOG_FILES_SIZE, + recommended_labels_for_role_group_resources, + recommended_labels_for_unversioned_role_group_resources, + role_group::RoleGroupInputs, + role_group_selector, }, crd::{CONFIG_DIR_NAME, HbaseRole, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME}, }; @@ -98,15 +94,13 @@ type Result = std::result::Result; /// /// The [`Pod`](stackable_operator::k8s_openapi::api::core::v1::Pod)s are accessible through the /// corresponding headless [`Service`](stackable_operator::k8s_openapi::api::core::v1::Service). -pub fn build_rolegroup_statefulset( - cluster: &ValidatedCluster, - hbase_role: &HbaseRole, - role_group_name: &RoleGroupName, - validated_rg_config: &HbaseRoleGroupConfig, -) -> Result { +pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result { + let cluster = inputs.cluster; + let hbase_role = &inputs.role; + let role_group_name = &inputs.role_group_name; + let resolved_product_image = &cluster.image; - let merged_config = &validated_rg_config.config.config; - let logging = &validated_rg_config.config.logging; + let logging = &inputs.logging; let resource_names = cluster.role_group_resource_names(hbase_role, role_group_name); let https_enabled = cluster.has_https_enabled(); @@ -154,11 +148,8 @@ pub fn build_rolegroup_statefulset( // required by phoenix (for cases where Kerberos is enabled): see https://issues.apache.org/jira/browse/PHOENIX-2369 .with_value(&HADOOP_CONF_DIR_ENV, CONFIG_DIR_NAME) // These env vars are set for all roles to avoid bash's "unbound variable" errors. - .with_value(®ION_MOVER_OPTS_ENV, merged_config.region_mover_args()) - .with_value( - &RUN_REGION_MOVER_ENV, - merged_config.run_region_mover().to_string(), - ) + .with_value(®ION_MOVER_OPTS_ENV, inputs.region_mover_opts.clone()) + .with_value(&RUN_REGION_MOVER_ENV, inputs.run_region_mover.to_string()) .with_value(&STACKABLE_LOG_DIR_ENV, STACKABLE_LOG_DIR) // Needed for the `containerdebug` process to log its tracing information to. .with_value( @@ -169,7 +160,7 @@ pub fn build_rolegroup_statefulset( .merge(kerberos::kerberos_env_vars(cluster)) // apply overrides last of all; `EnvVarSet` is keyed by name, so iteration is already // in a fixed (sorted-by-name) order - .merge(validated_rg_config.env_overrides.clone()); + .merge(inputs.env_overrides.clone()); let role_name = hbase_role.cli_role_name(); let mut hbase_container = new_container_builder(&HBASE_CONTAINER_NAME); @@ -197,7 +188,7 @@ pub fn build_rolegroup_statefulset( .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR) .expect("The mount paths are statically defined and there should be no duplicates.") .add_container_ports(ports) - .resources(merged_config.resources().clone().into()) + .resources(inputs.resources.clone().into()) .startup_probe(startup_probe) .liveness_probe(liveness_probe) .readiness_probe(readiness_probe); @@ -214,7 +205,7 @@ pub fn build_rolegroup_statefulset( pod_builder .metadata(pb_metadata) .image_pull_secrets_from_product_image(resolved_product_image) - .affinity(merged_config.affinity()) + .affinity(&inputs.affinity) .add_volume(Volume { name: HBASE_CONFIG_VOLUME_NAME.to_string(), config_map: Some(ConfigMapVolumeSource { @@ -272,15 +263,16 @@ pub fn build_rolegroup_statefulset( }) .context(AddVolumeSnafu)?; - add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?; + add_graceful_shutdown_config(inputs.graceful_shutdown_timeout, &mut pod_builder) + .context(GracefulShutdownSnafu)?; if cluster.has_kerberos_enabled() { add_kerberos_pod_config( cluster, resource_names.metrics_service_name().as_ref(), &mut hbase_container, &mut pod_builder, - merged_config - .requested_secret_lifetime() + inputs + .requested_secret_lifetime .context(MissingSecretLifetimeSnafu)?, ) .context(AddKerberosConfigSnafu)?; @@ -309,12 +301,18 @@ pub fn build_rolegroup_statefulset( role_group_name, ); - let listener_pvc = - super::listener::build_listener_pvc(hbase_role, merged_config, &unversioned_labels); + let listener_pvc = super::listener::build_listener_pvc( + hbase_role, + &inputs.listener_class, + &unversioned_labels, + ); - if let Some(listener_volume) = - super::listener::build_listener_volume(hbase_role, merged_config, &recommended_labels) - .context(ListenerVolumeSnafu)? + if let Some(listener_volume) = super::listener::build_listener_volume( + hbase_role, + &inputs.listener_class, + &recommended_labels, + ) + .context(ListenerVolumeSnafu)? { pod_builder .add_volume(listener_volume) @@ -323,7 +321,7 @@ pub fn build_rolegroup_statefulset( let mut pod_template = pod_builder.build_template(); - pod_template.merge_from(validated_rg_config.pod_overrides.clone()); + pod_template.merge_from(inputs.pod_overrides.clone()); let metadata = object_meta( cluster, @@ -337,7 +335,7 @@ pub fn build_rolegroup_statefulset( let statefulset_spec = StatefulSetSpec { pod_management_policy: Some("Parallel".to_string()), - replicas: validated_rg_config.replicas.map(i32::from), + replicas: inputs.replicas.map(i32::from), selector: LabelSelector { match_labels: Some(statefulset_match_labels.into()), ..LabelSelector::default() @@ -369,7 +367,7 @@ fn command() -> Vec { #[cfg(test)] mod tests { use super::*; - use crate::test_utils; + use crate::{controller::build::role_group::master, test_utils}; /// `envOverrides` are applied after every operator-set environment variable, so users can /// override any of them (previously the operator's value silently won for the variables set @@ -388,11 +386,15 @@ mod tests { )); let cluster = test_utils::validated_cluster_from(&hbase); let role_group_name = test_utils::role_group_name("default"); - let rg_config = &cluster.role_group_configs[&HbaseRole::Master][&role_group_name]; + let cluster_info = &test_utils::cluster_info(); + let inputs = master::inputs( + &cluster, + cluster_info, + &role_group_name, + test_utils::master_role_group_config(&cluster, "default"), + ); - let stateful_set = - build_rolegroup_statefulset(&cluster, &HbaseRole::Master, &role_group_name, rg_config) - .expect("the StatefulSet builds"); + let stateful_set = build_rolegroup_statefulset(&inputs).expect("the StatefulSet builds"); let env: Vec<(String, String)> = stateful_set .spec @@ -498,9 +500,15 @@ spec: let hbase = test_utils::hbase_from_yaml(yaml); let cluster = test_utils::validated_cluster_from(&hbase); let role_group_name = test_utils::role_group_name("default"); - let rg_config = &cluster.role_group_configs[&HbaseRole::Master][&role_group_name]; + let cluster_info = &test_utils::cluster_info(); + let inputs = master::inputs( + &cluster, + cluster_info, + &role_group_name, + test_utils::master_role_group_config(&cluster, "default"), + ); - build_rolegroup_statefulset(&cluster, &HbaseRole::Master, &role_group_name, rg_config) + build_rolegroup_statefulset(&inputs) .expect("the StatefulSet builds") .spec .expect("the StatefulSet has a spec") diff --git a/rust/operator-binary/src/controller/build/role_group/master.rs b/rust/operator-binary/src/controller/build/role_group/master.rs new file mode 100644 index 00000000..7af460eb --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/master.rs @@ -0,0 +1,53 @@ +use stackable_operator::{ + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, +}; + +use crate::{ + controller::{MasterRoleGroupConfig, ValidatedCluster, build::role_group::RoleGroupInputs}, + crd::{HbaseConfig, HbaseRole}, +}; + +pub(crate) const ROLE: HbaseRole = HbaseRole::Master; + +/// The inputs of one master role group. +pub(crate) fn inputs<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &MasterRoleGroupConfig, +) -> RoleGroupInputs<'a> { + let HbaseConfig { + hbase_rootdir, + resources, + // Validated up front into `rg_config.config.logging`. + logging: _, + affinity, + graceful_shutdown_timeout, + requested_secret_lifetime, + listener_class, + } = &rg_config.config.config; + + RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + affinity: affinity.clone(), + graceful_shutdown_timeout: *graceful_shutdown_timeout, + requested_secret_lifetime: *requested_secret_lifetime, + resources: resources.clone(), + hbase_rootdir: hbase_rootdir.clone(), + logging: rg_config.config.logging.clone(), + listener_class: listener_class.clone(), + region_mover_opts: String::new(), + run_region_mover: false, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs new file mode 100644 index 00000000..33ae0d38 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -0,0 +1,49 @@ +use stackable_operator::{ + commons::{ + affinity::StackableAffinity, + resources::{NoRuntimeLimits, Resources}, + }, + k8s_openapi::api::core::v1::PodTemplateSpec, + shared::time::Duration, + utils::cluster_info::KubernetesClusterInfo, + v2::{ + builder::pod::container::EnvVarSet, + jvm_argument_overrides::JvmArgumentOverrides, + types::{kubernetes::ListenerClassName, operator::RoleGroupName}, + }, +}; + +use crate::{ + controller::{ValidatedCluster, validate::ValidatedLogging}, + crd::{HbaseRole, HbaseStorageConfig, v1alpha1}, +}; + +pub(crate) mod master; +pub(crate) mod region_server; +pub(crate) mod rest_server; + +/// What one role group's resources are built from: the values every role has, resolved from +/// its typed role-group config by that role's module. +pub(crate) struct RoleGroupInputs<'a> { + pub(crate) cluster: &'a ValidatedCluster, + pub(crate) cluster_info: &'a KubernetesClusterInfo, + pub(crate) role: HbaseRole, + pub(crate) role_group_name: RoleGroupName, + pub(crate) affinity: StackableAffinity, + pub(crate) graceful_shutdown_timeout: Option, + pub(crate) requested_secret_lifetime: Option, + /// Container resources and the `HBASE_HEAPSIZE` derivation both read this. + pub(crate) resources: Resources, + pub(crate) hbase_rootdir: String, + pub(crate) logging: ValidatedLogging, + pub(crate) listener_class: ListenerClassName, + /// Set on every role because the entrypoint runs under `set -u`; only RegionServers run the + /// mover, so only they have a non-empty value. + pub(crate) region_mover_opts: String, + pub(crate) run_region_mover: bool, + pub(crate) replicas: Option, + pub(crate) config_overrides: v1alpha1::HbaseConfigOverrides, + pub(crate) env_overrides: EnvVarSet, + pub(crate) pod_overrides: PodTemplateSpec, + pub(crate) jvm_argument_overrides: JvmArgumentOverrides, +} diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs new file mode 100644 index 00000000..b6612520 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -0,0 +1,57 @@ +use stackable_operator::{ + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, +}; + +use crate::{ + controller::{ + RegionServerRoleGroupConfig, ValidatedCluster, + build::{region_mover, role_group::RoleGroupInputs}, + }, + crd::{HbaseRole, RegionServerConfig}, +}; + +pub(crate) const ROLE: HbaseRole = HbaseRole::RegionServer; + +/// The inputs of one region server role group. +pub(crate) fn inputs<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &RegionServerRoleGroupConfig, +) -> RoleGroupInputs<'a> { + let RegionServerConfig { + hbase_rootdir, + resources, + // Validated up front into `rg_config.config.logging`. + logging: _, + affinity, + graceful_shutdown_timeout, + requested_secret_lifetime, + listener_class, + region_mover, + } = &rg_config.config.config; + + RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + affinity: affinity.clone(), + graceful_shutdown_timeout: *graceful_shutdown_timeout, + requested_secret_lifetime: *requested_secret_lifetime, + resources: resources.clone(), + hbase_rootdir: hbase_rootdir.clone(), + logging: rg_config.config.logging.clone(), + listener_class: listener_class.clone(), + region_mover_opts: region_mover::region_mover_args(&rg_config.config.config), + run_region_mover: region_mover.run_before_shutdown, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs new file mode 100644 index 00000000..c6898c69 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -0,0 +1,53 @@ +use stackable_operator::{ + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, +}; + +use crate::{ + controller::{RestServerRoleGroupConfig, ValidatedCluster, build::role_group::RoleGroupInputs}, + crd::{HbaseConfig, HbaseRole}, +}; + +pub(crate) const ROLE: HbaseRole = HbaseRole::RestServer; + +/// The inputs of one rest server role group. +pub(crate) fn inputs<'a>( + cluster: &'a ValidatedCluster, + cluster_info: &'a KubernetesClusterInfo, + role_group_name: &RoleGroupName, + rg_config: &RestServerRoleGroupConfig, +) -> RoleGroupInputs<'a> { + let HbaseConfig { + hbase_rootdir, + resources, + // Validated up front into `rg_config.config.logging`. + logging: _, + affinity, + graceful_shutdown_timeout, + requested_secret_lifetime, + listener_class, + } = &rg_config.config.config; + + RoleGroupInputs { + cluster, + cluster_info, + role: ROLE, + role_group_name: role_group_name.clone(), + affinity: affinity.clone(), + graceful_shutdown_timeout: *graceful_shutdown_timeout, + requested_secret_lifetime: *requested_secret_lifetime, + resources: resources.clone(), + hbase_rootdir: hbase_rootdir.clone(), + logging: rg_config.config.logging.clone(), + listener_class: listener_class.clone(), + region_mover_opts: String::new(), + run_region_mover: false, + replicas: rg_config.replicas, + config_overrides: rg_config.config_overrides.clone(), + env_overrides: rg_config.env_overrides.clone(), + pod_overrides: rg_config.pod_overrides.clone(), + jvm_argument_overrides: rg_config + .product_specific_common_config + .jvm_argument_overrides + .clone(), + } +} diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index cc346c13..24e97d83 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -25,7 +25,7 @@ use stackable_operator::{ v2::{ HasName, HasUid, NameIsValidLabelValue, role_group_utils::ResourceNames, - role_utils, + role_utils::{self, JavaCommonConfig, RoleGroupConfig}, types::{ kubernetes::{ConfigMapName, NamespaceName, SecretClassName, Uid}, operator::{ClusterName, ControllerName, OperatorName, ProductName, ProductVersion}, @@ -34,8 +34,11 @@ use stackable_operator::{ }; use crate::{ - controller::{build::opa::HbaseOpaConfig, zookeeper::ZookeeperConnectionInformation}, - crd::{APP_NAME, AnyServiceConfig, HBASE_OPERATOR_NAME, HbaseRole, v1alpha1}, + controller::{ + build::opa::HbaseOpaConfig, validate::ValidatedLogging, + zookeeper::ZookeeperConnectionInformation, + }, + crd::{APP_NAME, HBASE_OPERATOR_NAME, HbaseConfig, HbaseRole, RegionServerConfig, v1alpha1}, }; pub const HBASE_CONTROLLER_NAME: &str = "hbasecluster"; @@ -46,12 +49,18 @@ constant!(PRODUCT_NAME: ProductName = APP_NAME); constant!(OPERATOR_NAME: OperatorName = HBASE_OPERATOR_NAME); constant!(CONTROLLER_NAME: ControllerName = HBASE_CONTROLLER_NAME); +pub(crate) const DEFAULT_REPLICAS: u16 = 1; + /// Marker for prepared Kubernetes resources which are not applied yet. pub struct Prepared; /// Marker for applied Kubernetes resources. pub struct Applied; +pub(crate) fn replicas_or_default(replicas: Option) -> u16 { + replicas.unwrap_or(DEFAULT_REPLICAS) +} + /// The complete set of Kubernetes resources built for a [`ValidatedCluster`], ready to be applied. /// /// hbase exposes its listeners as volume/PVC sources inside the `StatefulSet` rather than as @@ -70,6 +79,30 @@ pub struct KubernetesResources { pub status: PhantomData, } +/// A master role group's validated config together with its validated logging. +#[derive(Clone, Debug)] +pub struct ValidatedMasterConfig { + pub config: HbaseConfig, + pub logging: ValidatedLogging, +} +#[derive(Clone, Debug)] +pub struct ValidatedRegionServerConfig { + pub config: RegionServerConfig, + pub logging: ValidatedLogging, +} +#[derive(Clone, Debug)] +pub struct ValidatedRestServerConfig { + pub config: HbaseConfig, + pub logging: ValidatedLogging, +} + +pub type MasterRoleGroupConfig = + RoleGroupConfig; +pub type RegionServerRoleGroupConfig = + RoleGroupConfig; +pub type RestServerRoleGroupConfig = + RoleGroupConfig; + /// The validated cluster: proves that config merging and validation succeeded for /// every role and role group before any resources are created. #[derive(Clone, Debug)] @@ -90,41 +123,37 @@ pub struct ValidatedCluster { /// value. pub product_version: ProductVersion, pub cluster_config: ValidatedClusterConfig, - pub role_group_configs: BTreeMap>, - pub role_configs: BTreeMap, + /// The masters' role-level config. Masters are required. + pub master_config: ValidatedRoleConfig, + /// The validated config of every master role group, keyed by role group name. + pub master_role_group_configs: BTreeMap, + pub region_server_config: ValidatedRoleConfig, + pub region_server_role_group_configs: BTreeMap, + /// The rest servers' role-level config, or `None` if the role is absent. + pub rest_server_config: Option, + /// Empty if the role is absent. + pub rest_server_role_group_configs: BTreeMap, } impl ValidatedCluster { - #[allow(clippy::too_many_arguments)] - pub fn new( - name: ClusterName, - namespace: NamespaceName, - uid: Uid, - image: ResolvedProductImage, - cluster_config: ValidatedClusterConfig, - role_group_configs: BTreeMap>, - role_configs: BTreeMap, - ) -> Self { + pub(crate) fn object_meta( + name: &ClusterName, + namespace: &NamespaceName, + uid: &Uid, + ) -> ObjectMeta { + ObjectMeta { + name: Some(name.to_string()), + namespace: Some(namespace.to_string()), + uid: Some(uid.to_string()), + ..ObjectMeta::default() + } + } + + pub(crate) fn product_version(image: &ResolvedProductImage) -> ProductVersion { // `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()), - namespace: Some(namespace.to_string()), - uid: Some(uid.to_string()), - ..ObjectMeta::default() - }, - name, - namespace, - uid, - image, - product_version, - cluster_config, - role_group_configs, - role_configs, - } + ProductVersion::from_str(&image.app_version_label_value) + .expect("the app version label value is a valid product version") } /// Type-safe names for the per-cluster RBAC resources: the ServiceAccount shared by all @@ -230,25 +259,6 @@ pub struct ValidatedRoleConfig { pub pdb: stackable_operator::commons::pdb::PdbConfig, } -/// The validated per-rolegroup product configuration: the merged CRD config and the resolved -/// logging settings. The merged (role <- role group) `configOverrides`, `envOverrides` and -/// `podOverrides` live on the enclosing [`HbaseRoleGroupConfig`] (the `RoleGroupConfig` wrapper), -/// not here. -#[derive(Clone, Debug)] -pub struct ValidatedHbaseConfig { - /// The merged, role-specific product config. - pub config: AnyServiceConfig, - /// The validated logging configuration (HBase + optional Vector container), resolved up-front - /// during validation. - pub logging: validate::ValidatedLogging, -} - -pub type HbaseRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupConfig< - ValidatedHbaseConfig, - stackable_operator::v2::role_utils::JavaCommonConfig, - v1alpha1::HbaseConfigOverrides, ->; - #[cfg(test)] mod tests { use super::{CONTROLLER_NAME, OPERATOR_NAME, PRODUCT_NAME}; diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 63923183..d7198bd6 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -1,6 +1,6 @@ use std::{collections::BTreeMap, str::FromStr}; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{OptionExt, ResultExt, Snafu, ensure}; use stackable_operator::{ commons::product_image_selection::{self}, config::{ @@ -16,20 +16,24 @@ use stackable_operator::{ ValidatedContainerLogConfigChoice, VectorContainerLogConfig, validate_logging_configuration_for_container, }, - role_utils::{CommonConfiguration, JavaCommonConfig, Role, with_validated_config}, + role_utils::{ + CommonConfiguration, JavaCommonConfig, Role, RoleGroup, RoleGroupConfig, + with_validated_config, + }, types::{kubernetes::ConfigMapName, operator::RoleGroupName}, }, }; -use strum::IntoEnumIterator; use crate::{ controller::{ - HbaseRoleGroupConfig, ValidatedCluster, ValidatedClusterConfig, ValidatedHbaseConfig, - ValidatedRoleConfig, dereference::DereferencedObjects, + MasterRoleGroupConfig, RegionServerRoleGroupConfig, RestServerRoleGroupConfig, + ValidatedCluster, ValidatedClusterConfig, ValidatedMasterConfig, + ValidatedRegionServerConfig, ValidatedRestServerConfig, ValidatedRoleConfig, + dereference::DereferencedObjects, }, crd::{ - AnyServiceConfig, Container, HbaseConfigFragment, HbaseRole, RegionServerConfigFragment, - v1alpha1, + Container, HbaseConfig, HbaseConfigFragment, HbaseRole, RegionServerConfig, + RegionServerConfigFragment, v1alpha1, }, }; @@ -48,14 +52,18 @@ pub enum Error { }, #[snafu(display("the HbaseCluster has no {role} role defined"))] - MissingRequiredRole { role: String }, + MissingRequiredRole { role: HbaseRole }, - #[snafu(display("failed to merge and validate the role group config"))] - ValidateRoleGroupConfig { source: fragment::ValidationError }, + #[snafu(display("failed to merge and validate the {role} role group config"))] + ValidateRoleGroupConfig { + source: fragment::ValidationError, + role: HbaseRole, + }, - #[snafu(display("invalid role group name {role_group}"))] + #[snafu(display("invalid role group name {role_group} for role {role}"))] ParseRoleGroupName { source: stackable_operator::v2::macros::attributed_string_type::Error, + role: HbaseRole, role_group: String, }, @@ -124,9 +132,6 @@ pub fn validate_cluster( ) .context(ResolveProductImageSnafu)?; - let mut role_groups = BTreeMap::new(); - let mut role_configs = BTreeMap::new(); - let hdfs_discovery_cm_name = hbase.spec.cluster_config.hdfs_config_map_name.as_ref(); let cluster_name = hbase.name_any(); @@ -146,85 +151,118 @@ pub fn validate_cluster( .as_ref() .and_then(|authorization| authorization.opa.as_ref()); - for hbase_role in HbaseRole::iter() { - let group_configs = match hbase_role { - HbaseRole::Master => validate_role_group_configs( - hbase.spec.masters.as_ref(), - HbaseConfigFragment::default_config( - &hbase_role, - &cluster_name, - hdfs_discovery_cm_name, - opa_config, - ), - AnyServiceConfig::Master, - &vector_aggregator_config_map_name, - )?, - HbaseRole::RegionServer => validate_role_group_configs( - hbase.spec.region_servers.as_ref(), - RegionServerConfigFragment::default_config( - &hbase_role, - &cluster_name, - hdfs_discovery_cm_name, - opa_config, - ), - AnyServiceConfig::RegionServer, - &vector_aggregator_config_map_name, - )?, - HbaseRole::RestServer => validate_role_group_configs( - hbase.spec.rest_servers.as_ref(), - HbaseConfigFragment::default_config( - &hbase_role, - &cluster_name, - hdfs_discovery_cm_name, - opa_config, - ), - AnyServiceConfig::RestServer, - &vector_aggregator_config_map_name, - )?, - }; - - // masters and region servers are required; rest servers are optional. - if group_configs.is_empty() { - match hbase_role { - HbaseRole::Master | HbaseRole::RegionServer => { - return MissingRequiredRoleSnafu { - role: hbase_role.to_string(), - } - .fail(); - } - HbaseRole::RestServer => continue, - } + // The first failure propagates, so this order decides which misconfiguration the user is told + // about when more than one role is wrong. It is `HbaseRole`'s variant order. + let masters = hbase + .spec + .masters + .as_ref() + .context(MissingRequiredRoleSnafu { + role: HbaseRole::Master, + })?; + let master_role_group_configs = validate_master_role_group_configs( + masters, + &HbaseConfigFragment::default_config( + &HbaseRole::Master, + &cluster_name, + hdfs_discovery_cm_name, + opa_config, + ), + &vector_aggregator_config_map_name, + )?; + ensure!( + !master_role_group_configs.is_empty(), + MissingRequiredRoleSnafu { + role: HbaseRole::Master } + ); - if let Some(GenericRoleConfig { - pod_disruption_budget: pdb, - }) = hbase.role_config(&hbase_role) - { - role_configs.insert(hbase_role.clone(), ValidatedRoleConfig { pdb: pdb.clone() }); - } + let GenericRoleConfig { + pod_disruption_budget: master_pdb, + } = &masters.role_config; - role_groups.insert(hbase_role, group_configs); - } + let region_servers = hbase + .spec + .region_servers + .as_ref() + .context(MissingRequiredRoleSnafu { + role: HbaseRole::RegionServer, + })?; + let region_server_role_group_configs = validate_region_server_role_group_configs( + region_servers, + &RegionServerConfigFragment::default_config( + &HbaseRole::RegionServer, + &cluster_name, + hdfs_discovery_cm_name, + opa_config, + ), + &vector_aggregator_config_map_name, + )?; + ensure!( + !region_server_role_group_configs.is_empty(), + MissingRequiredRoleSnafu { + role: HbaseRole::RegionServer + } + ); + + let GenericRoleConfig { + pod_disruption_budget: region_server_pdb, + } = ®ion_servers.role_config; + + let rest_server_role_group_configs = match &hbase.spec.rest_servers { + Some(rest_servers) => validate_rest_server_role_group_configs( + rest_servers, + &HbaseConfigFragment::default_config( + &HbaseRole::RestServer, + &cluster_name, + hdfs_discovery_cm_name, + opa_config, + ), + &vector_aggregator_config_map_name, + )?, + None => BTreeMap::new(), + }; + let rest_server_config = match &hbase.spec.rest_servers { + Some(rest_servers) if !rest_server_role_group_configs.is_empty() => { + let GenericRoleConfig { + pod_disruption_budget, + } = &rest_servers.role_config; + Some(ValidatedRoleConfig { + pdb: pod_disruption_budget.clone(), + }) + } + _ => None, + }; let name = get_cluster_name(hbase).context(GetClusterIdentitySnafu)?; let namespace = get_namespace(hbase).context(GetClusterIdentitySnafu)?; let uid = get_uid(hbase).context(GetClusterIdentitySnafu)?; - Ok(ValidatedCluster::new( + Ok(ValidatedCluster { + metadata: ValidatedCluster::object_meta(&name, &namespace, &uid), name, namespace, uid, - resolved_product_image, - ValidatedClusterConfig { + product_version: ValidatedCluster::product_version(&resolved_product_image), + image: resolved_product_image, + cluster_config: ValidatedClusterConfig { zookeeper_connection_information: dereferenced_objects.zookeeper_connection_information, hbase_opa_config: dereferenced_objects.hbase_opa_config, kerberos_secret_class: hbase.kerberos_secret_class(), https_secret_class: hbase.https_secret_class(), hdfs_config_map_name: hbase.spec.cluster_config.hdfs_config_map_name.clone(), }, - role_groups, - role_configs, - )) + master_config: ValidatedRoleConfig { + pdb: master_pdb.clone(), + }, + master_role_group_configs, + region_server_config: ValidatedRoleConfig { + pdb: region_server_pdb.clone(), + }, + region_server_role_group_configs, + rest_server_config, + rest_server_role_group_configs, + }) } /// Validates every role group of a role into a map keyed by role group name. @@ -234,71 +272,179 @@ pub fn validate_cluster( /// which folds the CRD config fragment (default <- role <- role group) plus the /// `configOverrides`, `envOverrides`, `cliOverrides`, `podOverrides` and the /// `jvmArgumentOverrides` (role group wins) into a single merged -/// [`RoleGroup`](stackable_operator::v2::role_utils::RoleGroup). The per-role validated config -/// is wrapped into [`AnyServiceConfig`] via `wrap`; the merged `envOverrides` are converted -/// into an `EnvVarSet` (validating at deserialize time). The merged `jvmArgumentOverrides` are -/// kept in `product_specific_common_config` and applied at build time. +/// `RoleGroup`. /// /// Returns an empty map if the role is not configured. -fn validate_role_group_configs( - role: Option< - &Role, - >, - default_config: Config, - wrap: fn(ValidatedConfig) -> AnyServiceConfig, - vector_aggregator_config_map_name: &Option, -) -> Result, Error> +fn validate_role_group_config( + hbase_role: &HbaseRole, + role: &Role, + role_group: &RoleGroup, + default_config: &Config, +) -> Result, Error> where Config: Clone + Merge, ValidatedConfig: FromFragment, { - let Some(role) = role else { - return Ok(BTreeMap::new()); + let validated = with_validated_config::< + ValidatedConfig, + JavaCommonConfig, + Config, + GenericRoleConfig, + v1alpha1::HbaseConfigOverrides, + >(role_group, role, default_config) + .context(ValidateRoleGroupConfigSnafu { + role: hbase_role.clone(), + })?; + + let CommonConfiguration { + config, + config_overrides, + env_overrides, + cli_overrides, + pod_overrides, + product_specific_common_config, + } = validated.config; + + let validated = RoleGroupConfig { + replicas: validated.replicas, + config, + config_overrides, + env_overrides: env_overrides.into(), + cli_overrides, + pod_overrides, + product_specific_common_config, }; + Ok(validated) +} + +fn validate_master_role_group_configs( + role: &Role< + HbaseConfigFragment, + v1alpha1::HbaseConfigOverrides, + GenericRoleConfig, + JavaCommonConfig, + >, + default_config: &HbaseConfigFragment, + vector_aggregator_config_map_name: &Option, +) -> Result, Error> { + role.role_groups + .iter() + .map(|(role_group_name, role_group)| { + let merged = validate_role_group_config::<_, HbaseConfig>( + &HbaseRole::Master, + role, + role_group, + default_config, + )?; + let logging = + validate_logging(&merged.config.logging, vector_aggregator_config_map_name)?; + let validated = MasterRoleGroupConfig { + replicas: merged.replicas, + config: ValidatedMasterConfig { + config: merged.config, + logging, + }, + config_overrides: merged.config_overrides, + env_overrides: merged.env_overrides, + cli_overrides: merged.cli_overrides, + pod_overrides: merged.pod_overrides, + product_specific_common_config: merged.product_specific_common_config, + }; + let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| { + ParseRoleGroupNameSnafu { + role: HbaseRole::Master, + role_group: role_group_name.clone(), + } + })?; + Ok((role_group_name, validated)) + }) + .collect() +} +fn validate_region_server_role_group_configs( + role: &Role< + RegionServerConfigFragment, + v1alpha1::HbaseConfigOverrides, + GenericRoleConfig, + JavaCommonConfig, + >, + default_config: &RegionServerConfigFragment, + vector_aggregator_config_map_name: &Option, +) -> Result, Error> { role.role_groups .iter() .map(|(role_group_name, role_group)| { + let merged = validate_role_group_config::<_, RegionServerConfig>( + &HbaseRole::RegionServer, + role, + role_group, + default_config, + )?; + let logging = + validate_logging(&merged.config.logging, vector_aggregator_config_map_name)?; + let validated = RegionServerRoleGroupConfig { + replicas: merged.replicas, + config: ValidatedRegionServerConfig { + config: merged.config, + logging, + }, + config_overrides: merged.config_overrides, + env_overrides: merged.env_overrides, + cli_overrides: merged.cli_overrides, + pod_overrides: merged.pod_overrides, + product_specific_common_config: merged.product_specific_common_config, + }; let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| { ParseRoleGroupNameSnafu { + role: HbaseRole::RegionServer, role_group: role_group_name.clone(), } })?; - let validated = with_validated_config::< - ValidatedConfig, - JavaCommonConfig, - Config, - GenericRoleConfig, - v1alpha1::HbaseConfigOverrides, - >(role_group, role, &default_config) - .context(ValidateRoleGroupConfigSnafu)?; - - let CommonConfiguration { - config, - config_overrides, - env_overrides, - cli_overrides, - pod_overrides, - product_specific_common_config, - } = validated.config; - - let config = wrap(config); - - // Validate the logging configuration up-front so an invalid custom log ConfigMap name - // or a missing Vector aggregator discovery ConfigMap fails here rather than at build - // time. The build step then consumes the validated logging instead of the raw config. - let logging = validate_logging(config.logging(), vector_aggregator_config_map_name)?; - - let validated = HbaseRoleGroupConfig { - replicas: validated.replicas, - config: ValidatedHbaseConfig { config, logging }, - config_overrides, - env_overrides: env_overrides.into(), - cli_overrides, - pod_overrides, - product_specific_common_config, + Ok((role_group_name, validated)) + }) + .collect() +} + +fn validate_rest_server_role_group_configs( + role: &Role< + HbaseConfigFragment, + v1alpha1::HbaseConfigOverrides, + GenericRoleConfig, + JavaCommonConfig, + >, + default_config: &HbaseConfigFragment, + vector_aggregator_config_map_name: &Option, +) -> Result, Error> { + role.role_groups + .iter() + .map(|(role_group_name, role_group)| { + let merged = validate_role_group_config::<_, HbaseConfig>( + &HbaseRole::RestServer, + role, + role_group, + default_config, + )?; + let logging = + validate_logging(&merged.config.logging, vector_aggregator_config_map_name)?; + let validated = RestServerRoleGroupConfig { + replicas: merged.replicas, + config: ValidatedRestServerConfig { + config: merged.config, + logging, + }, + config_overrides: merged.config_overrides, + env_overrides: merged.env_overrides, + cli_overrides: merged.cli_overrides, + pod_overrides: merged.pod_overrides, + product_specific_common_config: merged.product_specific_common_config, }; - Ok((role_group_name.clone(), validated)) + let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| { + ParseRoleGroupNameSnafu { + role: HbaseRole::RestServer, + role_group: role_group_name.clone(), + } + })?; + Ok((role_group_name, validated)) }) .collect() } diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 6a5485e4..31ca23b6 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -157,9 +157,30 @@ mod tests { let hbase: v1alpha1::HbaseCluster = serde_yaml::from_str(input).expect("illegal test input"); let validated_cluster = crate::test_utils::validated_cluster_from(&hbase); - let affinity = crate::test_utils::merged_config(&validated_cluster, &role) - .affinity() - .clone(); + + let affinity = match role { + HbaseRole::Master => { + crate::test_utils::master_role_group_config(&validated_cluster, "default") + .config + .config + .affinity + .clone() + } + HbaseRole::RegionServer => { + crate::test_utils::region_server_role_group_config(&validated_cluster, "default") + .config + .config + .affinity + .clone() + } + HbaseRole::RestServer => { + crate::test_utils::rest_server_role_group_config(&validated_cluster, "default") + .config + .config + .affinity + .clone() + } + }; let mut expected_affinities = vec![WeightedPodAffinityTerm { pod_affinity_term: PodAffinityTerm { diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 3f864fd4..e42df1f7 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -1,3 +1,4 @@ +use core::fmt; use std::{ops::Deref, str::FromStr}; use security::AuthenticationConfig; @@ -210,14 +211,6 @@ impl HasStatusCondition for v1alpha1::HbaseCluster { } impl v1alpha1::HbaseCluster { - pub fn role_config(&self, role: &HbaseRole) -> Option<&GenericRoleConfig> { - match role { - HbaseRole::Master => self.spec.masters.as_ref().map(|m| &m.role_config), - HbaseRole::RegionServer => self.spec.region_servers.as_ref().map(|rs| &rs.role_config), - HbaseRole::RestServer => self.spec.rest_servers.as_ref().map(|rs| &rs.role_config), - } - } - pub fn kerberos_secret_class(&self) -> Option { self.spec .cluster_config @@ -247,6 +240,16 @@ pub enum HbaseRole { RestServer, } +impl fmt::Display for HbaseRole { + /// The role's name, e.g. `master`. + /// + /// `to_string()` already worked through the [`Deref`] to [`RoleName`]; this impl exists so a + /// role can be a `{role}` field in snafu `display` strings, which require `Display` itself. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.deref().fmt(f) + } +} + impl Deref for HbaseRole { type Target = RoleName; @@ -544,72 +547,6 @@ pub struct HbaseClusterStatus { pub conditions: Vec, } -#[derive(Clone, Debug)] -pub enum AnyServiceConfig { - Master(HbaseConfig), - RegionServer(RegionServerConfig), - RestServer(HbaseConfig), -} - -impl AnyServiceConfig { - pub fn resources(&self) -> &Resources { - match self { - AnyServiceConfig::Master(config) => &config.resources, - AnyServiceConfig::RegionServer(config) => &config.resources, - AnyServiceConfig::RestServer(config) => &config.resources, - } - } - - pub fn logging(&self) -> &Logging { - match self { - AnyServiceConfig::Master(config) => &config.logging, - AnyServiceConfig::RegionServer(config) => &config.logging, - AnyServiceConfig::RestServer(config) => &config.logging, - } - } - - pub fn affinity(&self) -> &StackableAffinity { - match self { - AnyServiceConfig::Master(config) => &config.affinity, - AnyServiceConfig::RegionServer(config) => &config.affinity, - AnyServiceConfig::RestServer(config) => &config.affinity, - } - } - - pub fn graceful_shutdown_timeout(&self) -> &Option { - match self { - AnyServiceConfig::Master(config) => &config.graceful_shutdown_timeout, - AnyServiceConfig::RegionServer(config) => &config.graceful_shutdown_timeout, - AnyServiceConfig::RestServer(config) => &config.graceful_shutdown_timeout, - } - } - - pub fn requested_secret_lifetime(&self) -> Option { - match self { - AnyServiceConfig::Master(config) => config.requested_secret_lifetime, - AnyServiceConfig::RegionServer(config) => config.requested_secret_lifetime, - AnyServiceConfig::RestServer(config) => config.requested_secret_lifetime, - } - } - - pub fn listener_class(&self) -> ListenerClassName { - match self { - AnyServiceConfig::Master(config) => config.listener_class.clone(), - AnyServiceConfig::RegionServer(config) => config.listener_class.clone(), - AnyServiceConfig::RestServer(config) => config.listener_class.clone(), - } - } - - /// The configured `hbase.rootdir`. - pub fn hbase_rootdir(&self) -> String { - match self { - AnyServiceConfig::Master(config) => config.hbase_rootdir.clone(), - AnyServiceConfig::RegionServer(config) => config.hbase_rootdir.clone(), - AnyServiceConfig::RestServer(config) => config.hbase_rootdir.clone(), - } - } -} - #[cfg(test)] mod tests { use indoc::indoc; @@ -617,6 +554,7 @@ mod tests { use stackable_operator::versioned::test_utils::RoundtripTestData; use super::*; + use crate::test_utils; #[test] fn test_constants() { @@ -679,23 +617,19 @@ spec: serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); let validated_cluster = crate::test_utils::validated_cluster_from(&hbase); - let merged_config = crate::test_utils::merged_config_for( - &validated_cluster, - &HbaseRole::RegionServer, - role_group_name, + + let config = + &test_utils::region_server_role_group_config(&validated_cluster, role_group_name) + .config + .config; + assert_eq!(run_before_shutdown, config.region_mover.run_before_shutdown); + assert_eq!(max_threads, config.region_mover.max_threads); + assert_eq!( + Some(RegionMoverExtraCliOpts { + additional_mover_options + }), + config.region_mover.cli_opts ); - if let AnyServiceConfig::RegionServer(config) = merged_config { - assert_eq!(run_before_shutdown, config.region_mover.run_before_shutdown); - assert_eq!(max_threads, config.region_mover.max_threads); - assert_eq!( - Some(RegionMoverExtraCliOpts { - additional_mover_options - }), - config.region_mover.cli_opts - ); - } else { - panic!("this shouldn't happen"); - }; } impl RoundtripTestData for v1alpha1::HbaseClusterSpec { diff --git a/rust/operator-binary/src/test_utils.rs b/rust/operator-binary/src/test_utils.rs index 5b327bfa..1b3e9d3e 100644 --- a/rust/operator-binary/src/test_utils.rs +++ b/rust/operator-binary/src/test_utils.rs @@ -14,10 +14,11 @@ use stackable_operator::{ use crate::{ controller::{ + MasterRoleGroupConfig, RegionServerRoleGroupConfig, RestServerRoleGroupConfig, ValidatedCluster, dereference::DereferencedObjects, validate::validate_cluster, zookeeper::ZookeeperConnectionInformation, }, - crd::{AnyServiceConfig, HbaseRole, v1alpha1}, + crd::v1alpha1, }; /// A minimal three-role `HbaseCluster` used to drive the builder/property tests. @@ -99,21 +100,23 @@ pub fn cluster_info() -> KubernetesClusterInfo { } } -/// The merged [`AnyServiceConfig`] for the given `role` and `role_group`. -pub fn merged_config_for<'a>( - validated_cluster: &'a ValidatedCluster, - role: &HbaseRole, +pub fn master_role_group_config<'a>( + cluster: &'a ValidatedCluster, role_group: &str, -) -> &'a AnyServiceConfig { - &validated_cluster.role_group_configs[role][&role_group_name(role_group)] - .config - .config +) -> &'a MasterRoleGroupConfig { + &cluster.master_role_group_configs[&role_group_name(role_group)] } -/// The merged [`AnyServiceConfig`] for the `default` role group of `role`. -pub fn merged_config<'a>( - validated_cluster: &'a ValidatedCluster, - role: &HbaseRole, -) -> &'a AnyServiceConfig { - merged_config_for(validated_cluster, role, "default") +pub fn region_server_role_group_config<'a>( + cluster: &'a ValidatedCluster, + role_group: &str, +) -> &'a RegionServerRoleGroupConfig { + &cluster.region_server_role_group_configs[&role_group_name(role_group)] +} + +pub fn rest_server_role_group_config<'a>( + cluster: &'a ValidatedCluster, + role_group: &str, +) -> &'a RestServerRoleGroupConfig { + &cluster.rest_server_role_group_configs[&role_group_name(role_group)] } From 1107993cb2dee6c9bbe88288eee3eb05f2c6f3d8 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 19 Oct 2026 14:16:25 +0200 Subject: [PATCH 03/13] refactor: move the per-role config tables into the role modules --- .../controller/build/properties/hbase_env.rs | 23 +-- .../controller/build/properties/hbase_site.rs | 153 ++++++++++-------- .../build/properties/security_properties.rs | 28 +--- .../controller/build/resource/config_map.rs | 16 +- .../src/controller/build/role_group/master.rs | 11 +- .../src/controller/build/role_group/mod.rs | 8 + .../build/role_group/region_server.rs | 9 +- .../build/role_group/rest_server.rs | 11 +- 8 files changed, 141 insertions(+), 118 deletions(-) diff --git a/rust/operator-binary/src/controller/build/properties/hbase_env.rs b/rust/operator-binary/src/controller/build/properties/hbase_env.rs index f816657d..ec752365 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_env.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_env.rs @@ -10,7 +10,7 @@ use stackable_operator::{ use crate::{ controller::build::jvm::{self, construct_global_jvm_args, construct_hbase_heapsize_env}, - crd::{HbaseRole, HbaseStorageConfig}, + crd::HbaseStorageConfig, }; /// The `hbase-env.sh` variable that carries a role's own JVM options; `bin/hbase` reads the one @@ -28,7 +28,7 @@ pub enum Error { /// Renders `hbase-env.sh` as `export VAR="VALUE"` lines. pub fn build( resources: &Resources, - role: &HbaseRole, + opts_variable: &str, kerberos_enabled: bool, non_heap_jvm_args: String, overrides: KeyValueConfigOverrides, @@ -44,18 +44,7 @@ pub fn build( "HBASE_OPTS".to_string(), construct_global_jvm_args(kerberos_enabled), ); - - match role { - HbaseRole::Master => { - env.insert(HBASE_MASTER_OPTS.to_string(), non_heap_jvm_args); - } - HbaseRole::RegionServer => { - env.insert(HBASE_REGIONSERVER_OPTS.to_string(), non_heap_jvm_args); - } - HbaseRole::RestServer => { - env.insert(HBASE_REST_OPTS.to_string(), non_heap_jvm_args); - } - } + env.insert(opts_variable.to_string(), non_heap_jvm_args); // configOverride come last env.extend(overrides); @@ -82,7 +71,7 @@ mod tests { .resources; let env = build( merged, - &HbaseRole::Master, + HBASE_MASTER_OPTS, false, "-Xtest".to_string(), KeyValueConfigOverrides::default(), @@ -101,7 +90,7 @@ mod tests { .resources; let env = build( merged, - &HbaseRole::RegionServer, + HBASE_REGIONSERVER_OPTS, false, "-Xtest".to_string(), KeyValueConfigOverrides::default(), @@ -119,7 +108,7 @@ mod tests { .resources; let env = build( merged, - &HbaseRole::Master, + HBASE_MASTER_OPTS, false, "-Xtest".to_string(), [("CUSTOM_VAR", "custom_value")].into(), diff --git a/rust/operator-binary/src/controller/build/properties/hbase_site.rs b/rust/operator-binary/src/controller/build/properties/hbase_site.rs index a982c41f..e31770e2 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_site.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_site.rs @@ -78,8 +78,8 @@ pub fn client_bootstrap_servers( /// Renders `hbase-site.xml`. pub fn build( - role: &HbaseRole, hbase_rootdir: &str, + role_hbase_site: BTreeMap, zookeeper_config: BTreeMap, kerberos_config: BTreeMap, opa_config: Option<&HbaseOpaConfig>, @@ -111,80 +111,91 @@ pub fn build( "false".to_string(), ); - match role { - HbaseRole::Master => { - config.insert( - HBASE_MASTER_IPC_ADDRESS.to_string(), - BIND_ALL_ADDRESSES.to_string(), - ); - config.insert( - HBASE_MASTER_IPC_PORT.to_string(), - HBASE_MASTER_PORT.to_string(), - ); - config.insert( - HBASE_MASTER_HOSTNAME.to_string(), - ENV_HBASE_SERVICE_HOST.to_string(), - ); - config.insert( - HBASE_MASTER_PORT_KEY.to_string(), - ENV_HBASE_SERVICE_PORT.to_string(), - ); - config.insert( - HBASE_MASTER_INFO_PORT.to_string(), - ENV_HBASE_INFO_PORT.to_string(), - ); - config.insert( - HBASE_MASTER_BOUND_INFO_PORT.to_string(), - HBASE_MASTER_UI_PORT.to_string(), - ); - } - HbaseRole::RegionServer => { - config.insert( - HBASE_REGIONSERVER_IPC_ADDRESS.to_string(), - BIND_ALL_ADDRESSES.to_string(), - ); - config.insert( - HBASE_REGIONSERVER_IPC_PORT.to_string(), - HBASE_REGIONSERVER_PORT.to_string(), - ); - config.insert( - HBASE_UNSAFE_REGIONSERVER_HOSTNAME.to_string(), - ENV_HBASE_SERVICE_HOST.to_string(), - ); - config.insert( - HBASE_REGIONSERVER_PORT_KEY.to_string(), - ENV_HBASE_SERVICE_PORT.to_string(), - ); - config.insert( - HBASE_REGIONSERVER_INFO_PORT.to_string(), - ENV_HBASE_INFO_PORT.to_string(), - ); - config.insert( - HBASE_REGIONSERVER_BOUND_INFO_PORT.to_string(), - HBASE_REGIONSERVER_UI_PORT.to_string(), - ); - } - HbaseRole::RestServer => { - config.insert( - // N.B. a custom tag, so as not to interfere with HBase internals. - // The other roles use a patch to correctly resolve host/port. - HBASE_REST_ENDPOINT.to_string(), - format!("{ENV_HBASE_SERVICE_HOST}:{ENV_HBASE_SERVICE_PORT}"), - ); - } - }; + config.extend(role_hbase_site); // configOverride come last build_xml_config(config, overrides) } +pub(crate) fn master_entries() -> BTreeMap { + BTreeMap::from([ + ( + HBASE_MASTER_IPC_ADDRESS.to_string(), + BIND_ALL_ADDRESSES.to_string(), + ), + ( + HBASE_MASTER_IPC_PORT.to_string(), + HBASE_MASTER_PORT.to_string(), + ), + ( + HBASE_MASTER_HOSTNAME.to_string(), + ENV_HBASE_SERVICE_HOST.to_string(), + ), + ( + HBASE_MASTER_PORT_KEY.to_string(), + ENV_HBASE_SERVICE_PORT.to_string(), + ), + ( + HBASE_MASTER_INFO_PORT.to_string(), + ENV_HBASE_INFO_PORT.to_string(), + ), + ( + HBASE_MASTER_BOUND_INFO_PORT.to_string(), + HBASE_MASTER_UI_PORT.to_string(), + ), + ]) +} + +pub(crate) fn region_server_entries() -> BTreeMap { + BTreeMap::from([ + ( + HBASE_REGIONSERVER_IPC_ADDRESS.to_string(), + BIND_ALL_ADDRESSES.to_string(), + ), + ( + HBASE_REGIONSERVER_IPC_PORT.to_string(), + HBASE_REGIONSERVER_PORT.to_string(), + ), + ( + HBASE_UNSAFE_REGIONSERVER_HOSTNAME.to_string(), + ENV_HBASE_SERVICE_HOST.to_string(), + ), + ( + HBASE_REGIONSERVER_PORT_KEY.to_string(), + ENV_HBASE_SERVICE_PORT.to_string(), + ), + ( + HBASE_REGIONSERVER_INFO_PORT.to_string(), + ENV_HBASE_INFO_PORT.to_string(), + ), + ( + HBASE_REGIONSERVER_BOUND_INFO_PORT.to_string(), + HBASE_REGIONSERVER_UI_PORT.to_string(), + ), + ]) +} + +pub(crate) fn rest_server_entries() -> BTreeMap { + BTreeMap::from([ + // N.B. a custom tag, so as not to interfere with HBase internals. + // The other roles use a patch to correctly resolve host/port. + ( + HBASE_REST_ENDPOINT.to_string(), + format!("{ENV_HBASE_SERVICE_HOST}:{ENV_HBASE_SERVICE_PORT}"), + ), + ]) +} + #[cfg(test)] mod tests { use indoc::indoc; use super::*; - use crate::test_utils::{ - self, cluster_info, hbase_from_yaml, validated_cluster, validated_cluster_from, + use crate::{ + controller::build::properties::hbase_site, + test_utils::{ + self, cluster_info, hbase_from_yaml, validated_cluster, validated_cluster_from, + }, }; fn bootstrap_servers_for(masters_yaml: &str) -> String { @@ -256,8 +267,8 @@ spec: .config .config; let xml = build( - &HbaseRole::Master, &config.hbase_rootdir, + hbase_site::master_entries(), BTreeMap::new(), BTreeMap::new(), None, @@ -285,8 +296,8 @@ spec: .config .config; let xml = build( - &HbaseRole::RegionServer, &config.hbase_rootdir, + hbase_site::region_server_entries(), BTreeMap::new(), BTreeMap::new(), None, @@ -314,8 +325,8 @@ spec: .config .config; let xml = build( - &HbaseRole::RestServer, &config.hbase_rootdir, + hbase_site::rest_server_entries(), BTreeMap::new(), BTreeMap::new(), None, @@ -337,8 +348,8 @@ spec: .config .config; let xml = build( - &HbaseRole::RestServer, &config.hbase_rootdir, + hbase_site::rest_server_entries(), BTreeMap::new(), BTreeMap::new(), None, @@ -360,8 +371,8 @@ spec: .config .config; let xml = build( - &HbaseRole::RestServer, &config.hbase_rootdir, + hbase_site::rest_server_entries(), BTreeMap::new(), BTreeMap::new(), None, @@ -383,8 +394,8 @@ spec: .config .config; let xml = build( - &HbaseRole::Master, &config.hbase_rootdir, + hbase_site::master_entries(), BTreeMap::new(), BTreeMap::new(), None, diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index 289e8a4b..beb95594 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -10,26 +10,18 @@ use stackable_operator::v2::{ config_overrides::KeyValueConfigOverrides, }; -use crate::crd::HbaseRole; - /// Renders `security.properties`: role-specific DNS cache TTLs plus user overrides. pub fn build( - role: &HbaseRole, + dns_cache_ttl_seconds: &str, overrides: KeyValueConfigOverrides, ) -> Result { - // Role-specific positive DNS cache TTLs. Caching forever (the JVM default for - // successful lookups) breaks failover when a peer's IP changes, so cap the + // Caching forever (the JVM default for successful lookups) + // breaks failover when a peer's IP changes, so cap the // positive cache and disable the negative cache. - let positive_ttl = match role { - HbaseRole::Master => "5", - HbaseRole::RegionServer => "10", - HbaseRole::RestServer => "30", - }; - let mut config: BTreeMap = BTreeMap::from([ ( "networkaddress.cache.ttl".to_string(), - positive_ttl.to_string(), + dns_cache_ttl_seconds.to_string(), ), ( "networkaddress.cache.negative.ttl".to_string(), @@ -48,7 +40,7 @@ mod tests { #[test] fn injects_master_dns_cache_ttl() { assert_eq!( - build(&HbaseRole::Master, KeyValueConfigOverrides::default()).unwrap(), + build("5", KeyValueConfigOverrides::default()).unwrap(), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=5\n" ); } @@ -56,7 +48,7 @@ mod tests { #[test] fn injects_regionserver_dns_cache_ttl() { assert_eq!( - build(&HbaseRole::RegionServer, KeyValueConfigOverrides::default()).unwrap(), + build("10", KeyValueConfigOverrides::default()).unwrap(), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=10\n" ); } @@ -64,7 +56,7 @@ mod tests { #[test] fn injects_restserver_dns_cache_ttl() { assert_eq!( - build(&HbaseRole::RestServer, KeyValueConfigOverrides::default()).unwrap(), + build("30", KeyValueConfigOverrides::default()).unwrap(), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=30\n" ); } @@ -72,11 +64,7 @@ mod tests { #[test] fn user_overrides_win_over_injected_defaults() { assert_eq!( - build( - &HbaseRole::Master, - [("networkaddress.cache.ttl", "60")].into() - ) - .unwrap(), + build("5", [("networkaddress.cache.ttl", "60")].into()).unwrap(), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=60\n" ); } 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 29218ffc..b79eff37 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -54,8 +54,8 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result let cluster_config = &cluster.cluster_config; let hbase_site_xml = hbase_site::build( - role, &inputs.hbase_rootdir, + inputs.role_hbase_site.clone(), cluster_config .zookeeper_connection_information .as_hbase_settings(), @@ -67,7 +67,7 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result let hbase_env_sh = hbase_env::build( &inputs.resources, - role, + inputs.hbase_env_opts_variable, cluster.has_kerberos_enabled(), construct_role_specific_non_heap_jvm_args(cluster, &inputs.jvm_argument_overrides), inputs.config_overrides.hbase_env_sh.clone(), @@ -83,11 +83,13 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result inputs.config_overrides.ssl_client_xml.clone(), ); - let security_properties = - security_properties::build(role, inputs.config_overrides.security_properties.clone()) - .with_context(|_| JvmSecurityPropertiesSnafu { - role_group: role_group_name.to_string(), - })?; + let security_properties = security_properties::build( + inputs.dns_cache_ttl_seconds, + inputs.config_overrides.security_properties.clone(), + ) + .with_context(|_| JvmSecurityPropertiesSnafu { + role_group: role_group_name.to_string(), + })?; let cm_metadata = object_meta( cluster, diff --git a/rust/operator-binary/src/controller/build/role_group/master.rs b/rust/operator-binary/src/controller/build/role_group/master.rs index 7af460eb..eb35640a 100644 --- a/rust/operator-binary/src/controller/build/role_group/master.rs +++ b/rust/operator-binary/src/controller/build/role_group/master.rs @@ -3,7 +3,13 @@ use stackable_operator::{ }; use crate::{ - controller::{MasterRoleGroupConfig, ValidatedCluster, build::role_group::RoleGroupInputs}, + controller::{ + MasterRoleGroupConfig, ValidatedCluster, + build::{ + properties::{hbase_env::HBASE_MASTER_OPTS, hbase_site}, + role_group::RoleGroupInputs, + }, + }, crd::{HbaseConfig, HbaseRole}, }; @@ -49,5 +55,8 @@ pub(crate) fn inputs<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), + dns_cache_ttl_seconds: "5", + hbase_env_opts_variable: HBASE_MASTER_OPTS, + role_hbase_site: hbase_site::master_entries(), } } diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index 33ae0d38..d22de1a6 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use stackable_operator::{ commons::{ affinity::StackableAffinity, @@ -46,4 +48,10 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) env_overrides: EnvVarSet, pub(crate) pod_overrides: PodTemplateSpec, pub(crate) jvm_argument_overrides: JvmArgumentOverrides, + /// The JVM's positive DNS cache TTL in seconds, written to `security.properties`. + pub(crate) dns_cache_ttl_seconds: &'static str, + /// The `hbase-env.sh` variable `bin/hbase` reads for this role's process. + pub(crate) hbase_env_opts_variable: &'static str, + /// The role's own `hbase-site.xml` entries (bind addresses and ports, or the REST endpoint). + pub(crate) role_hbase_site: BTreeMap, } diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index b6612520..972f1ddb 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -5,7 +5,11 @@ use stackable_operator::{ use crate::{ controller::{ RegionServerRoleGroupConfig, ValidatedCluster, - build::{region_mover, role_group::RoleGroupInputs}, + build::{ + properties::{hbase_env::HBASE_REGIONSERVER_OPTS, hbase_site}, + region_mover, + role_group::RoleGroupInputs, + }, }, crd::{HbaseRole, RegionServerConfig}, }; @@ -53,5 +57,8 @@ pub(crate) fn inputs<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), + dns_cache_ttl_seconds: "10", + hbase_env_opts_variable: HBASE_REGIONSERVER_OPTS, + role_hbase_site: hbase_site::region_server_entries(), } } diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index c6898c69..7e9f16ae 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -3,7 +3,13 @@ use stackable_operator::{ }; use crate::{ - controller::{RestServerRoleGroupConfig, ValidatedCluster, build::role_group::RoleGroupInputs}, + controller::{ + RestServerRoleGroupConfig, ValidatedCluster, + build::{ + properties::{hbase_env::HBASE_REST_OPTS, hbase_site}, + role_group::RoleGroupInputs, + }, + }, crd::{HbaseConfig, HbaseRole}, }; @@ -49,5 +55,8 @@ pub(crate) fn inputs<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), + dns_cache_ttl_seconds: "30", + hbase_env_opts_variable: HBASE_REST_OPTS, + role_hbase_site: hbase_site::rest_server_entries(), } } From 0d49369957f2e68dde2ebbbdb543def929c570a6 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 19 Oct 2026 16:08:45 +0200 Subject: [PATCH 04/13] refactor: the role modules decide the listener placement --- .../src/controller/build/mod.rs | 110 +++++++++++------- .../controller/build/resource/config_map.rs | 23 +--- .../src/controller/build/resource/listener.rs | 67 +++++------ .../controller/build/resource/statefulset.rs | 41 ++----- .../src/controller/build/role_group/master.rs | 21 +++- .../src/controller/build/role_group/mod.rs | 41 ++++++- .../build/role_group/region_server.rs | 21 +++- .../build/role_group/rest_server.rs | 19 ++- 8 files changed, 187 insertions(+), 156 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 0be2ae69..9246cf8f 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -21,12 +21,12 @@ use crate::{ ValidatedCluster, build::{ resource::{ - config_map::{self, build_rolegroup_config_map}, + config_map::build_rolegroup_config_map, discovery::{self, build_discovery_config_map}, pdb::build_pdb, rbac::{build_role_binding, build_service_account}, service::{build_rolegroup_metrics_service, build_rolegroup_service}, - statefulset::{self, build_rolegroup_statefulset}, + statefulset::build_rolegroup_statefulset, }, role_group::{master, region_server, rest_server}, }, @@ -55,16 +55,9 @@ pub(crate) fn object_meta( #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build ConfigMap for role {role} role group {role_group}"))] - ConfigMap { - source: config_map::Error, - role: HbaseRole, - role_group: RoleGroupName, - }, - - #[snafu(display("failed to build StatefulSet for role {role} role group {role_group}"))] - StatefulSet { - source: statefulset::Error, + #[snafu(display("failed to build the resources of {role} role group {role_group}"))] + RoleGroup { + source: role_group::Error, role: HbaseRole, role_group: RoleGroupName, }, @@ -88,7 +81,12 @@ pub fn build( let mut pod_disruption_budgets = vec![]; for (role_group_name, rg_config) in &cluster.master_role_group_configs { - let inputs = master::inputs(cluster, cluster_info, role_group_name, rg_config); + let inputs = master::inputs(cluster, cluster_info, role_group_name, rg_config) + .with_context(|_| RoleGroupSnafu { + role: master::ROLE, + role_group: role_group_name.clone(), + })?; + services.push(build_rolegroup_service( cluster, &master::ROLE, @@ -100,17 +98,21 @@ pub fn build( role_group_name, )); config_maps.push( - build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { - role: master::ROLE, - role_group: role_group_name.clone(), - })?, + build_rolegroup_config_map(&inputs) + .context(role_group::ConfigMapSnafu) + .with_context(|_| RoleGroupSnafu { + role: master::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push( + build_rolegroup_statefulset(&inputs) + .context(role_group::StatefulSetSnafu) + .with_context(|_| RoleGroupSnafu { + role: master::ROLE, + role_group: role_group_name.clone(), + })?, ); - stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { - StatefulSetSnafu { - role: master::ROLE, - role_group: role_group_name.clone(), - } - })?); } if let Some(pdb) = build_pdb(&cluster.master_config.pdb, cluster, &master::ROLE) { @@ -118,7 +120,12 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.region_server_role_group_configs { - let inputs = region_server::inputs(cluster, cluster_info, role_group_name, rg_config); + let inputs = region_server::inputs(cluster, cluster_info, role_group_name, rg_config) + .with_context(|_| RoleGroupSnafu { + role: region_server::ROLE, + role_group: role_group_name.clone(), + })?; + services.push(build_rolegroup_service( cluster, ®ion_server::ROLE, @@ -130,17 +137,21 @@ pub fn build( role_group_name, )); config_maps.push( - build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { - role: region_server::ROLE, - role_group: role_group_name.clone(), - })?, + build_rolegroup_config_map(&inputs) + .context(role_group::ConfigMapSnafu) + .with_context(|_| RoleGroupSnafu { + role: region_server::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push( + build_rolegroup_statefulset(&inputs) + .context(role_group::StatefulSetSnafu) + .with_context(|_| RoleGroupSnafu { + role: region_server::ROLE, + role_group: role_group_name.clone(), + })?, ); - stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { - StatefulSetSnafu { - role: region_server::ROLE, - role_group: role_group_name.clone(), - } - })?); } if let Some(pdb) = build_pdb( @@ -152,7 +163,12 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.rest_server_role_group_configs { - let inputs = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config); + let inputs = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config) + .with_context(|_| RoleGroupSnafu { + role: rest_server::ROLE, + role_group: role_group_name.clone(), + })?; + services.push(build_rolegroup_service( cluster, &rest_server::ROLE, @@ -164,17 +180,21 @@ pub fn build( role_group_name, )); config_maps.push( - build_rolegroup_config_map(&inputs).with_context(|_| ConfigMapSnafu { - role: rest_server::ROLE, - role_group: role_group_name.clone(), - })?, + build_rolegroup_config_map(&inputs) + .context(role_group::ConfigMapSnafu) + .with_context(|_| RoleGroupSnafu { + role: rest_server::ROLE, + role_group: role_group_name.clone(), + })?, + ); + stateful_sets.push( + build_rolegroup_statefulset(&inputs) + .context(role_group::StatefulSetSnafu) + .with_context(|_| RoleGroupSnafu { + role: rest_server::ROLE, + role_group: role_group_name.clone(), + })?, ); - stateful_sets.push(build_rolegroup_statefulset(&inputs).with_context(|_| { - StatefulSetSnafu { - role: rest_server::ROLE, - role_group: role_group_name.clone(), - } - })?); } if let Some(rest_server_config) = &cluster.rest_server_config 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 b79eff37..626d60f7 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -19,23 +19,15 @@ use crate::controller::build::{ #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("the validated cluster has no role group {role_group:?} for role {role:?}"))] - MissingRoleGroup { role: String, role_group: String }, - #[snafu(display("failed to build hbase-env.sh"))] BuildHbaseEnv { source: hbase_env::Error }, - #[snafu(display("failed to serialize {} for {role_group}", ConfigFileName::Security))] - JvmSecurityProperties { - source: PropertiesWriterError, - role_group: String, - }, + #[snafu(display("failed to serialize {}", ConfigFileName::Security))] + JvmSecurityProperties { source: PropertiesWriterError }, - #[snafu(display("cannot build config map for role {role:?} and role group {role_group:?}"))] + #[snafu(display("failed to assemble the ConfigMap"))] Assemble { source: stackable_operator::builder::configmap::Error, - role: String, - role_group: String, }, } @@ -87,9 +79,7 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result inputs.dns_cache_ttl_seconds, inputs.config_overrides.security_properties.clone(), ) - .with_context(|_| JvmSecurityPropertiesSnafu { - role_group: role_group_name.to_string(), - })?; + .context(JvmSecurityPropertiesSnafu)?; let cm_metadata = object_meta( cluster, @@ -129,8 +119,5 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result ); } - builder.build().with_context(|_| AssembleSnafu { - role: role.to_string(), - role_group: role_group_name.to_string(), - }) + builder.build().context(AssembleSnafu) } diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index a04d3cfa..b5ec6852 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -20,7 +20,7 @@ use stackable_operator::{ }, }; -use crate::crd::{HbaseRole, LISTENER_VOLUME_NAME}; +use crate::crd::LISTENER_VOLUME_NAME; constant!(pub(crate) LISTENER_PVC_NAME: PersistentVolumeClaimName = LISTENER_VOLUME_NAME); @@ -34,54 +34,39 @@ pub enum Error { type Result = std::result::Result; -/// The ephemeral listener [`Volume`] for the masters and region servers, or `None` for the rest -/// servers (which use a [`PersistentVolumeClaim`] instead, see [`build_listener_pvc`]). +/// The ephemeral listener [`Volume`] for the `listener_class`. pub fn build_listener_volume( - role: &HbaseRole, listener_class: &ListenerClassName, recommended_labels: &Labels, -) -> Result> { - let volume = match role { - // Master and regionservers should use ephemeral listener volumes - // since clients pull the latest address from ZooKeeper - HbaseRole::Master | HbaseRole::RegionServer => Some( - VolumeBuilder::new(LISTENER_VOLUME_NAME) - .ephemeral( - // The v2 framework only exposes a PVC builder - // (`listener_operator_volume_source_builder_build_pvc`, used by - // `build_listener_pvc`), so the ephemeral case still uses the legacy - // `ListenerOperatorVolumeSourceBuilder` and its stringly-typed - // `ListenerReference`. We keep the typed `listener_class` and convert to a - // `String` only at this boundary; switch to a v2 helper once one exists. - ListenerOperatorVolumeSourceBuilder::new( - &ListenerReference::ListenerClass(listener_class.to_string()), - recommended_labels, - ) - .build_ephemeral() - .context(BuildListenerVolumeSnafu)?, - ) - .build(), - ), - HbaseRole::RestServer => None, - }; - Ok(volume) +) -> Result { + Ok(VolumeBuilder::new(LISTENER_VOLUME_NAME) + .ephemeral( + // The v2 framework only exposes a PVC builder + // (`listener_operator_volume_source_builder_build_pvc`, used by + // `build_listener_pvc`), so the ephemeral case still uses the legacy + // `ListenerOperatorVolumeSourceBuilder` and its stringly-typed + // `ListenerReference`. We keep the typed `listener_class` and convert to a + // `String` only at this boundary; switch to a v2 helper once one exists. + ListenerOperatorVolumeSourceBuilder::new( + &ListenerReference::ListenerClass(listener_class.to_string()), + recommended_labels, + ) + .build_ephemeral() + .context(BuildListenerVolumeSnafu)?, + ) + .build()) } -/// The listener [`PersistentVolumeClaim`] template for the rest servers, or `None` for the masters -/// and region servers (which use an ephemeral [`Volume`] instead, see [`build_listener_volume`]). +/// A listener [`PersistentVolumeClaim`] template for the `listener_class`. pub fn build_listener_pvc( - role: &HbaseRole, listener_class: &ListenerClassName, recommended_labels: &Labels, -) -> Option> { - match role { - HbaseRole::Master | HbaseRole::RegionServer => None, - HbaseRole::RestServer => Some(vec![listener_operator_volume_source_builder_build_pvc( - &TypedListenerReference::ListenerClass(listener_class.clone()), - recommended_labels, - &LISTENER_PVC_NAME, - )]), - } +) -> PersistentVolumeClaim { + listener_operator_volume_source_builder_build_pvc( + &TypedListenerReference::ListenerClass(listener_class.clone()), + recommended_labels, + &LISTENER_PVC_NAME, + ) } #[cfg(test)] diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index b0935e39..e5c712ea 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -37,8 +37,7 @@ use crate::{ object_meta, properties::product_logging::MAX_HBASE_LOG_FILES_SIZE, recommended_labels_for_role_group_resources, - recommended_labels_for_unversioned_role_group_resources, - role_group::RoleGroupInputs, + role_group::{ListenerPlacement, RoleGroupInputs}, role_group_selector, }, crd::{CONFIG_DIR_NAME, HbaseRole, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME}, @@ -83,9 +82,6 @@ pub enum Error { #[snafu(display("failed to add needed volume"))] AddVolume { source: builder::pod::Error }, - - #[snafu(display("failed to build listener volume"))] - ListenerVolume { source: super::listener::Error }, } type Result = std::result::Result; @@ -292,32 +288,15 @@ pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result (Some(volume.clone()), None), + ListenerPlacement::ClaimTemplate(claim) => (None, Some(vec![claim.clone()])), + }; + if let Some(listener_volume) = listener_volume { pod_builder .add_volume(listener_volume) .context(AddVolumeSnafu)?; - }; + } let mut pod_template = pod_builder.build_template(); @@ -392,7 +371,8 @@ mod tests { cluster_info, &role_group_name, test_utils::master_role_group_config(&cluster, "default"), - ); + ) + .expect("inputs are valid"); let stateful_set = build_rolegroup_statefulset(&inputs).expect("the StatefulSet builds"); @@ -506,7 +486,8 @@ spec: cluster_info, &role_group_name, test_utils::master_role_group_config(&cluster, "default"), - ); + ) + .expect("inputs are valid"); build_rolegroup_statefulset(&inputs) .expect("the StatefulSet builds") diff --git a/rust/operator-binary/src/controller/build/role_group/master.rs b/rust/operator-binary/src/controller/build/role_group/master.rs index eb35640a..c5ef2d23 100644 --- a/rust/operator-binary/src/controller/build/role_group/master.rs +++ b/rust/operator-binary/src/controller/build/role_group/master.rs @@ -1,3 +1,4 @@ +use snafu::ResultExt; use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; @@ -7,7 +8,9 @@ use crate::{ MasterRoleGroupConfig, ValidatedCluster, build::{ properties::{hbase_env::HBASE_MASTER_OPTS, hbase_site}, - role_group::RoleGroupInputs, + recommended_labels_for_role_group_resources, + resource::listener::build_listener_volume, + role_group::{Error, ListenerPlacement, ListenerSnafu, RoleGroupInputs}, }, }, crd::{HbaseConfig, HbaseRole}, @@ -21,7 +24,7 @@ pub(crate) fn inputs<'a>( cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &MasterRoleGroupConfig, -) -> RoleGroupInputs<'a> { +) -> Result, Error> { let HbaseConfig { hbase_rootdir, resources, @@ -33,7 +36,15 @@ pub(crate) fn inputs<'a>( listener_class, } = &rg_config.config.config; - RoleGroupInputs { + // Masters get their listener from an ephemeral pod volume: clients pull the latest address + // from ZooKeeper, so the pods need no stable identity. + let recommended_labels = + recommended_labels_for_role_group_resources(cluster, &ROLE, role_group_name); + let listener = ListenerPlacement::PodVolume( + build_listener_volume(listener_class, &recommended_labels).context(ListenerSnafu)?, + ); + + Ok(RoleGroupInputs { cluster, cluster_info, role: ROLE, @@ -44,7 +55,7 @@ pub(crate) fn inputs<'a>( resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), - listener_class: listener_class.clone(), + listener, region_mover_opts: String::new(), run_region_mover: false, replicas: rg_config.replicas, @@ -58,5 +69,5 @@ pub(crate) fn inputs<'a>( dns_cache_ttl_seconds: "5", hbase_env_opts_variable: HBASE_MASTER_OPTS, role_hbase_site: hbase_site::master_entries(), - } + }) } diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index d22de1a6..c951c634 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -1,22 +1,22 @@ use std::collections::BTreeMap; +use snafu::Snafu; use stackable_operator::{ commons::{ affinity::StackableAffinity, resources::{NoRuntimeLimits, Resources}, }, - k8s_openapi::api::core::v1::PodTemplateSpec, + k8s_openapi::api::core::v1::{PersistentVolumeClaim, PodTemplateSpec, Volume}, shared::time::Duration, utils::cluster_info::KubernetesClusterInfo, v2::{ - builder::pod::container::EnvVarSet, - jvm_argument_overrides::JvmArgumentOverrides, - types::{kubernetes::ListenerClassName, operator::RoleGroupName}, + builder::pod::container::EnvVarSet, jvm_argument_overrides::JvmArgumentOverrides, + types::operator::RoleGroupName, }, }; use crate::{ - controller::{ValidatedCluster, validate::ValidatedLogging}, + controller::{ValidatedCluster, build::resource, validate::ValidatedLogging}, crd::{HbaseRole, HbaseStorageConfig, v1alpha1}, }; @@ -24,6 +24,35 @@ pub(crate) mod master; pub(crate) mod region_server; pub(crate) mod rest_server; +/// Why building one role group failed. The caller adds which role and role group it was. +#[derive(Snafu, Debug)] +#[snafu(visibility(pub(crate)))] +pub enum Error { + #[snafu(display("failed to build the listener volume"))] + Listener { source: resource::listener::Error }, + + #[snafu(display("failed to build ConfigMap"))] + ConfigMap { source: resource::config_map::Error }, + + #[snafu(display("failed to build StatefulSet"))] + StatefulSet { + source: resource::statefulset::Error, + }, +} + +/// Where a role group's listener comes from. +#[allow( + clippy::large_enum_variant, + reason = "one value per role group; its size does not matter" +)] +pub(crate) enum ListenerPlacement { + /// An ephemeral pod volume (Masters and RegionServers): clients pull the latest address from + /// ZooKeeper, so the pods need no stable identity. + PodVolume(Volume), + /// A persistent volume claim template (RestServers), for a stable per-pod address. + ClaimTemplate(PersistentVolumeClaim), +} + /// What one role group's resources are built from: the values every role has, resolved from /// its typed role-group config by that role's module. pub(crate) struct RoleGroupInputs<'a> { @@ -38,7 +67,7 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) resources: Resources, pub(crate) hbase_rootdir: String, pub(crate) logging: ValidatedLogging, - pub(crate) listener_class: ListenerClassName, + pub(crate) listener: ListenerPlacement, /// Set on every role because the entrypoint runs under `set -u`; only RegionServers run the /// mover, so only they have a non-empty value. pub(crate) region_mover_opts: String, diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index 972f1ddb..e8aae074 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -1,3 +1,4 @@ +use snafu::ResultExt; use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; @@ -7,8 +8,9 @@ use crate::{ RegionServerRoleGroupConfig, ValidatedCluster, build::{ properties::{hbase_env::HBASE_REGIONSERVER_OPTS, hbase_site}, - region_mover, - role_group::RoleGroupInputs, + recommended_labels_for_role_group_resources, region_mover, + resource::listener::build_listener_volume, + role_group::{Error, ListenerPlacement, ListenerSnafu, RoleGroupInputs}, }, }, crd::{HbaseRole, RegionServerConfig}, @@ -22,7 +24,7 @@ pub(crate) fn inputs<'a>( cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &RegionServerRoleGroupConfig, -) -> RoleGroupInputs<'a> { +) -> Result, Error> { let RegionServerConfig { hbase_rootdir, resources, @@ -35,7 +37,14 @@ pub(crate) fn inputs<'a>( region_mover, } = &rg_config.config.config; - RoleGroupInputs { + // Region servers, like masters, get their listener from an ephemeral pod volume. + let recommended_labels = + recommended_labels_for_role_group_resources(cluster, &ROLE, role_group_name); + let listener = ListenerPlacement::PodVolume( + build_listener_volume(listener_class, &recommended_labels).context(ListenerSnafu)?, + ); + + Ok(RoleGroupInputs { cluster, cluster_info, role: ROLE, @@ -46,7 +55,7 @@ pub(crate) fn inputs<'a>( resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), - listener_class: listener_class.clone(), + listener, region_mover_opts: region_mover::region_mover_args(&rg_config.config.config), run_region_mover: region_mover.run_before_shutdown, replicas: rg_config.replicas, @@ -60,5 +69,5 @@ pub(crate) fn inputs<'a>( dns_cache_ttl_seconds: "10", hbase_env_opts_variable: HBASE_REGIONSERVER_OPTS, role_hbase_site: hbase_site::region_server_entries(), - } + }) } diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index 7e9f16ae..846b97a9 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -7,7 +7,9 @@ use crate::{ RestServerRoleGroupConfig, ValidatedCluster, build::{ properties::{hbase_env::HBASE_REST_OPTS, hbase_site}, - role_group::RoleGroupInputs, + recommended_labels_for_unversioned_role_group_resources, + resource::listener::build_listener_pvc, + role_group::{Error, ListenerPlacement, RoleGroupInputs}, }, }, crd::{HbaseConfig, HbaseRole}, @@ -21,7 +23,7 @@ pub(crate) fn inputs<'a>( cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &RestServerRoleGroupConfig, -) -> RoleGroupInputs<'a> { +) -> Result, Error> { let HbaseConfig { hbase_rootdir, resources, @@ -33,7 +35,14 @@ pub(crate) fn inputs<'a>( listener_class, } = &rg_config.config.config; - RoleGroupInputs { + // Rest servers get their listener from a claim template, for a stable per-pod address. The + // template cannot be modified once deployed, so its labels omit the version. + let unversioned_labels = + recommended_labels_for_unversioned_role_group_resources(cluster, &ROLE, role_group_name); + let listener = + ListenerPlacement::ClaimTemplate(build_listener_pvc(listener_class, &unversioned_labels)); + + Ok(RoleGroupInputs { cluster, cluster_info, role: ROLE, @@ -44,7 +53,7 @@ pub(crate) fn inputs<'a>( resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), - listener_class: listener_class.clone(), + listener, region_mover_opts: String::new(), run_region_mover: false, replicas: rg_config.replicas, @@ -58,5 +67,5 @@ pub(crate) fn inputs<'a>( dns_cache_ttl_seconds: "30", hbase_env_opts_variable: HBASE_REST_OPTS, role_hbase_site: hbase_site::rest_server_entries(), - } + }) } From b941cfc2230635254c658eb88fd8bdaf2f8e9bc9 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 19 Oct 2026 17:08:20 +0200 Subject: [PATCH 05/13] resolve secret lifetime in role group --- .../src/controller/build/resource/statefulset.rs | 9 ++------- .../src/controller/build/role_group/master.rs | 10 +++++++--- .../src/controller/build/role_group/mod.rs | 5 ++++- .../src/controller/build/role_group/region_server.rs | 10 +++++++--- .../src/controller/build/role_group/rest_server.rs | 6 ++++-- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index e5c712ea..21399093 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -3,7 +3,7 @@ use std::str::FromStr; use indoc::formatdoc; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::{ self, @@ -71,9 +71,6 @@ const HBASE_LOG_CONFIG_TMP_DIR: &str = "/stackable/tmp/log_config"; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("missing secret lifetime"))] - MissingSecretLifetime, - #[snafu(display("failed to add kerberos config"))] AddKerberosConfig { source: kerberos::Error }, @@ -267,9 +264,7 @@ pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result( role_group_name: role_group_name.clone(), affinity: affinity.clone(), graceful_shutdown_timeout: *graceful_shutdown_timeout, - requested_secret_lifetime: *requested_secret_lifetime, + requested_secret_lifetime: (*requested_secret_lifetime) + .context(MissingSecretLifetimeSnafu)?, resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index c951c634..c70db63d 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -28,6 +28,9 @@ pub(crate) mod rest_server; #[derive(Snafu, Debug)] #[snafu(visibility(pub(crate)))] pub enum Error { + #[snafu(display("missing secret lifetime"))] + MissingSecretLifetime, + #[snafu(display("failed to build the listener volume"))] Listener { source: resource::listener::Error }, @@ -62,7 +65,7 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) role_group_name: RoleGroupName, pub(crate) affinity: StackableAffinity, pub(crate) graceful_shutdown_timeout: Option, - pub(crate) requested_secret_lifetime: Option, + pub(crate) requested_secret_lifetime: Duration, /// Container resources and the `HBASE_HEAPSIZE` derivation both read this. pub(crate) resources: Resources, pub(crate) hbase_rootdir: String, diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index e8aae074..88b98e1f 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -1,4 +1,4 @@ -use snafu::ResultExt; +use snafu::{OptionExt, ResultExt}; use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; @@ -10,7 +10,10 @@ use crate::{ properties::{hbase_env::HBASE_REGIONSERVER_OPTS, hbase_site}, recommended_labels_for_role_group_resources, region_mover, resource::listener::build_listener_volume, - role_group::{Error, ListenerPlacement, ListenerSnafu, RoleGroupInputs}, + role_group::{ + Error, ListenerPlacement, ListenerSnafu, MissingSecretLifetimeSnafu, + RoleGroupInputs, + }, }, }, crd::{HbaseRole, RegionServerConfig}, @@ -51,7 +54,8 @@ pub(crate) fn inputs<'a>( role_group_name: role_group_name.clone(), affinity: affinity.clone(), graceful_shutdown_timeout: *graceful_shutdown_timeout, - requested_secret_lifetime: *requested_secret_lifetime, + requested_secret_lifetime: (*requested_secret_lifetime) + .context(MissingSecretLifetimeSnafu)?, resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index 846b97a9..80fbe607 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -1,3 +1,4 @@ +use snafu::OptionExt; use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; @@ -9,7 +10,7 @@ use crate::{ properties::{hbase_env::HBASE_REST_OPTS, hbase_site}, recommended_labels_for_unversioned_role_group_resources, resource::listener::build_listener_pvc, - role_group::{Error, ListenerPlacement, RoleGroupInputs}, + role_group::{Error, ListenerPlacement, MissingSecretLifetimeSnafu, RoleGroupInputs}, }, }, crd::{HbaseConfig, HbaseRole}, @@ -49,7 +50,8 @@ pub(crate) fn inputs<'a>( role_group_name: role_group_name.clone(), affinity: affinity.clone(), graceful_shutdown_timeout: *graceful_shutdown_timeout, - requested_secret_lifetime: *requested_secret_lifetime, + requested_secret_lifetime: (*requested_secret_lifetime) + .context(MissingSecretLifetimeSnafu)?, resources: resources.clone(), hbase_rootdir: hbase_rootdir.clone(), logging: rg_config.config.logging.clone(), From 514802621d79194b50816707e49ff4e5d459599b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 19 Oct 2026 17:50:44 +0200 Subject: [PATCH 06/13] refactor: build the containers and their volumes in one place --- .../src/controller/build/container.rs | 308 ++++++++++++++++++ .../src/controller/build/kerberos.rs | 39 +-- .../src/controller/build/mod.rs | 34 +- .../controller/build/resource/statefulset.rs | 282 ++-------------- 4 files changed, 367 insertions(+), 296 deletions(-) create mode 100644 rust/operator-binary/src/controller/build/container.rs diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs new file mode 100644 index 00000000..c746ff66 --- /dev/null +++ b/rust/operator-binary/src/controller/build/container.rs @@ -0,0 +1,308 @@ +//! Builds the containers of a role group's pod and collects the volumes they mount. +//! +//! Every HBase role runs the same containers: the `hbase` container and, when the Vector agent is +//! enabled, the Vector sidecar. What differs between roles arrives resolved in the +//! [`RoleGroupInputs`]; the role itself is read only for the values that are functions of it (its +//! CLI name and ports) and for the object names. + +use std::str::FromStr; + +use indoc::formatdoc; +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + builder::pod::volume::VolumeBuilder, + constant, + k8s_openapi::{ + api::core::v1::{ + ConfigMapVolumeSource, Container, ContainerPort, PersistentVolumeClaim, Probe, + TCPSocketAction, Volume, + }, + apimachinery::pkg::util::intstr::IntOrString, + }, + product_logging, + v2::{ + builder::pod::container::{EnvVarName, EnvVarSet, new_container_builder}, + product_logging::framework::{ + STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container, + }, + types::kubernetes::{ContainerName, VolumeName}, + }, +}; + +use crate::{ + controller::build::{ + kerberos::{self, mount_kerberos_volumes}, + properties::product_logging::MAX_HBASE_LOG_FILES_SIZE, + role_group::{ListenerPlacement, RoleGroupInputs}, + }, + crd::{CONFIG_DIR_NAME, HbaseRole, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME}, +}; + +constant!(pub(crate) HBASE_CONTAINER_NAME: ContainerName = "hbase"); +constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector"); + +// Pod volume names. The Vector container reuses the `hbase-config` (rolegroup ConfigMap, which +// carries `vector.yaml`) and `log` volumes, so the produced volume mounts match the rest of the +// Pod. +constant!(pub(crate) HBASE_CONFIG_VOLUME_NAME: VolumeName = "hbase-config"); +constant!(pub(crate) HDFS_DISCOVERY_VOLUME_NAME: VolumeName = "hdfs-discovery"); +constant!(pub(crate) LOG_CONFIG_VOLUME_NAME: VolumeName = "log-config"); +constant!(pub(crate) LOG_VOLUME_NAME: VolumeName = "log"); + +// Environment variable names set on the HBase container. Declared as typed constants (instead of +// `EnvVarName::from_str_unsafe` at the use site) and validated by `test_constants`. +constant!(HBASE_CONF_DIR_ENV: EnvVarName = "HBASE_CONF_DIR"); +constant!(HADOOP_CONF_DIR_ENV: EnvVarName = "HADOOP_CONF_DIR"); +constant!(pub(crate) REGION_MOVER_OPTS_ENV: EnvVarName = "REGION_MOVER_OPTS"); +constant!(pub(crate) RUN_REGION_MOVER_ENV: EnvVarName = "RUN_REGION_MOVER"); +constant!(STACKABLE_LOG_DIR_ENV: EnvVarName = "STACKABLE_LOG_DIR"); +constant!(CONTAINERDEBUG_LOG_DIRECTORY_ENV: EnvVarName = "CONTAINERDEBUG_LOG_DIRECTORY"); + +// These constants are hard coded in hbase-entrypoint.sh +// You need to change them there too. +const HDFS_DISCOVERY_TMP_DIR: &str = "/stackable/tmp/hdfs"; +const HBASE_CONFIG_TMP_DIR: &str = "/stackable/tmp/hbase"; +const HBASE_LOG_CONFIG_TMP_DIR: &str = "/stackable/tmp/log_config"; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to mount the Kerberos volumes"))] + MountKerberosVolumes { source: kerberos::Error }, +} + +type Result = std::result::Result; + +/// The containers of a role group's pod and the volumes they mount, in pod order. +pub(crate) struct ContainersAndVolumes { + /// The `hbase` container, then the Vector sidecar when the Vector agent is enabled. + pub(crate) containers: Vec, + /// The pod volumes in the order the pod lists them; see [`containers_and_volumes`]. + pub(crate) pod_volumes: Vec, + /// The listener claim template of a rest server role group. `None` for the other roles, + /// which keeps their StatefulSets without a `volumeClaimTemplates` field. + pub(crate) volume_claim_templates: Option>, +} + +/// Builds the containers every role runs and collects the volumes they mount. +/// +/// The pod volumes are collected in the order the pod lists them: the role group's ConfigMap, +/// the HDFS discovery ConfigMap, the log directory, the log config, then the Kerberos keytab and +/// TLS keystore when configured, then the ephemeral listener volume of masters and region +/// servers. This order is part of the pod template, so changing it restarts every pod; it is +/// pinned by `pod_volumes_keep_their_order` in `build/mod.rs`. +pub(crate) fn containers_and_volumes(inputs: &RoleGroupInputs) -> Result { + let cluster = inputs.cluster; + let hbase_role = &inputs.role; + let resolved_product_image = &cluster.image; + let logging = &inputs.logging; + let resource_names = cluster.role_group_resource_names(hbase_role, &inputs.role_group_name); + let https_enabled = cluster.has_https_enabled(); + + let ports = hbase_role + .ports(https_enabled) + .into_iter() + .map(|(name, value)| ContainerPort { + name: Some(name), + container_port: i32::from(value), + protocol: Some("TCP".to_string()), + ..ContainerPort::default() + }) + .collect(); + + let probe_template = Probe { + tcp_socket: Some(TCPSocketAction { + port: IntOrString::String(hbase_role.data_port_name(https_enabled)), + ..TCPSocketAction::default() + }), + ..Probe::default() + }; + + let startup_probe = Probe { + failure_threshold: Some(120), + initial_delay_seconds: Some(4), + period_seconds: Some(5), + timeout_seconds: Some(3), + ..probe_template.clone() + }; + let liveness_probe = Probe { + failure_threshold: Some(3), + period_seconds: Some(10), + timeout_seconds: Some(3), + ..probe_template.clone() + }; + let readiness_probe = Probe { + failure_threshold: Some(1), + period_seconds: Some(10), + timeout_seconds: Some(2), + ..probe_template + }; + + let merged_env = EnvVarSet::new() + .with_value(&HBASE_CONF_DIR_ENV, CONFIG_DIR_NAME) + // required by phoenix (for cases where Kerberos is enabled): see https://issues.apache.org/jira/browse/PHOENIX-2369 + .with_value(&HADOOP_CONF_DIR_ENV, CONFIG_DIR_NAME) + // These env vars are set for all roles to avoid bash's "unbound variable" errors. + .with_value(®ION_MOVER_OPTS_ENV, inputs.region_mover_opts.clone()) + .with_value(&RUN_REGION_MOVER_ENV, inputs.run_region_mover.to_string()) + .with_value(&STACKABLE_LOG_DIR_ENV, STACKABLE_LOG_DIR) + // Needed for the `containerdebug` process to log its tracing information to. + .with_value( + &CONTAINERDEBUG_LOG_DIRECTORY_ENV, + format!("{STACKABLE_LOG_DIR}/containerdebug"), + ) + // Empty when Kerberos is disabled. + .merge(kerberos::kerberos_env_vars(cluster)) + // apply overrides last of all; `EnvVarSet` is keyed by name, so iteration is already + // in a fixed (sorted-by-name) order + .merge(inputs.env_overrides.clone()); + + let role_name = hbase_role.cli_role_name(); + let mut hbase_container = new_container_builder(&HBASE_CONTAINER_NAME); + + hbase_container + .image_from_product_image(resolved_product_image) + .command(command()) + .args(vec![formatdoc! {" + {entrypoint} {role} {port} {port_name} {ui_port_name}", + entrypoint = "/stackable/hbase/bin/hbase-entrypoint.sh".to_string(), + role = role_name, + port = hbase_role.data_port(), + port_name = hbase_role.data_port_name(https_enabled), + ui_port_name = HbaseRole::ui_port_name(https_enabled), + }]) + .add_env_vars(merged_env) + .add_volume_mount(&*HBASE_CONFIG_VOLUME_NAME, HBASE_CONFIG_TMP_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*HDFS_DISCOVERY_VOLUME_NAME, HDFS_DISCOVERY_TMP_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*LOG_CONFIG_VOLUME_NAME, HBASE_LOG_CONFIG_TMP_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*LOG_VOLUME_NAME, STACKABLE_LOG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_container_ports(ports) + .resources(inputs.resources.clone().into()) + .startup_probe(startup_probe) + .liveness_probe(liveness_probe) + .readiness_probe(readiness_probe); + + // The HBase container's log config ConfigMap: either the operator-generated one (the + // rolegroup ConfigMap, which carries the automatic `log4j2.properties`) or a user-provided + // custom ConfigMap. This branches on the *validated* logging choice (see `ValidatedLogging`). + let log_config_config_map = match &logging.hbase_container { + ValidatedContainerLogConfigChoice::Custom(config_map_name) => config_map_name.to_string(), + ValidatedContainerLogConfigChoice::Automatic(_) => { + resource_names.role_group_config_map().to_string() + } + }; + + let mut pod_volumes = vec![ + Volume { + name: HBASE_CONFIG_VOLUME_NAME.to_string(), + config_map: Some(ConfigMapVolumeSource { + name: resource_names.role_group_config_map().to_string(), + ..Default::default() + }), + ..Default::default() + }, + Volume { + name: HDFS_DISCOVERY_VOLUME_NAME.to_string(), + config_map: Some(ConfigMapVolumeSource { + name: cluster.cluster_config.hdfs_config_map_name.to_string(), + ..Default::default() + }), + ..Default::default() + }, + VolumeBuilder::new(&*LOG_VOLUME_NAME) + .with_empty_dir( + None::, + Some(product_logging::framework::calculate_log_volume_size_limit( + &[MAX_HBASE_LOG_FILES_SIZE], + )), + ) + .build(), + Volume { + name: LOG_CONFIG_VOLUME_NAME.to_string(), + config_map: Some(ConfigMapVolumeSource { + name: log_config_config_map, + ..ConfigMapVolumeSource::default() + }), + ..Volume::default() + }, + ]; + + if cluster.has_kerberos_enabled() { + pod_volumes.extend( + mount_kerberos_volumes( + cluster, + resource_names.metrics_service_name().as_ref(), + &mut hbase_container, + inputs.requested_secret_lifetime, + ) + .context(MountKerberosVolumesSnafu)?, + ); + } + + let volume_claim_templates = match &inputs.listener { + ListenerPlacement::PodVolume(volume) => { + pod_volumes.push(volume.clone()); + None + } + ListenerPlacement::ClaimTemplate(claim) => Some(vec![claim.clone()]), + }; + + let mut containers = vec![hbase_container.build()]; + + // Vector sidecar shall be the last container in the list. + if let Some(vector_log_config) = &logging.vector_container { + containers.push(vector_container( + &VECTOR_CONTAINER_NAME, + resolved_product_image, + vector_log_config, + &resource_names, + &HBASE_CONFIG_VOLUME_NAME, + &LOG_VOLUME_NAME, + EnvVarSet::new(), + )); + } + + Ok(ContainersAndVolumes { + containers, + pod_volumes, + volume_claim_templates, + }) +} + +/// Returns the container command. +fn command() -> Vec { + vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), + ] +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *HBASE_CONTAINER_NAME; + let _ = *VECTOR_CONTAINER_NAME; + let _ = *HBASE_CONFIG_VOLUME_NAME; + let _ = *HDFS_DISCOVERY_VOLUME_NAME; + let _ = *LOG_CONFIG_VOLUME_NAME; + let _ = *LOG_VOLUME_NAME; + let _ = *HBASE_CONF_DIR_ENV; + let _ = *HADOOP_CONF_DIR_ENV; + let _ = *REGION_MOVER_OPTS_ENV; + let _ = *RUN_REGION_MOVER_ENV; + let _ = *STACKABLE_LOG_DIR_ENV; + let _ = *CONTAINERDEBUG_LOG_DIRECTORY_ENV; + } +} diff --git a/rust/operator-binary/src/controller/build/kerberos.rs b/rust/operator-binary/src/controller/build/kerberos.rs index fc72e8d2..671ad813 100644 --- a/rust/operator-binary/src/controller/build/kerberos.rs +++ b/rust/operator-binary/src/controller/build/kerberos.rs @@ -2,16 +2,13 @@ use std::{collections::BTreeMap, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::{ - self, - pod::{ - PodBuilder, - container::ContainerBuilder, - volume::{SecretFormat, SecretOperatorVolumeSourceBuilder, VolumeBuilder}, - }, + builder::pod::{ + container::ContainerBuilder, + volume::{SecretFormat, SecretOperatorVolumeSourceBuilder, VolumeBuilder}, }, commons::secret_class::SecretClassVolumeProvisionParts, constant, + k8s_openapi::api::core::v1::Volume, shared::time::Duration, utils::cluster_info::KubernetesClusterInfo, v2::{ @@ -48,9 +45,6 @@ pub enum Error { BuildTlsSecretVolumeSource { source: stackable_operator::builder::pod::volume::SecretOperatorVolumeSourceBuilderError, }, - - #[snafu(display("failed to add needed volume"))] - AddVolume { source: builder::pod::Error }, } /// The `hbase-site.xml` Kerberos properties for `cluster`, gated on Kerberos being enabled @@ -224,22 +218,23 @@ pub fn kerberos_ssl_client_settings() -> BTreeMap { truststore_settings("client") } -/// Adds the Kerberos keytab and TLS keystore volumes to the [`PodBuilder`] and their mounts to the +/// Adds the Kerberos keytab and TLS keystore volumes to the volume collection and their mounts to the /// [`ContainerBuilder`], for whichever of the two secret classes are configured. /// /// # 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_kerberos_pod_config( +pub fn mount_kerberos_volumes( cluster: &ValidatedCluster, metrics_service_name: &str, cb: &mut ContainerBuilder, - pb: &mut PodBuilder, requested_secret_lifetime: Duration, -) -> Result<(), Error> { +) -> Result, Error> { + let mut volumes = Vec::new(); + if let Some(kerberos_secret_class) = &cluster.cluster_config.kerberos_secret_class { - // Mount keytab + // Add keytab let kerberos_secret_operator_volume = SecretOperatorVolumeSourceBuilder::new( kerberos_secret_class.clone(), // We need both public (krb5.conf) and private (keytab) parts. @@ -250,19 +245,18 @@ pub fn add_kerberos_pod_config( .with_kerberos_service_name("HTTP") .build() .context(BuildKerberosSecretVolumeSourceSnafu)?; - pb.add_volume( + volumes.push( VolumeBuilder::new(&*KERBEROS_VOLUME_NAME) .ephemeral(kerberos_secret_operator_volume) .build(), - ) - .context(AddVolumeSnafu)?; + ); cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR) .expect("The mount paths are statically defined and there should be no duplicates."); } if let Some(https_secret_class) = &cluster.cluster_config.https_secret_class { - // Mount TLS keystore - pb.add_volume( + // Add TLS keystore + volumes.push( VolumeBuilder::new(&*TLS_STORE_VOLUME_NAME) .ephemeral( SecretOperatorVolumeSourceBuilder::new( @@ -282,12 +276,11 @@ pub fn add_kerberos_pod_config( .context(BuildTlsSecretVolumeSourceSnafu)?, ) .build(), - ) - .context(AddVolumeSnafu)?; + ); cb.add_volume_mount(&*TLS_STORE_VOLUME_NAME, TLS_STORE_DIR) .expect("The mount paths are statically defined and there should be no duplicates."); } - Ok(()) + Ok(volumes) } /// The environment variables the Kerberos configuration requires on the HBase container, or an diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 9246cf8f..2e47211b 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -34,6 +34,17 @@ use crate::{ crd::HbaseRole, }; +pub mod container; +pub mod graceful_shutdown; +pub mod jvm; +pub mod kerberos; +pub mod opa; +pub mod properties; +pub mod region_mover; +pub mod resource; +pub mod role; +pub mod role_group; + /// Returns an [`ObjectMetaBuilder`] pre-filled with the cluster's namespace, an owner /// reference back to the cluster, the resource `name` and the given `recommended_labels`. /// @@ -282,16 +293,6 @@ pub(crate) fn role_group_selector( label::role_group_selector(&cluster.name, &PRODUCT_NAME, role_name, role_group_name) } -pub mod graceful_shutdown; -pub mod jvm; -pub mod kerberos; -pub mod opa; -pub mod properties; -pub mod region_mover; -pub mod resource; -pub mod role; -pub mod role_group; - #[cfg(test)] mod tests { use std::collections::BTreeMap; @@ -304,6 +305,10 @@ mod tests { use super::{ build, + container::{ + HBASE_CONFIG_VOLUME_NAME, HBASE_CONTAINER_NAME, HDFS_DISCOVERY_VOLUME_NAME, + LOG_CONFIG_VOLUME_NAME, LOG_VOLUME_NAME, REGION_MOVER_OPTS_ENV, RUN_REGION_MOVER_ENV, + }, kerberos::KERBEROS_VOLUME_NAME, opa::HbaseOpaConfig, properties::{ @@ -313,14 +318,7 @@ mod tests { HBASE_MASTER_IPC_ADDRESS, HBASE_REGIONSERVER_IPC_ADDRESS, HBASE_REST_ENDPOINT, }, }, - resource::{ - listener::LISTENER_PVC_NAME, - statefulset::{ - HBASE_CONFIG_VOLUME_NAME, HBASE_CONTAINER_NAME, HDFS_DISCOVERY_VOLUME_NAME, - LOG_CONFIG_VOLUME_NAME, LOG_VOLUME_NAME, REGION_MOVER_OPTS_ENV, - RUN_REGION_MOVER_ENV, - }, - }, + resource::listener::LISTENER_PVC_NAME, }; use crate::{ controller::{ diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 21399093..bb243f33 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -1,8 +1,5 @@ //! Build the per-rolegroup `StatefulSet` for the HbaseCluster. -use std::str::FromStr; - -use indoc::formatdoc; use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::{ @@ -10,69 +7,26 @@ use stackable_operator::{ meta::ObjectMetaBuilder, pod::{PodBuilder, security::PodSecurityContextBuilder}, }, - constant, constants::RESTART_CONTROLLER_ENABLED_LABEL, k8s_openapi::{ DeepMerge, - api::{ - apps::v1::{StatefulSet, StatefulSetSpec}, - core::v1::{ConfigMapVolumeSource, ContainerPort, Probe, TCPSocketAction, Volume}, - }, - apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString}, - }, - product_logging, - v2::{ - builder::pod::container::{EnvVarName, EnvVarSet, new_container_builder}, - product_logging::framework::{ - STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container, - }, - types::kubernetes::{ContainerName, VolumeName}, + api::apps::v1::{StatefulSet, StatefulSetSpec}, + apimachinery::pkg::apis::meta::v1::LabelSelector, }, }; -use crate::{ - controller::build::{ - graceful_shutdown::{self, add_graceful_shutdown_config}, - kerberos::{self, add_kerberos_pod_config}, - object_meta, - properties::product_logging::MAX_HBASE_LOG_FILES_SIZE, - recommended_labels_for_role_group_resources, - role_group::{ListenerPlacement, RoleGroupInputs}, - role_group_selector, - }, - crd::{CONFIG_DIR_NAME, HbaseRole, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME}, +use crate::controller::build::{ + container::{self, ContainersAndVolumes, containers_and_volumes}, + graceful_shutdown::{self, add_graceful_shutdown_config}, + object_meta, recommended_labels_for_role_group_resources, + role_group::RoleGroupInputs, + role_group_selector, }; -constant!(pub(crate) HBASE_CONTAINER_NAME: ContainerName = "hbase"); -constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector"); - -// Pod volume names. The Vector container reuses the `hbase-config` (rolegroup ConfigMap, which -// carries `vector.yaml`) and `log` volumes, so the produced volume mounts match the rest of the -// Pod. -constant!(pub(crate) HBASE_CONFIG_VOLUME_NAME: VolumeName = "hbase-config"); -constant!(pub(crate) HDFS_DISCOVERY_VOLUME_NAME: VolumeName = "hdfs-discovery"); -constant!(pub(crate) LOG_CONFIG_VOLUME_NAME: VolumeName = "log-config"); -constant!(pub(crate) LOG_VOLUME_NAME: VolumeName = "log"); - -// Environment variable names set on the HBase container. Declared as typed constants (instead of -// `EnvVarName::from_str_unsafe` at the use site) and validated by `test_constants`. -constant!(HBASE_CONF_DIR_ENV: EnvVarName = "HBASE_CONF_DIR"); -constant!(HADOOP_CONF_DIR_ENV: EnvVarName = "HADOOP_CONF_DIR"); -constant!(pub(crate) REGION_MOVER_OPTS_ENV: EnvVarName = "REGION_MOVER_OPTS"); -constant!(pub(crate) RUN_REGION_MOVER_ENV: EnvVarName = "RUN_REGION_MOVER"); -constant!(STACKABLE_LOG_DIR_ENV: EnvVarName = "STACKABLE_LOG_DIR"); -constant!(CONTAINERDEBUG_LOG_DIRECTORY_ENV: EnvVarName = "CONTAINERDEBUG_LOG_DIRECTORY"); - -// These constants are hard coded in hbase-entrypoint.sh -// You need to change them there too. -const HDFS_DISCOVERY_TMP_DIR: &str = "/stackable/tmp/hdfs"; -const HBASE_CONFIG_TMP_DIR: &str = "/stackable/tmp/hbase"; -const HBASE_LOG_CONFIG_TMP_DIR: &str = "/stackable/tmp/log_config"; - #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to add kerberos config"))] - AddKerberosConfig { source: kerberos::Error }, + #[snafu(display("failed to build the containers and volumes"))] + ContainersAndVolumes { source: container::Error }, #[snafu(display("failed to configure graceful shutdown"))] GracefulShutdown { source: graceful_shutdown::Error }, @@ -91,100 +45,13 @@ pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result Result Result config_map_name.to_string(), - ValidatedContainerLogConfigChoice::Automatic(_) => { - resource_names.role_group_config_map().to_string() - } - }; - pod_builder - .add_volume(Volume { - name: LOG_CONFIG_VOLUME_NAME.to_string(), - config_map: Some(ConfigMapVolumeSource { - name: log_config_config_map, - ..ConfigMapVolumeSource::default() - }), - ..Volume::default() - }) - .context(AddVolumeSnafu)?; - add_graceful_shutdown_config(inputs.graceful_shutdown_timeout, &mut pod_builder) .context(GracefulShutdownSnafu)?; - if cluster.has_kerberos_enabled() { - add_kerberos_pod_config( - cluster, - resource_names.metrics_service_name().as_ref(), - &mut hbase_container, - &mut pod_builder, - inputs.requested_secret_lifetime, - ) - .context(AddKerberosConfigSnafu)?; - } - pod_builder.add_container(hbase_container.build()); - // Vector sidecar shall be the last container in the list. - if let Some(vector_log_config) = &logging.vector_container { - pod_builder.add_container(vector_container( - &VECTOR_CONTAINER_NAME, - resolved_product_image, - vector_log_config, - &resource_names, - &HBASE_CONFIG_VOLUME_NAME, - &LOG_VOLUME_NAME, - EnvVarSet::new(), - )); - } - - let (listener_volume, listener_pvc) = match &inputs.listener { - ListenerPlacement::PodVolume(volume) => (Some(volume.clone()), None), - ListenerPlacement::ClaimTemplate(claim) => (None, Some(vec![claim.clone()])), - }; - if let Some(listener_volume) = listener_volume { - pod_builder - .add_volume(listener_volume) - .context(AddVolumeSnafu)?; + for container in containers { + pod_builder.add_container(container); } let mut pod_template = pod_builder.build_template(); @@ -316,7 +113,7 @@ pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result Result Vec { - vec![ - "/bin/bash".to_string(), - "-x".to_string(), - "-euo".to_string(), - "pipefail".to_string(), - "-c".to_string(), - ] -} - #[cfg(test)] mod tests { use super::*; - use crate::{controller::build::role_group::master, test_utils}; + use crate::{ + controller::build::{container::HBASE_CONTAINER_NAME, kerberos, role_group::master}, + test_utils, + }; /// `envOverrides` are applied after every operator-set environment variable, so users can /// override any of them (previously the operator's value silently won for the variables set @@ -524,21 +313,4 @@ spec: [("KRB5_CONFIG".to_string(), "/custom/krb5.conf".to_string())] ); } - - #[test] - fn test_constants() { - // Test that dereferencing the constants does not panic. - let _ = *HBASE_CONTAINER_NAME; - let _ = *VECTOR_CONTAINER_NAME; - let _ = *HBASE_CONFIG_VOLUME_NAME; - let _ = *HDFS_DISCOVERY_VOLUME_NAME; - let _ = *LOG_CONFIG_VOLUME_NAME; - let _ = *LOG_VOLUME_NAME; - let _ = *HBASE_CONF_DIR_ENV; - let _ = *HADOOP_CONF_DIR_ENV; - let _ = *REGION_MOVER_OPTS_ENV; - let _ = *RUN_REGION_MOVER_ENV; - let _ = *STACKABLE_LOG_DIR_ENV; - let _ = *CONTAINERDEBUG_LOG_DIRECTORY_ENV; - } } From 19ad4a9c1613f700afe18e3b7c1d06b12ccbef3c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 19 Oct 2026 18:15:42 +0200 Subject: [PATCH 07/13] render the config files separately from assembling the ConfigMap --- .../controller/build/resource/config_map.rs | 63 ++++++++++++------- 1 file changed, 40 insertions(+), 23 deletions(-) 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 626d60f7..ba3fb60f 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -1,5 +1,7 @@ //! Build the per-rolegroup `ConfigMap` for the HbaseCluster. +use std::collections::BTreeMap; + use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap, @@ -43,6 +45,31 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result role = role.to_string() ); + let data = config_map_data(inputs)?; + + let cm_metadata = object_meta( + cluster, + cluster + .role_group_resource_names(role, role_group_name) + .role_group_config_map() + .to_string(), + recommended_labels_for_role_group_resources(cluster, role, role_group_name), + ) + .build(); + + ConfigMapBuilder::new() + .metadata(cm_metadata) + .data(data) + .build() + .context(AssembleSnafu) +} + +/// Renders the role group's config files, keyed by file name. +/// +/// The `ssl-*.xml` files are present only when they carry settings and `log4j2.properties` only +/// for the automatic log config, so the set of keys is part of what this renders. +pub(crate) fn config_map_data(inputs: &RoleGroupInputs) -> Result> { + let cluster = inputs.cluster; let cluster_config = &cluster.cluster_config; let hbase_site_xml = hbase_site::build( @@ -81,43 +108,33 @@ pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result ) .context(JvmSecurityPropertiesSnafu)?; - let cm_metadata = object_meta( - cluster, - cluster - .role_group_resource_names(role, role_group_name) - .role_group_config_map() - .to_string(), - recommended_labels_for_role_group_resources(cluster, role, role_group_name), - ) - .build(); - - let mut builder = ConfigMapBuilder::new(); - builder - .metadata(cm_metadata) - .add_data(ConfigFileName::HbaseSite.to_string(), hbase_site_xml) - .add_data(ConfigFileName::HbaseEnv.to_string(), hbase_env_sh) - .add_data(ConfigFileName::Security.to_string(), security_properties); + let mut data = BTreeMap::from([ + (ConfigFileName::HbaseSite.to_string(), hbase_site_xml), + (ConfigFileName::HbaseEnv.to_string(), hbase_env_sh), + (ConfigFileName::Security.to_string(), security_properties), + ]); // HBase does not like empty config files, so the ssl-*.xml files are only // written when they actually carry settings (see `ssl_server::build`): - // Caused by: com.ctc.wstx.exc.WstxEOFException: Unexpected EOF in prolog at [row,col,system-id]: [1,0,"file:/stackable/conf/ssl-server.xml"] + // Caused by: com.ctc.wstx.exc.WstxEOFException: Unexpected EOF in prolog + // at [row,col,system-id]: [1,0,"file:/stackable/conf/ssl-server.xml"] if let Some(ssl_server_xml) = ssl_server_xml { - builder.add_data(ConfigFileName::SslServer.to_string(), ssl_server_xml); + data.insert(ConfigFileName::SslServer.to_string(), ssl_server_xml); } if let Some(ssl_client_xml) = ssl_client_xml { - builder.add_data(ConfigFileName::SslClient.to_string(), ssl_client_xml); + data.insert(ConfigFileName::SslClient.to_string(), ssl_client_xml); } if let Some(log4j2_properties) = product_logging::build_log4j2(&inputs.logging.hbase_container) { - builder.add_data(ConfigFileName::Log4j2.to_string(), log4j2_properties); + data.insert(ConfigFileName::Log4j2.to_string(), log4j2_properties); } if inputs.logging.enable_vector_agent { - builder.add_data( - VECTOR_CONFIG_FILE, + data.insert( + VECTOR_CONFIG_FILE.to_string(), product_logging::vector_config_file_content(), ); } - builder.build().context(AssembleSnafu) + Ok(data) } From 7bd4491778d704e0a82bf577c2d4841a7c3508fb Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 12:21:31 +0200 Subject: [PATCH 08/13] introduce RoleGroupBuilder as the shared resolve and emit step --- .../src/controller/build/graceful_shutdown.rs | 25 ---- .../src/controller/build/mod.rs | 73 ++++-------- .../controller/build/resource/config_map.rs | 12 +- .../controller/build/resource/statefulset.rs | 53 ++++----- .../src/controller/build/role_group/mod.rs | 107 +++++++++++++++++- 5 files changed, 155 insertions(+), 115 deletions(-) delete mode 100644 rust/operator-binary/src/controller/build/graceful_shutdown.rs diff --git a/rust/operator-binary/src/controller/build/graceful_shutdown.rs b/rust/operator-binary/src/controller/build/graceful_shutdown.rs deleted file mode 100644 index 9dc27a5f..00000000 --- a/rust/operator-binary/src/controller/build/graceful_shutdown.rs +++ /dev/null @@ -1,25 +0,0 @@ -use snafu::{ResultExt, Snafu}; -use stackable_operator::{builder::pod::PodBuilder, shared::time::Duration}; - -#[derive(Debug, Snafu)] -pub enum Error { - #[snafu(display("Failed to set terminationGracePeriod"))] - SetTerminationGracePeriod { - source: stackable_operator::builder::pod::Error, - }, -} - -pub fn add_graceful_shutdown_config( - graceful_shutdown_timeout: Option, - pod_builder: &mut PodBuilder, -) -> Result<(), Error> { - // This must be always set by the merge mechanism, as we provide a default value, - // users can not disable graceful shutdown. - if let Some(graceful_shutdown_timeout) = graceful_shutdown_timeout { - pod_builder - .termination_grace_period(&graceful_shutdown_timeout) - .context(SetTerminationGracePeriodSnafu)?; - } - - Ok(()) -} diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 2e47211b..ad4a13ea 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -21,21 +21,17 @@ use crate::{ ValidatedCluster, build::{ resource::{ - config_map::build_rolegroup_config_map, discovery::{self, build_discovery_config_map}, pdb::build_pdb, rbac::{build_role_binding, build_service_account}, - service::{build_rolegroup_metrics_service, build_rolegroup_service}, - statefulset::build_rolegroup_statefulset, }, - role_group::{master, region_server, rest_server}, + role_group::{RoleGroupBuilder, master, region_server, rest_server}, }, }, crd::HbaseRole, }; pub mod container; -pub mod graceful_shutdown; pub mod jvm; pub mod kerberos; pub mod opa; @@ -92,33 +88,24 @@ pub fn build( let mut pod_disruption_budgets = vec![]; for (role_group_name, rg_config) in &cluster.master_role_group_configs { - let inputs = master::inputs(cluster, cluster_info, role_group_name, rg_config) + let builder = master::inputs(cluster, cluster_info, role_group_name, rg_config) + .and_then(RoleGroupBuilder::new) .with_context(|_| RoleGroupSnafu { role: master::ROLE, role_group: role_group_name.clone(), })?; - - services.push(build_rolegroup_service( - cluster, - &master::ROLE, - role_group_name, - )); - services.push(build_rolegroup_metrics_service( - cluster, - &master::ROLE, - role_group_name, - )); + services.extend(builder.build_services()); config_maps.push( - build_rolegroup_config_map(&inputs) - .context(role_group::ConfigMapSnafu) + builder + .build_config_map() .with_context(|_| RoleGroupSnafu { role: master::ROLE, role_group: role_group_name.clone(), })?, ); stateful_sets.push( - build_rolegroup_statefulset(&inputs) - .context(role_group::StatefulSetSnafu) + builder + .build_statefulset() .with_context(|_| RoleGroupSnafu { role: master::ROLE, role_group: role_group_name.clone(), @@ -131,33 +118,25 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.region_server_role_group_configs { - let inputs = region_server::inputs(cluster, cluster_info, role_group_name, rg_config) + let builder = region_server::inputs(cluster, cluster_info, role_group_name, rg_config) + .and_then(RoleGroupBuilder::new) .with_context(|_| RoleGroupSnafu { role: region_server::ROLE, role_group: role_group_name.clone(), })?; - services.push(build_rolegroup_service( - cluster, - ®ion_server::ROLE, - role_group_name, - )); - services.push(build_rolegroup_metrics_service( - cluster, - ®ion_server::ROLE, - role_group_name, - )); + services.extend(builder.build_services()); config_maps.push( - build_rolegroup_config_map(&inputs) - .context(role_group::ConfigMapSnafu) + builder + .build_config_map() .with_context(|_| RoleGroupSnafu { role: region_server::ROLE, role_group: role_group_name.clone(), })?, ); stateful_sets.push( - build_rolegroup_statefulset(&inputs) - .context(role_group::StatefulSetSnafu) + builder + .build_statefulset() .with_context(|_| RoleGroupSnafu { role: region_server::ROLE, role_group: role_group_name.clone(), @@ -174,33 +153,25 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.rest_server_role_group_configs { - let inputs = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config) + let builder = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config) + .and_then(RoleGroupBuilder::new) .with_context(|_| RoleGroupSnafu { role: rest_server::ROLE, role_group: role_group_name.clone(), })?; - services.push(build_rolegroup_service( - cluster, - &rest_server::ROLE, - role_group_name, - )); - services.push(build_rolegroup_metrics_service( - cluster, - &rest_server::ROLE, - role_group_name, - )); + services.extend(builder.build_services()); config_maps.push( - build_rolegroup_config_map(&inputs) - .context(role_group::ConfigMapSnafu) + builder + .build_config_map() .with_context(|_| RoleGroupSnafu { role: rest_server::ROLE, role_group: role_group_name.clone(), })?, ); stateful_sets.push( - build_rolegroup_statefulset(&inputs) - .context(role_group::StatefulSetSnafu) + builder + .build_statefulset() .with_context(|_| RoleGroupSnafu { role: rest_server::ROLE, role_group: role_group_name.clone(), 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 ba3fb60f..7af68c80 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -16,7 +16,7 @@ use crate::controller::build::{ ssl_server, }, recommended_labels_for_role_group_resources, - role_group::RoleGroupInputs, + role_group::{RoleGroupBuilder, RoleGroupInputs}, }; #[derive(Snafu, Debug)] @@ -35,17 +35,17 @@ pub enum Error { type Result = std::result::Result; -pub fn build_rolegroup_config_map(inputs: &RoleGroupInputs) -> Result { - let cluster = inputs.cluster; - let role = &inputs.role; - let role_group_name = &inputs.role_group_name; +pub fn build_config_map(builder: &RoleGroupBuilder) -> Result { + let cluster = builder.cluster; + let role = &builder.role; + let role_group_name = &builder.role_group_name; tracing::info!( "Setting up ConfigMap for {role}/{role_group_name}", role = role.to_string() ); - let data = config_map_data(inputs)?; + let data = builder.config_map_data.clone(); let cm_metadata = object_meta( cluster, diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index bb243f33..1e7a0940 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -16,20 +16,14 @@ use stackable_operator::{ }; use crate::controller::build::{ - container::{self, ContainersAndVolumes, containers_and_volumes}, - graceful_shutdown::{self, add_graceful_shutdown_config}, - object_meta, recommended_labels_for_role_group_resources, - role_group::RoleGroupInputs, + object_meta, recommended_labels_for_role_group_resources, role_group::RoleGroupBuilder, role_group_selector, }; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build the containers and volumes"))] - ContainersAndVolumes { source: container::Error }, - - #[snafu(display("failed to configure graceful shutdown"))] - GracefulShutdown { source: graceful_shutdown::Error }, + #[snafu(display("failed to configure termination grace period"))] + TerminationGracePeriod { source: builder::pod::Error }, #[snafu(display("failed to add needed volume"))] AddVolume { source: builder::pod::Error }, @@ -41,18 +35,12 @@ type Result = std::result::Result; /// /// The [`Pod`](stackable_operator::k8s_openapi::api::core::v1::Pod)s are accessible through the /// corresponding headless [`Service`](stackable_operator::k8s_openapi::api::core::v1::Service). -pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result { - let cluster = inputs.cluster; - let hbase_role = &inputs.role; - let role_group_name = &inputs.role_group_name; +pub fn build_statefulset(builder: &RoleGroupBuilder) -> Result { + let cluster = builder.cluster; + let hbase_role = &builder.role; + let role_group_name = &builder.role_group_name; let resource_names = cluster.role_group_resource_names(hbase_role, role_group_name); - let ContainersAndVolumes { - containers, - pod_volumes, - volume_claim_templates, - } = containers_and_volumes(inputs).context(ContainersAndVolumesSnafu)?; - let mut pod_builder = PodBuilder::new(); let recommended_labels = @@ -65,11 +53,11 @@ pub fn build_rolegroup_statefulset(inputs: &RoleGroupInputs) -> Result Result Result = stateful_set .spec @@ -273,7 +268,7 @@ spec: ) .expect("inputs are valid"); - build_rolegroup_statefulset(&inputs) + build_statefulset(&RoleGroupBuilder::new(inputs).expect("the role group resolves")) .expect("the StatefulSet builds") .spec .expect("the StatefulSet has a spec") diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index c70db63d..f3469dd8 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -1,12 +1,21 @@ +//! Each role's module gathers its [`RoleGroupInputs`]; [`RoleGroupBuilder::new`] turns those +//! into finished containers, pod volumes and rendered `ConfigMap` entries. Then the `build_*` +//! methods emit the Kubernetes objects. Every HBase role runs the same containers, so the +//! resolve and emit phases live here once; everything that differs between roles happens in the +//! three role modules. + use std::collections::BTreeMap; -use snafu::Snafu; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ commons::{ affinity::StackableAffinity, resources::{NoRuntimeLimits, Resources}, }, - k8s_openapi::api::core::v1::{PersistentVolumeClaim, PodTemplateSpec, Volume}, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Container, PersistentVolumeClaim, PodTemplateSpec, Service, Volume}, + }, shared::time::Duration, utils::cluster_info::KubernetesClusterInfo, v2::{ @@ -16,7 +25,11 @@ use stackable_operator::{ }; use crate::{ - controller::{ValidatedCluster, build::resource, validate::ValidatedLogging}, + controller::{ + ValidatedCluster, + build::{container, resource}, + validate::ValidatedLogging, + }, crd::{HbaseRole, HbaseStorageConfig, v1alpha1}, }; @@ -26,7 +39,6 @@ pub(crate) mod rest_server; /// Why building one role group failed. The caller adds which role and role group it was. #[derive(Snafu, Debug)] -#[snafu(visibility(pub(crate)))] pub enum Error { #[snafu(display("missing secret lifetime"))] MissingSecretLifetime, @@ -41,6 +53,12 @@ pub enum Error { StatefulSet { source: resource::statefulset::Error, }, + + #[snafu(display("failed to render the config files"))] + ConfigMapData { source: resource::config_map::Error }, + + #[snafu(display("failed to build the containers and volumes"))] + ContainersAndVolumes { source: container::Error }, } /// Where a role group's listener comes from. @@ -87,3 +105,84 @@ pub(crate) struct RoleGroupInputs<'a> { /// The role's own `hbase-site.xml` entries (bind addresses and ports, or the REST endpoint). pub(crate) role_hbase_site: BTreeMap, } + +/// Everything one role group's Kubernetes objects are built from, already resolved. +/// +/// These fields are outputs rather than configuration: the containers are built and the +/// `ConfigMap` entries rendered. The `build_*` methods therefore read straight through, and +/// nothing about which role this is reaches them beyond the role's name. +pub(crate) struct RoleGroupBuilder<'a> { + pub(crate) cluster: &'a ValidatedCluster, + /// Read for the role group's object names and labels. + pub(crate) role: HbaseRole, + pub(crate) role_group_name: RoleGroupName, + /// Read for the pod's affinity. + pub(crate) affinity: StackableAffinity, + /// Read for the pod's termination grace period. + pub(crate) graceful_shutdown_timeout: Option, + /// The role group's replica count, passed to the StatefulSet as is; `None` makes Kubernetes + /// run one pod. + pub(crate) replicas: Option, + pub(crate) pod_overrides: PodTemplateSpec, + /// `Some` for rest servers only, which keeps the other roles' StatefulSets without a + /// `volumeClaimTemplates` field. + pub(crate) volume_claim_templates: Option>, + /// The `hbase` container, then the Vector sidecar when the Vector agent is enabled. + pub(crate) containers: Vec, + /// Every pod volume, in the order the pod lists them. + pub(crate) pod_volumes: Vec, + /// The rendered `ConfigMap` entries, keyed by file name. + pub(crate) config_map_data: BTreeMap, +} + +impl<'a> RoleGroupBuilder<'a> { + /// Resolves one role group: renders its config files, builds the containers every role runs + /// and collects their pod volumes. + pub(crate) fn new(inputs: RoleGroupInputs<'a>) -> Result { + let config_map_data = + resource::config_map::config_map_data(&inputs).context(ConfigMapDataSnafu)?; + let container::ContainersAndVolumes { + containers, + pod_volumes, + volume_claim_templates, + } = container::containers_and_volumes(&inputs).context(ContainersAndVolumesSnafu)?; + + Ok(Self { + cluster: inputs.cluster, + role: inputs.role, + role_group_name: inputs.role_group_name, + affinity: inputs.affinity, + graceful_shutdown_timeout: inputs.graceful_shutdown_timeout, + replicas: inputs.replicas, + pod_overrides: inputs.pod_overrides, + volume_claim_templates, + containers, + pod_volumes, + config_map_data, + }) + } + + /// The headless and metrics `Service`s. + pub(crate) fn build_services(&self) -> Vec { + vec![ + resource::service::build_rolegroup_service( + self.cluster, + &self.role, + &self.role_group_name, + ), + resource::service::build_rolegroup_metrics_service( + self.cluster, + &self.role, + &self.role_group_name, + ), + ] + } + + pub(crate) fn build_config_map(&self) -> Result { + resource::config_map::build_config_map(self).context(ConfigMapSnafu) + } + + pub(crate) fn build_statefulset(&self) -> Result { + resource::statefulset::build_statefulset(self).context(StatefulSetSnafu) + } +} From cb4f722b7c3c7885671f1d356d52ef841120dbb9 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 13:28:52 +0200 Subject: [PATCH 09/13] build each role group through per-role modules and a shared role group builder --- .../src/controller/build/mod.rs | 98 +++++++------------ .../controller/build/resource/statefulset.rs | 10 +- .../src/controller/build/role_group/master.rs | 11 ++- .../build/role_group/region_server.rs | 8 +- .../build/role_group/rest_server.rs | 11 ++- 5 files changed, 58 insertions(+), 80 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index ad4a13ea..b2cd48cd 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -6,6 +6,10 @@ use std::marker::PhantomData; use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service}, + }, kvp::Labels, utils::cluster_info::KubernetesClusterInfo, v2::{ @@ -73,6 +77,25 @@ pub enum Error { Discovery { source: discovery::Error }, } +/// The resources of the role groups, in the order the role-group loops of [`build`] add them. +#[derive(Default)] +struct RoleGroupResources { + services: Vec, + config_maps: Vec, + stateful_sets: Vec, +} + +impl RoleGroupResources { + /// Adds the headless and metrics Services, the ConfigMap and the StatefulSet of one role + /// group. + fn add(&mut self, builder: &RoleGroupBuilder) -> Result<(), role_group::Error> { + self.services.extend(builder.build_services()); + self.config_maps.push(builder.build_config_map()?); + self.stateful_sets.push(builder.build_statefulset()?); + Ok(()) + } +} + /// Builds every Kubernetes resource for the given validated cluster. /// /// Does not need a Kubernetes client: every reference to another Kubernetes resource is already @@ -82,35 +105,16 @@ pub fn build( cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, ) -> Result, Error> { - let mut stateful_sets = vec![]; - let mut services = vec![]; - let mut config_maps = vec![]; + let mut built = RoleGroupResources::default(); let mut pod_disruption_budgets = vec![]; for (role_group_name, rg_config) in &cluster.master_role_group_configs { - let builder = master::inputs(cluster, cluster_info, role_group_name, rg_config) - .and_then(RoleGroupBuilder::new) + master::build(cluster, cluster_info, role_group_name, rg_config) + .and_then(|builder| built.add(&builder)) .with_context(|_| RoleGroupSnafu { role: master::ROLE, role_group: role_group_name.clone(), })?; - services.extend(builder.build_services()); - config_maps.push( - builder - .build_config_map() - .with_context(|_| RoleGroupSnafu { - role: master::ROLE, - role_group: role_group_name.clone(), - })?, - ); - stateful_sets.push( - builder - .build_statefulset() - .with_context(|_| RoleGroupSnafu { - role: master::ROLE, - role_group: role_group_name.clone(), - })?, - ); } if let Some(pdb) = build_pdb(&cluster.master_config.pdb, cluster, &master::ROLE) { @@ -118,30 +122,12 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.region_server_role_group_configs { - let builder = region_server::inputs(cluster, cluster_info, role_group_name, rg_config) - .and_then(RoleGroupBuilder::new) + region_server::build(cluster, cluster_info, role_group_name, rg_config) + .and_then(|builder| built.add(&builder)) .with_context(|_| RoleGroupSnafu { role: region_server::ROLE, role_group: role_group_name.clone(), })?; - - services.extend(builder.build_services()); - config_maps.push( - builder - .build_config_map() - .with_context(|_| RoleGroupSnafu { - role: region_server::ROLE, - role_group: role_group_name.clone(), - })?, - ); - stateful_sets.push( - builder - .build_statefulset() - .with_context(|_| RoleGroupSnafu { - role: region_server::ROLE, - role_group: role_group_name.clone(), - })?, - ); } if let Some(pdb) = build_pdb( @@ -153,30 +139,12 @@ pub fn build( } for (role_group_name, rg_config) in &cluster.rest_server_role_group_configs { - let builder = rest_server::inputs(cluster, cluster_info, role_group_name, rg_config) - .and_then(RoleGroupBuilder::new) + rest_server::build(cluster, cluster_info, role_group_name, rg_config) + .and_then(|builder| built.add(&builder)) .with_context(|_| RoleGroupSnafu { role: rest_server::ROLE, role_group: role_group_name.clone(), })?; - - services.extend(builder.build_services()); - config_maps.push( - builder - .build_config_map() - .with_context(|_| RoleGroupSnafu { - role: rest_server::ROLE, - role_group: role_group_name.clone(), - })?, - ); - stateful_sets.push( - builder - .build_statefulset() - .with_context(|_| RoleGroupSnafu { - role: rest_server::ROLE, - role_group: role_group_name.clone(), - })?, - ); } if let Some(rest_server_config) = &cluster.rest_server_config @@ -185,6 +153,12 @@ pub fn build( pod_disruption_budgets.push(pdb); } + let RoleGroupResources { + services, + mut config_maps, + stateful_sets, + } = built; + // The role-level discovery ConfigMap advertises the cluster's connection information; it is // deterministic (derived only from the validated cluster and static cluster info). config_maps.push(build_discovery_config_map(cluster, cluster_info).context(DiscoverySnafu)?); diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 1e7a0940..1bfcf7b1 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -143,7 +143,7 @@ mod tests { let cluster = test_utils::validated_cluster_from(&hbase); let role_group_name = test_utils::role_group_name("default"); let cluster_info = &test_utils::cluster_info(); - let inputs = master::inputs( + let builder = master::build( &cluster, cluster_info, &role_group_name, @@ -151,9 +151,7 @@ mod tests { ) .expect("inputs are valid"); - let stateful_set = - build_statefulset(&RoleGroupBuilder::new(inputs).expect("the role group resolves")) - .expect("the StatefulSet builds"); + let stateful_set = build_statefulset(&builder).expect("the StatefulSet builds"); let env: Vec<(String, String)> = stateful_set .spec @@ -260,7 +258,7 @@ spec: let cluster = test_utils::validated_cluster_from(&hbase); let role_group_name = test_utils::role_group_name("default"); let cluster_info = &test_utils::cluster_info(); - let inputs = master::inputs( + let builder = master::build( &cluster, cluster_info, &role_group_name, @@ -268,7 +266,7 @@ spec: ) .expect("inputs are valid"); - build_statefulset(&RoleGroupBuilder::new(inputs).expect("the role group resolves")) + build_statefulset(&builder) .expect("the StatefulSet builds") .spec .expect("the StatefulSet has a spec") diff --git a/rust/operator-binary/src/controller/build/role_group/master.rs b/rust/operator-binary/src/controller/build/role_group/master.rs index ad972927..6f21b74f 100644 --- a/rust/operator-binary/src/controller/build/role_group/master.rs +++ b/rust/operator-binary/src/controller/build/role_group/master.rs @@ -1,3 +1,6 @@ +//! Resolves one master role group into its RoleGroupBuilder. Masters get their +//! listener from an ephemeral pod volume and do not run the region mover. + use snafu::{OptionExt, ResultExt}; use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, @@ -12,7 +15,7 @@ use crate::{ resource::listener::build_listener_volume, role_group::{ Error, ListenerPlacement, ListenerSnafu, MissingSecretLifetimeSnafu, - RoleGroupInputs, + RoleGroupBuilder, RoleGroupInputs, }, }, }, @@ -22,12 +25,12 @@ use crate::{ pub(crate) const ROLE: HbaseRole = HbaseRole::Master; /// The inputs of one master role group. -pub(crate) fn inputs<'a>( +pub(crate) fn build<'a>( cluster: &'a ValidatedCluster, cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &MasterRoleGroupConfig, -) -> Result, Error> { +) -> Result, Error> { let HbaseConfig { hbase_rootdir, resources, @@ -47,7 +50,7 @@ pub(crate) fn inputs<'a>( build_listener_volume(listener_class, &recommended_labels).context(ListenerSnafu)?, ); - Ok(RoleGroupInputs { + RoleGroupBuilder::new(RoleGroupInputs { cluster, cluster_info, role: ROLE, diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index 88b98e1f..2c924fbd 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -12,7 +12,7 @@ use crate::{ resource::listener::build_listener_volume, role_group::{ Error, ListenerPlacement, ListenerSnafu, MissingSecretLifetimeSnafu, - RoleGroupInputs, + RoleGroupBuilder, RoleGroupInputs, }, }, }, @@ -22,12 +22,12 @@ use crate::{ pub(crate) const ROLE: HbaseRole = HbaseRole::RegionServer; /// The inputs of one region server role group. -pub(crate) fn inputs<'a>( +pub(crate) fn build<'a>( cluster: &'a ValidatedCluster, cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &RegionServerRoleGroupConfig, -) -> Result, Error> { +) -> Result, Error> { let RegionServerConfig { hbase_rootdir, resources, @@ -47,7 +47,7 @@ pub(crate) fn inputs<'a>( build_listener_volume(listener_class, &recommended_labels).context(ListenerSnafu)?, ); - Ok(RoleGroupInputs { + RoleGroupBuilder::new(RoleGroupInputs { cluster, cluster_info, role: ROLE, diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index 80fbe607..1a302172 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -10,7 +10,10 @@ use crate::{ properties::{hbase_env::HBASE_REST_OPTS, hbase_site}, recommended_labels_for_unversioned_role_group_resources, resource::listener::build_listener_pvc, - role_group::{Error, ListenerPlacement, MissingSecretLifetimeSnafu, RoleGroupInputs}, + role_group::{ + Error, ListenerPlacement, MissingSecretLifetimeSnafu, RoleGroupBuilder, + RoleGroupInputs, + }, }, }, crd::{HbaseConfig, HbaseRole}, @@ -19,12 +22,12 @@ use crate::{ pub(crate) const ROLE: HbaseRole = HbaseRole::RestServer; /// The inputs of one rest server role group. -pub(crate) fn inputs<'a>( +pub(crate) fn build<'a>( cluster: &'a ValidatedCluster, cluster_info: &'a KubernetesClusterInfo, role_group_name: &RoleGroupName, rg_config: &RestServerRoleGroupConfig, -) -> Result, Error> { +) -> Result, Error> { let HbaseConfig { hbase_rootdir, resources, @@ -43,7 +46,7 @@ pub(crate) fn inputs<'a>( let listener = ListenerPlacement::ClaimTemplate(build_listener_pvc(listener_class, &unversioned_labels)); - Ok(RoleGroupInputs { + RoleGroupBuilder::new(RoleGroupInputs { cluster, cluster_info, role: ROLE, From a1d02ddcf710922f7751bcba0e04db0431df2367 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 14:49:49 +0200 Subject: [PATCH 10/13] validate the variable names of hbase-env.sh overrides --- .../src/controller/build/mod.rs | 6 +- .../controller/build/properties/hbase_env.rs | 46 ++++--- .../controller/build/resource/config_map.rs | 2 +- .../src/controller/build/role_group/master.rs | 2 +- .../src/controller/build/role_group/mod.rs | 11 +- .../build/role_group/region_server.rs | 2 +- .../build/role_group/rest_server.rs | 2 +- rust/operator-binary/src/controller/mod.rs | 20 ++- .../src/controller/validate.rs | 129 +++++++++++++++++- rust/operator-binary/src/test_utils.rs | 17 ++- 10 files changed, 188 insertions(+), 49 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index b2cd48cd..e11d6a1d 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -532,9 +532,9 @@ spec: // The hbase-site key that identifies each role's own section, and its OPTS variable. let own = |role: &HbaseRole| match role { - HbaseRole::Master => (HBASE_MASTER_IPC_ADDRESS, HBASE_MASTER_OPTS), - HbaseRole::RegionServer => (HBASE_REGIONSERVER_IPC_ADDRESS, HBASE_REGIONSERVER_OPTS), - HbaseRole::RestServer => (HBASE_REST_ENDPOINT, HBASE_REST_OPTS), + HbaseRole::Master => (HBASE_MASTER_IPC_ADDRESS, &*HBASE_MASTER_OPTS), + HbaseRole::RegionServer => (HBASE_REGIONSERVER_IPC_ADDRESS, &*HBASE_REGIONSERVER_OPTS), + HbaseRole::RestServer => (HBASE_REST_ENDPOINT, &*HBASE_REST_OPTS), }; for role in HbaseRole::iter() { let data = config_map_data(&resources, &cluster, &role); diff --git a/rust/operator-binary/src/controller/build/properties/hbase_env.rs b/rust/operator-binary/src/controller/build/properties/hbase_env.rs index ec752365..35ad631a 100644 --- a/rust/operator-binary/src/controller/build/properties/hbase_env.rs +++ b/rust/operator-binary/src/controller/build/properties/hbase_env.rs @@ -1,11 +1,12 @@ //! Builds the `hbase-env.sh` config file: JVM heap/options env vars + overrides. -use std::{collections::BTreeMap, fmt::Write}; +use std::{collections::BTreeMap, fmt::Write, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ commons::resources::{NoRuntimeLimits, Resources}, - v2::config_overrides::KeyValueConfigOverrides, + constant, + v2::builder::pod::container::EnvVarName, }; use crate::{ @@ -13,11 +14,13 @@ use crate::{ crd::HbaseStorageConfig, }; -/// The `hbase-env.sh` variable that carries a role's own JVM options; `bin/hbase` reads the one -/// matching the process it starts. -pub(crate) const HBASE_MASTER_OPTS: &str = "HBASE_MASTER_OPTS"; -pub(crate) const HBASE_REGIONSERVER_OPTS: &str = "HBASE_REGIONSERVER_OPTS"; -pub(crate) const HBASE_REST_OPTS: &str = "HBASE_REST_OPTS"; +constant!(pub(crate) HBASE_MASTER_OPTS: EnvVarName = "HBASE_MASTER_OPTS"); +constant!(pub(crate) HBASE_REGIONSERVER_OPTS: EnvVarName = "HBASE_REGIONSERVER_OPTS"); +constant!(pub(crate) HBASE_REST_OPTS: EnvVarName = "HBASE_REST_OPTS"); + +constant!(HBASE_MANAGES_ZK: EnvVarName = "HBASE_MANAGES_ZK"); +constant!(HBASE_HEAPSIZE: EnvVarName = "HBASE_HEAPSIZE"); +constant!(HBASE_OPTS: EnvVarName = "HBASE_OPTS"); #[derive(Snafu, Debug)] pub enum Error { @@ -28,23 +31,23 @@ pub enum Error { /// Renders `hbase-env.sh` as `export VAR="VALUE"` lines. pub fn build( resources: &Resources, - opts_variable: &str, + opts_variable: &EnvVarName, kerberos_enabled: bool, non_heap_jvm_args: String, - overrides: KeyValueConfigOverrides, + overrides: BTreeMap, ) -> Result { - let mut env: BTreeMap = BTreeMap::new(); + let mut env: BTreeMap = BTreeMap::new(); - env.insert("HBASE_MANAGES_ZK".to_string(), "false".to_string()); + env.insert(HBASE_MANAGES_ZK.clone(), "false".to_string()); env.insert( - "HBASE_HEAPSIZE".to_string(), + HBASE_HEAPSIZE.clone(), construct_hbase_heapsize_env(resources).context(ConstructHbaseHeapsizeEnvSnafu)?, ); env.insert( - "HBASE_OPTS".to_string(), + HBASE_OPTS.clone(), construct_global_jvm_args(kerberos_enabled), ); - env.insert(opts_variable.to_string(), non_heap_jvm_args); + env.insert(opts_variable.clone(), non_heap_jvm_args); // configOverride come last env.extend(overrides); @@ -71,10 +74,10 @@ mod tests { .resources; let env = build( merged, - HBASE_MASTER_OPTS, + &HBASE_MASTER_OPTS, false, "-Xtest".to_string(), - KeyValueConfigOverrides::default(), + BTreeMap::new(), ) .unwrap(); assert!(env.contains("export HBASE_MANAGES_ZK=\"false\""), "{env}"); @@ -90,10 +93,10 @@ mod tests { .resources; let env = build( merged, - HBASE_REGIONSERVER_OPTS, + &HBASE_REGIONSERVER_OPTS, false, "-Xtest".to_string(), - KeyValueConfigOverrides::default(), + BTreeMap::new(), ) .unwrap(); assert!(env.contains("export HBASE_REGIONSERVER_OPTS="), "{env}"); @@ -108,10 +111,13 @@ mod tests { .resources; let env = build( merged, - HBASE_MASTER_OPTS, + &HBASE_MASTER_OPTS, false, "-Xtest".to_string(), - [("CUSTOM_VAR", "custom_value")].into(), + BTreeMap::from([( + EnvVarName::from_str("CUSTOM_VAR").expect("valid variable name"), + "custom_value".to_string(), + )]), ) .unwrap(); assert!(env.contains("export CUSTOM_VAR=\"custom_value\""), "{env}"); 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 7af68c80..23d80201 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -86,7 +86,7 @@ pub(crate) fn config_map_data(inputs: &RoleGroupInputs) -> Result( .jvm_argument_overrides .clone(), dns_cache_ttl_seconds: "5", - hbase_env_opts_variable: HBASE_MASTER_OPTS, + hbase_env_opts_variable: HBASE_MASTER_OPTS.clone(), role_hbase_site: hbase_site::master_entries(), }) } diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index f3469dd8..cf1ecbbd 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -19,18 +19,19 @@ use stackable_operator::{ shared::time::Duration, utils::cluster_info::KubernetesClusterInfo, v2::{ - builder::pod::container::EnvVarSet, jvm_argument_overrides::JvmArgumentOverrides, + builder::pod::container::{EnvVarName, EnvVarSet}, + jvm_argument_overrides::JvmArgumentOverrides, types::operator::RoleGroupName, }, }; use crate::{ controller::{ - ValidatedCluster, + ValidatedCluster, ValidatedHbaseConfigOverrides, build::{container, resource}, validate::ValidatedLogging, }, - crd::{HbaseRole, HbaseStorageConfig, v1alpha1}, + crd::{HbaseRole, HbaseStorageConfig}, }; pub(crate) mod master; @@ -94,14 +95,14 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) region_mover_opts: String, pub(crate) run_region_mover: bool, pub(crate) replicas: Option, - pub(crate) config_overrides: v1alpha1::HbaseConfigOverrides, + pub(crate) config_overrides: ValidatedHbaseConfigOverrides, pub(crate) env_overrides: EnvVarSet, pub(crate) pod_overrides: PodTemplateSpec, pub(crate) jvm_argument_overrides: JvmArgumentOverrides, /// The JVM's positive DNS cache TTL in seconds, written to `security.properties`. pub(crate) dns_cache_ttl_seconds: &'static str, /// The `hbase-env.sh` variable `bin/hbase` reads for this role's process. - pub(crate) hbase_env_opts_variable: &'static str, + pub(crate) hbase_env_opts_variable: EnvVarName, /// The role's own `hbase-site.xml` entries (bind addresses and ports, or the REST endpoint). pub(crate) role_hbase_site: BTreeMap, } diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index 2c924fbd..13da69b7 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -71,7 +71,7 @@ pub(crate) fn build<'a>( .jvm_argument_overrides .clone(), dns_cache_ttl_seconds: "10", - hbase_env_opts_variable: HBASE_REGIONSERVER_OPTS, + hbase_env_opts_variable: HBASE_REGIONSERVER_OPTS.clone(), role_hbase_site: hbase_site::region_server_entries(), }) } diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index 1a302172..f5d9ca91 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -70,7 +70,7 @@ pub(crate) fn build<'a>( .jvm_argument_overrides .clone(), dns_cache_ttl_seconds: "30", - hbase_env_opts_variable: HBASE_REST_OPTS, + hbase_env_opts_variable: HBASE_REST_OPTS.clone(), role_hbase_site: hbase_site::rest_server_entries(), }) } diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 24e97d83..9f78b6a3 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -24,6 +24,8 @@ use stackable_operator::{ kube::Resource, v2::{ HasName, HasUid, NameIsValidLabelValue, + builder::pod::container::EnvVarName, + config_overrides::KeyValueConfigOverrides, role_group_utils::ResourceNames, role_utils::{self, JavaCommonConfig, RoleGroupConfig}, types::{ @@ -96,12 +98,24 @@ pub struct ValidatedRestServerConfig { pub logging: ValidatedLogging, } +/// The merged `configOverrides` of a role group, with the `hbase-env.sh` variable names +/// validated. `EnvVarName` is Kubernetes' rule (non-empty, printable ASCII, no `=`), not the +/// shell's: it rejects a name that would corrupt the `export` line, nothing stricter. +#[derive(Clone, Debug)] +pub struct ValidatedHbaseConfigOverrides { + pub hbase_site_xml: KeyValueConfigOverrides, + pub hbase_env_sh: BTreeMap, + pub ssl_server_xml: KeyValueConfigOverrides, + pub ssl_client_xml: KeyValueConfigOverrides, + pub security_properties: KeyValueConfigOverrides, +} + pub type MasterRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; pub type RegionServerRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; pub type RestServerRoleGroupConfig = - RoleGroupConfig; + RoleGroupConfig; /// The validated cluster: proves that config merging and validation succeeded for /// every role and role group before any resources are created. diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 3a9c7387..f1291469 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -11,6 +11,7 @@ use stackable_operator::{ product_logging::spec::Logging, role_utils::GenericRoleConfig, v2::{ + builder::pod::container::EnvVarName, controller_utils::{get_cluster_name, get_namespace, get_uid}, product_logging::framework::{ ValidatedContainerLogConfigChoice, VectorContainerLogConfig, @@ -27,9 +28,9 @@ use stackable_operator::{ use crate::{ controller::{ MasterRoleGroupConfig, RegionServerRoleGroupConfig, RestServerRoleGroupConfig, - ValidatedCluster, ValidatedClusterConfig, ValidatedMasterConfig, - ValidatedRegionServerConfig, ValidatedRestServerConfig, ValidatedRoleConfig, - dereference::DereferencedObjects, + ValidatedCluster, ValidatedClusterConfig, ValidatedHbaseConfigOverrides, + ValidatedMasterConfig, ValidatedRegionServerConfig, ValidatedRestServerConfig, + ValidatedRoleConfig, dereference::DereferencedObjects, }, crd::{ Container, HbaseConfig, HbaseConfigFragment, HbaseRole, RegionServerConfig, @@ -76,6 +77,16 @@ pub enum Error { "the Vector aggregator discovery ConfigMap name is required when the Vector agent is enabled" ))] MissingVectorAggregatorConfigMapName, + + #[snafu(display( + "invalid hbase-env.sh variable name {variable:?} in the configOverrides of {role} role group {role_group}" + ))] + ParseHbaseEnvVariableName { + source: stackable_operator::v2::macros::attributed_string_type::Error, + role: HbaseRole, + role_group: String, + variable: String, + }, } /// Validated logging configuration for the HBase and (optional) Vector container. @@ -317,6 +328,43 @@ where Ok(validated) } +/// Validates the merged `configOverrides` of one role group: the `hbase-env.sh` keys become +/// `EnvVarName`s, the other files' overrides are passed through. +fn validate_config_overrides( + hbase_role: &HbaseRole, + role_group_name: &str, + overrides: v1alpha1::HbaseConfigOverrides, +) -> Result { + let v1alpha1::HbaseConfigOverrides { + hbase_site_xml, + hbase_env_sh, + ssl_server_xml, + ssl_client_xml, + security_properties, + } = overrides; + let hbase_env_sh = hbase_env_sh + .overrides + .into_iter() + .map(|(variable, value)| { + let name = EnvVarName::from_str(&variable).with_context(|_| { + ParseHbaseEnvVariableNameSnafu { + role: hbase_role.clone(), + role_group: role_group_name.to_owned(), + variable: variable.clone(), + } + })?; + Ok((name, value)) + }) + .collect::, Error>>()?; + Ok(ValidatedHbaseConfigOverrides { + hbase_site_xml, + hbase_env_sh, + ssl_server_xml, + ssl_client_xml, + security_properties, + }) +} + fn validate_master_role_group_configs( role: &Role< HbaseConfigFragment, @@ -344,7 +392,11 @@ fn validate_master_role_group_configs( config: merged.config, logging, }, - config_overrides: merged.config_overrides, + config_overrides: validate_config_overrides( + &HbaseRole::Master, + role_group_name, + merged.config_overrides, + )?, env_overrides: merged.env_overrides, cli_overrides: merged.cli_overrides, pod_overrides: merged.pod_overrides, @@ -388,7 +440,11 @@ fn validate_region_server_role_group_configs( config: merged.config, logging, }, - config_overrides: merged.config_overrides, + config_overrides: validate_config_overrides( + &HbaseRole::RegionServer, + role_group_name, + merged.config_overrides, + )?, env_overrides: merged.env_overrides, cli_overrides: merged.cli_overrides, pod_overrides: merged.pod_overrides, @@ -432,7 +488,11 @@ fn validate_rest_server_role_group_configs( config: merged.config, logging, }, - config_overrides: merged.config_overrides, + config_overrides: validate_config_overrides( + &HbaseRole::RestServer, + role_group_name, + merged.config_overrides, + )?, env_overrides: merged.env_overrides, cli_overrides: merged.cli_overrides, pod_overrides: merged.pod_overrides, @@ -454,7 +514,7 @@ mod tests { use indoc::indoc; use super::*; - use crate::crd::HbaseConfig; + use crate::{crd::HbaseConfig, test_utils}; /// Role-level `envOverrides` are merged with role-group-level ones, with the role /// group winning on key collisions. @@ -589,4 +649,59 @@ spec: let aggregator = ConfigMapName::from_str("vector-aggregator").expect("valid name"); assert!(validate_logging(&logging, &Some(aggregator)).is_ok()); } + + /// An `hbase-env.sh` override whose variable name cannot be an environment variable name + /// (here: it contains `=`) is rejected when the cluster is validated, naming the role, the + /// role group and the key, instead of being written into the file. + #[test] + fn hbase_env_override_with_invalid_variable_name_is_rejected() { + let hbase = test_utils::hbase_from_yaml( + r#" +--- +apiVersion: hbase.stackable.tech/v1alpha1 +kind: HbaseCluster +metadata: + name: hbase + namespace: default + uid: c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f +spec: + image: + productVersion: 2.6.3 + clusterConfig: + hdfsConfigMapName: simple-hdfs + zookeeperConfigMapName: simple-znode + masters: + roleGroups: + default: + replicas: 1 + regionServers: + configOverrides: + hbase-env.sh: + "BAD=NAME": value + roleGroups: + default: + replicas: 1 +"#, + ); + + let error = validate_cluster( + &hbase, + "oci.example.org", + test_utils::dereferenced_objects(), + ) + .expect_err("an hbase-env.sh variable name containing '=' should be rejected"); + + assert!( + matches!( + &error, + Error::ParseHbaseEnvVariableName { + role: HbaseRole::RegionServer, + role_group, + variable, + .. + } if role_group == "default" && variable == "BAD=NAME" + ), + "{error:?}" + ); + } } diff --git a/rust/operator-binary/src/test_utils.rs b/rust/operator-binary/src/test_utils.rs index 3ac3a62d..c86bf873 100644 --- a/rust/operator-binary/src/test_utils.rs +++ b/rust/operator-binary/src/test_utils.rs @@ -63,13 +63,16 @@ pub fn minimal_hbase() -> v1alpha1::HbaseCluster { /// Runs the real validation pipeline over `hbase`, with a fixed dereferenced ZooKeeper connection /// and no OPA. pub fn validated_cluster_from(hbase: &v1alpha1::HbaseCluster) -> ValidatedCluster { - validated_cluster_with( - hbase, - DereferencedObjects { - zookeeper_connection_information: ZookeeperConnectionInformation::for_tests(), - hbase_opa_config: None, - }, - ) + validated_cluster_with(hbase, dereferenced_objects()) +} + +/// A fixed dereferenced ZooKeeper connection and no OPA, for tests that call `validate_cluster` +/// directly to inspect its error. +pub fn dereferenced_objects() -> DereferencedObjects { + DereferencedObjects { + zookeeper_connection_information: ZookeeperConnectionInformation::for_tests(), + hbase_opa_config: None, + } } /// Runs the real validation pipeline over `hbase` with the given dereferenced objects, for From 7b9cef1be670c88dcaf23cbc2a109e84485f6c86 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 14:57:08 +0200 Subject: [PATCH 11/13] use constants for role-specific dns_cache_ttl_seconds values --- .../build/properties/security_properties.rs | 14 +++++++++----- .../src/controller/build/role_group/master.rs | 4 +++- .../src/controller/build/role_group/mod.rs | 2 +- .../controller/build/role_group/region_server.rs | 4 +++- .../src/controller/build/role_group/rest_server.rs | 4 +++- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index beb95594..af564d37 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -12,7 +12,7 @@ use stackable_operator::v2::{ /// Renders `security.properties`: role-specific DNS cache TTLs plus user overrides. pub fn build( - dns_cache_ttl_seconds: &str, + dns_cache_ttl_seconds: u32, overrides: KeyValueConfigOverrides, ) -> Result { // Caching forever (the JVM default for successful lookups) @@ -40,7 +40,8 @@ mod tests { #[test] fn injects_master_dns_cache_ttl() { assert_eq!( - build("5", KeyValueConfigOverrides::default()).unwrap(), + build(5, KeyValueConfigOverrides::default()) + .expect("the default security.properties should render"), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=5\n" ); } @@ -48,7 +49,8 @@ mod tests { #[test] fn injects_regionserver_dns_cache_ttl() { assert_eq!( - build("10", KeyValueConfigOverrides::default()).unwrap(), + build(10, KeyValueConfigOverrides::default()) + .expect("the default security.properties should render"), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=10\n" ); } @@ -56,7 +58,8 @@ mod tests { #[test] fn injects_restserver_dns_cache_ttl() { assert_eq!( - build("30", KeyValueConfigOverrides::default()).unwrap(), + build(30, KeyValueConfigOverrides::default()) + .expect("the default security.properties should render"), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=30\n" ); } @@ -64,7 +67,8 @@ mod tests { #[test] fn user_overrides_win_over_injected_defaults() { assert_eq!( - build("5", [("networkaddress.cache.ttl", "60")].into()).unwrap(), + build(5, [("networkaddress.cache.ttl", "60")].into()) + .expect("security.properties with an override should render"), "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=60\n" ); } diff --git a/rust/operator-binary/src/controller/build/role_group/master.rs b/rust/operator-binary/src/controller/build/role_group/master.rs index a1b25ed1..9b3bc436 100644 --- a/rust/operator-binary/src/controller/build/role_group/master.rs +++ b/rust/operator-binary/src/controller/build/role_group/master.rs @@ -23,6 +23,8 @@ use crate::{ }; pub(crate) const ROLE: HbaseRole = HbaseRole::Master; +/// The JVM's positive DNS cache TTL for this role, written to `security.properties`. +const DNS_CACHE_TTL_SECONDS: u32 = 5; /// The inputs of one master role group. pub(crate) fn build<'a>( @@ -73,7 +75,7 @@ pub(crate) fn build<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), - dns_cache_ttl_seconds: "5", + dns_cache_ttl_seconds: DNS_CACHE_TTL_SECONDS, hbase_env_opts_variable: HBASE_MASTER_OPTS.clone(), role_hbase_site: hbase_site::master_entries(), }) diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs index cf1ecbbd..d4705e5b 100644 --- a/rust/operator-binary/src/controller/build/role_group/mod.rs +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -100,7 +100,7 @@ pub(crate) struct RoleGroupInputs<'a> { pub(crate) pod_overrides: PodTemplateSpec, pub(crate) jvm_argument_overrides: JvmArgumentOverrides, /// The JVM's positive DNS cache TTL in seconds, written to `security.properties`. - pub(crate) dns_cache_ttl_seconds: &'static str, + pub(crate) dns_cache_ttl_seconds: u32, /// The `hbase-env.sh` variable `bin/hbase` reads for this role's process. pub(crate) hbase_env_opts_variable: EnvVarName, /// The role's own `hbase-site.xml` entries (bind addresses and ports, or the REST endpoint). diff --git a/rust/operator-binary/src/controller/build/role_group/region_server.rs b/rust/operator-binary/src/controller/build/role_group/region_server.rs index 13da69b7..7bba6c0a 100644 --- a/rust/operator-binary/src/controller/build/role_group/region_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/region_server.rs @@ -20,6 +20,8 @@ use crate::{ }; pub(crate) const ROLE: HbaseRole = HbaseRole::RegionServer; +/// The JVM's positive DNS cache TTL for this role, written to `security.properties`. +const DNS_CACHE_TTL_SECONDS: u32 = 10; /// The inputs of one region server role group. pub(crate) fn build<'a>( @@ -70,7 +72,7 @@ pub(crate) fn build<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), - dns_cache_ttl_seconds: "10", + dns_cache_ttl_seconds: DNS_CACHE_TTL_SECONDS, hbase_env_opts_variable: HBASE_REGIONSERVER_OPTS.clone(), role_hbase_site: hbase_site::region_server_entries(), }) diff --git a/rust/operator-binary/src/controller/build/role_group/rest_server.rs b/rust/operator-binary/src/controller/build/role_group/rest_server.rs index f5d9ca91..8b65105d 100644 --- a/rust/operator-binary/src/controller/build/role_group/rest_server.rs +++ b/rust/operator-binary/src/controller/build/role_group/rest_server.rs @@ -20,6 +20,8 @@ use crate::{ }; pub(crate) const ROLE: HbaseRole = HbaseRole::RestServer; +/// The JVM's positive DNS cache TTL for this role, written to `security.properties`. +const DNS_CACHE_TTL_SECONDS: u32 = 30; /// The inputs of one rest server role group. pub(crate) fn build<'a>( @@ -69,7 +71,7 @@ pub(crate) fn build<'a>( .product_specific_common_config .jvm_argument_overrides .clone(), - dns_cache_ttl_seconds: "30", + dns_cache_ttl_seconds: DNS_CACHE_TTL_SECONDS, hbase_env_opts_variable: HBASE_REST_OPTS.clone(), role_hbase_site: hbase_site::rest_server_entries(), }) From 57425f0aa8fd2b7218b024598b59f58c5477ec79 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 15:09:12 +0200 Subject: [PATCH 12/13] chore: remove the render-diff harness, document the hbase-env.sh validation, name the DNS cache properties --- CHANGELOG.md | 7 ++++++ .../hbase/pages/usage-guide/overrides.adoc | 4 +++- .../src/controller/build/mod.rs | 23 ------------------- .../build/properties/security_properties.rs | 19 +++++++++------ 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f4f5fed..aa476cb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,12 @@ - test: Update OPA to 1.20.2 in integration tests ([#823]). - All pods now set `securityContext.seccompProfile.type` to `RuntimeDefault` ([#824]). - Replace the unmaintained `serde_yaml` with `yaml_serde` ([#824]). +- Internal operator refactoring: build each role group through per-role modules (masters, + region servers, rest servers) and a shared role group builder, with each role's configuration + held in typed fields of the validated cluster ([#827]). +- Variable names in `configOverrides` for `hbase-env.sh` are now validated (non-empty, printable + ASCII, no `=`); an invalid name is rejected when the HbaseCluster is validated instead of + producing a broken `hbase-env.sh` ([#827]). ### Fixed @@ -67,6 +73,7 @@ [#823]: https://github.com/stackabletech/hbase-operator/pull/823 [#824]: https://github.com/stackabletech/hbase-operator/pull/824 [#825]: https://github.com/stackabletech/hbase-operator/pull/825 +[#827]: https://github.com/stackabletech/hbase-operator/pull/827 ## [26.7.0] - 2026-07-21 diff --git a/docs/modules/hbase/pages/usage-guide/overrides.adoc b/docs/modules/hbase/pages/usage-guide/overrides.adoc index 4dd653ea..50b2f87b 100644 --- a/docs/modules/hbase/pages/usage-guide/overrides.adoc +++ b/docs/modules/hbase/pages/usage-guide/overrides.adoc @@ -16,6 +16,8 @@ For a role or role group, at the same level of `config`, you can specify: `confi * `ssl-client.xml` * `security.properties` +Variable names in `hbase-env.sh` overrides must be valid environment variable names (non-empty, printable ASCII, no `=`); an invalid name is rejected when the HbaseCluster is validated. + NOTE: `hdfs-site.xml` is not listed here, the file is always taken from the referenced HDFS cluster. If you want to modify it, take a look at xref:hdfs:usage-guide/configuration-environment-overrides.adoc[HDFS configuration overrides]. @@ -52,7 +54,7 @@ restServers: ---- All override property values must be strings. -The properties are formatted and escaped correctly into the XML file, respectively inserted as is into the `hbase-env.sh` file. +The properties are formatted and escaped correctly into the XML file, respectively inserted as is into the `hbase-env.sh` file after the variable names have been validated. For a full list of configuration options we refer to the HBase https://hbase.apache.org/book.html#config.files[configuration documentation]. diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index e11d6a1d..0ebfd63b 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -367,29 +367,6 @@ spec: ) } - /// Not a test: dumps every built resource as JSON to `$RENDER_DUMP`, so the refactoring can be - /// diffed against the baseline. Run with `cargo test dump_rendered_resources -- --ignored`. - #[test] - #[ignore = "not a test: writes the rendered resources to $RENDER_DUMP for the render diff"] - fn dump_rendered_resources() { - let path = std::env::var("RENDER_DUMP").expect("RENDER_DUMP should be set"); - let resources = - build(&rich_validated_cluster(), &test_utils::cluster_info()).expect("build succeeds"); - let dump = serde_json::json!({ - "services": resources.services, - "config_maps": resources.config_maps, - "pod_disruption_budgets": resources.pod_disruption_budgets, - "stateful_sets": resources.stateful_sets, - "service_accounts": resources.service_accounts, - "role_bindings": resources.role_bindings, - }); - std::fs::write( - path, - serde_json::to_string_pretty(&dump).expect("the resources should serialise"), - ) - .expect("the dump should be written"); - } - /// The `default` role group's StatefulSet spec for `role`, found by the name the builder /// derives from the cluster. fn stateful_set_spec<'a>( diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index af564d37..bcb0c7a1 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -10,6 +10,11 @@ use stackable_operator::v2::{ config_overrides::KeyValueConfigOverrides, }; +/// The JVM's positive DNS cache TTL in seconds. +const NETWORKADDRESS_CACHE_TTL: &str = "networkaddress.cache.ttl"; +/// The JVM's negative DNS cache TTL in seconds. +const NETWORKADDRESS_CACHE_NEGATIVE_TTL: &str = "networkaddress.cache.negative.ttl"; + /// Renders `security.properties`: role-specific DNS cache TTLs plus user overrides. pub fn build( dns_cache_ttl_seconds: u32, @@ -20,11 +25,11 @@ pub fn build( // positive cache and disable the negative cache. let mut config: BTreeMap = BTreeMap::from([ ( - "networkaddress.cache.ttl".to_string(), + NETWORKADDRESS_CACHE_TTL.to_string(), dns_cache_ttl_seconds.to_string(), ), ( - "networkaddress.cache.negative.ttl".to_string(), + NETWORKADDRESS_CACHE_NEGATIVE_TTL.to_string(), "0".to_string(), ), ]); @@ -42,7 +47,7 @@ mod tests { assert_eq!( build(5, KeyValueConfigOverrides::default()) .expect("the default security.properties should render"), - "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=5\n" + format!("{NETWORKADDRESS_CACHE_NEGATIVE_TTL}=0\n{NETWORKADDRESS_CACHE_TTL}=5\n") ); } @@ -51,7 +56,7 @@ mod tests { assert_eq!( build(10, KeyValueConfigOverrides::default()) .expect("the default security.properties should render"), - "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=10\n" + format!("{NETWORKADDRESS_CACHE_NEGATIVE_TTL}=0\n{NETWORKADDRESS_CACHE_TTL}=10\n") ); } @@ -60,16 +65,16 @@ mod tests { assert_eq!( build(30, KeyValueConfigOverrides::default()) .expect("the default security.properties should render"), - "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=30\n" + format!("{NETWORKADDRESS_CACHE_NEGATIVE_TTL}=0\n{NETWORKADDRESS_CACHE_TTL}=30\n") ); } #[test] fn user_overrides_win_over_injected_defaults() { assert_eq!( - build(5, [("networkaddress.cache.ttl", "60")].into()) + build(5, [(NETWORKADDRESS_CACHE_TTL, "60")].into()) .expect("security.properties with an override should render"), - "networkaddress.cache.negative.ttl=0\nnetworkaddress.cache.ttl=60\n" + format!("{NETWORKADDRESS_CACHE_NEGATIVE_TTL}=0\n{NETWORKADDRESS_CACHE_TTL}=60\n") ); } } From 04c6d7d0370029b9ab937c774da4296248c4282f Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 20 Oct 2026 15:15:16 +0200 Subject: [PATCH 13/13] pass the graceful shutdown timeout into the shared config defaults --- .../src/controller/validate.rs | 3 ++ rust/operator-binary/src/crd/mod.rs | 31 +++++++------------ 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index f1291469..1008d75c 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -175,6 +175,7 @@ pub fn validate_cluster( masters, &HbaseConfigFragment::default_config( &HbaseRole::Master, + HbaseRole::DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT, &cluster_name, hdfs_discovery_cm_name, opa_config, @@ -225,6 +226,7 @@ pub fn validate_cluster( rest_servers, &HbaseConfigFragment::default_config( &HbaseRole::RestServer, + HbaseRole::DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT, &cluster_name, hdfs_discovery_cm_name, opa_config, @@ -570,6 +572,7 @@ spec: let role_group = role.role_groups.get("default").unwrap(); let default_config = HbaseConfigFragment::default_config( &HbaseRole::Master, + HbaseRole::DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT, &hbase.name_any(), hbase.spec.cluster_config.hdfs_config_map_name.as_ref(), None, diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 5200fb28..936f28de 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -263,15 +263,14 @@ impl Deref for HbaseRole { } impl HbaseRole { - const DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(20); - // Auto TLS certificate lifetime - const DEFAULT_MASTER_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(1); - const DEFAULT_REGION_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(1); - const DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = + pub(crate) const DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = + Duration::from_minutes_unchecked(20); + pub(crate) const DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(60); - const DEFAULT_REST_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(1); - const DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = + pub(crate) const DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(5); + /// The lifetime requested for auto-TLS certificates, the same for every role. + const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(1); } fn default_resources(role: &HbaseRole) -> ResourcesFragment { @@ -314,30 +313,22 @@ fn default_resources(role: &HbaseRole) -> ResourcesFragment, ) -> Self { - let graceful_shutdown_timeout = match role { - HbaseRole::Master => HbaseRole::DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT, - HbaseRole::RegionServer => HbaseRole::DEFAULT_REGION_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT, - HbaseRole::RestServer => HbaseRole::DEFAULT_REST_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT, - }; - let requested_secret_lifetime = match role { - HbaseRole::Master => HbaseRole::DEFAULT_MASTER_SECRET_LIFETIME, - HbaseRole::RegionServer => HbaseRole::DEFAULT_REGION_SECRET_LIFETIME, - HbaseRole::RestServer => HbaseRole::DEFAULT_REST_SECRET_LIFETIME, - }; HbaseConfigFragment { hbase_rootdir: Some(default_hbase_rootdir()), resources: default_resources(role), logging: product_logging::spec::default_logging(), affinity: get_affinity(cluster_name, role, hdfs_discovery_cm_name, opa_config), graceful_shutdown_timeout: Some(graceful_shutdown_timeout), - requested_secret_lifetime: Some(requested_secret_lifetime), + requested_secret_lifetime: Some(HbaseRole::DEFAULT_SECRET_LIFETIME), listener_class: Some( ListenerClassName::from_str(DEFAULT_LISTENER_CLASS) .expect("DEFAULT_LISTENER_CLASS is a valid listener class name"), @@ -368,7 +359,7 @@ impl RegionServerConfigFragment { ack: Some(true), cli_opts: None, }, - requested_secret_lifetime: Some(HbaseRole::DEFAULT_REGION_SECRET_LIFETIME), + requested_secret_lifetime: Some(HbaseRole::DEFAULT_SECRET_LIFETIME), listener_class: Some( ListenerClassName::from_str(DEFAULT_LISTENER_CLASS) .expect("DEFAULT_LISTENER_CLASS is a valid listener class name"),