setting nodestate annotation with 'sriovnetwork.openshift.io/use-external-drainer=true' in case exteranl drainer is enabled

The motivation is for external drainer verification, that SRIOV operator is set with external drainer
Signed-off-by: Ido Heyvi <[email protected]>

Author: heyvister1

bindata/manifests/daemon/daemonset.yaml

@@ -169,6 +169,10 @@ spec:
             value: "{{.ClusterType}}"
           - name: DEV_MODE
             value: "{{.DevMode}}"
+        {{- if .UseExternalDrainer }}
+          - name: USE_EXTERNAL_DRAINER
+            value: "{{.UseExternalDrainer}}"
+        {{- end }}
         resources:
           requests:
             cpu: 100m

controllers/drain_controller.go

@@ -106,6 +106,13 @@ func (dr *DrainReconcile) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return ctrl.Result{}, nil
 	}
 
+	// check if use-external-drainer is set for a specific node state
+	if utils.ObjectHasAnnotation(nodeNetworkState, constants.NodeStateExternalDrainerAnnotation, "true") {
+		reqLogger.Info(`nodestate is set with external drainer annotation, don't requeue the request. 
+		Draining will be done externally`, "nodeState", req.Name)
+		return ctrl.Result{}, nil
+	}
+
 	// 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 {

controllers/drain_controller_test.go

