Skip to content

Commit 3ca11ac

Browse files
maltesanderclaude
andcommitted
refactor: add typed ClusterName to ValidatedCluster
Adds a top-level name: ClusterName field to ValidatedCluster (mirroring trino-operator), constructed and validated in the validate step. The per-file builders and build_rolegroup_config_map now take the whole &ValidatedCluster, reading cluster.name and cluster.cluster_config. product_version is intentionally not added: unlike Trino (integer versions parsed to u16), HDFS uses semver product versions, so the image keeps carrying the version string. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 09eb912 commit 3ca11ac

6 files changed

Lines changed: 35 additions & 22 deletions

File tree

rust/operator-binary/src/controller/build/config_map.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ pub fn build_rolegroup_config_map(
8585
let cluster_config = &cluster.cluster_config;
8686

8787
let hdfs_site_xml = hdfs_site::build(
88-
cluster_config,
88+
cluster,
8989
cluster_info,
9090
merged_config,
9191
namenode_podrefs,
9292
journalnode_podrefs,
9393
config_overrides.hdfs_site_xml.clone(),
9494
);
9595
let core_site_xml = core_site::build(
96-
cluster_config,
96+
cluster,
9797
role,
9898
cluster_info,
9999
config_overrides.core_site_xml.clone(),

rust/operator-binary/src/controller/build/properties/core_site.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
config::CoreSiteConfigBuilder,
1212
controller::build::properties::resolved_overrides,
1313
crd::HdfsNodeRole,
14-
hdfs_controller::ValidatedClusterConfig,
14+
hdfs_controller::ValidatedCluster,
1515
security::kerberos::{self, KerberosConfig},
1616
};
1717

@@ -24,20 +24,21 @@ pub enum Error {
2424
/// Renders `core-site.xml`: operator defaults + kerberos/OPA security config,
2525
/// with user `configOverrides` applied last.
2626
pub fn build(
27-
cluster_config: &ValidatedClusterConfig,
27+
cluster: &ValidatedCluster,
2828
role: HdfsNodeRole,
2929
cluster_info: &KubernetesClusterInfo,
3030
overrides: KeyValueConfigOverrides,
3131
) -> Result<String, Error> {
32+
let cluster_config = &cluster.cluster_config;
3233
let kerberos = KerberosConfig {
33-
cluster_name: &cluster_config.name,
34+
cluster_name: cluster.name.as_ref(),
3435
cluster_namespace: cluster_config.namespace.as_deref(),
3536
authentication_enabled: cluster_config.authentication_enabled,
3637
kerberos_enabled: cluster_config.kerberos_enabled,
3738
authorization_enabled: cluster_config.authorization_enabled,
3839
};
3940

40-
let mut core_site = CoreSiteConfigBuilder::new(cluster_config.name.clone());
41+
let mut core_site = CoreSiteConfigBuilder::new(cluster.name.as_ref().to_owned());
4142
core_site
4243
.fs_default_fs()
4344
.ha_zookeeper_quorum()
@@ -67,13 +68,13 @@ pub fn build(
6768
mod tests {
6869
use super::*;
6970
use crate::controller::build::properties::test_support::{
70-
cluster_info, config_overrides, validated_cluster_config,
71+
cluster_info, config_overrides, validated_cluster,
7172
};
7273

7374
#[test]
7475
fn renders_operator_defaults() {
7576
let xml = build(
76-
&validated_cluster_config(),
77+
&validated_cluster(),
7778
HdfsNodeRole::Name,
7879
&cluster_info(),
7980
config_overrides(&[]),
@@ -98,7 +99,7 @@ mod tests {
9899
#[test]
99100
fn user_overrides_win_over_defaults() {
100101
let xml = build(
101-
&validated_cluster_config(),
102+
&validated_cluster(),
102103
HdfsNodeRole::Name,
103104
&cluster_info(),
104105
config_overrides(&[("io.file.buffer.size", "65536")]),

rust/operator-binary/src/controller/build/properties/hdfs_site.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,20 @@ use crate::{
1010
config::HdfsSiteConfigBuilder,
1111
controller::build::properties::resolved_overrides,
1212
crd::{AnyNodeConfig, HdfsPodRef},
13-
hdfs_controller::ValidatedClusterConfig,
13+
hdfs_controller::ValidatedCluster,
1414
};
1515

1616
/// Renders `hdfs-site.xml`: operator defaults, HA wiring derived from the pod
1717
/// refs, kerberos/OPA security config, with user `configOverrides` applied last.
1818
pub fn build(
19-
cluster_config: &ValidatedClusterConfig,
19+
cluster: &ValidatedCluster,
2020
cluster_info: &KubernetesClusterInfo,
2121
merged_config: &AnyNodeConfig,
2222
namenode_podrefs: &[HdfsPodRef],
2323
journalnode_podrefs: &[HdfsPodRef],
2424
overrides: KeyValueConfigOverrides,
2525
) -> String {
26+
let cluster_config = &cluster.cluster_config;
2627
// IMPORTANT: these folders must be under the volume mount point, otherwise they will not
2728
// be formatted by the namenode, or used by the other services.
2829
// See also: https://github.com/apache-spark-on-k8s/kubernetes-HDFS/commit/aef9586ecc8551ca0f0a468c3b917d8c38f494a0
@@ -34,7 +35,7 @@ pub fn build(
3435
// https://hadoop.apache.org/docs/stable/hadoop-project-dist/hadoop-hdfs/HDFSHighAvailabilityWithNFS.html
3536
// This caused a deadlock with no namenode becoming active during a startup after
3637
// HDFS was completely down for a while.
37-
let mut hdfs_site = HdfsSiteConfigBuilder::new(cluster_config.name.clone());
38+
let mut hdfs_site = HdfsSiteConfigBuilder::new(cluster.name.as_ref().to_owned());
3839
hdfs_site
3940
.dfs_namenode_name_dir()
4041
.dfs_datanode_data_dir(
@@ -114,7 +115,7 @@ mod tests {
114115
use super::*;
115116
use crate::{
116117
controller::build::properties::test_support::{
117-
cluster_info, config_overrides, minimal_hdfs, validated_cluster_config,
118+
cluster_info, config_overrides, minimal_hdfs, validated_cluster,
118119
},
119120
crd::{HdfsNodeRole, v1alpha1},
120121
};
@@ -129,7 +130,7 @@ mod tests {
129130
fn renders_operator_defaults() {
130131
let merged = namenode_merged_config(&minimal_hdfs());
131132
let xml = build(
132-
&validated_cluster_config(),
133+
&validated_cluster(),
133134
&cluster_info(),
134135
&merged,
135136
&[],
@@ -150,7 +151,7 @@ mod tests {
150151
fn user_overrides_win_over_defaults() {
151152
let merged = namenode_merged_config(&minimal_hdfs());
152153
let xml = build(
153-
&validated_cluster_config(),
154+
&validated_cluster(),
154155
&cluster_info(),
155156
&merged,
156157
&[],

rust/operator-binary/src/controller/build/properties/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ pub(crate) mod test_support {
7373
v2::config_overrides::KeyValueConfigOverrides,
7474
};
7575

76-
use crate::{crd::v1alpha1, hdfs_controller::ValidatedClusterConfig};
76+
use crate::{
77+
controller::validate::validate_cluster, crd::v1alpha1, hdfs_controller::ValidatedCluster,
78+
};
7779

7880
/// Builds a [`KeyValueConfigOverrides`] from `(key, value)` pairs for tests.
7981
pub fn config_overrides(pairs: &[(&str, &str)]) -> KeyValueConfigOverrides {
@@ -121,7 +123,8 @@ spec:
121123
}
122124
}
123125

124-
pub fn validated_cluster_config() -> ValidatedClusterConfig {
125-
ValidatedClusterConfig::resolve(&minimal_hdfs(), None)
126+
pub fn validated_cluster() -> ValidatedCluster {
127+
validate_cluster(&minimal_hdfs(), "oci.example.org", None)
128+
.expect("validate should succeed for the minimal fixture")
126129
}
127130
}

rust/operator-binary/src/controller/validate.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77
//! validated via [`HdfsNodeRole::merged_config`], and the per-file
88
//! `configOverrides` / `envOverrides` are merged here (role group wins).
99
10-
use std::collections::BTreeMap;
10+
use std::{collections::BTreeMap, str::FromStr};
1111

1212
use snafu::{ResultExt, Snafu};
1313
use stackable_operator::{
1414
commons::product_image_selection,
1515
config::merge::Merge,
16+
kube::ResourceExt,
1617
role_utils::{GenericRoleConfig, JavaCommonConfig, Role},
18+
v2::types::operator::ClusterName,
1719
};
1820
use strum::IntoEnumIterator;
1921

@@ -33,6 +35,11 @@ pub enum Error {
3335
source: product_image_selection::Error,
3436
},
3537

38+
#[snafu(display("invalid cluster name"))]
39+
InvalidClusterName {
40+
source: stackable_operator::v2::macros::attributed_string_type::Error,
41+
},
42+
3643
#[snafu(display("failed to resolve and merge config for role and role group"))]
3744
FailedToResolveConfig { source: crate::crd::Error },
3845
}
@@ -97,6 +104,7 @@ pub fn validate_cluster(
97104
}
98105

99106
Ok(ValidatedCluster {
107+
name: ClusterName::from_str(&hdfs.name_any()).context(InvalidClusterNameSnafu)?,
100108
image: resolved_product_image,
101109
cluster_config: ValidatedClusterConfig::resolve(hdfs, hdfs_opa_config),
102110
role_groups,

rust/operator-binary/src/hdfs_controller.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use stackable_operator::{
3838
rollout::check_statefulset_rollout_complete,
3939
},
4040
utils::cluster_info::KubernetesClusterInfo,
41+
v2::types::operator::ClusterName,
4142
};
4243
use strum::{EnumDiscriminants, IntoEnumIterator, IntoStaticStr};
4344

@@ -70,6 +71,8 @@ pub const CONTAINER_IMAGE_BASE_NAME: &str = "hadoop";
7071
/// the controller.
7172
#[derive(Clone, Debug)]
7273
pub struct ValidatedCluster {
74+
/// The logical (and Kubernetes object) name of the cluster.
75+
pub name: ClusterName,
7376
pub image: ResolvedProductImage,
7477
pub cluster_config: ValidatedClusterConfig,
7578
pub role_groups: BTreeMap<HdfsNodeRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
@@ -81,8 +84,6 @@ pub struct ValidatedCluster {
8184
/// the same `HdfsCluster` predicates used previously, just resolved up-front.
8285
#[derive(Clone, Debug)]
8386
pub struct ValidatedClusterConfig {
84-
/// The logical (and Kubernetes object) name of the cluster.
85-
pub name: String,
8687
/// The cluster namespace, used to build kerberos principals.
8788
pub namespace: Option<String>,
8889
pub dfs_replication: u8,
@@ -100,7 +101,6 @@ impl ValidatedClusterConfig {
100101
authorization: Option<HdfsOpaConfig>,
101102
) -> ValidatedClusterConfig {
102103
ValidatedClusterConfig {
103-
name: hdfs.name_any(),
104104
namespace: hdfs.namespace(),
105105
dfs_replication: hdfs.spec.cluster_config.dfs_replication,
106106
https_enabled: hdfs.has_https_enabled(),

0 commit comments

Comments
 (0)