Skip to content

Commit b8d6046

Browse files
maltesanderclaude
andcommitted
refactor: Split config_map into per-file property builders
Restructure zk_controller/build to mirror trino-operator and hdfs-operator: - build/properties/mod.rs holds the ConfigFileName enum and shared helpers (apply_overrides, into_optional_values). - build/properties/zoo_cfg.rs and security_properties.rs each render one file; metrics_http_port moves alongside the zoo.cfg builder. - build/properties/logging.rs takes over the logback/log4j and vector.yaml rendering, replacing the top-level product_logging.rs module (now removed). - build/config_map.rs becomes a thin orchestrator that assembles the data map via ConfigFileName and the property/logging builders, then .data(data).build(). Pure restructuring: no behavior change. The generated CRD is unchanged and all 16 unit tests (which render the full ConfigMap) still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c2a2bab commit b8d6046

8 files changed

Lines changed: 312 additions & 198 deletions

File tree

rust/operator-binary/src/main.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ mod discovery;
4646
mod framework;
4747
mod listener;
4848
mod operations;
49-
mod product_logging;
5049
mod service;
5150
mod utils;
5251
mod webhooks;

rust/operator-binary/src/zk_controller.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ pub async fn reconcile_zk(
358358
for (rolegroup_name, rolegroup_config) in server_role_group_configs {
359359
let rolegroup = zk.server_rolegroup_ref(rolegroup_name);
360360
let merged_config = &rolegroup_config.config;
361-
let metrics_port = build::config_map::metrics_http_port(&validated_cluster, rolegroup_config);
361+
let metrics_port =
362+
build::properties::zoo_cfg::metrics_http_port(&validated_cluster, rolegroup_config);
362363

363364
let rg_headless_service =
364365
build_server_rolegroup_headless_service(zk, &rolegroup, resolved_product_image)

rust/operator-binary/src/zk_controller/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@
66
//! configuration.
77
88
pub mod config_map;
9+
pub mod properties;
Lines changed: 61 additions & 174 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,28 @@
1-
//! Builds the rolegroup `ConfigMap` (`zoo.cfg` + `security.properties`) from the
2-
//! [`ValidatedCluster`], without reaching into the
3-
//! [`v1alpha1::ZookeeperCluster`] except for the owner reference and metadata.
1+
//! Assembles the per-rolegroup `ConfigMap` from the [`ValidatedCluster`],
2+
//! without reaching into the [`v1alpha1::ZookeeperCluster`] except for the owner
3+
//! reference and object metadata.
44
//!
5-
//! The operator-injected defaults seeded here used to live in
6-
//! `deploy/config-spec/properties.yaml` and were injected by the `product-config`
7-
//! crate during validation. They are reproduced here so the rendered config is
8-
//! byte-identical to the previous implementation.
5+
//! The individual files are rendered by the [`properties`](super::properties)
6+
//! submodules; this module only orchestrates them into the ConfigMap.
97
108
use std::collections::BTreeMap;
119

1210
use snafu::{ResultExt, Snafu};
1311
use stackable_operator::{
14-
builder::{
15-
configmap::ConfigMapBuilder,
16-
meta::ObjectMetaBuilder,
17-
},
18-
config_overrides::KeyValueOverridesProvider,
12+
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
1913
k8s_openapi::api::core::v1::ConfigMap,
2014
role_utils::RoleGroupRef,
2115
};
2216

2317
use crate::{
24-
crd::{
25-
JVM_SECURITY_PROPERTIES_FILE, METRICS_PROVIDER_HTTP_PORT, METRICS_PROVIDER_HTTP_PORT_KEY,
26-
STACKABLE_DATA_DIR, ZOOKEEPER_PROPERTIES_FILE, ZookeeperRole,
27-
security::ZookeeperSecurity,
28-
v1alpha1::{self, ZookeeperConfig},
29-
},
18+
crd::{ZookeeperRole, v1alpha1},
3019
framework::writer::{PropertiesWriterError, to_java_properties_string},
31-
product_logging::extend_role_group_config_map,
3220
utils::build_recommended_labels,
3321
zk_controller::{
3422
ZK_CONTROLLER_NAME,
23+
build::properties::{
24+
ConfigFileName, into_optional_values, logging, security_properties, zoo_cfg,
25+
},
3526
validate::{ValidatedCluster, ZookeeperRoleGroupConfig},
3627
},
3728
};
@@ -57,7 +48,7 @@ pub enum Error {
5748

5849
#[snafu(display("failed to add the logging configuration to the ConfigMap [{cm_name}]"))]
5950
InvalidLoggingConfig {
60-
source: crate::product_logging::Error,
51+
source: logging::Error,
6152
cm_name: String,
6253
},
6354

@@ -81,14 +72,42 @@ pub fn build_server_rolegroup_config_map(
8172
rolegroup_config: &ZookeeperRoleGroupConfig,
8273
owner: &v1alpha1::ZookeeperCluster,
8374
) -> Result<ConfigMap> {
84-
let zoo_cfg = build_zoo_cfg(cluster, rolegroup_config);
85-
let security_properties = build_security_properties(rolegroup_config);
75+
let mut data: BTreeMap<String, String> = BTreeMap::new();
76+
77+
// zoo.cfg
78+
data.insert(
79+
ConfigFileName::ZooCfg.to_string(),
80+
render(
81+
ConfigFileName::ZooCfg,
82+
zoo_cfg::build(cluster, rolegroup_config),
83+
rolegroup_ref,
84+
)?,
85+
);
8686

87-
let zoo_cfg_data = into_optional_values(zoo_cfg);
88-
let security_properties_data = into_optional_values(security_properties);
87+
// security.properties
88+
data.insert(
89+
ConfigFileName::SecurityProperties.to_string(),
90+
render(
91+
ConfigFileName::SecurityProperties,
92+
security_properties::build(rolegroup_config),
93+
rolegroup_ref,
94+
)?,
95+
);
8996

90-
let mut cm_builder = ConfigMapBuilder::new();
91-
cm_builder
97+
// logback.xml / log4j.properties and vector.yaml
98+
data.extend(
99+
logging::build(
100+
owner,
101+
role.clone(),
102+
rolegroup_ref,
103+
&cluster.image.product_version,
104+
)
105+
.context(InvalidLoggingConfigSnafu {
106+
cm_name: rolegroup_ref.object_name(),
107+
})?,
108+
);
109+
110+
ConfigMapBuilder::new()
92111
.metadata(
93112
ObjectMetaBuilder::new()
94113
.name_and_namespace(owner)
@@ -105,155 +124,23 @@ pub fn build_server_rolegroup_config_map(
105124
.context(ObjectMetaSnafu)?
106125
.build(),
107126
)
108-
.add_data(
109-
JVM_SECURITY_PROPERTIES_FILE,
110-
to_java_properties_string(security_properties_data.iter()).with_context(|_| {
111-
SerializePropertiesSnafu {
112-
file: JVM_SECURITY_PROPERTIES_FILE.to_string(),
113-
rolegroup: rolegroup_ref.clone(),
114-
}
115-
})?,
116-
)
117-
.add_data(
118-
ZOOKEEPER_PROPERTIES_FILE,
119-
to_java_properties_string(zoo_cfg_data.iter()).with_context(|_| {
120-
SerializePropertiesSnafu {
121-
file: ZOOKEEPER_PROPERTIES_FILE.to_string(),
122-
rolegroup: rolegroup_ref.clone(),
123-
}
124-
})?,
125-
);
126-
127-
extend_role_group_config_map(
128-
owner,
129-
role.clone(),
130-
rolegroup_ref,
131-
&mut cm_builder,
132-
&cluster.image.product_version,
133-
)
134-
.context(InvalidLoggingConfigSnafu {
135-
cm_name: rolegroup_ref.object_name(),
136-
})?;
137-
138-
cm_builder.build().context(BuildConfigMapSnafu {
139-
rolegroup: rolegroup_ref.clone(),
140-
})
127+
.data(data)
128+
.build()
129+
.context(BuildConfigMapSnafu {
130+
rolegroup: rolegroup_ref.clone(),
131+
})
141132
}
142133

143-
/// Builds the `zoo.cfg` contents for a role group.
144-
///
145-
/// Precedence (lowest to highest):
146-
/// 1. `server.<myid>` quorum entries (precomputed in validate)
147-
/// 2. operator-injected defaults (formerly `product-config` `properties.yaml`)
148-
/// 3. TLS / quorum settings from [`ZookeeperSecurity`]
149-
/// 4. user-set merged config (`initLimit` / `syncLimit` / `tickTime`)
150-
/// 5. `configOverrides` for `zoo.cfg`
151-
pub fn build_zoo_cfg(
152-
cluster: &ValidatedCluster,
153-
rolegroup_config: &ZookeeperRoleGroupConfig,
154-
) -> BTreeMap<String, String> {
155-
let security = &cluster.cluster_config.zookeeper_security;
156-
let config = &rolegroup_config.config;
157-
158-
let mut zoo_cfg = cluster.cluster_config.server_addresses.clone();
159-
160-
// Operator-injected defaults (former properties.yaml recommended/default values
161-
// and the `Configuration::compute_files` output).
162-
zoo_cfg.insert("admin.serverPort".to_string(), "8080".to_string());
163-
zoo_cfg.insert(
164-
ZookeeperSecurity::CLIENT_PORT_NAME.to_string(),
165-
security.client_port().to_string(),
166-
);
167-
zoo_cfg.insert(
168-
ZookeeperConfig::DATA_DIR.to_string(),
169-
STACKABLE_DATA_DIR.to_string(),
170-
);
171-
zoo_cfg.insert(ZookeeperConfig::INIT_LIMIT.to_string(), "5".to_string());
172-
zoo_cfg.insert(ZookeeperConfig::SYNC_LIMIT.to_string(), "2".to_string());
173-
zoo_cfg.insert(ZookeeperConfig::TICK_TIME.to_string(), "3000".to_string());
174-
zoo_cfg.insert(
175-
"metricsProvider.className".to_string(),
176-
"org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider".to_string(),
177-
);
178-
zoo_cfg.insert(
179-
METRICS_PROVIDER_HTTP_PORT_KEY.to_string(),
180-
METRICS_PROVIDER_HTTP_PORT.to_string(),
181-
);
182-
183-
// TLS / quorum settings.
184-
zoo_cfg.extend(security.config_settings());
185-
186-
// User-set merged config overrides the seeded defaults above.
187-
if let Some(init_limit) = config.init_limit {
188-
zoo_cfg.insert(ZookeeperConfig::INIT_LIMIT.to_string(), init_limit.to_string());
189-
}
190-
if let Some(sync_limit) = config.sync_limit {
191-
zoo_cfg.insert(ZookeeperConfig::SYNC_LIMIT.to_string(), sync_limit.to_string());
192-
}
193-
if let Some(tick_time) = config.tick_time {
194-
zoo_cfg.insert(ZookeeperConfig::TICK_TIME.to_string(), tick_time.to_string());
195-
}
196-
197-
// configOverrides go last so they win.
198-
apply_overrides(
199-
&mut zoo_cfg,
200-
rolegroup_config
201-
.config_overrides
202-
.get_key_value_overrides(ZOOKEEPER_PROPERTIES_FILE),
203-
);
204-
205-
zoo_cfg
206-
}
207-
208-
/// Builds the `security.properties` contents for a role group.
209-
pub fn build_security_properties(
210-
rolegroup_config: &ZookeeperRoleGroupConfig,
211-
) -> BTreeMap<String, String> {
212-
let mut security_properties = BTreeMap::new();
213-
214-
// Operator-injected defaults (former properties.yaml recommended values).
215-
security_properties.insert("networkaddress.cache.ttl".to_string(), "5".to_string());
216-
security_properties.insert(
217-
"networkaddress.cache.negative.ttl".to_string(),
218-
"0".to_string(),
219-
);
220-
221-
apply_overrides(
222-
&mut security_properties,
223-
rolegroup_config
224-
.config_overrides
225-
.get_key_value_overrides(JVM_SECURITY_PROPERTIES_FILE),
226-
);
227-
228-
security_properties
229-
}
230-
231-
/// Resolves the metrics HTTP port for a role group, honoring a
232-
/// `metricsProvider.httpPort` `configOverride` if present.
233-
pub fn metrics_http_port(
234-
cluster: &ValidatedCluster,
235-
rolegroup_config: &ZookeeperRoleGroupConfig,
236-
) -> u16 {
237-
build_zoo_cfg(cluster, rolegroup_config)
238-
.get(METRICS_PROVIDER_HTTP_PORT_KEY)
239-
.and_then(|port| port.parse().ok())
240-
.unwrap_or(METRICS_PROVIDER_HTTP_PORT)
241-
}
242-
243-
/// Applies key-value overrides: `Some(value)` sets the key, `None` removes it.
244-
fn apply_overrides(target: &mut BTreeMap<String, String>, overrides: BTreeMap<String, Option<String>>) {
245-
for (key, value) in overrides {
246-
match value {
247-
Some(value) => {
248-
target.insert(key, value);
249-
}
250-
None => {
251-
target.remove(&key);
252-
}
134+
/// Serializes a property map to its Java-properties on-wire representation.
135+
fn render(
136+
file: ConfigFileName,
137+
properties: BTreeMap<String, String>,
138+
rolegroup_ref: &RoleGroupRef<v1alpha1::ZookeeperCluster>,
139+
) -> Result<String> {
140+
to_java_properties_string(into_optional_values(properties).iter()).with_context(|_| {
141+
SerializePropertiesSnafu {
142+
file: file.to_string(),
143+
rolegroup: rolegroup_ref.clone(),
253144
}
254-
}
255-
}
256-
257-
fn into_optional_values(map: BTreeMap<String, String>) -> BTreeMap<String, Option<String>> {
258-
map.into_iter().map(|(k, v)| (k, Some(v))).collect()
145+
})
259146
}

0 commit comments

Comments
 (0)