[e2e] test softtainter (#495)

* [BUG] fix softtainter cleanup

When running in the cleanup mode,
LowNodeUtilizationArgs can be completely missing
from the Descheduler policy.
Let's ignore it to try completing the
softtaints cleanup.

Signed-off-by: Simone Tiraboschi <[email protected]>

* [BUG] make softtainter VAP stricter on taint effect

Signed-off-by: Simone Tiraboschi <[email protected]>

* Refactor test/e2e/operator_test.go

Refactor test/e2e/operator_test.go to make
adding more tests in the future. easier.

Signed-off-by: Simone Tiraboschi <[email protected]>

* [e2e] test softtainter deployment

Cover also softtainter depployment
and its cleanup in e2e tests.

Signed-off-by: Simone Tiraboschi <[email protected]>

* [e2e] test softtainter VAP

Cover also softtainter ValidatingAdmissionPolicy
trying to adde and remove allowed and not allowed
taints impersonating softtainer SA.

Signed-off-by: Simone Tiraboschi <[email protected]>

* Address review comments

Signed-off-by: Simone Tiraboschi <[email protected]>

---------

Signed-off-by: Simone Tiraboschi <[email protected]>

Author: tiraboschi

bindata/assets/kube-descheduler/softtaintervalidatingadmissionpolicy.yaml

@@ -20,16 +20,18 @@ spec:
   variables:
     - name: "deschedulerTaintPrefix"
       expression: "'descheduler.openshift.io'"
+    - name: "deschedulerTaintEffect"
+      expression: "'PreferNoSchedule'"
     - name: "oldNonDeschedulerTaints"
-      expression: "has(oldObject.spec.taints) ? oldObject.spec.taints.filter(t, !t.key.contains(variables.deschedulerTaintPrefix)) : []"
+      expression: "has(oldObject.spec.taints) ? oldObject.spec.taints.filter(t, !t.key.contains(variables.deschedulerTaintPrefix) || t.effect != variables.deschedulerTaintEffect) : []"
     - name: "oldTaints"
       expression: "has(oldObject.spec.taints) ? oldObject.spec.taints : []"
     - name: "newNonDeschedulerTaints"
-      expression: "has(object.spec.taints) ? object.spec.taints.filter(t, !t.key.contains(variables.deschedulerTaintPrefix)) : []"
+      expression: "has(object.spec.taints) ? object.spec.taints.filter(t, !t.key.contains(variables.deschedulerTaintPrefix) || t.effect != variables.deschedulerTaintEffect) : []"
     - name: "newTaints"
       expression: "has(object.spec.taints) ? object.spec.taints : []"
     - name: "newDeschedulerTaints"
-      expression: "has(object.spec.taints) ? object.spec.taints.filter(t, t.key.contains(variables.deschedulerTaintPrefix)) : []"
+      expression: "has(object.spec.taints) ? object.spec.taints.filter(t, t.key.contains(variables.deschedulerTaintPrefix) && t.effect == variables.deschedulerTaintEffect) : []"
   validations:
     - expression: |
         oldObject.metadata.filter(k, k != "resourceVersion" && k != "generation" && k != "managedFields").all(k, k in object.metadata) &&

pkg/softtainter/softtainter.go

@@ -50,7 +50,6 @@ type softTainterArgs struct {
 	useDeviationThresholds bool
 	thresholds             api.ResourceThresholds
 	targetThresholds       api.ResourceThresholds
-	applySoftTaints        bool
 	mode                   desv1.Mode
 }
 
@@ -108,6 +107,39 @@ func (st *softTainter) Reconcile(ctx context.Context, request reconcile.Request)
 	}
 	st.resyncPeriod = time.Duration(*des.Spec.DeschedulingIntervalSeconds) * time.Second
 
