add updateConditionAndPhase comment

kubernetes-sigs · Jun 7, 2024 · 9e78074 · 9e78074
1 parent 25396be
commit 9e78074
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 8 deletions.
diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go
@@ -898,11 +898,13 @@ func setCondition(js *jobset.JobSet, condOpts *conditionOpts, updateStatusOpts *
 	enqueueEvent(updateStatusOpts, event)
 }
 
-// updateConditionAndPhase accepts a given condition and does one of the following:
+// updateConditionAndPhase accepts a condition and a phase, and does the following:
 //  1. If an identical condition already exists, do nothing and return false (indicating
 //     no change was made).
 //  2. If a condition of the same type exists but with a different status, update
 //     the condition in place and return true (indicating a condition change was made).
+//  3. If the specified phase is different from the current phase of the JobSet,
+//     update the JobSet Status Phase
 func updateConditionAndPhase(js *jobset.JobSet, opts *conditionOpts) bool {
 	if opts == nil || opts.condition == nil {
 		return false
@@ -943,7 +945,7 @@ func updateConditionAndPhase(js *jobset.JobSet, opts *conditionOpts) bool {
 		shouldUpdate = true
 	}
 
-	// Update the JobSet phase if necessary.
+	// Update the JobSet Status Phase if necessary.
 	if opts.phase != "" && js.Status.Phase != opts.phase {
 		js.Status.Phase = opts.phase
 		shouldUpdate = true

diff --git a/pkg/controllers/jobset_controller_test.go b/pkg/controllers/jobset_controller_test.go
@@ -696,7 +696,7 @@ func TestUpdateConditions(t *testing.T) {
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
 					Replicas(1).
-					Obj()).Obj(),
+					Obj()).Phase(jobset.JobSetRunning).Obj(),
 			opts:           makeCompletedConditionsOpts(),
 			expectedUpdate: true,
 		},
@@ -706,7 +706,7 @@ func TestUpdateConditions(t *testing.T) {
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
 					Replicas(1).
-					Obj()).Obj(),
+					Obj()).Phase(jobset.JobSetRunning).Obj(),
 			opts:           makeSuspendedConditionOpts(),
 			expectedUpdate: true,
 		},
@@ -716,7 +716,7 @@ func TestUpdateConditions(t *testing.T) {
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
 					Replicas(1).
-					Obj()).
+					Obj()).Phase(jobset.JobSetRunning).
 				Conditions([]metav1.Condition{
 					// JobSet is currrently suspended.
 					{
@@ -736,7 +736,7 @@ func TestUpdateConditions(t *testing.T) {
 				ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
 					Job(testutils.MakeJobTemplate(jobName, ns).Obj()).
 					Replicas(1).
-					Obj()).Phase(string(jobset.JobSetCompleted)).
+					Obj()).Phase(jobset.JobSetCompleted).
 				Conditions([]metav1.Condition{
 					// JobSet is completed..
 					{

diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go
@@ -160,8 +160,8 @@ func (j *JobSetWrapper) FailedCondition(failedAt metav1.Time) *JobSetWrapper {
 }
 
 // Phase sets the value of JobSet.Status.Phase.
-func (j *JobSetWrapper) Phase(phase string) *JobSetWrapper {
-	j.Status.Phase = phase
+func (j *JobSetWrapper) Phase(phase jobset.JobSetConditionType) *JobSetWrapper {
+	j.Status.Phase = string(phase)
 	return j
 }