Fix equivalence cache hash tests.

2025-07-21 19:01:49 +00:00 · 2018-01-23 15:41:28 -08:00 · 2018-01-23 15:41:28 -08:00 · e9a3815a6c
commit e9a3815a6c
parent 466a499fcb
2 changed files with 77 additions and 190 deletions
--- a/pkg/scheduler/core/equivalence_cache.go
+++ b/pkg/scheduler/core/equivalence_cache.go
@ -238,7 +238,7 @@ type equivalencePod struct {

 // getEquivalencePod returns the equivalencePod for a Pod.
 func getEquivalencePod(pod *v1.Pod) *equivalencePod {
-	return &equivalencePod{
+	ep := &equivalencePod{
 		Namespace:      &pod.Namespace,
 		Labels:         pod.Labels,
 		Affinity:       pod.Spec.Affinity,
@ -249,4 +249,26 @@ func getEquivalencePod(pod *v1.Pod) *equivalencePod {
 		Tolerations:    pod.Spec.Tolerations,
 		Volumes:        pod.Spec.Volumes,
 	}
+	// DeepHashObject considers nil and empy slices to be different. Normalize them.
+	if len(ep.Containers) == 0 {
+		ep.Containers = nil
+	}
+	if len(ep.InitContainers) == 0 {
+		ep.InitContainers = nil
+	}
+	if len(ep.Tolerations) == 0 {
+		ep.Tolerations = nil
+	}
+	if len(ep.Volumes) == 0 {
+		ep.Volumes = nil
+	}
+	// Normalize empty maps also.
+	if len(ep.Labels) == 0 {
+		ep.Labels = nil
+	}
+	if len(ep.NodeSelector) == 0 {
+		ep.NodeSelector = nil
+	}
+	// TODO: Also normalize nested maps and slices.
+	return ep
 }
--- a/pkg/scheduler/core/equivalence_cache_test.go
+++ b/pkg/scheduler/core/equivalence_cache_test.go
@ -29,11 +29,11 @@ import (
 )

 // makeBasicPod returns a Pod object with many of the fields populated.
-func makeBasicPod() *v1.Pod {
+func makeBasicPod(name string) *v1.Pod {
 	isController := true
 	return &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-pod",
+			Name:      name,
 			Namespace: "test-ns",
 			Labels:    map[string]string{"app": "web", "env": "prod"},
 			OwnerReferences: []metav1.OwnerReference{
@ -353,185 +353,46 @@ func TestPredicateWithECache(t *testing.T) {
 	}
 }

-func TestGetHashEquivalencePod(t *testing.T) {
-
-	testNamespace := "test"
+func TestGetEquivalenceHash(t *testing.T) {

 	ecache := NewEquivalenceCache()

-	isController := true
+	pod1 := makeBasicPod("pod1")
+	pod2 := makeBasicPod("pod2")

-	pod1 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod1",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "123",
-					Controller: &isController,
-				},
-			},
-		},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol1",
-						},
-					},
-				},
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol2",
-						},
-					},
+	pod3 := makeBasicPod("pod3")
+	pod3.Spec.Volumes = []v1.Volume{
+		{
+			VolumeSource: v1.VolumeSource{
+				PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
+					ClaimName: "someEBSVol111",
 				},
 			},
 		},
 	}

-	pod2 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod2",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "123",
-					Controller: &isController,
-				},
-			},
-		},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol2",
-						},
-					},
-				},
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol1",
-						},
-					},
+	pod4 := makeBasicPod("pod4")
+	pod4.Spec.Volumes = []v1.Volume{
+		{
+			VolumeSource: v1.VolumeSource{
+				PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
+					ClaimName: "someEBSVol222",
 				},
 			},
 		},
 	}

-	pod3 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod3",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "567",
-					Controller: &isController,
-				},
-			},
-		},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol3-1",
-						},
-					},
-				},
-			},
-		},
-	}
+	pod5 := makeBasicPod("pod5")
+	pod5.Spec.Volumes = []v1.Volume{}

-	pod4 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod4",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "567",
-					Controller: &isController,
-				},
-			},
-		},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "someEBSVol3-0",
-						},
-					},
-				},
-			},
-		},
-	}
+	pod6 := makeBasicPod("pod6")
+	pod6.Spec.Volumes = nil

-	pod5 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod5",
-			Namespace: testNamespace,
-		},
-	}
+	pod7 := makeBasicPod("pod7")
+	pod7.Spec.NodeSelector = nil

