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

@@ -187,6 +187,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

@@ -131,7 +131,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
 	Context("when there is only one node", func() {
 
 		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)
 
@@ -145,7 +145,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)
 
@@ -158,8 +158,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)
 
@@ -175,9 +175,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)
@@ -215,9 +215,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{}
@@ -266,9 +266,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{}
@@ -291,9 +291,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)
@@ -319,7 +319,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")
 
@@ -342,7 +342,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")
@@ -424,7 +424,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,
@@ -448,6 +448,10 @@ func createNode(ctx context.Context, nodeName string) (*corev1.Node, *sriovnetwo
 		},
 	}
 
+	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

@@ -86,8 +86,10 @@ spec:
               value: {{ .Values.operator.metricsExporter.certificates.secretName }}
             - name: METRICS_EXPORTER_KUBE_RBAC_PROXY_IMAGE
               value: {{ .Values.images.metricsExporterKubeRbacProxy }}
-            - name: USE_EXTERNAL_DRAINER
-              value: {{ .Values.operator.externalDrainer.enabled | quote }}
+            {{- 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

@@ -274,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

@@ -92,14 +92,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

@@ -424,6 +424,16 @@ func (dn *NodeReconciler) apply(ctx context.Context, desiredNodeState *sriovnetw
 		return ctrl.Result{}, err
 	}
 
+	// remove external drainer nodestate annotation
+	err = utils.AnnotateObject(ctx, desiredNodeState,
+		consts.NodeStateExternalDrainerAnnotation, "", dn.client)
+	if err != nil {
+		if !errors.IsNotFound(err) {
+			reqLogger.Error(err, "failed to remove node external drainer annotation")
+			return ctrl.Result{}, err
+		}
+	}
+
 	reqLogger.Info("sync succeeded")
 	syncStatus := consts.SyncStatusSucceeded
 	lastSyncError := ""
@@ -548,6 +558,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 node external drainer annotation")
+			return false, err
+		}
+	}
+
 	// annotate both node and node state with drain or reboot
 	annotation := consts.DrainRequired
 	if reqReboot {

pkg/daemon/daemon_test.go

@@ -69,6 +69,7 @@ var (
 const (
 	waitTime  = 2 * time.Minute
 	retryTime = 5 * time.Second
+	nodeName  = "node1"
 )
 
 var _ = Describe("Daemon Controller", Ordered, func() {
@@ -168,7 +169,7 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 		daemonReconciler = createDaemon(hostHelper, platformHelper, featureGates, []string{})
 		startDaemon(daemonReconciler)
 
-		_, nodeState = createNode("node1")
+		_, nodeState = createNode(nodeName)
 	})
 
 	AfterAll(func() {
@@ -270,6 +271,11 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 					ToNot(HaveOccurred())
 
 				g.Expect(nodeState.Annotations[constants.NodeStateDrainAnnotation]).To(Equal(constants.DrainRequired))
+				// verify that external drainer annotation doesn't exist
+				node := &corev1.Node{}
+				g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: nodeName}, node)).
+					ToNot(HaveOccurred())
+				g.Expect(nodeState.Annotations).NotTo(ContainElement(constants.NodeStateExternalDrainerAnnotation))
 			}, waitTime, retryTime).Should(Succeed())
 
 			patchAnnotation(nodeState, constants.NodeStateDrainAnnotationCurrent, constants.DrainComplete)
@@ -351,6 +357,67 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 			eventuallySyncStatusEqual(nodeState, constants.SyncStatusSucceeded)
 			assertLastStatusTransitionsContains(nodeState, 2, constants.SyncStatusInProgress)
 		})
+
+		It("Should apply external drainer annotation when useExternalDrainer is true", func(ctx context.Context) {
+			DeferCleanup(func(x bool) { vars.UseExternalDrainer = x }, vars.UseExternalDrainer)
+			vars.UseExternalDrainer = true
+
+			discoverSriovReturn.Store(&[]sriovnetworkv1.InterfaceExt{
+				{
+					Name:           "eno1",
+					Driver:         "ice",
+					PciAddress:     "0000:16:00.0",
+					DeviceID:       "1593",
+					Vendor:         "8086",
+					EswitchMode:    "legacy",
+					LinkAdminState: "up",
+					LinkSpeed:      "10000 Mb/s",
+					LinkType:       "ETH",
+					Mac:            "aa:bb:cc:dd:ee:ff",
+					Mtu:            1500,
+					TotalVfs:       2,
+					NumVfs:         0,
+				},
+			})
+
+			By("waiting for state to be in progress")
+			eventuallySyncStatusEqual(nodeState, constants.SyncStatusInProgress)
+
+			err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("waiting to require drain")
+			EventuallyWithOffset(1, func(g Gomega) {
+				g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)).
+					ToNot(HaveOccurred())
+
+				g.Expect(nodeState.Annotations[constants.NodeStateDrainAnnotation]).To(Equal(constants.DrainRequired))
+			}, waitTime, retryTime).Should(Succeed())
+
+			// Validate status
+			EventuallyWithOffset(1, func(g Gomega) {
+				// get node by name
+				node := &corev1.Node{}
+				g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: nodeName}, node)).
+					ToNot(HaveOccurred())
+				g.Expect(nodeState.Annotations[constants.NodeStateExternalDrainerAnnotation]).To(Equal("true"))
+			}, waitTime, retryTime).Should(Succeed())
+
+			patchAnnotation(nodeState, constants.NodeStateDrainAnnotationCurrent, constants.DrainComplete)
+			// Validate status
+			EventuallyWithOffset(1, func(g Gomega) {
+				g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)).
+					ToNot(HaveOccurred())
+
+				g.Expect(nodeState.Status.SyncStatus).To(Equal(constants.SyncStatusInProgress))
+				node := &corev1.Node{}
+				g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: nodeName}, node)).
+					ToNot(HaveOccurred())
+				// verify that external drainer annotation is removed
+				g.Expect(nodeState.Annotations).NotTo(ContainElement(constants.NodeStateExternalDrainerAnnotation))
+			}, waitTime, retryTime).Should(Succeed())
+
+		})
 	})
 })