Merge pull request #919 from Thor-wl/0708-cherrypick

UpdateJob when Job annotations or labels changed
volcano-sh · Jul 8, 2020 · cb4aa33 · cb4aa33
2 parents eeb33e7 + 042c351
commit cb4aa33
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 12 deletions.
diff --git a/pkg/controllers/job/job_controller_handler.go b/pkg/controllers/job/job_controller_handler.go
@@ -83,10 +83,9 @@ func (cc *Controller) updateJob(oldObj, newObj interface{}) {
 		return
 	}
 
-	// NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
-	// For Job status, it's used internally and always been updated via our controller.
-	if reflect.DeepEqual(newJob.Spec, oldJob.Spec) && newJob.Status.State.Phase == oldJob.Status.State.Phase {
-		klog.V(6).Infof("Job update event is ignored since no update in 'Spec'.")
+	// No need to update if ResourceVersion is not changed
+	if newJob.ResourceVersion == oldJob.ResourceVersion {
+		klog.V(6).Infof("No need to update because job is not modified.")
 		return
 	}
 
@@ -95,6 +94,13 @@ func (cc *Controller) updateJob(oldObj, newObj interface{}) {
 			newJob.Namespace, newJob.Name, err)
 	}
 
+	// NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
+	// For Job status, it's used internally and always been updated via our controller.
+	if reflect.DeepEqual(newJob.Spec, oldJob.Spec) && newJob.Status.State.Phase == oldJob.Status.State.Phase {
+		klog.V(6).Infof("Job update event is ignored since no update in 'Spec'.")
+		return
+	}
+
 	req := apis.Request{
 		Namespace: newJob.Namespace,
 		JobName:   newJob.Name,

diff --git a/pkg/controllers/job/job_controller_handler_test.go b/pkg/controllers/job/job_controller_handler_test.go
@@ -187,8 +187,9 @@ func TestUpdateJobFunc(t *testing.T) {
 			Name: "Job Update Success Case",
 			oldJob: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "job1",
-					Namespace: namespace,
+					Name:            "job1",
+					Namespace:       namespace,
+					ResourceVersion: "54467984",
 				},
 				Spec: batch.JobSpec{
 					SchedulerName: "volcano",
@@ -202,8 +203,9 @@ func TestUpdateJobFunc(t *testing.T) {
 			},
 			newJob: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "job1",
-					Namespace: namespace,
+					Name:            "job1",
+					Namespace:       namespace,
+					ResourceVersion: "54469999",
 				},
 				Spec: batch.JobSpec{
 					SchedulerName: "volcano",
@@ -220,8 +222,9 @@ func TestUpdateJobFunc(t *testing.T) {
 			Name: "Job Update Failure Case",
 			oldJob: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "job1",
-					Namespace: namespace,
+					Name:            "job1",
+					Namespace:       namespace,
+					ResourceVersion: "54469999",
 				},
 				Spec: batch.JobSpec{
 					SchedulerName: "volcano",
@@ -235,8 +238,9 @@ func TestUpdateJobFunc(t *testing.T) {
 			},
 			newJob: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "job1",
-					Namespace: namespace,
+					Name:            "job1",
+					Namespace:       namespace,
+					ResourceVersion: "54469999",
 				},
 				Spec: batch.JobSpec{
 					SchedulerName: "volcano",