Ensures that the DaemonSet controller does not launch a Pod on a Node…

… while waiting for a Pod that it has previously created to terminate.
kubernetes · Aug 24, 2017 · 4248f2a · 4248f2a
1 parent baf2f85
commit 4248f2a
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 15 deletions.
diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go
@@ -743,7 +743,7 @@ func (dsc *DaemonSetsController) getDaemonPods(ds *extensions.DaemonSet) ([]*v1.
 	}
 	// If any adoptions are attempted, we should first recheck for deletion with
 	// an uncached quorum read sometime after listing Pods (see #42639).
-	canAdoptFunc := controller.RecheckDeletionTimestamp(func() (metav1.Object, error) {
+	dsNotDeleted := controller.RecheckDeletionTimestamp(func() (metav1.Object, error) {
 		fresh, err := dsc.kubeClient.ExtensionsV1beta1().DaemonSets(ds.Namespace).Get(ds.Name, metav1.GetOptions{})
 		if err != nil {
 			return nil, err
@@ -753,8 +753,9 @@ func (dsc *DaemonSetsController) getDaemonPods(ds *extensions.DaemonSet) ([]*v1.
 		}
 		return fresh, nil
 	})
+
 	// Use ControllerRefManager to adopt/orphan as needed.
-	cm := controller.NewPodControllerRefManager(dsc.podControl, ds, selector, controllerKind, canAdoptFunc)
+	cm := controller.NewPodControllerRefManager(dsc.podControl, ds, selector, controllerKind, dsNotDeleted)
 	return cm.ClaimPods(pods)
 }
 
@@ -770,10 +771,6 @@ func (dsc *DaemonSetsController) getNodesToDaemonPods(ds *extensions.DaemonSet)
 	// Group Pods by Node name.
 	nodeToDaemonPods := make(map[string][]*v1.Pod)
 	for _, pod := range claimedPods {
-		// Skip terminating pods
-		if pod.DeletionTimestamp != nil {
-			continue
-		}
 		nodeName := pod.Spec.NodeName
 		nodeToDaemonPods[nodeName] = append(nodeToDaemonPods[nodeName], pod)
 	}

diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go
@@ -1211,15 +1211,30 @@ func TestNodeDaemonLaunchesToleratePod(t *testing.T) {
 		ds.Spec.UpdateStrategy = *strategy
 		setDaemonSetToleration(ds, noScheduleTolerations)
 		manager, podControl, _ := newTestController(ds)
-
-		node := newNode("untainted", nil)
-		manager.nodeStore.Add(node)
+		addNodes(manager.nodeStore, 0, 1, nil)
 		manager.dsStore.Add(ds)
 
 		syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0, 0)
 	}
 }
 
+// DaemonSet should launch a pod on a not ready node with taint notReady:NoExecute.
+func TestDaemonSetRespectsTermination(t *testing.T) {
+	for _, strategy := range updateStrategies() {
+		ds := newDaemonSet("foo")
+		ds.Spec.UpdateStrategy = *strategy
+		manager, podControl, _ := newTestController(ds)
+
+		addNodes(manager.nodeStore, 0, 1, simpleNodeLabel)
+		pod := newPod(fmt.Sprintf("%s-", "node-0"), "node-0", simpleDaemonSetLabel, ds)
+		dt := metav1.Now()
+		pod.DeletionTimestamp = &dt
+		manager.podStore.Add(pod)
+		manager.dsStore.Add(ds)
+		syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0, 0)
+	}
+}
+
 func setNodeTaint(node *v1.Node, taints []v1.Taint) {
 	node.Spec.Taints = taints
 }
@@ -1816,12 +1831,6 @@ func TestGetNodesToDaemonPods(t *testing.T) {
 			newPod("non-matching-owned-0-", "node-0", simpleDaemonSetLabel2, ds),
 			newPod("non-matching-orphan-1-", "node-1", simpleDaemonSetLabel2, nil),
 			newPod("matching-owned-by-other-0-", "node-0", simpleDaemonSetLabel, ds2),
-			func() *v1.Pod {
-				pod := newPod("matching-owned-2-but-set-for-deletion", "node-2", simpleDaemonSetLabel, ds)
-				now := metav1.Now()
-				pod.DeletionTimestamp = &now
-				return pod
-			}(),
 		}
 		for _, pod := range ignoredPods {
 			manager.podStore.Add(pod)