diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index 040251918b9..40fec239a19 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -24,8 +24,6 @@ import ( "sync" "time" - "k8s.io/klog/v2" - apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -49,6 +47,7 @@ import ( "k8s.io/client-go/util/workqueue" v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" + "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/daemon/util" @@ -654,42 +653,11 @@ func (dsc *DaemonSetsController) addNode(logger klog.Logger, obj interface{}) { } } -// nodeInSameCondition returns true if all effective types ("Status" is true) equals; -// otherwise, returns false. -func nodeInSameCondition(old []v1.NodeCondition, cur []v1.NodeCondition) bool { - if len(old) == 0 && len(cur) == 0 { - return true - } - - c1map := map[v1.NodeConditionType]v1.ConditionStatus{} - for _, c := range old { - if c.Status == v1.ConditionTrue { - c1map[c.Type] = c.Status - } - } - - for _, c := range cur { - if c.Status != v1.ConditionTrue { - continue - } - - if _, found := c1map[c.Type]; !found { - return false - } - - delete(c1map, c.Type) - } - - return len(c1map) == 0 -} - +// shouldIgnoreNodeUpdate returns true if Node labels and taints have not changed, otherwise returns false. +// If other calling functions need to use other properties of Node, shouldIgnoreNodeUpdate needs to be updated. func shouldIgnoreNodeUpdate(oldNode, curNode v1.Node) bool { - if !nodeInSameCondition(oldNode.Status.Conditions, curNode.Status.Conditions) { - return false - } - oldNode.ResourceVersion = curNode.ResourceVersion - oldNode.Status.Conditions = curNode.Status.Conditions - return apiequality.Semantic.DeepEqual(oldNode, curNode) + return apiequality.Semantic.DeepEqual(oldNode.Labels, curNode.Labels) && + apiequality.Semantic.DeepEqual(oldNode.Spec.Taints, curNode.Spec.Taints) } func (dsc *DaemonSetsController) updateNode(logger klog.Logger, old, cur interface{}) { @@ -706,6 +674,7 @@ func (dsc *DaemonSetsController) updateNode(logger klog.Logger, old, cur interfa } // TODO: it'd be nice to pass a hint with these enqueues, so that each ds would only examine the added node (unless it has other work to do, too). for _, ds := range dsList { + // If NodeShouldRunDaemonPod needs to uses other than Labels and Taints (mutable) properties of node, it needs to update shouldIgnoreNodeUpdate. oldShouldRun, oldShouldContinueRunning := NodeShouldRunDaemonPod(oldNode, ds) currentShouldRun, currentShouldContinueRunning := NodeShouldRunDaemonPod(curNode, ds) if (oldShouldRun != currentShouldRun) || (oldShouldContinueRunning != currentShouldContinueRunning) { diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 965df59edeb..1779077ac97 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -3580,3 +3580,43 @@ func TestStoreDaemonSetStatus(t *testing.T) { }) } } + +func TestShouldIgnoreNodeUpdate(t *testing.T) { + cases := []struct { + name string + newNode *v1.Node + oldNode *v1.Node + expectedResult bool + }{ + { + name: "Nothing changed", + oldNode: newNode("node1", nil), + newNode: newNode("node1", nil), + expectedResult: true, + }, + { + name: "Node labels changed", + oldNode: newNode("node1", nil), + newNode: newNode("node1", simpleNodeLabel), + expectedResult: false, + }, + { + name: "Node taints changed", + oldNode: func() *v1.Node { + node := newNode("node1", nil) + setNodeTaint(node, noScheduleTaints) + return node + }(), + newNode: newNode("node1", nil), + expectedResult: false, + }, + } + + for _, c := range cases { + result := shouldIgnoreNodeUpdate(*c.oldNode, *c.newNode) + + if result != c.expectedResult { + t.Errorf("[%s] unexpected results: %v", c.name, result) + } + } +}