From b2e2fb8d89d48259188bea1fb537096cd4289cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Wr=C3=B3blewski?= Date: Thu, 3 Mar 2022 11:17:01 +0000 Subject: [PATCH] Make daemon.NodeShouldRunDaemonPod function public --- pkg/controller/daemon/daemon_controller.go | 20 +++++++++---------- .../daemon/daemon_controller_test.go | 2 +- pkg/controller/daemon/update.go | 2 +- test/e2e/framework/daemonset/fixtures.go | 10 ++-------- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index d73f1ea6572..e31dde1eacc 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -641,7 +641,7 @@ func (dsc *DaemonSetsController) addNode(obj interface{}) { } node := obj.(*v1.Node) for _, ds := range dsList { - if shouldRun, _ := dsc.nodeShouldRunDaemonPod(node, ds); shouldRun { + if shouldRun, _ := NodeShouldRunDaemonPod(node, ds); shouldRun { dsc.enqueueDaemonSet(ds) } } @@ -699,8 +699,8 @@ func (dsc *DaemonSetsController) updateNode(old, cur interface{}) { } // 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 { - oldShouldRun, oldShouldContinueRunning := dsc.nodeShouldRunDaemonPod(oldNode, ds) - currentShouldRun, currentShouldContinueRunning := dsc.nodeShouldRunDaemonPod(curNode, ds) + oldShouldRun, oldShouldContinueRunning := NodeShouldRunDaemonPod(oldNode, ds) + currentShouldRun, currentShouldContinueRunning := NodeShouldRunDaemonPod(curNode, ds) if (oldShouldRun != currentShouldRun) || (oldShouldContinueRunning != currentShouldContinueRunning) { dsc.enqueueDaemonSet(ds) } @@ -798,7 +798,7 @@ func (dsc *DaemonSetsController) podsShouldBeOnNode( hash string, ) (nodesNeedingDaemonPods, podsToDelete []string) { - shouldRun, shouldContinueRunning := dsc.nodeShouldRunDaemonPod(node, ds) + shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(node, ds) daemonPods, exists := nodeToDaemonPods[node.Name] switch { @@ -1118,7 +1118,7 @@ func (dsc *DaemonSetsController) updateDaemonSetStatus(ctx context.Context, ds * var desiredNumberScheduled, currentNumberScheduled, numberMisscheduled, numberReady, updatedNumberScheduled, numberAvailable int now := dsc.failedPodsBackoff.Clock.Now() for _, node := range nodeList { - shouldRun, _ := dsc.nodeShouldRunDaemonPod(node, ds) + shouldRun, _ := NodeShouldRunDaemonPod(node, ds) scheduled := len(nodeToDaemonPods[node.Name]) > 0 if shouldRun { @@ -1254,7 +1254,7 @@ func (dsc *DaemonSetsController) syncDaemonSet(ctx context.Context, key string) return dsc.updateDaemonSetStatus(ctx, ds, nodeList, hash, true) } -// nodeShouldRunDaemonPod checks a set of preconditions against a (node,daemonset) and returns a +// NodeShouldRunDaemonPod checks a set of preconditions against a (node,daemonset) and returns a // summary. Returned booleans are: // * shouldRun: // Returns true when a daemonset should run on the node if a daemonset pod is not already @@ -1262,7 +1262,7 @@ func (dsc *DaemonSetsController) syncDaemonSet(ctx context.Context, key string) // * shouldContinueRunning: // Returns true when a daemonset should continue running on a node if a daemonset pod is already // running on that node. -func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) { +func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) { pod := NewPod(ds, node.Name) // If the daemon set specifies a node name, check that it matches with node.Name. @@ -1271,7 +1271,7 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *apps. } taints := node.Spec.Taints - fitsNodeName, fitsNodeAffinity, fitsTaints := Predicates(pod, node, taints) + fitsNodeName, fitsNodeAffinity, fitsTaints := predicates(pod, node, taints) if !fitsNodeName || !fitsNodeAffinity { return false, false } @@ -1287,8 +1287,8 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *apps. return true, true } -// Predicates checks if a DaemonSet's pod can run on a node. -func Predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) { +// predicates checks if a DaemonSet's pod can run on a node. +func predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) { fitsNodeName = len(pod.Spec.NodeName) == 0 || pod.Spec.NodeName == node.Name // Ignore parsing errors for backwards compatibility. fitsNodeAffinity, _ = nodeaffinity.GetRequiredNodeAffinity(pod).Match(node) diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 0e873676aeb..9328a0a05dc 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -2397,7 +2397,7 @@ func TestNodeShouldRunDaemonPod(t *testing.T) { manager.podNodeIndex.Add(p) } c.ds.Spec.UpdateStrategy = *strategy - shouldRun, shouldContinueRunning := manager.nodeShouldRunDaemonPod(node, c.ds) + shouldRun, shouldContinueRunning := NodeShouldRunDaemonPod(node, c.ds) if shouldRun != c.shouldRun { t.Errorf("[%v] strategy: %v, predicateName: %v expected shouldRun: %v, got: %v", i, c.ds.Spec.UpdateStrategy.Type, c.predicateName, c.shouldRun, shouldRun) diff --git a/pkg/controller/daemon/update.go b/pkg/controller/daemon/update.go index 4f5bcafbfc5..3983fbb87b4 100644 --- a/pkg/controller/daemon/update.go +++ b/pkg/controller/daemon/update.go @@ -528,7 +528,7 @@ func (dsc *DaemonSetsController) updatedDesiredNodeCounts(ds *apps.DaemonSet, no var desiredNumberScheduled int for i := range nodeList { node := nodeList[i] - wantToRun, _ := dsc.nodeShouldRunDaemonPod(node, ds) + wantToRun, _ := NodeShouldRunDaemonPod(node, ds) if !wantToRun { continue } diff --git a/test/e2e/framework/daemonset/fixtures.go b/test/e2e/framework/daemonset/fixtures.go index 1fc81fba325..765335c0d1f 100644 --- a/test/e2e/framework/daemonset/fixtures.go +++ b/test/e2e/framework/daemonset/fixtures.go @@ -84,7 +84,8 @@ func SchedulableNodes(c clientset.Interface, ds *appsv1.DaemonSet) []string { framework.ExpectNoError(err) nodeNames := make([]string, 0) for _, node := range nodeList.Items { - if !canScheduleOnNode(node, ds) { + shouldRun, _ := daemon.NodeShouldRunDaemonPod(&node, ds) + if !shouldRun { framework.Logf("DaemonSet pods can't tolerate node %s with taints %+v, skip checking this node", node.Name, node.Spec.Taints) continue } @@ -93,13 +94,6 @@ func SchedulableNodes(c clientset.Interface, ds *appsv1.DaemonSet) []string { return nodeNames } -// canScheduleOnNode checks if a given DaemonSet can schedule pods on the given node -func canScheduleOnNode(node v1.Node, ds *appsv1.DaemonSet) bool { - newPod := daemon.NewPod(ds, node.Name) - fitsNodeName, fitsNodeAffinity, fitsTaints := daemon.Predicates(newPod, &node, node.Spec.Taints) - return fitsNodeName && fitsNodeAffinity && fitsTaints -} - func CheckDaemonPodOnNodes(f *framework.Framework, ds *appsv1.DaemonSet, nodeNames []string) func() (bool, error) { return func() (bool, error) { return checkDaemonPodStateOnNodes(f.ClientSet, ds, f.Namespace.Name, nodeNames, func(pod *v1.Pod) bool {