Fix comments

DNM before squash

Signed-off-by: Sebastian Sch <[email protected]>

.github/workflows/test.yml

@@ -40,16 +40,19 @@ jobs:
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2
 
-    - name: test pkg
-      run: make test-pkg
+    - name: test pkg on kubernetes
+      run: CLUSTER_TYPE=kubernetes make test-pkg
+
+    - name: test pkg on openshift
+      run: CLUSTER_TYPE=openshift make test-pkg
 
     - name: test cmd
       run: make test-cmd
 
     - name: test api
       run: make test-api
 
-    - name: test controllers on opensfhit
+    - name: test controllers on openshift
       run: CLUSTER_TYPE=openshift make test-controllers
 
     - name: test controllers on kubernetes

cmd/sriov-network-config-daemon/start.go

@@ -145,7 +145,7 @@ func configGlobalVariables() error {
 	return nil
 }
 
-func UseKubeletKubeConfig() {
+func useKubeletKubeConfig() {
 	fnLogger := log.Log.WithName("sriov-network-config-daemon")
 
 	kubeconfig, err := clientcmd.LoadFromFile("/host/etc/kubernetes/kubeconfig")
@@ -174,7 +174,6 @@ func UseKubeletKubeConfig() {
 }
 
 func getOperatorConfig(kClient runtimeclient.Client) (*sriovnetworkv1.SriovOperatorConfig, error) {
-	// Init feature gates once to prevent race conditions.
 	defaultConfig := &sriovnetworkv1.SriovOperatorConfig{}
 	err := kClient.Get(context.Background(), types.NamespacedName{Namespace: vars.Namespace, Name: consts.DefaultConfigName}, defaultConfig)
 	if err != nil {
@@ -228,7 +227,7 @@ func runStartCmd(cmd *cobra.Command, args []string) error {
 	// On openshift we use the kubeconfig from kubelet on the node where the daemon is running
 	// this allow us to improve security as every daemon has access only to its own node
 	if vars.ClusterType == consts.ClusterTypeOpenshift {
-		UseKubeletKubeConfig()
+		useKubeletKubeConfig()
 	}
 
 	kubeconfig := os.Getenv("KUBECONFIG")
@@ -343,19 +342,19 @@ func runStartCmd(cmd *cobra.Command, args []string) error {
 		startOpts.disabledPlugins)
 
 	// Init Daemon configuration on the node
-	if err = dm.DaemonInitialization(); err != nil {
+	if err = dm.Init(); err != nil {
 		setupLog.Error(err, "unable to initialize daemon")
 		os.Exit(1)
 	}
 
 	// Setup reconcile loop with manager
 	if err = dm.SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create setup daemon manager for SriovNetworkNodeState")
+		setupLog.Error(err, "unable to setup daemon with manager for SriovNetworkNodeState")
 		os.Exit(1)
 	}
 
 	// Setup reconcile loop with manager
-	if err = daemon.NewOperatorConfigReconcile(kClient).SetupWithManager(mgr); err != nil {
+	if err = daemon.NewOperatorConfigNodeReconcile(kClient).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create setup daemon manager for OperatorConfig")
 		os.Exit(1)
 	}

controllers/drain_controller.go

@@ -108,7 +108,7 @@ func (dr *DrainReconcile) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	// create the drain state annotation if it doesn't exist in the sriovNetworkNodeState object
 	nodeStateDrainAnnotationCurrent, currentNodeStateExist, err := dr.ensureAnnotationExists(ctx, nodeNetworkState, constants.NodeStateDrainAnnotationCurrent)
 	if err != nil {
-		reqLogger.Error(err, "failed to ensure nodeStateDrainAnnotation")
+		reqLogger.Error(err, "failed to ensure nodeStateDrainAnnotationCurrent")
 		return ctrl.Result{}, err
 	}
 	_, desireNodeStateExist, err := dr.ensureAnnotationExists(ctx, nodeNetworkState, constants.NodeStateDrainAnnotation)

controllers/drain_controller_helper.go

@@ -3,10 +3,8 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"github.com/go-logr/logr"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -44,7 +42,7 @@ func (dr *DrainReconcile) handleNodeIdleNodeStateDrainingOrCompleted(ctx context
 			"DrainController",
 			"node complete drain was not completed")
 		// TODO: make this time configurable
-		return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
+		return reconcile.Result{RequeueAfter: constants.DrainControllerRequeueTime}, nil
 	}
 
 	// move the node state back to idle
@@ -106,7 +104,7 @@ func (dr *DrainReconcile) handleNodeDrainOrReboot(ctx context.Context,
 			corev1.EventTypeWarning,
 			"DrainController",
 			"node drain operation was not completed")
-		return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
+		return reconcile.Result{RequeueAfter: constants.DrainControllerRequeueTime}, nil
 	}
 
 	// if we manage to drain we label the node state with drain completed and finish
@@ -180,7 +178,7 @@ func (dr *DrainReconcile) tryDrainNode(ctx context.Context, node *corev1.Node) (
 		// the node requested to be drained, but we are at the limit so we re-enqueue the request
 		reqLogger.Info("MaxParallelNodeConfiguration limit reached for draining nodes re-enqueue the request")
 		// TODO: make this time configurable
-		return &reconcile.Result{RequeueAfter: 5 * time.Second}, nil
+		return &reconcile.Result{RequeueAfter: constants.DrainControllerRequeueTime}, nil
 	}
 
 	if currentSnns == nil {

controllers/sriovnetworknodepolicy_controller.go

@@ -88,7 +88,7 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct
 	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{RequeueAfter: constants.DrainControllerRequeueTime}, nil
 		}
 		return reconcile.Result{}, err
 	}
@@ -226,7 +226,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncDevicePluginConfigMap(ctx context
 		}
 		configData[node.Name] = string(config)
 
-		if data.ResourceList == nil || len(data.ResourceList) == 0 {
+		if len(data.ResourceList) == 0 {
 			// if we don't have policies we should add the disabled label for the device plugin
 			err = utils.LabelNode(ctx, node.Name, constants.SriovDevicePluginLabel, constants.SriovDevicePluginLabelDisabled, r.Client)
 			if err != nil {

pkg/consts/constants.go

@@ -14,7 +14,10 @@ const (
 	Chroot = "/host"
 	Host   = "/host"
 
-	ResyncPeriod                       = 5 * time.Minute
+	ResyncPeriod               = 5 * time.Minute
+	DaemonRequeueTime          = 30 * time.Second
+	DrainControllerRequeueTime = 5 * time.Second
+
 	DefaultConfigName                  = "default"
 	ConfigDaemonPath                   = "./bindata/manifests/daemon"
 	InjectorWebHookPath                = "./bindata/manifests/webhook"

pkg/daemon/config.go

@@ -14,16 +14,19 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
 )
 
-type OperatorConfigReconcile struct {
+// OperatorConfigNodeReconcile represents the reconcile struct for the OperatorConfig.
+type OperatorConfigNodeReconcile struct {
 	client             client.Client
 	latestFeatureGates map[string]bool
 }
 
-func NewOperatorConfigReconcile(client client.Client) *OperatorConfigReconcile {
-	return &OperatorConfigReconcile{client: client, latestFeatureGates: make(map[string]bool)}
+// NewOperatorConfigNodeReconcile creates a new instance of OperatorConfigNodeReconcile with the given client.
+func NewOperatorConfigNodeReconcile(client client.Client) *OperatorConfigNodeReconcile {
+	return &OperatorConfigNodeReconcile{client: client, latestFeatureGates: make(map[string]bool)}
 }
 
-func (oc *OperatorConfigReconcile) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+// Reconcile reconciles the OperatorConfig resource. It updates log level and feature gates as necessary.
+func (oc *OperatorConfigNodeReconcile) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	reqLogger := log.FromContext(ctx).WithName("Reconcile")
 	operatorConfig := &sriovnetworkv1.SriovOperatorConfig{}
 	err := oc.client.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: req.Name}, operatorConfig)
@@ -32,7 +35,7 @@ func (oc *OperatorConfigReconcile) Reconcile(ctx context.Context, req ctrl.Reque
 			reqLogger.Info("OperatorConfig doesn't exist", "name", req.Name, "namespace", req.Namespace)
 			return ctrl.Result{}, nil
 		}
-		reqLogger.Error(err, "Failed to operatorConfig", "name", req.Name, "namespace", req.Namespace)
+		reqLogger.Error(err, "Failed to get OperatorConfig", "name", req.Name, "namespace", req.Namespace)
 		return ctrl.Result{}, err
 	}
 
@@ -54,7 +57,8 @@ func (oc *OperatorConfigReconcile) Reconcile(ctx context.Context, req ctrl.Reque
 	return ctrl.Result{}, nil
 }
 
-func (oc *OperatorConfigReconcile) SetupWithManager(mgr ctrl.Manager) error {
+// SetupWithManager sets up the reconciliation logic for this controller using the given manager.
+func (oc *OperatorConfigNodeReconcile) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sriovnetworkv1.SriovOperatorConfig{}).
 		Complete(oc)

pkg/daemon/config_test.go

@@ -31,7 +31,7 @@ var _ = Describe("Daemon OperatorConfig Controller", Ordered, func() {
 		})
 		Expect(err).ToNot(HaveOccurred())
 
-		configController := daemon.NewOperatorConfigReconcile(k8sClient)
+		configController := daemon.NewOperatorConfigNodeReconcile(k8sClient)
 		err = configController.SetupWithManager(k8sManager)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -57,7 +57,7 @@ var _ = Describe("Daemon OperatorConfig Controller", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
-	BeforeEach(func() {
+	AfterEach(func() {
 		Expect(k8sClient.DeleteAllOf(context.Background(), &sriovnetworkv1.SriovOperatorConfig{}, client.InNamespace(testNamespace))).ToNot(HaveOccurred())
 	})