Skip updating cache on pod update if the node was deleted

alculquicondor · alculquicondor · commit 23c6b68f8422 · 2020-04-08T09:40:43.000-04:00
Signed-off-by: Aldo Culquicondor &lt;acondor@google.com&gt;
diff --git a/pkg/scheduler/internal/cache/cache.go b/pkg/scheduler/internal/cache/cache.go
@@ -428,6 +428,13 @@ func (cache *schedulerCache) addPod(pod *v1.Pod) {
 
 // Assumes that lock is already acquired.
 func (cache *schedulerCache) updatePod(oldPod, newPod *v1.Pod) error {
+	if _, ok := cache.nodes[newPod.Spec.NodeName]; !ok {
+		// The node might have been deleted already.
+		// This is not a problem in the case where a pod update arrives before the
+		// node creation, because we will always have a create pod event before
+		// that, which will create the placeholder node item.
+		return nil
+	}
 	if err := cache.removePod(oldPod); err != nil {
 		return err
 	}
diff --git a/pkg/scheduler/internal/cache/cache_test.go b/pkg/scheduler/internal/cache/cache_test.go
@@ -1232,15 +1232,15 @@ func TestSchedulerCache_UpdateSnapshot(t *testing.T) {
 
 	// Create a few pods for tests.
 	pods := []*v1.Pod{}
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 20; i++ {
 		pod := &v1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      fmt.Sprintf("test-pod%v", i),
 				Namespace: "test-ns",
 				UID:       types.UID(fmt.Sprintf("test-puid%v", i)),
 			},
 			Spec: v1.PodSpec{
-				NodeName: fmt.Sprintf("test-node%v", i),
+				NodeName: fmt.Sprintf("test-node%v", i%10),
 			},
 		}
 		pods = append(pods, pod)
@@ -1313,6 +1313,13 @@ func TestSchedulerCache_UpdateSnapshot(t *testing.T) {
 			}
 		}
 	}
+	removePod := func(i int) operation {
+		return func() {
+			if err := cache.RemovePod(pods[i]); err != nil {
+				t.Error(err)
+			}
+		}
+	}
 	removePodWithAffinity := func(i int) operation {
 		return func() {
 			if err := cache.RemovePod(podsWithAffinity[i]); err != nil {
@@ -1381,13 +1388,6 @@ func TestSchedulerCache_UpdateSnapshot(t *testing.T) {
 			},
 			expected: []*v1.Node{nodes[6], nodes[5], nodes[2], nodes[0]},
 		},
-		{
-			name: "Remove non-existing node",
-			operations: []operation{
-				addNode(0), addNode(1), updateSnapshot(),
-			},
-			expected: []*v1.Node{nodes[1], nodes[0]},
-		},
 		{
 			name: "Update some nodes",
 			operations: []operation{
@@ -1444,11 +1444,19 @@ func TestSchedulerCache_UpdateSnapshot(t *testing.T) {
 			expected: []*v1.Node{nodes[0], nodes[4], nodes[2]},
 		},
 		{
-			name: "Remove pod from non-existing node",
+			name: "Add pod before its node",
+			operations: []operation{
+				addNode(0), addPod(1), updatePod(1), addNode(1),
+			},
+			expected: []*v1.Node{nodes[1], nodes[0]},
+		},
+		{
+			name: "Remove node before its pods",
 			operations: []operation{
-				addNode(0), addPod(0), addNode(2), updateSnapshot(),
+				addNode(0), addNode(1), addPod(1), addPod(11),
+				removeNode(1), updatePod(1), updatePod(11), removePod(1), removePod(11),
 			},
-			expected: []*v1.Node{nodes[2], nodes[0]},
+			expected: []*v1.Node{nodes[0]},
 		},
 		{
 			name: "Add Pods with affinity",
