Skip to content

Commit

Permalink
add sort for the policy on the sriovOperatorConfig controller
Browse files Browse the repository at this point in the history
we need to be consistent with the policy order

Signed-off-by: Sebastian Sch <[email protected]>
  • Loading branch information
SchSeba authored and zeeke committed Jun 28, 2024
1 parent 42905de commit 7eaa583
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
3 changes: 3 additions & 0 deletions controllers/sriovnetworknodepolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions controllers/sriovoperatorconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"os"
"sort"
"strings"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -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 {
Expand Down
49 changes: 49 additions & 0 deletions controllers/sriovoperatorconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package controllers

import (
"context"
"fmt"
"os"
"strings"
"sync"

admv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/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"

Expand Down Expand Up @@ -389,5 +392,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())
})
})
})

0 comments on commit 7eaa583

Please sign in to comment.