Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate job labels and annotations #737

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/controllers/jobset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R

// Set labels on the object.
labels := make(map[string]string)
maps.Copy(labels, obj.GetLabels())
labels[jobset.JobSetNameKey] = js.Name
labels[jobset.ReplicatedJobNameKey] = rjob.Name
labels[constants.RestartsKey] = strconv.Itoa(int(js.Status.Restarts))
Expand All @@ -748,6 +749,7 @@ func labelAndAnnotateObject(obj metav1.Object, js *jobset.JobSet, rjob *jobset.R
labels[jobset.JobGlobalIndexKey] = globalJobIndex(js, rjob.Name, jobIdx)

annotations := make(map[string]string)
maps.Copy(annotations, obj.GetAnnotations())
annotations[jobset.JobSetNameKey] = js.Name
annotations[jobset.ReplicatedJobNameKey] = rjob.Name
annotations[constants.RestartsKey] = strconv.Itoa(int(js.Status.Restarts))
Expand Down
56 changes: 52 additions & 4 deletions pkg/controllers/jobset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"context"
"errors"
"fmt"
"maps"
ahg-g marked this conversation as resolved.
Show resolved Hide resolved
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -127,10 +128,18 @@ func TestIsJobFinished(t *testing.T) {

func TestConstructJobsFromTemplate(t *testing.T) {
var (
jobSetName = "test-jobset"
replicatedJobName = "replicated-job"
jobName = "test-job"
ns = "default"
jobSetName = "test-jobset"
replicatedJobName = "replicated-job"
jobName = "test-job"
ns = "default"
annotations = map[string]string{
"foo": "bar",
"foo1": "bar",
imreddy13 marked this conversation as resolved.
Show resolved Hide resolved
}
labels = map[string]string{
"foo": "bar",
"foo1": "bar",
imreddy13 marked this conversation as resolved.
Show resolved Hide resolved
}
topologyDomain = "test-topology-domain"
coordinatorKeyValue = map[string]string{
jobset.CoordinatorKey: fmt.Sprintf("%s-%s-%d-%d.%s", jobSetName, replicatedJobName, 0, 0, jobSetName),
Expand Down Expand Up @@ -183,6 +192,41 @@ func TestConstructJobsFromTemplate(t *testing.T) {
Suspend(false).Obj(),
},
},
{
name: "all jobs/pods created with labels and annotations",
js: testutils.MakeJobSet(jobSetName, ns).
SetAnnotations(annotations).
imreddy13 marked this conversation as resolved.
Show resolved Hide resolved
ReplicatedJob(testutils.MakeReplicatedJob(replicatedJobName).
Job(testutils.MakeJobTemplate(jobName, ns).
SetLabels(labels).
SetAnnotations(annotations).
Obj()).
Replicas(2).
Obj()).Obj(),
ownedJobs: &childJobs{},
want: []*batchv1.Job{
makeJob(&makeJobArgs{
jobSetName: jobSetName,
replicatedJobName: replicatedJobName,
jobName: "test-jobset-replicated-job-0",
ns: ns,
labels: labels,
annotations: annotations,
replicas: 2,
jobIdx: 0}).
Suspend(false).Obj(),
makeJob(&makeJobArgs{
jobSetName: jobSetName,
replicatedJobName: replicatedJobName,
jobName: "test-jobset-replicated-job-1",
ns: ns,
labels: labels,
annotations: annotations,
replicas: 2,
jobIdx: 1}).
Suspend(false).Obj(),
},
},
{
name: "one job created, one job not created (already active)",
js: testutils.MakeJobSet(jobSetName, ns).
Expand Down Expand Up @@ -1251,6 +1295,8 @@ type makeJobArgs struct {
replicatedJobName string
jobName string
ns string
labels map[string]string
annotations map[string]string
replicas int
jobIdx int
restarts int
Expand All @@ -1268,6 +1314,7 @@ func makeJob(args *makeJobArgs) *testutils.JobWrapper {
constants.RestartsKey: strconv.Itoa(args.restarts),
jobset.JobKey: jobHashKey(args.ns, args.jobName),
}
maps.Copy(labels, args.labels)
annotations := map[string]string{
jobset.JobSetNameKey: args.jobSetName,
jobset.ReplicatedJobNameKey: args.replicatedJobName,
Expand All @@ -1276,6 +1323,7 @@ func makeJob(args *makeJobArgs) *testutils.JobWrapper {
constants.RestartsKey: strconv.Itoa(args.restarts),
jobset.JobKey: jobHashKey(args.ns, args.jobName),
}
maps.Copy(annotations, args.annotations)
// Only set exclusive key if we are using exclusive placement per topology.
if args.topology != "" {
annotations[jobset.ExclusiveKey] = args.topology
Expand Down
8 changes: 8 additions & 0 deletions pkg/util/testing/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ func (j *JobTemplateWrapper) PodSpec(podSpec corev1.PodSpec) *JobTemplateWrapper
// SetAnnotations sets the annotations on the Job template.
func (j *JobTemplateWrapper) SetAnnotations(annotations map[string]string) *JobTemplateWrapper {
j.Annotations = annotations
j.Spec.Template.SetAnnotations(annotations)
return j
}

// SetLabels sets the labels on the Job template.
func (j *JobTemplateWrapper) SetLabels(labels map[string]string) *JobTemplateWrapper {
j.Labels = labels
j.Spec.Template.SetLabels(labels)
return j
}

Expand Down