-	pod6 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod6",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "567",
-					Controller: &isController,
-				},
-			},
-		},
-		Spec: v1.PodSpec{
-			Volumes: []v1.Volume{
-				{
-					VolumeSource: v1.VolumeSource{
-						PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
-							ClaimName: "no-exists-pvc",
-						},
-					},
-				},
-			},
-		},
-	}
-
-	pod7 := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod7",
-			Namespace: testNamespace,
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "v1",
-					Kind:       "ReplicationController",
-					Name:       "rc",
-					UID:        "567",
-					Controller: &isController,
-				},
-			},
-		},
-	}
+	pod8 := makeBasicPod("pod8")
+	pod8.Spec.NodeSelector = make(map[string]string)

 	type podInfo struct {
 		pod         *v1.Pod
@ -539,39 +400,41 @@ func TestGetHashEquivalencePod(t *testing.T) {
 	}

 	tests := []struct {
+		name         string
 		podInfoList  []podInfo
 		isEquivalent bool
 	}{
-		// pods with same controllerRef and same pvc claim
 		{
+			name: "pods with everything the same except name",
 			podInfoList: []podInfo{
 				{pod: pod1, hashIsValid: true},
 				{pod: pod2, hashIsValid: true},
 			},
 			isEquivalent: true,
 		},
-		// pods with same controllerRef but different pvc claim
 		{
+			name: "pods that only differ in their PVC volume sources",
 			podInfoList: []podInfo{
 				{pod: pod3, hashIsValid: true},
 				{pod: pod4, hashIsValid: true},
 			},
 			isEquivalent: false,
 		},
-		// pod without controllerRef
 		{
+			name: "pods that have no volumes, but one uses nil and one uses an empty slice",
 			podInfoList: []podInfo{
-				{pod: pod5, hashIsValid: false},
+				{pod: pod5, hashIsValid: true},
+				{pod: pod6, hashIsValid: true},
 			},
-			isEquivalent: false,
+			isEquivalent: true,
 		},
-		// pods with same controllerRef but one has non-exists pvc claim
 		{
+			name: "pods that have no NodeSelector, but one uses nil and one uses an empty map",
 			podInfoList: []podInfo{
-				{pod: pod6, hashIsValid: false},
 				{pod: pod7, hashIsValid: true},
+				{pod: pod8, hashIsValid: true},
 			},
-			isEquivalent: false,
+			isEquivalent: true,
 		},
 	}

@ -581,25 +444,27 @@ func TestGetHashEquivalencePod(t *testing.T) {
 	)

 	for _, test := range tests {
-		for i, podInfo := range test.podInfoList {
-			testPod := podInfo.pod
-			hash, isValid := ecache.getEquivalenceHash(testPod)
-			if isValid != podInfo.hashIsValid {
-				t.Errorf("Failed: pod %v is expected to have valid hash", testPod)
-			}
-			// NOTE(harry): the first element will be used as target so
-			// this logic can't verify more than two inequivalent pods
-			if i == 0 {
-				targetHash = hash
-				targetPodInfo = podInfo
-			} else {
-				if targetHash != hash {
-					if test.isEquivalent {
-						t.Errorf("Failed: pod: %v is expected to be equivalent to: %v", testPod, targetPodInfo.pod)
+		t.Run(test.name, func(t *testing.T) {
+			for i, podInfo := range test.podInfoList {
+				testPod := podInfo.pod
+				hash, isValid := ecache.getEquivalenceHash(testPod)
+				if isValid != podInfo.hashIsValid {
+					t.Errorf("Failed: pod %v is expected to have valid hash", testPod)
+				}
+				// NOTE(harry): the first element will be used as target so
+				// this logic can't verify more than two inequivalent pods
+				if i == 0 {
+					targetHash = hash
+					targetPodInfo = podInfo
+				} else {
+					if targetHash != hash {
+						if test.isEquivalent {
+							t.Errorf("Failed: pod: %v is expected to be equivalent to: %v", testPod, targetPodInfo.pod)
+						}
 					}
 				}
 			}
-		}
+		})
 	}
 }

@ -736,7 +601,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {

 func BenchmarkEquivalenceHash(b *testing.B) {
 	cache := NewEquivalenceCache()
-	pod := makeBasicPod()
+	pod := makeBasicPod("test")
 	for i := 0; i < b.N; i++ {
 		cache.getEquivalenceHash(pod)
 	}