From dc8c040cffd99ae708057c73292df3ba25e3811c Mon Sep 17 00:00:00 2001 From: adrianc Date: Tue, 6 Feb 2024 18:43:42 +0200 Subject: [PATCH] Remove the creation of default SriovNetworkNodePolicy There is no need to create it anymore. Signed-off-by: adrianc --- controllers/helper.go | 15 +++++++- .../sriovnetworknodepolicy_controller.go | 33 ++++------------- controllers/sriovoperatorconfig_controller.go | 7 ---- .../sriovoperatorconfig_controller_test.go | 27 +++----------- .../templates/sriovnetworknodepolicy.yaml | 10 ----- main.go | 37 ------------------- 6 files changed, 28 insertions(+), 101 deletions(-) delete mode 100644 deployment/sriov-network-operator/templates/sriovnetworknodepolicy.yaml diff --git a/controllers/helper.go b/controllers/helper.go index f341f5f73..cf63c8b26 100644 --- a/controllers/helper.go +++ b/controllers/helper.go @@ -78,6 +78,19 @@ func GetDefaultNodeSelector() map[string]string { "kubernetes.io/os": "linux"} } +// hasNoValidPolicy returns true if no SriovNetworkNodePolicy +// or only the (deprecated) "default" policy is present +func hasNoValidPolicy(pl []sriovnetworkv1.SriovNetworkNodePolicy) bool { + switch len(pl) { + case 0: + return true + case 1: + return pl[0].Name == constants.DefaultPolicyName + default: + return false + } +} + func syncPluginDaemonObjs(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, @@ -101,7 +114,7 @@ func syncPluginDaemonObjs(ctx context.Context, return err } - if len(pl.Items) < 2 { + if hasNoValidPolicy(pl.Items) { for _, obj := range objs { err := deleteK8sResource(ctx, client, obj) if err != nil { diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index 0b59097ff..6efbd0b8e 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -83,30 +83,6 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct reqLogger := log.FromContext(ctx) reqLogger.Info("Reconciling") - defaultPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} - err := r.Get(ctx, types.NamespacedName{Name: constants.DefaultPolicyName, Namespace: vars.Namespace}, defaultPolicy) - if err != nil { - if errors.IsNotFound(err) { - // Default policy object not found, create it. - defaultPolicy.SetNamespace(vars.Namespace) - defaultPolicy.SetName(constants.DefaultPolicyName) - defaultPolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{ - NumVfs: 0, - NodeSelector: make(map[string]string), - NicSelector: sriovnetworkv1.SriovNetworkNicSelector{}, - } - err = r.Create(ctx, defaultPolicy) - if err != nil { - reqLogger.Error(err, "Failed to create default Policy", "Namespace", vars.Namespace, "Name", constants.DefaultPolicyName) - return reconcile.Result{}, err - } - reqLogger.Info("Default policy created") - return reconcile.Result{}, nil - } - // Error reading the object - requeue the request. - return reconcile.Result{}, err - } - // Fetch the default SriovOperatorConfig defaultOpConf := &sriovnetworkv1.SriovOperatorConfig{} if err := r.Get(ctx, types.NamespacedName{Namespace: vars.Namespace, Name: constants.DefaultConfigName}, defaultOpConf); err != nil { @@ -119,7 +95,7 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct // Fetch the SriovNetworkNodePolicyList policyList := &sriovnetworkv1.SriovNetworkNodePolicyList{} - err = r.List(ctx, policyList, &client.ListOptions{}) + err := r.List(ctx, policyList, &client.ListOptions{}) if err != nil { if errors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -344,6 +320,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncSriovNetworkNodeState(ctx context // it should not matter since the flag used in p.Apply() will only be applied when VF partition is detected. ppp := 100 for _, p := range npl.Items { + // Note(adrianc): default policy is deprecated and ignored. if p.Name == constants.DefaultPolicyName { continue } @@ -394,6 +371,11 @@ func setDsNodeAffinity(pl *sriovnetworkv1.SriovNetworkNodePolicyList, ds *appsv1 func nodeSelectorTermsForPolicyList(policies []sriovnetworkv1.SriovNetworkNodePolicy) []corev1.NodeSelectorTerm { terms := []corev1.NodeSelectorTerm{} for _, p := range policies { + // Note(adrianc): default policy is deprecated and ignored. + if p.Name == constants.DefaultPolicyName { + continue + } + if len(p.Spec.NodeSelector) == 0 { continue } @@ -437,6 +419,7 @@ func (r *SriovNetworkNodePolicyReconciler) renderDevicePluginConfigData(ctx cont logger.V(1).Info("Start to render device plugin config data", "node", node.Name) rcl := dptypes.ResourceConfList{} for _, p := range pl.Items { + // Note(adrianc): default policy is deprecated and ignored. if p.Name == constants.DefaultPolicyName { continue } diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index c4eeec339..dcf13c46a 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -103,13 +103,6 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl. return reconcile.Result{}, err } - defaultPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} - err = r.Get(ctx, types.NamespacedName{Name: consts.DefaultPolicyName, Namespace: vars.Namespace}, defaultPolicy) - if err != nil { - // Error reading the object - requeue the request. - return reconcile.Result{}, err - } - // Render and sync webhook objects if err = r.syncWebhookObjs(ctx, defaultConfig); err != nil { return reconcile.Result{}, err diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go index bbb777e45..cb3f4817b 100644 --- a/controllers/sriovoperatorconfig_controller_test.go +++ b/controllers/sriovoperatorconfig_controller_test.go @@ -42,33 +42,18 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { Expect(err).ToNot(HaveOccurred()) }) - // Create default SriovNetworkNodePolicy - defaultPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} - defaultPolicy.SetNamespace(testNamespace) - defaultPolicy.SetName(constants.DefaultPolicyName) - defaultPolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{ - NumVfs: 0, - NodeSelector: make(map[string]string), - NicSelector: sriovnetworkv1.SriovNetworkNicSelector{}, - } - Expect(k8sClient.Create(context.Background(), defaultPolicy)).Should(Succeed()) - DeferCleanup(func() { - err := k8sClient.Delete(context.Background(), defaultPolicy) - Expect(err).ToNot(HaveOccurred()) - }) - - otherPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} - otherPolicy.SetNamespace(testNamespace) - otherPolicy.SetName("other-policy") - otherPolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{ + somePolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} + somePolicy.SetNamespace(testNamespace) + somePolicy.SetName("some-policy") + somePolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{ NumVfs: 5, NodeSelector: map[string]string{"foo": "bar"}, NicSelector: sriovnetworkv1.SriovNetworkNicSelector{}, Priority: 20, } - Expect(k8sClient.Create(context.Background(), otherPolicy)).ToNot(HaveOccurred()) + Expect(k8sClient.Create(context.Background(), somePolicy)).ToNot(HaveOccurred()) DeferCleanup(func() { - err := k8sClient.Delete(context.Background(), otherPolicy) + err := k8sClient.Delete(context.Background(), somePolicy) Expect(err).ToNot(HaveOccurred()) }) diff --git a/deployment/sriov-network-operator/templates/sriovnetworknodepolicy.yaml b/deployment/sriov-network-operator/templates/sriovnetworknodepolicy.yaml deleted file mode 100644 index 7f2934033..000000000 --- a/deployment/sriov-network-operator/templates/sriovnetworknodepolicy.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: sriovnetwork.openshift.io/v1 -kind: SriovNetworkNodePolicy -metadata: - name: default - namespace: {{ .Release.Namespace }} -spec: - nicSelector: {} - nodeSelector: {} - numVfs: 0 - resourceName: "" diff --git a/main.go b/main.go index 4912d93b9..f1ca9c368 100644 --- a/main.go +++ b/main.go @@ -24,7 +24,6 @@ import ( netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" openshiftconfigv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - "k8s.io/apimachinery/pkg/api/errors" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" @@ -34,7 +33,6 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -48,7 +46,6 @@ import ( sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" "github.com/k8snetworkplumbingwg/sriov-network-operator/controllers" - constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/leaderelection" snolog "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/log" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" @@ -182,13 +179,6 @@ func main() { } // +kubebuilder:scaffold:builder - // Create a default SriovNetworkNodePolicy - err = createDefaultPolicy(kubeClient) - if err != nil { - setupLog.Error(err, "unable to create default SriovNetworkNodePolicy") - os.Exit(1) - } - if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") os.Exit(1) @@ -225,30 +215,3 @@ func initNicIDMap() error { return nil } - -func createDefaultPolicy(c client.Client) error { - logger := setupLog.WithName("createDefaultPolicy") - policy := &sriovnetworkv1.SriovNetworkNodePolicy{ - Spec: sriovnetworkv1.SriovNetworkNodePolicySpec{ - NumVfs: 0, - NodeSelector: make(map[string]string), - NicSelector: sriovnetworkv1.SriovNetworkNicSelector{}, - }, - } - namespace := os.Getenv("NAMESPACE") - err := c.Get(context.TODO(), types.NamespacedName{Name: constants.DefaultPolicyName, Namespace: namespace}, policy) - if err != nil { - if errors.IsNotFound(err) { - logger.Info("Create a default SriovNetworkNodePolicy") - policy.Namespace = namespace - policy.Name = constants.DefaultPolicyName - err = c.Create(context.TODO(), policy) - if err != nil { - return err - } - } - // Error reading the object - requeue the request. - return err - } - return nil -}