From 94c57aa842e762291668169eb120bf5cfcbea125 Mon Sep 17 00:00:00 2001 From: adrianc Date: Mon, 5 Feb 2024 19:41:49 +0200 Subject: [PATCH] Change sriov device plugin owner ref Use default SriovOperatorConfig object instead of default SriovNetworkNodePolicy. This change is required as subsequent changes will drop the creation of the default policy. Signed-off-by: adrianc --- controllers/helper.go | 21 ++++++++++++------- .../sriovnetworknodepolicy_controller.go | 2 +- controllers/sriovoperatorconfig_controller.go | 2 +- .../sriovoperatorconfig_controller_test.go | 20 ++++++++++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/controllers/helper.go b/controllers/helper.go index b15c11d8b..f341f5f73 100644 --- a/controllers/helper.go +++ b/controllers/helper.go @@ -82,7 +82,6 @@ func syncPluginDaemonObjs(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dc *sriovnetworkv1.SriovOperatorConfig, - dp *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList) error { logger := log.Log.WithName("syncPluginDaemonObjs") logger.V(1).Info("Start to sync sriov daemons objects") @@ -129,7 +128,7 @@ func syncPluginDaemonObjs(ctx context.Context, return err } } - err = syncDsObject(ctx, client, scheme, dp, pl, obj) + err = syncDsObject(ctx, client, scheme, dc, pl, obj) if err != nil { logger.Error(err, "Couldn't sync SR-IoV daemons objects") return err @@ -146,13 +145,13 @@ func deleteK8sResource(ctx context.Context, client k8sclient.Client, in *uns.Uns return nil } -func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dp *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList, obj *uns.Unstructured) error { +func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dc *sriovnetworkv1.SriovOperatorConfig, pl *sriovnetworkv1.SriovNetworkNodePolicyList, obj *uns.Unstructured) error { logger := log.Log.WithName("syncDsObject") kind := obj.GetKind() logger.V(1).Info("Start to sync Objects", "Kind", kind) switch kind { case constants.ServiceAccount, constants.Role, constants.RoleBinding: - if err := controllerutil.SetControllerReference(dp, obj, scheme); err != nil { + if err := controllerutil.SetControllerReference(dc, obj, scheme); err != nil { return err } if err := apply.ApplyObject(ctx, client, obj); err != nil { @@ -166,7 +165,7 @@ func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime. logger.Error(err, "Fail to convert to DaemonSet") return err } - err = syncDaemonSet(ctx, client, scheme, dp, pl, ds) + err = syncDaemonSet(ctx, client, scheme, dc, pl, ds) if err != nil { logger.Error(err, "Fail to sync DaemonSet", "Namespace", ds.Namespace, "Name", ds.Name) return err @@ -175,7 +174,7 @@ func syncDsObject(ctx context.Context, client k8sclient.Client, scheme *runtime. return nil } -func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, cr *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList, in *appsv1.DaemonSet) error { +func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dc *sriovnetworkv1.SriovOperatorConfig, pl *sriovnetworkv1.SriovNetworkNodePolicyList, in *appsv1.DaemonSet) error { logger := log.Log.WithName("syncDaemonSet") logger.V(1).Info("Start to sync DaemonSet", "Namespace", in.Namespace, "Name", in.Name) var err error @@ -185,7 +184,7 @@ func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime return err } } - if err = controllerutil.SetControllerReference(cr, in, scheme); err != nil { + if err = controllerutil.SetControllerReference(dc, in, scheme); err != nil { return err } ds := &appsv1.DaemonSet{} @@ -207,7 +206,13 @@ func syncDaemonSet(ctx context.Context, client k8sclient.Client, scheme *runtime // DeepDerivative checks for changes only comparing non-zero fields in the source struct. // This skips default values added by the api server. // References in https://github.com/kubernetes-sigs/kubebuilder/issues/592#issuecomment-625738183 - if equality.Semantic.DeepDerivative(in.Spec, ds.Spec) { + + // Note(Adrianc): we check Equality of OwnerReference as we changed sriov-device-plugin owner ref + // from SriovNetworkNodePolicy to SriovOperatorConfig, hence even if there is no change in spec, + // we need to update the obj's owner reference. + + if equality.Semantic.DeepEqual(in.OwnerReferences, ds.OwnerReferences) && + equality.Semantic.DeepDerivative(in.Spec, ds.Spec) { // DeepDerivative has issue detecting nodeAffinity change // https://bugzilla.redhat.com/show_bug.cgi?id=1914066 // DeepDerivative doesn't detect changes in containers args section diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index 16c1b7449..fcc62dec2 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -157,7 +157,7 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct return reconcile.Result{}, err } // Render and sync Daemon objects - if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultOpConf, defaultPolicy, policyList); err != nil { + if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultOpConf, policyList); err != nil { return reconcile.Result{}, err } diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index 556bbf44d..c4eeec339 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -120,7 +120,7 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl. return reconcile.Result{}, err } - if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, defaultPolicy, policyList); err != nil { + if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, policyList); err != nil { return reconcile.Result{}, err } diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go index 1db6f51c8..bbb777e45 100644 --- a/controllers/sriovoperatorconfig_controller_test.go +++ b/controllers/sriovoperatorconfig_controller_test.go @@ -14,6 +14,7 @@ import ( . "github.com/onsi/gomega" sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" mock_platforms "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/mock" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/openshift" @@ -56,6 +57,21 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { Expect(err).ToNot(HaveOccurred()) }) + otherPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} + otherPolicy.SetNamespace(testNamespace) + otherPolicy.SetName("other-policy") + otherPolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{ + NumVfs: 5, + NodeSelector: map[string]string{"foo": "bar"}, + NicSelector: sriovnetworkv1.SriovNetworkNicSelector{}, + Priority: 20, + } + Expect(k8sClient.Create(context.Background(), otherPolicy)).ToNot(HaveOccurred()) + DeferCleanup(func() { + err := k8sClient.Delete(context.Background(), otherPolicy) + Expect(err).ToNot(HaveOccurred()) + }) + // setup controller manager By("Setup controller manager") k8sManager, err := setupK8sManagerForTest() @@ -125,10 +141,14 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { daemonSet := &appsv1.DaemonSet{} err := util.WaitForNamespacedObject(daemonSet, k8sClient, testNamespace, dsName, util.RetryInterval, util.APITimeout) Expect(err).NotTo(HaveOccurred()) + Expect(daemonSet.OwnerReferences).To(HaveLen(1)) + Expect(daemonSet.OwnerReferences[0].Kind).To(Equal("SriovOperatorConfig")) + Expect(daemonSet.OwnerReferences[0].Name).To(Equal(consts.DefaultConfigName)) }, Entry("operator-webhook", "operator-webhook"), Entry("network-resources-injector", "network-resources-injector"), Entry("sriov-network-config-daemon", "sriov-network-config-daemon"), + Entry("sriov-device-plugin", "sriov-device-plugin"), ) It("should be able to turn network-resources-injector on/off", func() {