From e88532fe64f2bb36837e88d3e7c2b24f928ea7e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Sun, 23 Oct 2022 15:43:13 +0200 Subject: [PATCH 01/14] Addition of Affinity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sönke Liebau --- src/builder/pod/mod.rs | 9 ++++++++- src/product_config_utils.rs | 15 +++++++++++++++ src/role_utils.rs | 3 +++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index ab4687251..516550db9 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -8,7 +8,7 @@ use crate::error::{Error, OperatorResult}; use k8s_openapi::{ api::core::v1::{ Affinity, Container, LocalObjectReference, NodeAffinity, NodeSelector, - NodeSelectorRequirement, NodeSelectorTerm, Pod, PodAffinity, PodCondition, + NodeSelectorRequirement, NodeSelectorTerm, Pod, PodAffinity, PodAntiAffinity, PodCondition, PodSecurityContext, PodSpec, PodStatus, PodTemplateSpec, Toleration, Volume, }, apimachinery::pkg::apis::meta::v1::{LabelSelector, LabelSelectorRequirement, ObjectMeta}, @@ -26,6 +26,7 @@ pub struct PodBuilder { node_name: Option, node_selector: Option, pod_affinity: Option, + pod_anti_affinity: Option, status: Option, security_context: Option, tolerations: Option>, @@ -84,6 +85,11 @@ impl PodBuilder { self } + pub fn pod_anti_affinity(&mut self, anti_affinity: PodAntiAffinity) -> &mut Self { + self.pod_anti_affinity = Some(anti_affinity); + self + } + pub fn node_selector(&mut self, node_selector: LabelSelector) -> &mut Self { self.node_selector = Some(node_selector); self @@ -210,6 +216,7 @@ impl PodBuilder { .map(|node_affinity| Affinity { node_affinity: Some(node_affinity), pod_affinity: self.pod_affinity.clone(), + pod_anti_affinity: self.pod_anti_affinity.clone(), ..Affinity::default() }) .or_else(|| { diff --git a/src/product_config_utils.rs b/src/product_config_utils.rs index 1bdf0e39d..5da013117 100644 --- a/src/product_config_utils.rs +++ b/src/product_config_utils.rs @@ -628,6 +628,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (true, true, true, false) => Role { @@ -642,6 +643,7 @@ mod tests { config: build_common_config( build_test_config(GROUP_CONFIG, GROUP_ENV, GROUP_CLI), None, None, None), selector: None, + affinity: None, }}, }, (true, true, false, true) => Role { @@ -659,6 +661,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (true, true, false, false) => Role { @@ -676,6 +679,7 @@ mod tests { None, None), selector: None, + affinity: None, }}, }, (true, false, true, true) => Role { @@ -693,6 +697,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (true, false, true, false) => Role { @@ -706,6 +711,7 @@ mod tests { replicas: Some(1), config: CommonConfiguration::default(), selector: None, + affinity: None, }}, }, (true, false, false, true) => Role { @@ -724,6 +730,7 @@ mod tests { build_cli_override(GROUP_CLI_OVERRIDE) ), selector: None, + affinity: None, }}, }, (true, false, false, false) => Role { @@ -737,6 +744,7 @@ mod tests { replicas: Some(1), config: CommonConfiguration::default(), selector: None, + affinity: None, }}, }, (false, true, true, true) => Role { @@ -754,6 +762,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (false, true, true, false) => Role { @@ -783,6 +792,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (false, true, false, false) => Role { @@ -795,6 +805,7 @@ mod tests { None, None), selector: None, + affinity: None, }}, }, (false, false, true, true) => Role { @@ -812,6 +823,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (false, false, true, false) => Role { @@ -825,6 +837,7 @@ mod tests { replicas: Some(1), config: CommonConfiguration::default(), selector: None, + affinity: None, }}, }, (false, false, false, true) => Role { @@ -837,6 +850,7 @@ mod tests { build_env_override(GROUP_ENV_OVERRIDE), build_cli_override(GROUP_CLI_OVERRIDE)), selector: None, + affinity: None, }}, }, (false, false, false, false) => Role { @@ -845,6 +859,7 @@ mod tests { replicas: Some(1), config: CommonConfiguration::default(), selector: None, + affinity: None, }}, }, } diff --git a/src/role_utils.rs b/src/role_utils.rs index 3d4796b9e..3fccb8e4d 100644 --- a/src/role_utils.rs +++ b/src/role_utils.rs @@ -87,6 +87,7 @@ use std::{ use crate::product_config_utils::Configuration; use derivative::Derivative; +use k8s_openapi::api::core::v1::Affinity; use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector; use kube::{runtime::reflector::ObjectRef, Resource}; use schemars::JsonSchema; @@ -159,6 +160,7 @@ impl Role { }, replicas: group.replicas, selector: group.selector, + affinity: group.affinity, }, ) }) @@ -177,6 +179,7 @@ pub struct RoleGroup { pub config: CommonConfiguration, pub replicas: Option, pub selector: Option, + pub affinity: Option, } /// A reference to a named role group of a given cluster object From a0314178ad28143d628aba146e5bc61856a0cc3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Sun, 23 Oct 2022 23:50:12 +0200 Subject: [PATCH 02/14] Added maybe_ variants of builder methods that take an Option<_> to make using them from some contexts easier. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sönke Liebau --- src/builder/pod/mod.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 516550db9..9587eec16 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -90,11 +90,26 @@ impl PodBuilder { self } + pub fn maybe_pod_affinity(&mut self, affinity: Option) -> &mut Self { + self.pod_affinity = affinity; + self + } + + pub fn maybe_pod_anti_affinity(&mut self, anti_affinity: Option) -> &mut Self { + self.pod_anti_affinity = anti_affinity; + self + } + pub fn node_selector(&mut self, node_selector: LabelSelector) -> &mut Self { self.node_selector = Some(node_selector); self } + pub fn maybe_node_selector(&mut self, node_selector: Option) -> &mut Self { + self.node_selector = node_selector; + self + } + pub fn phase(&mut self, phase: &str) -> &mut Self { let mut status = self.status.get_or_insert_with(PodStatus::default); status.phase = Some(phase.to_string()); @@ -222,6 +237,7 @@ impl PodBuilder { .or_else(|| { Some(Affinity { pod_affinity: self.pod_affinity.clone(), + pod_anti_affinity: self.pod_anti_affinity.clone(), ..Affinity::default() }) }), From 296ad675afe9ba61db9e070363b68314d29cbd79 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 09:53:25 +0100 Subject: [PATCH 03/14] Renamed maybe_* to *_opt --- src/builder/pod/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 9587eec16..8e4ac7419 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -15,8 +15,7 @@ use k8s_openapi::{ }; use std::collections::BTreeMap; -/// A builder to build [`Pod`] objects. -/// +/// A builder to build [`Pod`] or [`PodTemplateSpec`] objects. #[derive(Clone, Default)] pub struct PodBuilder { containers: Vec, @@ -90,12 +89,12 @@ impl PodBuilder { self } - pub fn maybe_pod_affinity(&mut self, affinity: Option) -> &mut Self { + pub fn pod_affinity_opt(&mut self, affinity: Option) -> &mut Self { self.pod_affinity = affinity; self } - pub fn maybe_pod_anti_affinity(&mut self, anti_affinity: Option) -> &mut Self { + pub fn pod_anti_affinity_opt(&mut self, anti_affinity: Option) -> &mut Self { self.pod_anti_affinity = anti_affinity; self } @@ -105,7 +104,7 @@ impl PodBuilder { self } - pub fn maybe_node_selector(&mut self, node_selector: Option) -> &mut Self { + pub fn node_selector_opt(&mut self, node_selector: Option) -> &mut Self { self.node_selector = node_selector; self } From 0826b6078bc17c445611c59ce26c35e2c7dcb10e Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 09:53:33 +0100 Subject: [PATCH 04/14] Updated changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca2dd802d..8efc8900f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Extended the `PodBuilder` with `pod_affinity`, `pod_anti_affinity`, `node_selector` and their `*_opt` variants ([#520]) + +[#520]: https://github.com/stackabletech/operator-rs/pull/520 + ## [0.25.3] - 2022-10-13 ### Added From b9716f0d54fb15aad7b61d61a4fb0143edb1800e Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 10:11:47 +0100 Subject: [PATCH 05/14] Updated tests --- src/product_config_utils.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/product_config_utils.rs b/src/product_config_utils.rs index 5da013117..aa20fb439 100644 --- a/src/product_config_utils.rs +++ b/src/product_config_utils.rs @@ -780,6 +780,7 @@ mod tests { None, None), selector: None, + affinity: None, }}, }, (false, true, false, true) => Role { @@ -1063,7 +1064,8 @@ mod tests { build_config_override(file_name, "file"), build_env_override(GROUP_ENV_OVERRIDE), None), - selector: None, + selector: None, + affinity: None, }}, }; @@ -1123,6 +1125,7 @@ mod tests { None ), selector: None, + affinity: None, }, role_group_2.to_string() => RoleGroup { replicas: Some(1), @@ -1133,6 +1136,7 @@ mod tests { None ), selector: None, + affinity: None, }} }), role_2.to_string() => (vec![PropertyNameKind::Cli], Role { @@ -1151,6 +1155,7 @@ mod tests { None ), selector: None, + affinity: None, }}, }) }; @@ -1213,6 +1218,7 @@ mod tests { None ), selector: None, + affinity: None, }} } ), @@ -1228,6 +1234,7 @@ mod tests { None ), selector: None, + affinity: None, }} } ), From d5aa901674e2dcbb1e9d33f8c382218d46eb6a2c Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 11:31:59 +0100 Subject: [PATCH 06/14] Added node_selector test --- Cargo.toml | 2 +- src/builder/pod/mod.rs | 119 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 107 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d4868996c..33b896d06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ opentelemetry-jaeger = { version = "0.16.0", features = ["rt-tokio"] } stackable-operator-derive = { path = "stackable-operator-derive" } [dev-dependencies] -rstest = "0.15.0" +rstest = "0.16.0" tempfile = "3.3.0" [features] diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 8e4ac7419..9abde4734 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -280,14 +280,72 @@ impl PodBuilder { #[cfg(test)] mod tests { - use crate::builder::{ - meta::ObjectMetaBuilder, - pod::{container::ContainerBuilder, volume::VolumeBuilder, PodBuilder}, + use super::*; + use crate::{ + builder::{ + meta::ObjectMetaBuilder, + pod::{container::ContainerBuilder, volume::VolumeBuilder}, + }, }; use k8s_openapi::{ api::core::v1::{LocalObjectReference, PodAffinity, PodAffinityTerm}, apimachinery::pkg::apis::meta::v1::{LabelSelector, LabelSelectorRequirement}, }; + use rstest::*; + + // A fixture for a simple container with a name and image + #[fixture] + fn dummy_container() -> Container { + ContainerBuilder::new("container") + .expect("ContainerBuilder not created") + .image("private-comapany/product:2.4.14") + .build() + } + + /// A [`PodBuilder`] that already contains the minum setup to build a Pod (name and container). + #[fixture] + fn pod_builder_with_name_and_container(dummy_container: Container) -> PodBuilder { + let mut builder = PodBuilder::new(); + builder + .metadata(ObjectMetaBuilder::new().name("testpod").build()) + .add_container(dummy_container); + builder + } + + // A fixture for a node selector to use on a Pod, and the resulting node selector labels and affinity. + #[fixture] + fn node_selector1() -> ( + LabelSelector, + Option>, + Option, + ) { + let labels = BTreeMap::from([("key".to_owned(), "value".to_owned())]); + let label_selector = LabelSelector { + match_expressions: Some(vec![LabelSelectorRequirement { + key: "security".to_owned(), + operator: "In".to_owned(), + values: Some(vec!["S1".to_owned(), "S2".to_owned()]), + }]), + match_labels: Some(labels.clone()), + }; + let affinity = Some(Affinity { + node_affinity: Some(NodeAffinity { + required_during_scheduling_ignored_during_execution: Some(NodeSelector { + node_selector_terms: vec![NodeSelectorTerm { + match_expressions: Some(vec![NodeSelectorRequirement { + key: "security".to_owned(), + operator: "In".to_owned(), + values: Some(vec!["S1".to_owned(), "S2".to_owned()]), + }]), + ..Default::default() + }], + }), + ..Default::default() + }), + ..Default::default() + }); + (label_selector, Some(labels), affinity) + } #[test] fn test_pod_builder() { @@ -369,16 +427,9 @@ mod tests { assert_eq!(pod.metadata.name.unwrap(), "foo"); } - #[test] - fn test_pod_builder_image_pull_secrets() { - let container = ContainerBuilder::new("container") - .expect("ContainerBuilder not created") - .image("private-comapany/product:2.4.14") - .build(); - - let pod = PodBuilder::new() - .metadata(ObjectMetaBuilder::new().name("testpod").build()) - .add_container(container) + #[rstest] + fn test_pod_builder_image_pull_secrets(mut pod_builder_with_name_and_container: PodBuilder) { + let pod = pod_builder_with_name_and_container .image_pull_secrets(vec!["company-registry-secret".to_string()].into_iter()) .build() .unwrap(); @@ -390,4 +441,46 @@ mod tests { }] ); } + + #[rstest] + fn test_pod_builder_node_selector( + mut pod_builder_with_name_and_container: PodBuilder, + node_selector1: ( + LabelSelector, + Option>, + Option, + ), + ) { + let (node_selector, expected_labels, expected_affinity) = node_selector1; + let pod = pod_builder_with_name_and_container + .node_selector_opt(Some(node_selector)) + .build() + .unwrap(); + + // asserts + let spec = pod.spec.unwrap(); + assert_eq!(spec.node_selector, expected_labels); + assert_eq!(spec.affinity, expected_affinity); + } + + #[rstest] + fn test_pod_builder_node_selector_opt( + mut pod_builder_with_name_and_container: PodBuilder, + node_selector1: ( + LabelSelector, + Option>, + Option, + ), + ) { + let (node_selector, expected_labels, expected_affinity) = node_selector1; + let pod = pod_builder_with_name_and_container + .node_selector(node_selector) + .build() + .unwrap(); + + // asserts + let spec = pod.spec.unwrap(); + assert_eq!(spec.node_selector, expected_labels); + assert_eq!(spec.affinity, expected_affinity); + } } From be5b477efc6767703dee0cccbf1f3b95ac181436 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:02:12 +0100 Subject: [PATCH 07/14] Added affinity tests --- src/builder/pod/mod.rs | 139 ++++++++++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 45 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 9abde4734..7bac63f13 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -281,11 +281,9 @@ impl PodBuilder { #[cfg(test)] mod tests { use super::*; - use crate::{ - builder::{ - meta::ObjectMetaBuilder, - pod::{container::ContainerBuilder, volume::VolumeBuilder}, - }, + use crate::builder::{ + meta::ObjectMetaBuilder, + pod::{container::ContainerBuilder, volume::VolumeBuilder}, }; use k8s_openapi::{ api::core::v1::{LocalObjectReference, PodAffinity, PodAffinityTerm}, @@ -293,7 +291,7 @@ mod tests { }; use rstest::*; - // A fixture for a simple container with a name and image + // A simple [`Container`] with a name and image. #[fixture] fn dummy_container() -> Container { ContainerBuilder::new("container") @@ -312,12 +310,12 @@ mod tests { builder } - // A fixture for a node selector to use on a Pod, and the resulting node selector labels and affinity. + // A fixture for a node selector to use on a Pod, and the resulting node selector labels and node affinity. #[fixture] fn node_selector1() -> ( LabelSelector, Option>, - Option, + Option, ) { let labels = BTreeMap::from([("key".to_owned(), "value".to_owned())]); let label_selector = LabelSelector { @@ -328,27 +326,52 @@ mod tests { }]), match_labels: Some(labels.clone()), }; - let affinity = Some(Affinity { - node_affinity: Some(NodeAffinity { - required_during_scheduling_ignored_during_execution: Some(NodeSelector { - node_selector_terms: vec![NodeSelectorTerm { - match_expressions: Some(vec![NodeSelectorRequirement { - key: "security".to_owned(), - operator: "In".to_owned(), - values: Some(vec!["S1".to_owned(), "S2".to_owned()]), - }]), - ..Default::default() - }], - }), - ..Default::default() + let affinity = Some(NodeAffinity { + required_during_scheduling_ignored_during_execution: Some(NodeSelector { + node_selector_terms: vec![NodeSelectorTerm { + match_expressions: Some(vec![NodeSelectorRequirement { + key: "security".to_owned(), + operator: "In".to_owned(), + values: Some(vec!["S1".to_owned(), "S2".to_owned()]), + }]), + ..Default::default() + }], }), ..Default::default() }); (label_selector, Some(labels), affinity) } - #[test] - fn test_pod_builder() { + #[fixture] + fn pod_affinity() -> PodAffinity { + PodAffinity { + preferred_during_scheduling_ignored_during_execution: None, + required_during_scheduling_ignored_during_execution: Some(vec![PodAffinityTerm { + label_selector: Some(LabelSelector { + match_expressions: Some(vec![LabelSelectorRequirement { + key: "security".to_string(), + operator: "In".to_string(), + values: Some(vec!["S1".to_string()]), + }]), + match_labels: None, + }), + topology_key: "topology.kubernetes.io/zone".to_string(), + ..Default::default() + }]), + } + } + + #[fixture] + fn pod_anti_affinity(pod_affinity: PodAffinity) -> PodAntiAffinity { + PodAntiAffinity { + preferred_during_scheduling_ignored_during_execution: None, + required_during_scheduling_ignored_during_execution: pod_affinity + .required_during_scheduling_ignored_during_execution, + } + } + + #[rstest] + fn test_pod_builder(pod_affinity: PodAffinity) { let container = ContainerBuilder::new("containername") .expect("ContainerBuilder not created") .image("stackable/zookeeper:2.4.14") @@ -364,22 +387,6 @@ mod tests { .args(vec!["12345".to_string()]) .build(); - let pod_affinity = PodAffinity { - preferred_during_scheduling_ignored_during_execution: None, - required_during_scheduling_ignored_during_execution: Some(vec![PodAffinityTerm { - label_selector: Some(LabelSelector { - match_expressions: Some(vec![LabelSelectorRequirement { - key: "security".to_string(), - operator: "In".to_string(), - values: Some(vec!["S1".to_string()]), - }]), - match_labels: None, - }), - topology_key: "topology.kubernetes.io/zone".to_string(), - ..Default::default() - }]), - }; - let pod = PodBuilder::new() .pod_affinity(pod_affinity.clone()) .metadata(ObjectMetaBuilder::new().name("testpod").build()) @@ -442,16 +449,30 @@ mod tests { ); } + /// Test if setting a node selector generates the correct node selector labels and node affinity on the Pod. #[rstest] fn test_pod_builder_node_selector( mut pod_builder_with_name_and_container: PodBuilder, node_selector1: ( LabelSelector, Option>, - Option, + Option, ), ) { + // destruct fixture let (node_selector, expected_labels, expected_affinity) = node_selector1; + // first test with the normal node_selector function + let pod = pod_builder_with_name_and_container + .clone() + .node_selector(node_selector.clone()) + .build() + .unwrap(); + + let spec = pod.spec.unwrap(); + assert_eq!(spec.node_selector, expected_labels); + assert_eq!(spec.affinity.unwrap().node_affinity, expected_affinity); + + // test the node_selector_opt function let pod = pod_builder_with_name_and_container .node_selector_opt(Some(node_selector)) .build() @@ -460,27 +481,55 @@ mod tests { // asserts let spec = pod.spec.unwrap(); assert_eq!(spec.node_selector, expected_labels); - assert_eq!(spec.affinity, expected_affinity); + assert_eq!(spec.affinity.unwrap().node_affinity, expected_affinity); } + /// Test if setting a node selector generates the correct node selector labels and node affinity on the Pod, + /// while keeping the manually set Pod affinities. Since they are mangled together, it makes sense to make sure that + /// one is not replacing the other #[rstest] - fn test_pod_builder_node_selector_opt( + fn test_pod_builder_node_selector_and_affinity( mut pod_builder_with_name_and_container: PodBuilder, node_selector1: ( LabelSelector, Option>, - Option, + Option, ), + pod_affinity: PodAffinity, + pod_anti_affinity: PodAntiAffinity, ) { + // destruct fixture let (node_selector, expected_labels, expected_affinity) = node_selector1; + // first test with the normal functions let pod = pod_builder_with_name_and_container - .node_selector(node_selector) + .clone() + .node_selector(node_selector.clone()) + .pod_affinity(pod_affinity.clone()) + .pod_anti_affinity(pod_anti_affinity.clone()) + .build() + .unwrap(); + + let spec = pod.spec.unwrap(); + assert_eq!(spec.node_selector, expected_labels); + let affinity = spec.affinity.unwrap(); + assert_eq!(affinity.node_affinity, expected_affinity); + assert_eq!(affinity.pod_affinity, Some(pod_affinity.clone())); + assert_eq!(affinity.pod_anti_affinity, Some(pod_anti_affinity.clone())); + + // test the *_opt functions + let pod = pod_builder_with_name_and_container + .node_selector_opt(Some(node_selector)) + .pod_affinity_opt(Some(pod_affinity.clone())) + .pod_anti_affinity_opt(Some(pod_anti_affinity.clone())) .build() .unwrap(); // asserts let spec = pod.spec.unwrap(); assert_eq!(spec.node_selector, expected_labels); - assert_eq!(spec.affinity, expected_affinity); + let affinity = spec.affinity.unwrap(); + assert_eq!(affinity.node_affinity, expected_affinity); + assert_eq!(affinity.pod_affinity, Some(pod_affinity)); + assert_eq!(affinity.pod_anti_affinity, Some(pod_anti_affinity)); } } From b7246bb2525670a0c3012b5773d4efcee1b15618 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:08:30 +0100 Subject: [PATCH 08/14] Clippy fix --- src/builder/pod/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index bfcfe158b..d94183e15 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -378,7 +378,6 @@ impl PodBuilder { node_affinity: Some(node_affinity), pod_affinity: self.pod_affinity.clone(), pod_anti_affinity: self.pod_anti_affinity.clone(), - ..Affinity::default() }) .or_else(|| { Some(Affinity { From 6283329e5f2ebc4056cdf6a226fab5136c46e3af Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:30:41 +0100 Subject: [PATCH 09/14] Update src/builder/pod/mod.rs Co-authored-by: Malte Sander --- src/builder/pod/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index d94183e15..8abaa932e 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -442,7 +442,7 @@ mod tests { fn dummy_container() -> Container { ContainerBuilder::new("container") .expect("ContainerBuilder not created") - .image("private-comapany/product:2.4.14") + .image("private-company/product:2.4.14") .build() } From cef7ac7723e4019af0655769e46c881aaf1a86d8 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:35:18 +0100 Subject: [PATCH 10/14] Cleaned up affinity creation --- src/builder/pod/mod.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 8abaa932e..89c017dce 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -373,19 +373,11 @@ impl PodBuilder { init_containers: self.init_containers.clone(), node_name: self.node_name.clone(), node_selector: node_selector_labels, - affinity: node_affinity - .map(|node_affinity| Affinity { - node_affinity: Some(node_affinity), - pod_affinity: self.pod_affinity.clone(), - pod_anti_affinity: self.pod_anti_affinity.clone(), - }) - .or_else(|| { - Some(Affinity { - pod_affinity: self.pod_affinity.clone(), - pod_anti_affinity: self.pod_anti_affinity.clone(), - ..Affinity::default() - }) - }), + affinity: Some(Affinity { + node_affinity: node_affinity, + pod_affinity: self.pod_affinity.clone(), + pod_anti_affinity: self.pod_anti_affinity.clone(), + }), security_context: self.security_context.clone(), tolerations: self.tolerations.clone(), volumes: self.volumes.clone(), From 8f4eef351a82cc6de5b2ccebe4bcf414370623d8 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:37:07 +0100 Subject: [PATCH 11/14] Changed order of methods --- src/builder/pod/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 89c017dce..4c5f2db77 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -86,13 +86,13 @@ impl PodBuilder { self } - pub fn pod_anti_affinity(&mut self, anti_affinity: PodAntiAffinity) -> &mut Self { - self.pod_anti_affinity = Some(anti_affinity); + pub fn pod_affinity_opt(&mut self, affinity: Option) -> &mut Self { + self.pod_affinity = affinity; self } - pub fn pod_affinity_opt(&mut self, affinity: Option) -> &mut Self { - self.pod_affinity = affinity; + pub fn pod_anti_affinity(&mut self, anti_affinity: PodAntiAffinity) -> &mut Self { + self.pod_anti_affinity = Some(anti_affinity); self } From 9af360a600fc5885d9e3079f3a95f41a4770b060 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:41:49 +0100 Subject: [PATCH 12/14] Cleaned up tst --- src/builder/pod/mod.rs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 4c5f2db77..afaaf559e 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -509,26 +509,26 @@ mod tests { } #[rstest] - fn test_pod_builder(pod_affinity: PodAffinity) { - let container = ContainerBuilder::new("containername") - .expect("ContainerBuilder not created") - .image("stackable/zookeeper:2.4.14") - .command(vec!["zk-server-start.sh".to_string()]) - .args(vec!["stackable/conf/zk.properties".to_string()]) - .add_volume_mount("zk-worker-1", "conf/") - .build(); + fn test_pod_builder_pod_name() { + let pod = PodBuilder::new() + .metadata_builder(|builder| builder.name("foo")) + .build() + .unwrap(); + + assert_eq!(pod.metadata.name.unwrap(), "foo"); + } + #[rstest] + fn test_pod_builder(pod_affinity: PodAffinity, dummy_container: Container) { let init_container = ContainerBuilder::new("init-containername") .expect("ContainerBuilder not created") .image("stackable/zookeeper:2.4.14") - .command(vec!["wrapper.sh".to_string()]) - .args(vec!["12345".to_string()]) .build(); let pod = PodBuilder::new() .pod_affinity(pod_affinity.clone()) .metadata(ObjectMetaBuilder::new().name("testpod").build()) - .add_container(container) + .add_container(dummy_container) .add_init_container(init_container) .node_name("worker-1.stackable.demo") .add_volume( @@ -563,13 +563,6 @@ mod tests { .and_then(|volume| volume.config_map.as_ref()?.name.clone())), Some("configmap".to_string()) ); - - let pod = PodBuilder::new() - .metadata_builder(|builder| builder.name("foo")) - .build() - .unwrap(); - - assert_eq!(pod.metadata.name.unwrap(), "foo"); } #[rstest] From 8922df6068f751e31e1d4acc6f91e24d3b7eb00f Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:42:42 +0100 Subject: [PATCH 13/14] clippy fix --- src/builder/pod/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index afaaf559e..766768ef9 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -374,7 +374,7 @@ impl PodBuilder { node_name: self.node_name.clone(), node_selector: node_selector_labels, affinity: Some(Affinity { - node_affinity: node_affinity, + node_affinity, pod_affinity: self.pod_affinity.clone(), pod_anti_affinity: self.pod_anti_affinity.clone(), }), From d7f34c2379220b35289ced0b9938b844af3f24b9 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Mon, 19 Dec 2022 12:43:45 +0100 Subject: [PATCH 14/14] cargo fmt --- src/builder/pod/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/builder/pod/mod.rs b/src/builder/pod/mod.rs index 766768ef9..ddb70f224 100644 --- a/src/builder/pod/mod.rs +++ b/src/builder/pod/mod.rs @@ -511,11 +511,11 @@ mod tests { #[rstest] fn test_pod_builder_pod_name() { let pod = PodBuilder::new() - .metadata_builder(|builder| builder.name("foo")) - .build() - .unwrap(); + .metadata_builder(|builder| builder.name("foo")) + .build() + .unwrap(); - assert_eq!(pod.metadata.name.unwrap(), "foo"); + assert_eq!(pod.metadata.name.unwrap(), "foo"); } #[rstest]