From 7544e124731ba383f70224ed4bcd2e7bf9572541 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Fri, 16 Aug 2024 18:49:52 +0000 Subject: [PATCH] change name to job global index --- api/jobset/v1alpha2/jobset_types.go | 4 +- pkg/controllers/jobset_controller.go | 22 ++++----- pkg/controllers/jobset_controller_test.go | 54 +++++++++++------------ 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/api/jobset/v1alpha2/jobset_types.go b/api/jobset/v1alpha2/jobset_types.go index 0bd1dcbd..01f2f7fe 100644 --- a/api/jobset/v1alpha2/jobset_types.go +++ b/api/jobset/v1alpha2/jobset_types.go @@ -28,9 +28,9 @@ const ( // For each replicatedJob, this value will range from 0 to replicas-1, where `replicas` // is equal to jobset.spec.replicatedJobs[*].replicas. JobIndexKey string = "jobset.sigs.k8s.io/job-index" - // JobIDKey is a label/annotation set to an integer that is unique across the entire JobSet. + // JobGlobalIndexKey is a label/annotation set to an integer that is unique across the entire JobSet. // For each JobSet, this value will range from 0 to N-1, where N=total number of jobs in the jobset. - JobIDKey string = "jobset.sigs.k8s.io/job-id" + JobGlobalIndexKey string = "jobset.sigs.k8s.io/job-global-index" // JobKey holds the SHA256 hash of the namespaced job name, which can be used to uniquely identify the job. JobKey string = "jobset.sigs.k8s.io/job-key" // ExclusiveKey is an annotation that can be set on the JobSet or on a ReplicatedJob template. diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go index c895838d..f41ea3ba 100644 --- a/pkg/controllers/jobset_controller.go +++ b/pkg/controllers/jobset_controller.go @@ -730,7 +730,7 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R labels[jobset.ReplicatedJobReplicas] = strconv.Itoa(int(rjob.Replicas)) labels[jobset.JobIndexKey] = strconv.Itoa(jobIdx) labels[jobset.JobKey] = jobHashKey(js.Namespace, jobName) - labels[jobset.JobIDKey] = calculateJobID(js, rjob.Name, jobIdx) + labels[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjob.Name, jobIdx) // Set annotations on the object. annotations := collections.CloneMap(obj.GetAnnotations()) @@ -740,7 +740,7 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R annotations[jobset.ReplicatedJobReplicas] = strconv.Itoa(int(rjob.Replicas)) annotations[jobset.JobIndexKey] = strconv.Itoa(jobIdx) annotations[jobset.JobKey] = jobHashKey(js.Namespace, jobName) - annotations[jobset.JobIDKey] = calculateJobID(js, rjob.Name, jobIdx) + annotations[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjob.Name, jobIdx) // Apply coordinator annotation/label if a coordinator is defined in the JobSet spec. if js.Spec.Coordinator != nil { @@ -1035,25 +1035,25 @@ func coordinatorEndpoint(js *jobset.JobSet) string { return fmt.Sprintf("%s-%s-%d-%d.%s", js.Name, js.Spec.Coordinator.ReplicatedJob, js.Spec.Coordinator.JobIndex, js.Spec.Coordinator.PodIndex, GetSubdomain(js)) } -// calculateJobID determines the job ID for a given job. The job ID is a unique +// globalJobIndex determines the job global index for a given job. The job global index is a unique // global index for the job, with values ranging from 0 to N-1, -// where N=total number of jobs in the jobset. The job ID is calculated by +// where N=total number of jobs in the jobset. The job global index is calculated by // iterating through the replicatedJobs in the order, as defined in the JobSet // spec, keeping a cumulative sum of total replicas seen so far, then when we // arrive at the parent replicatedJob of the target job, we add the local job // index to our running sum of total jobs seen so far, in order to arrive at -// the final job ID value. +// the final job global index value. // -// Below is a diagram illustrating how job IDs differ from job indexes. +// Below is a diagram illustrating how job global indexs differ from job indexes. // -// | my-jobset | -// | replicated job A | replicated job B | -// | job ID 0 | job ID 1 | job ID 2 | job ID 3 | -// | job index 0 | job index 1 | job index 0 | job index 1 | +// | my-jobset | +// | replicated job A | replicated job B | +// | job index 0 | job index 1 | job index 0 | job index 1 | +// | global index 0 | global index 2 | global index 3 | global index 4 | // // Returns an empty string if the parent replicated Job does not exist, // although this should never happen in practice. -func calculateJobID(js *jobset.JobSet, replicatedJobName string, jobIdx int) string { +func globalJobIndex(js *jobset.JobSet, replicatedJobName string, jobIdx int) string { currTotalJobs := 0 for _, rjob := range js.Spec.ReplicatedJobs { if rjob.Name == replicatedJobName { diff --git a/pkg/controllers/jobset_controller_test.go b/pkg/controllers/jobset_controller_test.go index e26eeb0b..42519af1 100644 --- a/pkg/controllers/jobset_controller_test.go +++ b/pkg/controllers/jobset_controller_test.go @@ -688,10 +688,10 @@ func TestConstructJobsFromTemplate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Here we update the expected Jobs with certain features which require // direct access to the JobSet object itself to calculate. For example, - // the `jobset.sigs.k8s.io/job-id` annotation requires access to the + // the `jobset.sigs.k8s.io/job-global-index` annotation requires access to the // full JobSet spec to calculate a unique ID for each Job. for _, expectedJob := range tc.want { - addJobID(t, tc.js, expectedJob) + addJobGlobalIndex(t, tc.js, expectedJob) } // Now get the actual output of constructJobsFromTemplate, and diff the results. @@ -708,10 +708,10 @@ func TestConstructJobsFromTemplate(t *testing.T) { } } -// addJobID modifies the Job object in place by adding -// the `jobset.sigs.k8s.io/job-id` label/annotation to both the +// addJobGlobalIndex modifies the Job object in place by adding +// the `jobset.sigs.k8s.io/job-global-index` label/annotation to both the // Job itself and the Job template spec.` -func addJobID(t *testing.T, js *jobset.JobSet, job *batchv1.Job) { +func addJobGlobalIndex(t *testing.T, js *jobset.JobSet, job *batchv1.Job) { t.Helper() rjobName := job.Annotations[jobset.ReplicatedJobNameKey] @@ -720,12 +720,12 @@ func addJobID(t *testing.T, js *jobset.JobSet, job *batchv1.Job) { t.Fatalf("invalid test case: %v", err) } // Job label/annotation - job.Labels[jobset.JobIDKey] = calculateJobID(js, rjobName, jobIdx) - job.Annotations[jobset.JobIDKey] = calculateJobID(js, rjobName, jobIdx) + job.Labels[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjobName, jobIdx) + job.Annotations[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjobName, jobIdx) // Job template spec label/annotation - job.Spec.Template.Labels[jobset.JobIDKey] = calculateJobID(js, rjobName, jobIdx) - job.Spec.Template.Annotations[jobset.JobIDKey] = calculateJobID(js, rjobName, jobIdx) + job.Spec.Template.Labels[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjobName, jobIdx) + job.Spec.Template.Annotations[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjobName, jobIdx) } func TestUpdateConditions(t *testing.T) { @@ -1411,13 +1411,13 @@ func TestCreateHeadlessSvcIfNecessary(t *testing.T) { } } -func TestCalculateJobID(t *testing.T) { +func TestglobalJobIndex(t *testing.T) { tests := []struct { - name string - jobSet *jobset.JobSet - replicatedJob string - jobIdx int - expectedJobID string + name string + jobSet *jobset.JobSet + replicatedJob string + jobIdx int + expectedJobGlobalIndex string }{ { name: "single replicated job", @@ -1428,9 +1428,9 @@ func TestCalculateJobID(t *testing.T) { }, }, }, - replicatedJob: "rjob", - jobIdx: 1, - expectedJobID: "1", + replicatedJob: "rjob", + jobIdx: 1, + expectedJobGlobalIndex: "1", }, { name: "multiple replicated jobs", @@ -1443,9 +1443,9 @@ func TestCalculateJobID(t *testing.T) { }, }, }, - replicatedJob: "rjob2", - jobIdx: 3, - expectedJobID: "5", + replicatedJob: "rjob2", + jobIdx: 3, + expectedJobGlobalIndex: "5", }, { name: "replicated job not found", @@ -1456,17 +1456,17 @@ func TestCalculateJobID(t *testing.T) { }, }, }, - replicatedJob: "rjob2", - jobIdx: 0, - expectedJobID: "", + replicatedJob: "rjob2", + jobIdx: 0, + expectedJobGlobalIndex: "", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - actualJobID := calculateJobID(tc.jobSet, tc.replicatedJob, tc.jobIdx) - if diff := cmp.Diff(tc.expectedJobID, actualJobID); diff != "" { - t.Errorf("unexpected job ID (-want/+got): %s", diff) + actualJobGlobalIndex := globalJobIndex(tc.jobSet, tc.replicatedJob, tc.jobIdx) + if diff := cmp.Diff(tc.expectedJobGlobalIndex, actualJobGlobalIndex); diff != "" { + t.Errorf("unexpected global job index (-want/+got): %s", diff) } }) }