Skip to content

Commit

Permalink
Update job-uid label validation
Browse files Browse the repository at this point in the history
* Replace len label check with IsValidLabelValue from k8s
  apimachinery, which is used on the server side to
  validate the label values.

* Add a log message, for the case, when UID label
  validation returns any errors.
  • Loading branch information
achernevskii committed Aug 3, 2023
1 parent 866a09a commit 0350ad8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 7 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ const (
ParentWorkloadAnnotation = "kueue.x-k8s.io/parent-workload"

// JobUIDLabel is the label key in the workload resource, that holds the UID of
// a parent job.
// the owner job.
JobUIDLabel = "kueue.x-k8s.io/job-uid"
)
14 changes: 12 additions & 2 deletions pkg/controller/jobframework/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -456,6 +457,8 @@ func (r *JobReconciler) stopJob(ctx context.Context, job GenericJob, object clie

// constructWorkload will derive a workload from the corresponding job.
func (r *JobReconciler) constructWorkload(ctx context.Context, job GenericJob, object client.Object) (*kueue.Workload, error) {
log := ctrl.LoggerFrom(ctx)

podSets := job.PodSets()

wl := &kueue.Workload{
Expand All @@ -470,8 +473,15 @@ func (r *JobReconciler) constructWorkload(ctx context.Context, job GenericJob, o
},
}

if uid := string(job.Object().GetUID()); len(uid) < 64 && uid != "" {
wl.Labels[controllerconsts.JobUIDLabel] = uid
jobUid := string(job.Object().GetUID())
if errs := validation.IsValidLabelValue(jobUid); len(errs) == 0 {
wl.Labels[controllerconsts.JobUIDLabel] = jobUid
} else {
log.V(2).Info(
"Validation of the owner job UID label has failed. Creating workload without the label.",
"ValidationErrors", errs,
"LabelValue", jobUid,
)
}

priorityClassName, p, err := r.extractPriority(ctx, podSets, job)
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/jobs/job/job_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ func TestReconciler(t *testing.T) {
PodSets(*utiltesting.MakePodSet(kueue.DefaultPodSetName, 10).Request(corev1.ResourceCPU, "1").Obj()).
Queue("test-queue").
Priority(0).
SetLabels(map[string]string{
Labels(map[string]string{
controllerconsts.JobUIDLabel: "test-uid",
}).
Obj(),
Expand All @@ -464,7 +464,7 @@ func TestReconciler(t *testing.T) {
PodSets(*utiltesting.MakePodSet(kueue.DefaultPodSetName, 10).Request(corev1.ResourceCPU, "1").Obj()).
Queue("test-queue").
Priority(0).
SetLabels(map[string]string{}).
Labels(map[string]string{}).
Obj(),
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/testing/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ func (w *WorkloadWrapper) ReclaimablePods(rps ...kueue.ReclaimablePod) *Workload
return w
}

func (w *WorkloadWrapper) SetLabels(l map[string]string) *WorkloadWrapper {
w.Labels = l
func (w *WorkloadWrapper) Labels(l map[string]string) *WorkloadWrapper {
w.ObjectMeta.Labels = l
return w
}

Expand Down

0 comments on commit 0350ad8

Please sign in to comment.