@@ -113,9 +113,19 @@ var _ = Describe("Drain Controller", Ordered, func() {
 	})
 
 	Context("when there is only one node", func() {
+		It("should not drain node on drain require while use-external-drainer annotation is set",
+			func(ctx context.Context) {
+				node, nodeState := createNode(ctx, "node1", true)
+
+				simulateDaemonSetAnnotation(node, constants.DrainRequired)
+
+				expectNodeStateAnnotation(nodeState, constants.DrainIdle)
+				expectNodeIsSchedulable(node)
+
+			})
 
 		It("should drain single node on drain require", func(ctx context.Context) {
-			node, nodeState := createNode(ctx, "node1")
+			node, nodeState := createNode(ctx, "node1", false)
 
 			simulateDaemonSetAnnotation(node, constants.DrainRequired)
 
@@ -129,7 +139,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should not drain on reboot for single node", func(ctx context.Context) {
-			node, nodeState := createNode(ctx, "node1")
+			node, nodeState := createNode(ctx, "node1", false)
 
 			simulateDaemonSetAnnotation(node, constants.RebootRequired)
 
@@ -142,8 +152,8 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should drain on reboot for multiple node", func(ctx context.Context) {
-			node, nodeState := createNode(ctx, "node1")
-			createNode(ctx, "node2")
+			node, nodeState := createNode(ctx, "node1", false)
+			createNode(ctx, "node2", false)
 
 			simulateDaemonSetAnnotation(node, constants.RebootRequired)
 
@@ -159,9 +169,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
 	Context("when there are multiple nodes", func() {
 
 		It("should drain nodes serially with default pool selector", func(ctx context.Context) {
-			node1, nodeState1 := createNode(ctx, "node1")
-			node2, nodeState2 := createNode(ctx, "node2")
-			node3, nodeState3 := createNode(ctx, "node3")
+			node1, nodeState1 := createNode(ctx, "node1", false)
+			node2, nodeState2 := createNode(ctx, "node2", false)
+			node3, nodeState3 := createNode(ctx, "node3", false)
 
 			// Two nodes require to drain at the same time
 			simulateDaemonSetAnnotation(node1, constants.DrainRequired)
@@ -199,9 +209,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should drain nodes in parallel with a custom pool selector", func(ctx context.Context) {
-			node1, nodeState1 := createNode(ctx, "node1")
-			node2, nodeState2 := createNode(ctx, "node2")
-			node3, nodeState3 := createNode(ctx, "node3")
+			node1, nodeState1 := createNode(ctx, "node1", false)
+			node2, nodeState2 := createNode(ctx, "node2", false)
+			node3, nodeState3 := createNode(ctx, "node3", false)
 
 			maxun := intstr.Parse("2")
 			poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
@@ -250,9 +260,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should drain nodes in parallel with a custom pool selector and honor MaxUnavailable", func(ctx context.Context) {
-			node1, nodeState1 := createNode(ctx, "node1")
-			node2, nodeState2 := createNode(ctx, "node2")
-			node3, nodeState3 := createNode(ctx, "node3")
+			node1, nodeState1 := createNode(ctx, "node1", false)
+			node2, nodeState2 := createNode(ctx, "node2", false)
+			node3, nodeState3 := createNode(ctx, "node3", false)
 
 			maxun := intstr.Parse("2")
 			poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
@@ -275,9 +285,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should drain all nodes in parallel with a custom pool using nil in max unavailable", func(ctx context.Context) {
-			node1, nodeState1 := createNode(ctx, "node1")
-			node2, nodeState2 := createNode(ctx, "node2")
-			node3, nodeState3 := createNode(ctx, "node3")
+			node1, nodeState1 := createNode(ctx, "node1", false)
+			node2, nodeState2 := createNode(ctx, "node2", false)
+			node3, nodeState3 := createNode(ctx, "node3", false)
 
 			poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
 			poolConfig.SetNamespace(testNamespace)
@@ -303,7 +313,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should drain in parallel nodes from two different pools, one custom and one default", func() {
-			node1, nodeState1 := createNode(ctx, "node1")
+			node1, nodeState1 := createNode(ctx, "node1", false)
 			node2, nodeState2 := createNodeWithLabel(ctx, "node2", "pool")
 			createPodOnNode(ctx, "test-node-2", "node2")
 
@@ -326,7 +336,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
 		})
 
 		It("should select all the nodes to drain in parallel when the selector is empty", func() {
-			node1, nodeState1 := createNode(ctx, "node3")
+			node1, nodeState1 := createNode(ctx, "node3", false)
 			node2, nodeState2 := createNodeWithLabel(ctx, "node4", "pool")
 			createPodOnNode(ctx, "test-empty-1", "node3")
 			createPodOnNode(ctx, "test-empty-2", "node4")
@@ -408,7 +418,7 @@ func simulateDaemonSetAnnotation(node *corev1.Node, drainAnnotationValue string)
 		ToNot(HaveOccurred())
 }
 
-func createNode(ctx context.Context, nodeName string) (*corev1.Node, *sriovnetworkv1.SriovNetworkNodeState) {
+func createNode(ctx context.Context, nodeName string, useExternalDrainer bool) (*corev1.Node, *sriovnetworkv1.SriovNetworkNodeState) {
 	node := corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: nodeName,
@@ -426,12 +436,16 @@ func createNode(ctx context.Context, nodeName string) (*corev1.Node, *sriovnetwo
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      nodeName,
 			Namespace: vars.Namespace,
-			Labels: map[string]string{
+			Annotations: map[string]string{
 				constants.NodeStateDrainAnnotationCurrent: constants.DrainIdle,
 			},
 		},
 	}
 
+	if useExternalDrainer {
+		nodeState.Annotations[constants.NodeStateExternalDrainerAnnotation] = "true"
+	}
+
 	Expect(k8sClient.Create(ctx, &node)).ToNot(HaveOccurred())
 	Expect(k8sClient.Create(ctx, &nodeState)).ToNot(HaveOccurred())
 

controllers/sriovoperatorconfig_controller.go

@@ -196,6 +196,7 @@ func (r *SriovOperatorConfigReconciler) syncConfigDaemonSet(ctx context.Context,
 	data.Data["ReleaseVersion"] = os.Getenv("RELEASEVERSION")
 	data.Data["ClusterType"] = vars.ClusterType
 	data.Data["DevMode"] = os.Getenv("DEV_MODE")
+	data.Data["UseExternalDrainer"] = vars.UseExternalDrainer
 	data.Data["ImagePullSecrets"] = GetImagePullSecrets()
 	if dc.Spec.ConfigurationMode == sriovnetworkv1.SystemdConfigurationMode {
 		data.Data["UsedSystemdMode"] = true

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

@@ -78,8 +78,10 @@ spec:
               value: {{ .Values.operator.metricsExporter.certificates.secretName }}
             - name: METRICS_EXPORTER_KUBE_RBAC_PROXY_IMAGE
               value: {{ .Values.images.metricsExporterKubeRbacProxy }}
+            {{- if .Values.operator.externalDrainer.enabled }}
             - name: USE_EXTERNAL_DRAINER
               value: {{ .Values.operator.externalDrainer.enabled | quote }}
+            {{- end }}
             {{- if .Values.operator.metricsExporter.prometheusOperator.enabled }}
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED
               value: {{ .Values.operator.metricsExporter.prometheusOperator.enabled | quote}}

main.go

@@ -208,33 +208,8 @@ func main() {
 		os.Exit(1)
 	}
 
-	// we need a client that doesn't use the local cache for the objects
-	drainKClient, err := client.New(restConfig, client.Options{
-		Scheme: scheme,
-		Cache: &client.CacheOptions{
-			DisableFor: []client.Object{
-				&sriovnetworkv1.SriovNetworkNodeState{},
-				&corev1.Node{},
-				&mcfgv1.MachineConfigPool{},
-			},
-		},
-	})
-	if err != nil {
-		setupLog.Error(err, "unable to create drain kubernetes client")
-		os.Exit(1)
-	}
-
-	drainController, err := controllers.NewDrainReconcileController(drainKClient,
-		mgr.GetScheme(),
-		mgr.GetEventRecorderFor("SR-IOV operator"),
-		platformsHelper)
-	if err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "DrainReconcile")
-		os.Exit(1)
-	}
-
-	if err = drainController.SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to setup controller with manager", "controller", "DrainReconcile")
+	if err := setupDrainController(mgr, restConfig, platformsHelper, mgr.GetScheme()); err != nil {
+		setupLog.Error(err, "unable to setup drain controller")
 		os.Exit(1)
 	}
 	// +kubebuilder:scaffold:builder
@@ -299,8 +274,7 @@ func main() {
 func setupDrainController(mgr ctrl.Manager, restConfig *rest.Config,
 	platformsHelper platforms.Interface, scheme *runtime.Scheme) error {
 	if vars.UseExternalDrainer {
-		setupLog.Info("internal drain controller is disabled, draining will be done externally by the maintenance operator")
-		return nil
+		setupLog.Info("'UseExternalDrainer' is set, draining will be done externally")
 	}
 
 	// we need a client that doesn't use the local cache for the objects

pkg/consts/constants.go

@@ -76,14 +76,15 @@ const (
 	SriovDevicePluginLabelEnabled  = "Enabled"
 	SriovDevicePluginLabelDisabled = "Disabled"
 
-	NodeDrainAnnotation             = "sriovnetwork.openshift.io/state"
-	NodeStateDrainAnnotation        = "sriovnetwork.openshift.io/desired-state"
-	NodeStateDrainAnnotationCurrent = "sriovnetwork.openshift.io/current-state"
-	DrainIdle                       = "Idle"
-	DrainRequired                   = "Drain_Required"
-	RebootRequired                  = "Reboot_Required"
-	Draining                        = "Draining"
-	DrainComplete                   = "DrainComplete"
+	NodeDrainAnnotation                = "sriovnetwork.openshift.io/state"
+	NodeStateDrainAnnotation           = "sriovnetwork.openshift.io/desired-state"
+	NodeStateDrainAnnotationCurrent    = "sriovnetwork.openshift.io/current-state"
+	NodeStateExternalDrainerAnnotation = "sriovnetwork.openshift.io/use-external-drainer"
+	DrainIdle                          = "Idle"
+	DrainRequired                      = "Drain_Required"
+	RebootRequired                     = "Reboot_Required"
+	Draining                           = "Draining"
+	DrainComplete                      = "DrainComplete"
 
 	SyncStatusSucceeded  = "Succeeded"
 	SyncStatusFailed     = "Failed"

pkg/daemon/daemon.go

@@ -408,6 +408,19 @@ func (dn *NodeReconciler) apply(ctx context.Context, desiredNodeState *sriovnetw
 		return ctrl.Result{}, err
 	}
 
+	// remove external drainer nodestate annotation if exists
+	annotations := desiredNodeState.GetAnnotations()
+	if _, ok := annotations[consts.NodeStateExternalDrainerAnnotation]; ok {
+		reqLogger.Info("remove external drainer nodestate annotation", "annotation", consts.NodeStateExternalDrainerAnnotation)
+		original := desiredNodeState.DeepCopy()
+		delete(annotations, consts.NodeStateExternalDrainerAnnotation)
+		// Patch only the annotations
+		if err := dn.client.Patch(ctx, desiredNodeState, client.MergeFrom(original)); err != nil {
+			reqLogger.Error(err, "failed to patch nodestate after removing external drainer annotation")
+			return ctrl.Result{}, err
+		}
+	}
+
 	reqLogger.Info("sync succeeded")
 	syncStatus := consts.SyncStatusSucceeded
 	lastSyncError := ""
@@ -532,6 +545,16 @@ func (dn *NodeReconciler) handleDrain(ctx context.Context, desiredNodeState *sri
 		return false, nil
 	}
 
+	// add external drainer nodestate annotation if flag is enabled
+	if vars.UseExternalDrainer {
+		err := utils.AnnotateObject(ctx, desiredNodeState,
+			consts.NodeStateExternalDrainerAnnotation, "true", dn.client)
+		if err != nil {
+			funcLog.Error(err, "failed to add nodestate external drainer annotation")
+			return false, err
+		}
+	}
+
 	// annotate both node and node state with drain or reboot
 	annotation := consts.DrainRequired
 	if reqReboot {