Clean up duplicated and obsolete logic

k8snetworkplumbingwg · Jul 5, 2023 · e16b84a · e16b84a
1 parent 8fa1758
commit e16b84a
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 39 deletions.
diff --git a/controllers/drain_controller.go b/controllers/drain_controller.go
@@ -66,7 +66,7 @@ func (dr *DrainReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 	drainingNodes := 0
 	for _, node := range nodeList.Items {
-		if utils.NodeHasAnnotation(node, constants.NodeDrainAnnotation, "Draining") || utils.NodeHasAnnotation(node, constants.NodeDrainAnnotation, "Draining_MCP_Paused") {
+		if utils.NodeHasAnnotation(node, constants.NodeDrainAnnotation, constants.AnnoDraining) || utils.NodeHasAnnotation(node, constants.NodeDrainAnnotation, constants.AnnoMcpPaused) {
 			drainingNodes++
 		}
 	}

diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go
@@ -23,6 +23,12 @@ const (
 	DPConfigFileName                   = "config.json"
 	OVSHWOLMachineConfigNameSuffix     = "ovs-hw-offload"
 
+	NodeDrainAnnotation = "sriovnetwork.openshift.io/state"
+	AnnoIdle            = "Idle"
+	AnnoDrainRequired   = "Drain_Required"
+	AnnoMcpPaused       = "Draining_MCP_Paused"
+	AnnoDraining        = "Draining"
+
 	LinkTypeEthernet   = "ether"
 	LinkTypeInfiniband = "infiniband"
 

diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go
@@ -24,7 +24,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
@@ -98,9 +97,6 @@ type Daemon struct {
 
 	node *corev1.Node
 
-	// TODO(e0ne): remove it
-	drainable bool
-
 	disableDrain bool
 
 	nodeLister listerv1.NodeLister
@@ -112,11 +108,6 @@ type Daemon struct {
 
 const (
 	udevScriptsPath      = "/bindata/scripts/load-udev.sh"
-	annoKey              = "sriovnetwork.openshift.io/state"
-	annoIdle             = "Idle"
-	annoDrainRequired    = "Drain_Required"
-	annoMcpPaused        = "Draining_MCP_Paused"
-	annoDraining         = "Draining"
 	syncStatusSucceeded  = "Succeeded"
 	syncStatusFailed     = "Failed"
 	syncStatusInProgress = "InProgress"
@@ -401,17 +392,6 @@ func (dn *Daemon) nodeUpdateHandler(old, new interface{}) {
 		return
 	}
 	dn.node = node.DeepCopy()
-	nodes, err := dn.nodeLister.List(labels.Everything())
-	if err != nil {
-		return
-	}
-	for _, node := range nodes {
-		if node.GetName() != dn.name && (node.Annotations[annoKey] == annoDraining || node.Annotations[annoKey] == annoMcpPaused) {
-			dn.drainable = false
-			return
-		}
-	}
-	dn.drainable = true
 }
 
 func (dn *Daemon) operatorConfigAddHandler(obj interface{}) {
@@ -583,7 +563,7 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 		}
 	}
 
-	if dn.nodeHasAnnotation(annoKey, annoDrainRequired) {
+	if utils.NodeHasAnnotation(*dn.node, consts.NodeDrainAnnotation, consts.AnnoDrainRequired) {
 		glog.Info("nodeStateSyncHandler(): waiting for drain")
 		return nil
 	}
@@ -658,8 +638,8 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 			return err
 		}
 	} else {
-		if !dn.nodeHasAnnotation(annoKey, annoIdle) {
-			if err := dn.annotateNode(dn.name, annoIdle); err != nil {
+		if !utils.NodeHasAnnotation(*dn.node, consts.NodeDrainAnnotation, consts.AnnoIdle) {
+			if err := dn.annotateNode(dn.name, consts.AnnoIdle); err != nil {
 				glog.Errorf("nodeStateSyncHandler(): failed to annotate node: %v", err)
 				return err
 			}
@@ -683,15 +663,6 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 	return nil
 }
 
-func (dn *Daemon) nodeHasAnnotation(annoKey string, value string) bool {
-	// TODO(e0ne): re-use cluster.NodeHasAnnotation function
-	// Check if node already contains annotation
-	if anno, ok := dn.node.Annotations[annoKey]; ok && (anno == value) {
-		return true
-	}
-	return false
-}
-
 // isNodeDraining: check if the node is draining
 // both Draining and MCP paused labels will return true
 func (dn *Daemon) isNodeDraining() bool {
@@ -700,7 +671,7 @@ func (dn *Daemon) isNodeDraining() bool {
 		return false
 	}
 
-	return anno == annoDraining || anno == annoMcpPaused
+	return anno == consts.AnnoDraining || anno == consts.AnnoMcpPaused
 }
 
 func (dn *Daemon) completeDrain() error {
@@ -719,7 +690,7 @@ func (dn *Daemon) completeDrain() error {
 		}
 	}
 
-	if err := dn.annotateNode(dn.name, annoIdle); err != nil {
+	if err := dn.annotateNode(dn.name, consts.AnnoIdle); err != nil {
 		glog.Errorf("completeDrain(): failed to annotate node: %v", err)
 		return err
 	}
@@ -868,7 +839,7 @@ func (dn *Daemon) getNodeMachinePool() error {
 
 func (dn *Daemon) applyDrainRequired() error {
 	glog.V(2).Info("applyDrainRequired(): no other node is draining")
-	err := dn.annotateNode(dn.name, annoDrainRequired)
+	err := dn.annotateNode(dn.name, consts.AnnoDrainRequired)
 	if err != nil {
 		glog.Errorf("applyDrainRequired(): Failed to annotate node: %v", err)
 		return err
@@ -887,7 +858,7 @@ func (dn *Daemon) pauseMCP() error {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	paused := dn.node.Annotations[annoKey] == annoMcpPaused
+	paused := dn.node.Annotations[consts.NodeDrainAnnotation] == consts.AnnoMcpPaused
 
 	mcpEventHandler := func(obj interface{}) {
 		mcp := obj.(*mcfgv1.MachineConfigPool)
@@ -920,7 +891,7 @@ func (dn *Daemon) pauseMCP() error {
 				glog.V(2).Infof("pauseMCP(): Failed to pause MCP %s: %v", dn.mcpName, err)
 				return
 			}
-			err = dn.annotateNode(dn.name, annoMcpPaused)
+			err = dn.annotateNode(dn.name, consts.AnnoMcpPaused)
 			if err != nil {
 				glog.V(2).Infof("pauseMCP(): Failed to annotate node: %v", err)
 				return
@@ -936,7 +907,7 @@ func (dn *Daemon) pauseMCP() error {
 				glog.V(2).Infof("pauseMCP(): fail to resume MCP %s: %v", dn.mcpName, err)
 				return
 			}
-			err = dn.annotateNode(dn.name, annoDraining)
+			err = dn.annotateNode(dn.name, consts.AnnoDraining)
 			if err != nil {
 				glog.V(2).Infof("pauseMCP(): Failed to annotate node: %v", err)
 				return