Skip to content

Commit

Permalink
refactor!: Remove legacy node selector on RoleGroup
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbernauer
sbernauer committed Sep 15, 2023
1 parent f846302 commit b61c14d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 312 deletions.
285 changes: 2 additions & 283 deletions src/commons/affinity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ use std::collections::BTreeMap;

use k8s_openapi::{
api::core::v1::{
NodeAffinity, NodeSelector, NodeSelectorRequirement, NodeSelectorTerm, PodAffinity,
PodAffinityTerm, PodAntiAffinity, WeightedPodAffinityTerm,
NodeAffinity, PodAffinity, PodAffinityTerm, PodAntiAffinity, WeightedPodAffinityTerm,
},
apimachinery::pkg::apis::meta::v1::{LabelSelector, LabelSelectorRequirement},
apimachinery::pkg::apis::meta::v1::LabelSelector,
};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -41,79 +40,6 @@ pub struct StackableAffinity {
pub node_selector: Option<StackableNodeSelector>,
}

impl StackableAffinityFragment {
#[deprecated(
since = "0.36.0",
note = "During https://github.com/stackabletech/issues/issues/323 we moved from the previous selector field on a rolegroup to a more generic affinity handling. \
We still need to support the old selector field, which has some custom magic (see the code in this function). \
So we need a way to transform the old into the mechanism which this function offers. \
It will be removed once we stop supporting the old mechanism."
)]
pub fn add_legacy_selector(&mut self, label_selector: &LabelSelector) {
tracing::warn!("Deprecated field `selector` was specified. Please use the new `affinity` field instead, as support for `selector` will be removed in the future. See https://docs.stackable.tech/home/stable/contributor/adr/ADR026-affinities.html for details");

let node_labels = label_selector.match_labels.clone();
let node_label_exprs = label_selector.match_expressions.clone();

let node_affinity = node_label_exprs.map(|node_label_exprs| NodeAffinity {
required_during_scheduling_ignored_during_execution: Some(NodeSelector {
node_selector_terms: vec![NodeSelectorTerm {
match_expressions: Some(
node_label_exprs
.into_iter()
.map(
|LabelSelectorRequirement {
key,
operator,
values,
}| {
NodeSelectorRequirement {
key,
operator,
values,
}
},
)
.collect(),
),
..NodeSelectorTerm::default()
}],
}),
..NodeAffinity::default()
});

if let Some(node_labels) = node_labels {
self.node_selector
.get_or_insert(StackableNodeSelector {
node_selector: BTreeMap::new(),
})
.node_selector
.extend(node_labels);
}

if let Some(NodeAffinity {
required_during_scheduling_ignored_during_execution:
Some(NodeSelector {
node_selector_terms,
}),
..
}) = node_affinity
{
self.node_affinity
.get_or_insert(NodeAffinity {
preferred_during_scheduling_ignored_during_execution: None,
required_during_scheduling_ignored_during_execution: None,
})
.required_during_scheduling_ignored_during_execution
.get_or_insert(NodeSelector {
node_selector_terms: Vec::new(),
})
.node_selector_terms
.extend(node_selector_terms);
}
}
}

#[derive(Clone, Debug, Eq, Deserialize, JsonSchema, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct StackableNodeSelector {
Expand Down Expand Up @@ -183,7 +109,6 @@ mod tests {
api::core::v1::{NodeSelector, NodeSelectorRequirement, NodeSelectorTerm},
apimachinery::pkg::apis::meta::v1::LabelSelectorRequirement,
};
use rstest::rstest;

use crate::config::fragment;

Expand Down Expand Up @@ -430,210 +355,4 @@ mod tests {
}
);
}

#[rstest]
#[case::legacy_selector_labels_specified(
r#"
matchLabels:
disktype: ssd
"#,
r#"
nodeSelector:
generation: new
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#,
r#"
nodeSelector:
disktype: ssd
generation: new
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#
)]
#[case::legacy_selector_expression_specified(
r#"
matchExpressions:
- key: topology.kubernetes.io/continent
operator: In
values:
- europe
"#,
r#"
nodeSelector:
generation: new
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#,
r#"
nodeSelector:
generation: new
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
- matchExpressions:
- key: topology.kubernetes.io/continent
operator: In
values:
- europe
"#
)]
#[case::legacy_selector_expression_and_labels_specified(
r#"
matchLabels:
disktype: ssd
matchExpressions:
- key: topology.kubernetes.io/continent
operator: In
values:
- europe
"#,
r#"
nodeSelector:
generation: new
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#,
r#"
nodeSelector:
generation: new
disktype: ssd
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
- matchExpressions:
- key: topology.kubernetes.io/continent
operator: In
values:
- europe
"#
)]
#[case::legacy_selector_labels_no_new_labels(
r#"
matchLabels:
disktype: ssd
"#,
r#"
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#,
r#"
nodeSelector:
disktype: ssd
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#
)]
#[case::legacy_selector_labels_no_new_labels(
r#"
matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- africa-east1
"#,
r#"
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
"#,
r#"
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- antarctica-east1
- antarctica-west1
- matchExpressions:
- key: topology.kubernetes.io/zone
operator: In
values:
- africa-east1
"#
)]
fn test_add_legacy_selector_fn(
#[case] legacy_selector: &str,
#[case] new_selector: &str,
#[case] expected_result: &str,
) {
let legacy_selector: LabelSelector =
serde_yaml::from_str(legacy_selector).expect("illegal test input for legacy selector");

let mut new_selector: StackableAffinityFragment =
serde_yaml::from_str(new_selector).expect("illegal test input for new selector");

let expected_result: StackableAffinityFragment =
serde_yaml::from_str(expected_result).expect("illegal test input for expected result");

// Merge legacy and new node selectors
#[allow(deprecated)]
new_selector.add_legacy_selector(&legacy_selector);

assert_eq!(expected_result, new_selector);
}
}
Loading

0 comments on commit b61c14d

Please sign in to comment.