Skip to content

Commit 97325eb

Browse files
authored
fix: make container name accessor infallible (#874)
* fix: make container name accessor infallible * changelog * review feedback
1 parent 6083781 commit 97325eb

3 files changed

Lines changed: 32 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ All notable changes to this project will be documented in this file.
2525
StatefulSets created by older operator versions cannot be updated in place: after the
2626
operator upgrade, delete each broker, coordinator and router StatefulSet so that the operator
2727
immediately recreates it with the new labels ([#865]).
28-
- Make operations infallible where dependent on static inputs ([#869]).
28+
- Make operations infallible where dependent on static inputs ([#869], [#874]).
2929

3030
### Fixed
3131

@@ -43,6 +43,7 @@ All notable changes to this project will be documented in this file.
4343
[#865]: https://github.com/stackabletech/druid-operator/pull/865
4444
[#867]: https://github.com/stackabletech/druid-operator/pull/867
4545
[#869]: https://github.com/stackabletech/druid-operator/pull/869
46+
[#874]: https://github.com/stackabletech/druid-operator/pull/874
4647

4748
## [26.7.0] - 2026-07-21
4849

‎rust/operator-binary/src/controller/build/resource/statefulset.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,9 @@ pub fn build_rolegroup_statefulset(
130130
let druid_tls_security = &cluster.cluster_config.druid_tls_security;
131131
let druid_auth_config = &cluster.cluster_config.druid_auth_config;
132132
// prepare container builder
133-
let prepare_container_name = Container::Prepare.to_container_name();
134-
let mut cb_prepare = new_container_builder(&prepare_container_name);
133+
let mut cb_prepare = new_container_builder(Container::Prepare.name());
135134
// druid container builder
136-
let druid_container_name = Container::Druid.to_container_name();
137-
let mut cb_druid = new_container_builder(&druid_container_name);
135+
let mut cb_druid = new_container_builder(Container::Druid.name());
138136
// init pod builder
139137
let mut pb = PodBuilder::new();
140138
pb.affinity(&merged_rolegroup_config.affinity);
@@ -158,7 +156,7 @@ pub fn build_rolegroup_statefulset(
158156
// otherwise the output of the following commands will not be captured!
159157
prepare_container_commands.push(product_logging::framework::capture_shell_output(
160158
STACKABLE_LOG_DIR,
161-
prepare_container_name.as_ref(),
159+
Container::Prepare.name().as_ref(),
162160
log_config,
163161
));
164162
}
@@ -362,7 +360,7 @@ pub fn build_rolegroup_statefulset(
362360
// config volume; the validated aggregator address comes from the up-front `ValidatedLogging`.
363361
if let Some(vector_log_config) = &merged_rolegroup_config.logging.vector_container {
364362
pb.add_container(vector_container(
365-
&Container::Vector.to_container_name(),
363+
Container::Vector.name(),
366364
resolved_product_image,
367365
vector_log_config,
368366
&resource_names,

‎rust/operator-binary/src/crd/mod.rs‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -497,11 +497,20 @@ pub enum Container {
497497
Vector,
498498
}
499499

500+
// Typed container names. They must match the strum `Display` (kebab-case) of the variants above,
501+
// which is pinned by a unit test.
502+
constant!(DRUID_CONTAINER_NAME: ContainerName = "druid");
503+
constant!(PREPARE_CONTAINER_NAME: ContainerName = "prepare");
504+
constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector");
505+
500506
impl Container {
501-
/// Returns the typed container name for this container.
502-
pub fn to_container_name(&self) -> ContainerName {
503-
ContainerName::from_str(&self.to_string())
504-
.expect("a Container always serializes to a valid container name")
507+
/// The typed container name of this variant.
508+
pub fn name(&self) -> &'static ContainerName {
509+
match self {
510+
Container::Druid => &DRUID_CONTAINER_NAME,
511+
Container::Prepare => &PREPARE_CONTAINER_NAME,
512+
Container::Vector => &VECTOR_CONTAINER_NAME,
513+
}
505514
}
506515
}
507516

@@ -901,10 +910,20 @@ pub fn build_string_list(strings: &[String]) -> String {
901910
#[cfg(test)]
902911
mod tests {
903912
use stackable_operator::versioned::test_utils::RoundtripTestData;
913+
use strum::IntoEnumIterator;
904914

905915
use super::*;
906916
use crate::crd::v1alpha1;
907917

918+
/// The typed container names returned by `name` must agree with the strum `Display`
919+
/// of `Container`, which the logging configuration still uses as the per-container key.
920+
#[test]
921+
fn container_names_match_display() {
922+
for container in Container::iter() {
923+
assert_eq!(container.name().to_string(), container.to_string());
924+
}
925+
}
926+
908927
#[test]
909928
fn test_constants() {
910929
// Test that dereferencing the constants does not panic.
@@ -916,6 +935,9 @@ mod tests {
916935
let _ = *COOKIE_PASSPHRASE_ENV;
917936
let _ = *COOKIE_PASSPHRASE_SECRET_KEY;
918937
let _ = *DRUID_DEFAULT_LISTENER_CLASS;
938+
let _ = *DRUID_CONTAINER_NAME;
939+
let _ = *PREPARE_CONTAINER_NAME;
940+
let _ = *VECTOR_CONTAINER_NAME;
919941
}
920942

921943
impl RoundtripTestData for v1alpha1::DruidClusterSpec {

0 commit comments

Comments
 (0)