add sort for the policy on the sriovOperatorConfig controller

we need to be consistent with the policy order

Signed-off-by: Sebastian Sch <[email protected]>

controllers/sriovnetworknodepolicy_controller.go

@@ -121,6 +121,9 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct
 	}
 
 	// Sort the policies with priority, higher priority ones is applied later
+	// We need to use the sort so we always get the policies in the same order
+	// That is needed so when we create the node Affinity for the sriov-device plugin
+	// it will remain in the same order and not trigger a pod recreation
 	sort.Sort(sriovnetworkv1.ByPriority(policyList.Items))
 	// Sync SriovNetworkNodeState objects
 	if err = r.syncAllSriovNetworkNodeStates(ctx, defaultOpConf, policyList, nodeList); err != nil {

controllers/sriovoperatorconfig_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"sort"
 	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -107,6 +108,11 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 		// Error reading the object - requeue the request.
 		return reconcile.Result{}, err
 	}
+	// Sort the policies with priority, higher priority ones is applied later
+	// We need to use the sort so we always get the policies in the same order
+	// That is needed so when we create the node Affinity for the sriov-device plugin
+	// it will remain in the same order and not trigger a pod recreation
+	sort.Sort(sriovnetworkv1.ByPriority(policyList.Items))
 
 	// Render and sync webhook objects
 	if err = r.syncWebhookObjs(ctx, defaultConfig); err != nil {

controllers/sriovoperatorconfig_controller_test.go

@@ -2,14 +2,16 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 	"sync"
 
 	admv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 
@@ -354,7 +356,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 			err = util.WaitForNamespacedObject(&appsv1.DaemonSet{}, k8sClient, testNamespace, "sriov-network-metrics-exporter", util.RetryInterval, util.APITimeout)
 			Expect(err).NotTo(HaveOccurred())
 
-			err = util.WaitForNamespacedObject(&v1.Service{}, k8sClient, testNamespace, "sriov-network-metrics-exporter-service", util.RetryInterval, util.APITimeout)
+			err = util.WaitForNamespacedObject(&corev1.Service{}, k8sClient, testNamespace, "sriov-network-metrics-exporter-service", util.RetryInterval, util.APITimeout)
 			Expect(err).ToNot(HaveOccurred())
 
 			By("Turn `metricsExporter` feature gate off")
@@ -364,7 +366,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 			err = util.WaitForNamespacedObjectDeleted(&appsv1.DaemonSet{}, k8sClient, testNamespace, "sriov-network-metrics-exporter", util.RetryInterval, util.APITimeout)
 			Expect(err).NotTo(HaveOccurred())
 
-			err = util.WaitForNamespacedObjectDeleted(&v1.Service{}, k8sClient, testNamespace, "sriov-network-metrics-exporter-service", util.RetryInterval, util.APITimeout)
+			err = util.WaitForNamespacedObjectDeleted(&corev1.Service{}, k8sClient, testNamespace, "sriov-network-metrics-exporter-service", util.RetryInterval, util.APITimeout)
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -427,5 +429,51 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 				g.Expect(injectorCfg.Webhooks[0].ClientConfig.CABundle).To(Equal([]byte("ca-bundle-2\n")))
 			}, "1s").Should(Succeed())
 		})
+		It("should reconcile to a converging state when multiple node policies are set", func() {
+			By("Creating a consistent number of node policies")
+			for i := 0; i < 30; i++ {
+				p := &sriovnetworkv1.SriovNetworkNodePolicy{
+					ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: fmt.Sprintf("p%d", i)},
+					Spec: sriovnetworkv1.SriovNetworkNodePolicySpec{
+						Priority:     99,
+						NodeSelector: map[string]string{"foo": fmt.Sprintf("v%d", i)},
+					},
+				}
+				err := k8sClient.Create(context.Background(), p)
+				Expect(err).NotTo(HaveOccurred())
+			}
+
+			By("Triggering a the reconcile loop")
+			config := &sriovnetworkv1.SriovOperatorConfig{}
+			err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "default", Namespace: testNamespace}, config)
+			Expect(err).NotTo(HaveOccurred())
+			if config.ObjectMeta.Labels == nil {
+				config.ObjectMeta.Labels = make(map[string]string)
+			}
+			config.ObjectMeta.Labels["trigger-test"] = "test-reconcile-daemonset"
+			err = k8sClient.Update(context.Background(), config)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Wait until device-plugin Daemonset's affinity has been calculated")
+			var expectedAffinity *corev1.Affinity
+
+			Eventually(func(g Gomega) {
+				daemonSet := &appsv1.DaemonSet{}
+				err = k8sClient.Get(context.Background(), types.NamespacedName{Name: "sriov-device-plugin", Namespace: testNamespace}, daemonSet)
+				g.Expect(err).NotTo(HaveOccurred())
+				// Wait until the last policy (with NodeSelector foo=v29) has been considered at least one time
+				g.Expect(daemonSet.Spec.Template.Spec.Affinity.String()).To(ContainSubstring("v29"))
+				expectedAffinity = daemonSet.Spec.Template.Spec.Affinity
+			}, "3s", "1s").Should(Succeed())
+
+			By("Verify device-plugin Daemonset's affinity doesn't change over time")
+			Consistently(func(g Gomega) {
+				daemonSet := &appsv1.DaemonSet{}
+				err = k8sClient.Get(context.Background(), types.NamespacedName{Name: "sriov-device-plugin", Namespace: testNamespace}, daemonSet)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(daemonSet.Spec.Template.Spec.Affinity).
+					To(Equal(expectedAffinity))
+			}, "3s", "1s").Should(Succeed())
+		})
 	})
 })