Merge pull request #622 from adrianchiris/remove-default-policy

[Remove Default Objs  3/4] Remove default policy

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 {

controllers/sriovnetworknodepolicy_controller.go

@@ -42,6 +42,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	utils "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
@@ -83,30 +84,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 +96,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.
@@ -192,11 +169,23 @@ func (r *SriovNetworkNodePolicyReconciler) SetupWithManager(mgr ctrl.Manager) er
 				Info("Enqueuing sync for delete event", "resource", e.Object.GetName())
 			qHandler(q)
 		},
+		GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) {
+			log.Log.WithName("SriovNetworkNodePolicy").
+				Info("Enqueuing sync for generic event", "resource", e.Object.GetName())
+			qHandler(q)
+		},
 	}
 
+	// send initial sync event to trigger reconcile when controller is started
+	var eventChan = make(chan event.GenericEvent, 1)
+	eventChan <- event.GenericEvent{Object: &sriovnetworkv1.SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: nodePolicySyncEventName, Namespace: ""}}}
+	close(eventChan)
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sriovnetworkv1.SriovNetworkNodePolicy{}).
 		Watches(&sriovnetworkv1.SriovNetworkNodePolicy{}, delayedEventHandler).
+		WatchesRawSource(&source.Channel{Source: eventChan}, delayedEventHandler).
 		Complete(r)
 }
 
@@ -344,6 +333,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 +384,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 +432,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
 		}

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

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())
 		})
 

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
-}