Adding toleration to the job doesn't trigger workload change

kubernetes-sigs · Nov 13, 2023 · 1a99a26 · 1a99a26
1 parent 1ecdf3b
commit 1a99a26
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 12 deletions.
diff --git a/pkg/controller/jobframework/reconciler.go b/pkg/controller/jobframework/reconciler.go
@@ -487,12 +487,13 @@ func (r *JobReconciler) equivalentToWorkload(job GenericJob, object client.Objec
 	jobPodSets := resetMinCounts(job.PodSets())
 
 	if !workload.CanBePartiallyAdmitted(wl) || !workload.HasQuotaReservation(wl) {
+		changePodSpecFields := job.IsActive() && !job.IsSuspended()
 		// the two sets should fully match.
-		return equality.ComparePodSetSlices(jobPodSets, wl.Spec.PodSets, true)
+		return equality.ComparePodSetSlices(jobPodSets, wl.Spec.PodSets, true, changePodSpecFields)
 	}
 
 	// Check everything but the pod counts.
-	if !equality.ComparePodSetSlices(jobPodSets, wl.Spec.PodSets, false) {
+	if !equality.ComparePodSetSlices(jobPodSets, wl.Spec.PodSets, false, false) {
 		return false
 	}
 

diff --git a/pkg/util/equality/podset.go b/pkg/util/equality/podset.go
@@ -26,30 +26,33 @@ import (
 
 // TODO: Revisit this, maybe we should extend the check to everything that could potentially impact
 // the workload scheduling (priority, nodeSelectors(when suspended), tolerations and maybe more)
-func comparePodTemplate(a, b *corev1.PodSpec) bool {
+func comparePodTemplate(a, b *corev1.PodSpec, checkCount, changePodSpecFields bool) bool {
+	if changePodSpecFields && !equality.Semantic.DeepEqual(a.Tolerations, b.Tolerations) {
+		return false
+	}
 	if !equality.Semantic.DeepEqual(a.InitContainers, b.InitContainers) {
 		return false
 	}
 	return equality.Semantic.DeepEqual(a.Containers, b.Containers)
 }
 
-func ComparePodSets(a, b *kueue.PodSet, checkCount bool) bool {
+func ComparePodSets(a, b *kueue.PodSet, checkCount, changePodSpecFields bool) bool {
 	if checkCount && a.Count != b.Count {
 		return false
 	}
 	if ptr.Deref(a.MinCount, -1) != ptr.Deref(b.MinCount, -1) {
 		return false
 	}
 
-	return comparePodTemplate(&a.Template.Spec, &b.Template.Spec)
+	return comparePodTemplate(&a.Template.Spec, &b.Template.Spec, checkCount, changePodSpecFields)
 }
 
-func ComparePodSetSlices(a, b []kueue.PodSet, checkCount bool) bool {
+func ComparePodSetSlices(a, b []kueue.PodSet, checkCount, changePodSpecFields bool) bool {
 	if len(a) != len(b) {
 		return false
 	}
 	for i := range a {
-		if !ComparePodSets(&a[i], &b[i], checkCount) {
+		if !ComparePodSets(&a[i], &b[i], checkCount, changePodSpecFields) {
 			return false
 		}
 	}

diff --git a/pkg/util/equality/podset_test.go b/pkg/util/equality/podset_test.go
@@ -28,10 +28,11 @@ import (
 
 func TestComparePodSetSlices(t *testing.T) {
 	cases := map[string]struct {
-		a           []kueue.PodSet
-		b           []kueue.PodSet
-		checkCounts bool
-		wantEqual   bool
+		a                   []kueue.PodSet
+		b                   []kueue.PodSet
+		checkCounts         bool
+		changePodSpecFields bool
+		wantEqual           bool
 	}{
 		"different name": {
 			a:         []kueue.PodSet{*utiltestting.MakePodSet("ps", 10).SetMinimumCount(5).Obj()},
@@ -77,6 +78,22 @@ func TestComparePodSetSlices(t *testing.T) {
 			}).Obj()},
 			wantEqual: false,
 		},
+		"different requests in toleration": {
+			a: []kueue.PodSet{*utiltestting.MakePodSet("ps", 10).SetMinimumCount(5).Toleration(corev1.Toleration{
+				Key:      "instance",
+				Operator: corev1.TolerationOpEqual,
+				Value:    "spot",
+				Effect:   corev1.TaintEffectNoSchedule,
+			}).Obj()},
+			b: []kueue.PodSet{*utiltestting.MakePodSet("ps", 10).SetMinimumCount(5).Toleration(corev1.Toleration{
+				Key:      "instance",
+				Operator: corev1.TolerationOpEqual,
+				Value:    "demand",
+				Effect:   corev1.TaintEffectNoSchedule,
+			}).Obj()},
+			changePodSpecFields: true,
+			wantEqual:           false,
+		},
 		"different count when checked": {
 			a:           []kueue.PodSet{*utiltestting.MakePodSet("ps", 10).SetMinimumCount(5).Obj()},
 			b:           []kueue.PodSet{*utiltestting.MakePodSet("ps", 20).SetMinimumCount(5).Obj()},
@@ -92,7 +109,7 @@ func TestComparePodSetSlices(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			got := ComparePodSetSlices(tc.a, tc.b, tc.checkCounts)
+			got := ComparePodSetSlices(tc.a, tc.b, tc.checkCounts, tc.changePodSpecFields)
 			if got != tc.wantEqual {
 				t.Errorf("Unexpected result, want %v", tc.wantEqual)
 			}