From c7c80f62ab60c573f12466e44bb8225b4413dff9 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 27 Sep 2024 09:24:35 +0200 Subject: [PATCH 1/5] fix: Don't always return an error stating volumeMounts are colliding --- crates/stackable-operator/CHANGELOG.md | 7 +++++++ .../stackable-operator/src/builder/pod/container.rs | 13 +++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 3d27d6165..8f9adaa1f 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -8,6 +8,13 @@ All notable changes to this project will be documented in this file. ### Fixed +- Fix always returning an error stating that volumeMounts are colliding. Instead move the error + creation to the correct location within an `if` statement + +## [0.77.1] - 2024-09-26 + +### Fixed + - Fix the logback configuration for logback versions from 1.3.6/1.4.6 to 1.3.11/1.4.11 ([#874]). - BREAKING: Avoid colliding volumes and mounts by only adding volumes or mounts if they do not already exist. This makes functions such as `PodBuilder::add_volume` or `ContainerBuilder::add_volume_mount` as well as related ones fallible ([#871]). diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs index 8357493ac..8a969cd39 100644 --- a/crates/stackable-operator/src/builder/pod/container.rs +++ b/crates/stackable-operator/src/builder/pod/container.rs @@ -232,13 +232,14 @@ impl ContainerBuilder { ?existing_volume_mount, "Colliding mountPath {colliding_mount_path:?} in volumeMounts with different content" ); + + MountPathCollisionSnafu { + mount_path: &volume_mount.mount_path, + existing_volume_name: &existing_volume_mount.name, + new_volume_name: &volume_mount.name, + } + .fail()?; } - MountPathCollisionSnafu { - mount_path: &volume_mount.mount_path, - existing_volume_name: &existing_volume_mount.name, - new_volume_name: &volume_mount.name, - } - .fail()?; } else { self.volume_mounts .insert(volume_mount.mount_path.clone(), volume_mount); From 39b3e846ae35a4d1a881418f6f36ee4c9d4a96a9 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 27 Sep 2024 09:29:33 +0200 Subject: [PATCH 2/5] refactoring --- .../src/builder/pod/container.rs | 6 +++--- crates/stackable-operator/src/builder/pod/mod.rs | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs index 8a969cd39..273356313 100644 --- a/crates/stackable-operator/src/builder/pod/container.rs +++ b/crates/stackable-operator/src/builder/pod/container.rs @@ -28,11 +28,11 @@ pub enum Error { }, #[snafu(display( - "Colliding mountPath {mount_path:?} in volumeMounts with different content. \ + "Colliding mountPath {colliding_mount_path:?} in volumeMounts with different content. \ Existing volume name {existing_volume_name:?}, new volume name {new_volume_name:?}" ))] MountPathCollision { - mount_path: String, + colliding_mount_path: String, existing_volume_name: String, new_volume_name: String, }, @@ -234,7 +234,7 @@ impl ContainerBuilder { ); MountPathCollisionSnafu { - mount_path: &volume_mount.mount_path, + colliding_mount_path, existing_volume_name: &existing_volume_mount.name, new_volume_name: &volume_mount.name, } diff --git a/crates/stackable-operator/src/builder/pod/mod.rs b/crates/stackable-operator/src/builder/pod/mod.rs index fb5bda51d..cb8e18b8b 100644 --- a/crates/stackable-operator/src/builder/pod/mod.rs +++ b/crates/stackable-operator/src/builder/pod/mod.rs @@ -52,8 +52,10 @@ pub enum Error { #[snafu(display("object is missing key {key:?}"))] MissingObjectKey { key: &'static str }, - #[snafu(display("Colliding volume name {volume_name:?} in volumes with different content"))] - VolumeNameCollision { volume_name: String }, + #[snafu(display( + "Colliding volume name {colliding_volume_name:?} in volumes with different content" + ))] + VolumeNameCollision { colliding_volume_name: String }, } /// A builder to build [`Pod`] or [`PodTemplateSpec`] objects. @@ -292,16 +294,16 @@ impl PodBuilder { pub fn add_volume(&mut self, volume: Volume) -> Result<&mut Self> { if let Some(existing_volume) = self.volumes.get(&volume.name) { if existing_volume != &volume { - let colliding_name = &volume.name; + let colliding_volume_name = &volume.name; // We don't want to include the details in the error message, but instead trace them tracing::error!( - colliding_name, + colliding_volume_name, ?existing_volume, - "Colliding volume name {colliding_name:?} in volumes with different content" + "Colliding volume name {colliding_volume_name:?} in volumes with different content" ); VolumeNameCollisionSnafu { - volume_name: &volume.name, + colliding_volume_name, } .fail()?; } From 7390577d5c4806bf4e0eda8acdb4b264b1c5ef4e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 27 Sep 2024 09:30:15 +0200 Subject: [PATCH 3/5] changelog --- crates/stackable-operator/CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 8f9adaa1f..6d7ebe9f4 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,14 +4,16 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -## [0.77.0] - 2024-09-26 +## [0.77.1] - 2024-09-27 ### Fixed - Fix always returning an error stating that volumeMounts are colliding. Instead move the error - creation to the correct location within an `if` statement + creation to the correct location within an `if` statement ([#879]). + +[#879]: https://github.com/stackabletech/operator-rs/pull/879 -## [0.77.1] - 2024-09-26 +## [0.77.0] - 2024-09-26 ### Fixed From 8859ce3fc920af01cd224574ed4c8494b1c94ce8 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 27 Sep 2024 09:34:23 +0200 Subject: [PATCH 4/5] Bump crate to 0.77.1 --- Cargo.lock | 2 +- crates/stackable-operator/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 627b95b87..7923582b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2926,7 +2926,7 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.77.0" +version = "0.77.1" dependencies = [ "chrono", "clap", diff --git a/crates/stackable-operator/Cargo.toml b/crates/stackable-operator/Cargo.toml index 5a19c8ec8..4619582b0 100644 --- a/crates/stackable-operator/Cargo.toml +++ b/crates/stackable-operator/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stackable-operator" description = "Stackable Operator Framework" -version = "0.77.0" +version = "0.77.1" authors.workspace = true license.workspace = true edition.workspace = true From 1c98e96ce4a21a971f7b5bd9a3eba44e43d9e09a Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 27 Sep 2024 09:45:43 +0200 Subject: [PATCH 5/5] remove details from tracing error message --- crates/stackable-operator/src/builder/pod/container.rs | 2 +- crates/stackable-operator/src/builder/pod/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs index 273356313..49b847b13 100644 --- a/crates/stackable-operator/src/builder/pod/container.rs +++ b/crates/stackable-operator/src/builder/pod/container.rs @@ -230,7 +230,7 @@ impl ContainerBuilder { tracing::error!( colliding_mount_path, ?existing_volume_mount, - "Colliding mountPath {colliding_mount_path:?} in volumeMounts with different content" + "Colliding mountPath in volumeMounts with different content" ); MountPathCollisionSnafu { diff --git a/crates/stackable-operator/src/builder/pod/mod.rs b/crates/stackable-operator/src/builder/pod/mod.rs index cb8e18b8b..54b34bd3a 100644 --- a/crates/stackable-operator/src/builder/pod/mod.rs +++ b/crates/stackable-operator/src/builder/pod/mod.rs @@ -299,7 +299,7 @@ impl PodBuilder { tracing::error!( colliding_volume_name, ?existing_volume, - "Colliding volume name {colliding_volume_name:?} in volumes with different content" + "Colliding volume name in volumes with different content" ); VolumeNameCollisionSnafu {