+	nodeSelector := labels.Everything()
+	if policy.NodeSelector != nil {
+		sel, err := labels.Parse(*policy.NodeSelector)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+		nodeSelector = sel
+	}
+
+	nl := corev1.NodeList{}
+	err = st.client.List(ctx, &nl, &client.ListOptions{LabelSelector: nodeSelector})
+	if err != nil {
+		return reconcile.Result{}, err
+	}
+	nodeList := make([]*corev1.Node, len(nl.Items))
+	for i, node := range nl.Items {
+		nodeList[i] = &node
+	}
+
+	enableSoftTainter := false
+	if des.Spec.ProfileCustomizations != nil && des.Spec.ProfileCustomizations.DevEnableSoftTainter {
+		enableSoftTainter = true
+	}
+
+	if !enableSoftTainter {
+		logger.Info("SoftTainter is disabled, cleaning up eventual leftover taints")
+		err = st.cleanAllSoftTaints(ctx, nodeList)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+		return reconcile.Result{RequeueAfter: st.resyncPeriod}, nil
+	}
+
 	var lnargs *nodeutilization.LowNodeUtilizationArgs
 
 	for _, p := range policy.Profiles {
@@ -124,17 +156,10 @@ func (st *softTainter) Reconcile(ctx context.Context, request reconcile.Request)
 		logger.Error(err, "reconciliation failed")
 		return reconcile.Result{}, err
 	}
-
-	enableSoftTainter := false
-	if des.Spec.ProfileCustomizations != nil && des.Spec.ProfileCustomizations.DevEnableSoftTainter {
-		enableSoftTainter = true
-	}
-
 	st.args = &softTainterArgs{
 		useDeviationThresholds: lnargs.UseDeviationThresholds,
 		thresholds:             lnargs.Thresholds,
 		targetThresholds:       lnargs.TargetThresholds,
-		applySoftTaints:        enableSoftTainter,
 		mode:                   des.Spec.Mode,
 	}
 
@@ -172,37 +197,10 @@ func (st *softTainter) Reconcile(ctx context.Context, request reconcile.Request)
 
 	st.resourceNames = getResourceNames(st.args.thresholds)
 
-	nodeSelector := labels.Everything()
-	if policy.NodeSelector != nil {
-		sel, err := labels.Parse(*policy.NodeSelector)
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-		nodeSelector = sel
-	}
-
-	nl := corev1.NodeList{}
-	err = st.client.List(ctx, &nl, &client.ListOptions{LabelSelector: nodeSelector})
+	err = st.syncSoftTaints(ctx, nodeList)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-	nodeList := make([]*corev1.Node, len(nl.Items))
-	for i, node := range nl.Items {
-		nodeList[i] = &node
-	}
-
-	if st.args.applySoftTaints {
-		err = st.syncSoftTaints(ctx, nodeList)
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-	} else {
-		logger.Info("SoftTainter is disabled, cleaning up eventual leftover taints")
-		err = st.cleanAllSoftTaints(ctx, nodeList)
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-	}
 	return reconcile.Result{RequeueAfter: st.resyncPeriod}, nil
 }
 

test/e2e/bindata/assets/00_kube-descheduler-operator-crd.yaml

@@ -117,6 +117,9 @@ spec:
                     - Low
                     - Medium
                     - High
+                    - AsymmetricLow
+                    - AsymmetricMedium
+                    - AsymmetricHigh
                     - ""
                     type: string
                   devEnableEvictionsInBackground:
@@ -125,6 +128,12 @@ spec:
                       The EvictionsInBackground alpha feature is a subject to change.
                       Currently provided as an experimental feature.
                     type: boolean
+                  devEnableSoftTainter:
+                    description: |-
+                      DevEnableSoftTainter enables SoftTainter alpha feature.
+                      The EnableSoftTainter alpha feature is a subject to change.
+                      Currently provided as an experimental feature.
+                    type: boolean
                   devHighNodeUtilizationThresholds:
                     description: |-
                       devHighNodeUtilizationThresholds enumerates thresholds for node utilization levels.
@@ -137,7 +146,7 @@ spec:
                     - ""
                     type: string
                   devLowNodeUtilizationThresholds:
-                    description: LowNodeUtilizationThresholds enumerates predefined
+                    description: DevLowNodeUtilizationThresholds enumerates predefined
                       experimental thresholds
                     enum:
                     - Low

test/e2e/bindata/assets/07_descheduler-operator.cr.devKubeVirtRelieveAndMigrate.yaml

@@ -0,0 +1,17 @@
+apiVersion: operator.openshift.io/v1
+kind: KubeDescheduler
+metadata:
+  name: cluster
+  namespace: openshift-kube-descheduler-operator
+spec:
+  managementState: Managed
+  deschedulingIntervalSeconds: 30
+  mode: "Automatic"
+  profiles:
+    - DevKubeVirtRelieveAndMigrate
+  profileCustomizations:
+    podLifetime: 10s
+    devEnableSoftTainter: true
+    namespaces:
+      included:
+      - e2e-testdescheduling