refactor!: Remove legacy node selector on RoleGroup (#652)

* refactor!: Remove legacy node selector on RoleGroup * changelog
stackabletech · Dec 21, 2023 · 273fcc8 · 273fcc8
1 parent 38cb237
commit 273fcc8
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 312 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,8 +14,10 @@ All notable changes to this project will be documented in this file.
 - Move `stackable_operator::label_selector::convert_label_selector_to_query_string` into `kvp` module. The conversion
   functionality now is encapsulated in a new trait `LabelSelectorExt`. An instance of a `LabelSelector` can now be
   converted into a query string by calling the associated function `ls.to_query_string()` ([#684]).
+- BREAKING: Remove legacy node selector on `RoleGroup` ([#652]).
 
 [#684]: https://github.com/stackabletech/operator-rs/pull/684
+[#652]: https://github.com/stackabletech/operator-rs/pull/652
 
 ## [0.58.1] - 2023-12-12
 

diff --git a/src/commons/affinity.rs b/src/commons/affinity.rs
@@ -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};
@@ -43,79 +42,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 {
@@ -185,7 +111,6 @@ mod tests {
         api::core::v1::{NodeSelector, NodeSelectorRequirement, NodeSelectorTerm},
         apimachinery::pkg::apis::meta::v1::LabelSelectorRequirement,
     };
-    use rstest::rstest;
 
     use crate::config::fragment;
 
@@ -432,210 +357,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);
-    }
 }