Merge pull request #617 from adrianchiris/dont-create-operator-config

[Remove Default Objs  1/4] Dont create default SriovOperatorConfig

controllers/helper.go

@@ -78,7 +78,12 @@ func GetDefaultNodeSelector() map[string]string {
 		"kubernetes.io/os": "linux"}
 }
 
-func syncPluginDaemonObjs(ctx context.Context, client k8sclient.Client, scheme *runtime.Scheme, dp *sriovnetworkv1.SriovNetworkNodePolicy, pl *sriovnetworkv1.SriovNetworkNodePolicyList) error {
+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")
 
@@ -90,14 +95,7 @@ func syncPluginDaemonObjs(ctx context.Context, client k8sclient.Client, scheme *
 	data.Data["ResourcePrefix"] = os.Getenv("RESOURCE_PREFIX")
 	data.Data["ImagePullSecrets"] = GetImagePullSecrets()
 	data.Data["NodeSelectorField"] = GetDefaultNodeSelector()
-
-	defaultConfig := &sriovnetworkv1.SriovOperatorConfig{}
-	err := client.Get(ctx, types.NamespacedName{
-		Name: constants.DefaultConfigName, Namespace: vars.Namespace}, defaultConfig)
-	if err != nil {
-		return err
-	}
-	data.Data["UseCDI"] = defaultConfig.Spec.UseCDI
+	data.Data["UseCDI"] = dc.Spec.UseCDI
 	objs, err := renderDsForCR(constants.PluginPath, &data)
 	if err != nil {
 		logger.Error(err, "Fail to render SR-IoV manifests")
@@ -116,15 +114,15 @@ func syncPluginDaemonObjs(ctx context.Context, client k8sclient.Client, scheme *
 
 	// Sync DaemonSets
 	for _, obj := range objs {
-		if obj.GetKind() == constants.DaemonSet && len(defaultConfig.Spec.ConfigDaemonNodeSelector) > 0 {
+		if obj.GetKind() == constants.DaemonSet && len(dc.Spec.ConfigDaemonNodeSelector) > 0 {
 			scheme := kscheme.Scheme
 			ds := &appsv1.DaemonSet{}
 			err = scheme.Convert(obj, ds, nil)
 			if err != nil {
 				logger.Error(err, "Fail to convert to DaemonSet")
 				return err
 			}
-			ds.Spec.Template.Spec.NodeSelector = defaultConfig.Spec.ConfigDaemonNodeSelector
+			ds.Spec.Template.Spec.NodeSelector = dc.Spec.ConfigDaemonNodeSelector
 			err = scheme.Convert(ds, obj, nil)
 			if err != nil {
 				logger.Error(err, "Fail to convert to Unstructured")

controllers/sriovnetworknodepolicy_controller.go

@@ -106,6 +106,16 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct
 		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 {
+		if errors.IsNotFound(err) {
+			reqLogger.Info("default SriovOperatorConfig object not found, cannot reconcile SriovNetworkNodePolicies. Requeue.")
+			return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
+		}
+		return reconcile.Result{}, err
+	}
+
 	// Fetch the SriovNetworkNodePolicyList
 	policyList := &sriovnetworkv1.SriovNetworkNodePolicyList{}
 	err = r.List(ctx, policyList, &client.ListOptions{})
@@ -125,10 +135,6 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct
 		"node-role.kubernetes.io/worker": "",
 		"kubernetes.io/os":               "linux",
 	}
-	defaultOpConf := &sriovnetworkv1.SriovOperatorConfig{}
-	if err := r.Get(ctx, types.NamespacedName{Namespace: vars.Namespace, Name: constants.DefaultConfigName}, defaultOpConf); err != nil {
-		return reconcile.Result{}, err
-	}
 	if len(defaultOpConf.Spec.ConfigDaemonNodeSelector) > 0 {
 		labels := client.MatchingLabels(defaultOpConf.Spec.ConfigDaemonNodeSelector)
 		lo = &labels
@@ -151,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, defaultPolicy, policyList); err != nil {
+	if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultOpConf, defaultPolicy, policyList); err != nil {
 		return reconcile.Result{}, err
 	}
 

controllers/sriovoperatorconfig_controller.go

@@ -30,9 +30,11 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	kscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
+	ctrl_builder "sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	machinev1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
@@ -43,7 +45,6 @@ import (
 	snolog "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/log"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms"
 	render "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render"
-	utils "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
 )
 
@@ -69,48 +70,31 @@ type SriovOperatorConfigReconciler struct {
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.8.3/pkg/reconcile
 func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := log.FromContext(ctx).WithValues("sriovoperatorconfig", req.NamespacedName)
-
 	logger.Info("Reconciling SriovOperatorConfig")
 
-	if !vars.EnableAdmissionController {
-		logger.Info("SR-IOV Network Resource Injector and Operator Webhook are disabled.")
-	}
+	// Note: in SetupWithManager we setup manager to enqueue only default config obj
 	defaultConfig := &sriovnetworkv1.SriovOperatorConfig{}
-	err := r.Get(ctx, types.NamespacedName{
-		Name: consts.DefaultConfigName, Namespace: vars.Namespace}, defaultConfig)
+	err := r.Get(ctx, req.NamespacedName, defaultConfig)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			singleNode, err := utils.IsSingleNodeCluster(r.Client)
-			if err != nil {
-				return reconcile.Result{}, fmt.Errorf("couldn't get cluster single node status: %s", err)
-			}
-
-			// Default Config object not found, create it.
-			defaultConfig.SetNamespace(vars.Namespace)
-			defaultConfig.SetName(consts.DefaultConfigName)
-			defaultConfig.Spec = sriovnetworkv1.SriovOperatorConfigSpec{
-				EnableInjector:           vars.EnableAdmissionController,
-				EnableOperatorWebhook:    vars.EnableAdmissionController,
-				ConfigDaemonNodeSelector: map[string]string{},
-				LogLevel:                 2,
-				DisableDrain:             singleNode,
-				ConfigurationMode:        sriovnetworkv1.DaemonConfigurationMode,
-			}
-
-			err = r.Create(ctx, defaultConfig)
-			if err != nil {
-				logger.Error(err, "Failed to create default Operator Config", "Namespace",
-					vars.Namespace, "Name", consts.DefaultConfigName)
-				return reconcile.Result{}, err
-			}
+			logger.Info("default SriovOperatorConfig object not found. waiting for creation.")
 			return reconcile.Result{}, nil
 		}
 		// Error reading the object - requeue the request.
+		logger.Error(err, "Failed to get default SriovOperatorConfig object")
 		return reconcile.Result{}, err
 	}
 
 	snolog.SetLogLevel(defaultConfig.Spec.LogLevel)
 
+	if !defaultConfig.Spec.EnableInjector {
+		logger.Info("SR-IOV Network Resource Injector is disabled.")
+	}
+
+	if !defaultConfig.Spec.EnableOperatorWebhook {
+		logger.Info("SR-IOV Network Operator Webhook is disabled.")
+	}
+
 	// Fetch the SriovNetworkNodePolicyList
 	policyList := &sriovnetworkv1.SriovNetworkNodePolicyList{}
 	err = r.List(ctx, policyList, &client.ListOptions{})
@@ -126,10 +110,6 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 		return reconcile.Result{}, err
 	}
 
-	if req.Namespace != vars.Namespace {
-		return reconcile.Result{}, nil
-	}
-
 	// Render and sync webhook objects
 	if err = r.syncWebhookObjs(ctx, defaultConfig); err != nil {
 		return reconcile.Result{}, err
@@ -140,7 +120,7 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 		return reconcile.Result{}, err
 	}
 
-	if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultPolicy, policyList); err != nil {
+	if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, defaultPolicy, policyList); err != nil {
 		return reconcile.Result{}, err
 	}
 
@@ -160,10 +140,21 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 	return reconcile.Result{RequeueAfter: consts.ResyncPeriod}, nil
 }
 
+// defaultConfigPredicate creates a predicate.Predicate that will return true
+// only for the default sriovoperatorconfig obj.
+func defaultConfigPredicate() predicate.Predicate {
+	return predicate.NewPredicateFuncs(func(object client.Object) bool {
+		if object.GetName() == consts.DefaultConfigName && object.GetNamespace() == vars.Namespace {
+			return true
+		}
+		return false
+	})
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *SriovOperatorConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&sriovnetworkv1.SriovOperatorConfig{}).
+		For(&sriovnetworkv1.SriovOperatorConfig{}, ctrl_builder.WithPredicates(defaultConfigPredicate())).
 		Owns(&appsv1.DaemonSet{}).
 		Owns(&corev1.ConfigMap{}).
 		Complete(r)

controllers/sriovoperatorconfig_controller_test.go

@@ -134,11 +134,10 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 		It("should be able to turn network-resources-injector on/off", func() {
 			By("set disable to enableInjector")
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 
 			config.Spec.EnableInjector = false
-			err = k8sClient.Update(ctx, config)
+			err := k8sClient.Update(ctx, config)
 			Expect(err).NotTo(HaveOccurred())
 
 			daemonSet := &appsv1.DaemonSet{}
@@ -170,11 +169,10 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 
 			By("set disable to enableOperatorWebhook")
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 
 			config.Spec.EnableOperatorWebhook = false
-			err = k8sClient.Update(ctx, config)
+			err := k8sClient.Update(ctx, config)
 			Expect(err).NotTo(HaveOccurred())
 
 			daemonSet := &appsv1.DaemonSet{}
@@ -190,8 +188,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			By("set disable to enableOperatorWebhook")
-			err = util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 
 			config.Spec.EnableOperatorWebhook = true
 			err = k8sClient.Update(ctx, config)
@@ -213,10 +210,10 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 		It("should be able to update the node selector of sriov-network-config-daemon", func() {
 			By("specify the configDaemonNodeSelector")
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
+
 			config.Spec.ConfigDaemonNodeSelector = map[string]string{"node-role.kubernetes.io/worker": ""}
-			err = k8sClient.Update(ctx, config)
+			err := k8sClient.Update(ctx, config)
 			Expect(err).NotTo(HaveOccurred())
 
 			daemonSet := &appsv1.DaemonSet{}
@@ -233,10 +230,9 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 		It("should be able to do multiple updates to the node selector of sriov-network-config-daemon", func() {
 			By("changing the configDaemonNodeSelector")
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 			config.Spec.ConfigDaemonNodeSelector = map[string]string{"labelA": "", "labelB": "", "labelC": ""}
-			err = k8sClient.Update(ctx, config)
+			err := k8sClient.Update(ctx, config)
 			Expect(err).NotTo(HaveOccurred())
 			config.Spec.ConfigDaemonNodeSelector = map[string]string{"labelA": "", "labelB": ""}
 			err = k8sClient.Update(ctx, config)
@@ -254,8 +250,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 
 		It("should not render disable-plugins cmdline flag of sriov-network-config-daemon if disablePlugin not provided in spec", func() {
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 
 			Eventually(func() string {
 				daemonSet := &appsv1.DaemonSet{}
@@ -269,11 +264,10 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 
 		It("should render disable-plugins cmdline flag of sriov-network-config-daemon if disablePlugin provided in spec", func() {
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
 
 			config.Spec.DisablePlugins = sriovnetworkv1.PluginNameSlice{"mellanox"}
-			err = k8sClient.Update(ctx, config)
+			err := k8sClient.Update(ctx, config)
 			Expect(err).NotTo(HaveOccurred())
 
 			Eventually(func() string {

controllers/suite_test.go

@@ -122,7 +122,6 @@ var _ = BeforeSuite(func() {
 	By("setting up env variables for tests")
 	os.Setenv("RESOURCE_PREFIX", "openshift.io")
 	os.Setenv("NAMESPACE", "openshift-sriov-network-operator")
-	os.Setenv("ADMISSION_CONTROLLERS_ENABLED", "true")
 	os.Setenv("ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME", "operator-webhook-cert")
 	os.Setenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME", "network-resources-injector-cert")
 	os.Setenv("SRIOV_CNI_IMAGE", "mock-image")

deploy/operator.yaml

@@ -66,8 +66,6 @@ spec:
               value: $SRIOV_NETWORK_WEBHOOK_IMAGE
             - name: RESOURCE_PREFIX
               value: $RESOURCE_PREFIX
-            - name: ADMISSION_CONTROLLERS_ENABLED
-              value: "$ADMISSION_CONTROLLERS_ENABLED"
             - name: DEV_MODE
               value: "$DEV_MODE"
             - name: NAMESPACE

deploy/sriovoperatorconfig.yaml

@@ -0,0 +1,11 @@
+apiVersion: sriovnetwork.openshift.io/v1
+kind: SriovOperatorConfig
+metadata:
+  name: default
+spec:
+  enableInjector: $ADMISSION_CONTROLLERS_ENABLED
+  enableOperatorWebhook: $ADMISSION_CONTROLLERS_ENABLED
+  configDaemonNodeSelector: {}
+  logLevel: 2
+  disableDrain: false
+  configurationMode: daemon

deployment/sriov-network-operator/README.md

@@ -104,6 +104,18 @@ controllers, which can be found in the table below. In a nutshell, the modes tha
 | `operator.admissionControllers.certificates.custom.injector.tlsCrt` | string | `` | The public part of the certificate to be used by the Network Resources Injector's admission controller |
 | `operator.admissionControllers.certificates.custom.injector.tlsKey` | string | `` | The private part of the certificate to be used by the Network Resources Injector's admission controller |
 
+### SR-IOV Operator Configuration Parameters
+
+This section contains general parameters that apply to both the operator and daemon componets of SR-IOV Network Operator.
+
+| Name | Type | Default | description |
+| ---- | ---- | ------- | ----------- |
+| `sriovOperatorConfig.deploy` | bool | `false` | deploy SriovOperatorConfig custom resource |
+| `sriovOperatorConfig.configDaemonNodeSelector` | map[string]string | `{}` | node slectors for sriov-network-config-daemon |
+| `sriovOperatorConfig.logLevel` | int | `2` | log level for both operator and sriov-network-config-daemon |
+| `sriovOperatorConfig.disableDrain` | bool | `false` | disable node draining when configuring SR-IOV, set to true in case of a single node cluster or any other justifiable reason |
+| `sriovOperatorConfig.configurationMode` | string | `daemon` | sriov-network-config-daemon configuration mode. either `daemon` or `systemd` |
+
 ### Images parameters
 
 | Name | description |

deployment/sriov-network-operator/templates/operator.yaml

@@ -88,8 +88,6 @@ spec:
               value: {{ .Values.operator.cniBinPath }}
             - name: CLUSTER_TYPE
               value: {{ .Values.operator.clusterType }}
-            - name: ADMISSION_CONTROLLERS_ENABLED
-              value: {{ .Values.operator.admissionControllers.enabled | quote }}
         {{- if .Values.operator.admissionControllers.enabled }}
             - name: ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_SECRET_NAME
               value: {{ .Values.operator.admissionControllers.certificates.secretNames.operator }}

deployment/sriov-network-operator/templates/sriovoperatorconfig.yaml

@@ -0,0 +1,17 @@
+{{ if .Values.sriovOperatorConfig.deploy }}
+apiVersion: sriovnetwork.openshift.io/v1
+kind: SriovOperatorConfig
+metadata:
+  name: default
+  namespace: {{ .Release.Namespace }}
+spec:
+  enableInjector: {{ .Values.operator.admissionControllers.enabled }}
+  enableOperatorWebhook: {{ .Values.operator.admissionControllers.enabled }}
+  {{- with .Values.sriovOperatorConfig.configDaemonNodeSelector }}
+  configDaemonNodeSelector:
+    {{- range $k, $v := .}}{{printf "%s: %s" $k $v | nindent 4 }}{{ end }}
+  {{- end }}
+  logLevel: {{ .Values.sriovOperatorConfig.logLevel }}
+  disableDrain: {{ .Values.sriovOperatorConfig.disableDrain }}
+  configurationMode: {{ .Values.sriovOperatorConfig.configurationMode }}
+{{ end }}

deployment/sriov-network-operator/values.yaml

@@ -76,6 +76,19 @@ operator:
     #       ...
     #      -----END EC PRIVATE KEY-----
 
+sriovOperatorConfig:
+  # deploy sriovOperatorConfig CR with the below values
+  deploy: false
+  # node slectors for sriov-network-config-daemon
+  configDaemonNodeSelector: {}
+  # log level for both operator and sriov-network-config-daemon
+  logLevel: 2
+  # disable node draining when configuring SR-IOV, set to true in case of a single node
+  # cluster or any other justifiable reason
+  disableDrain: false
+  # sriov-network-config-daemon configuration mode. either "daemon" or "systemd"
+  configurationMode: daemon
+
 # Image URIs for sriov-network-operator components
 images:
   operator: ghcr.io/k8snetworkplumbingwg/sriov-network-operator