node labels and taints do not change, node events are ignored in daemonset controller
Signed-off-by: xigang <wangxigang2014@gmail.com>
This commit is contained in:
		| @@ -24,8 +24,6 @@ import ( | |||||||
| 	"sync" | 	"sync" | ||||||
| 	"time" | 	"time" | ||||||
|  |  | ||||||
| 	"k8s.io/klog/v2" |  | ||||||
|  |  | ||||||
| 	apps "k8s.io/api/apps/v1" | 	apps "k8s.io/api/apps/v1" | ||||||
| 	v1 "k8s.io/api/core/v1" | 	v1 "k8s.io/api/core/v1" | ||||||
| 	apiequality "k8s.io/apimachinery/pkg/api/equality" | 	apiequality "k8s.io/apimachinery/pkg/api/equality" | ||||||
| @@ -49,6 +47,7 @@ import ( | |||||||
| 	"k8s.io/client-go/util/workqueue" | 	"k8s.io/client-go/util/workqueue" | ||||||
| 	v1helper "k8s.io/component-helpers/scheduling/corev1" | 	v1helper "k8s.io/component-helpers/scheduling/corev1" | ||||||
| 	"k8s.io/component-helpers/scheduling/corev1/nodeaffinity" | 	"k8s.io/component-helpers/scheduling/corev1/nodeaffinity" | ||||||
|  | 	"k8s.io/klog/v2" | ||||||
| 	podutil "k8s.io/kubernetes/pkg/api/v1/pod" | 	podutil "k8s.io/kubernetes/pkg/api/v1/pod" | ||||||
| 	"k8s.io/kubernetes/pkg/controller" | 	"k8s.io/kubernetes/pkg/controller" | ||||||
| 	"k8s.io/kubernetes/pkg/controller/daemon/util" | 	"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; | // shouldIgnoreNodeUpdate returns true if Node labels and taints have not changed, otherwise returns false. | ||||||
| // otherwise, returns false. | // If other calling functions need to use other properties of Node, shouldIgnoreNodeUpdate needs to be updated. | ||||||
| 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 |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func shouldIgnoreNodeUpdate(oldNode, curNode v1.Node) bool { | func shouldIgnoreNodeUpdate(oldNode, curNode v1.Node) bool { | ||||||
| 	if !nodeInSameCondition(oldNode.Status.Conditions, curNode.Status.Conditions) { | 	return apiequality.Semantic.DeepEqual(oldNode.Labels, curNode.Labels) && | ||||||
| 		return false | 		apiequality.Semantic.DeepEqual(oldNode.Spec.Taints, curNode.Spec.Taints) | ||||||
| 	} |  | ||||||
| 	oldNode.ResourceVersion = curNode.ResourceVersion |  | ||||||
| 	oldNode.Status.Conditions = curNode.Status.Conditions |  | ||||||
| 	return apiequality.Semantic.DeepEqual(oldNode, curNode) |  | ||||||
| } | } | ||||||
|  |  | ||||||
| func (dsc *DaemonSetsController) updateNode(logger klog.Logger, old, cur interface{}) { | 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). | 	// 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 { | 	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) | 		oldShouldRun, oldShouldContinueRunning := NodeShouldRunDaemonPod(oldNode, ds) | ||||||
| 		currentShouldRun, currentShouldContinueRunning := NodeShouldRunDaemonPod(curNode, ds) | 		currentShouldRun, currentShouldContinueRunning := NodeShouldRunDaemonPod(curNode, ds) | ||||||
| 		if (oldShouldRun != currentShouldRun) || (oldShouldContinueRunning != currentShouldContinueRunning) { | 		if (oldShouldRun != currentShouldRun) || (oldShouldContinueRunning != currentShouldContinueRunning) { | ||||||
|   | |||||||
| @@ -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) | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 xigang
					xigang