Skip to content

Commit 82feee6

Browse files
kannon92kannon92
committed
promote local queue defaulting to ga
1 parent f48bbd0 commit 82feee6

File tree

23 files changed

+158
-289
lines changed

23 files changed

+158
-289
lines changed

‎keps/2936-local-queue-defaulting/kep.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,16 @@ stage: alpha
1717
# The most recent milestone for which work toward delivery of this KEP has been
1818
# done. This can be the current (upcoming) milestone, if it is being actively
1919
# worked on.
20-
latest-milestone: "v0.10"
20+
latest-milestone: "v0.17"
2121

2222
# The milestone at which this feature was, or is targeted to be, at each stage.
2323
milestone:
2424
alpha: "v0.10"
25-
beta:
26-
stable:
25+
beta: "v0.11"
26+
stable: "v0.17"
2727

2828
# The following PRR answers are required at alpha release
2929
# List the feature gate name and the components for which it must be enabled
3030
feature-gates:
3131
- name: LocalQueueDefaulting
32-
disable-supported: true
32+
disable-supported: false

‎pkg/controller/constants/constants.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const (
2323
QueueLabel = "kueue.x-k8s.io/queue-name"
2424

2525
// DefaultLocalQueueName is the name for default LocalQueue that is applied
26-
// if the feature LocalQueueDefaulting is enabled and QueueLabel is not specified.
26+
// if QueueLabel is not specified.
2727
DefaultLocalQueueName kueue.LocalQueueName = "default"
2828

2929
// PrebuiltWorkloadLabel is the label key of the job holding the name of the pre-built workload to use.

‎pkg/controller/jobframework/base_webhook_test.go‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import (
4343
func TestBaseWebhookDefault(t *testing.T) {
4444
testcases := map[string]struct {
4545
manageJobsWithoutQueueName bool
46-
localQueueDefaulting bool
4746
defaultLqExist bool
4847
enableMultiKueue bool
4948
job *batchv1.Job
@@ -58,25 +57,22 @@ func TestBaseWebhookDefault(t *testing.T) {
5857
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Obj(),
5958
want: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Suspend(true).Obj(),
6059
},
61-
"LocalQueueDefaulting enabled, default lq is created, job doesn't have queue label": {
62-
localQueueDefaulting: true,
63-
defaultLqExist: true,
64-
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
60+
"default lq is created, job doesn't have queue label": {
61+
defaultLqExist: true,
62+
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
6563
want: utiljob.MakeJob("job", metav1.NamespaceDefault).
6664
Label(constants.QueueLabel, "default").
6765
Obj(),
6866
},
69-
"LocalQueueDefaulting enabled, default lq is created, job has queue label": {
70-
localQueueDefaulting: true,
71-
defaultLqExist: true,
72-
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Obj(),
73-
want: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Obj(),
67+
"default lq is created, job has queue label": {
68+
defaultLqExist: true,
69+
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Obj(),
70+
want: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("queue").Obj(),
7471
},
75-
"LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label": {
76-
localQueueDefaulting: true,
77-
defaultLqExist: false,
78-
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
79-
want: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
72+
"default lq isn't created, job doesn't have queue label": {
73+
defaultLqExist: false,
74+
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
75+
want: utiljob.MakeJob("job", metav1.NamespaceDefault).Obj(),
8076
},
8177
"ManagedByDefaulting, targeting multikueue local queue": {
8278
job: utiljob.MakeJob("job", metav1.NamespaceDefault).Queue("multikueue").Obj(),
@@ -110,7 +106,6 @@ func TestBaseWebhookDefault(t *testing.T) {
110106
for name, tc := range testcases {
111107
t.Run(name, func(t *testing.T) {
112108
ctx, log := utiltesting.ContextWithLog(t)
113-
features.SetFeatureGateDuringTest(t, features.LocalQueueDefaulting, tc.localQueueDefaulting)
114109
features.SetFeatureGateDuringTest(t, features.MultiKueue, tc.enableMultiKueue)
115110
clientBuilder := utiltesting.NewClientBuilder().
116111
WithObjects(

‎pkg/controller/jobframework/defaults.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
qcache "sigs.k8s.io/kueue/pkg/cache/queue"
3131
schdcache "sigs.k8s.io/kueue/pkg/cache/scheduler"
3232
"sigs.k8s.io/kueue/pkg/controller/constants"
33-
"sigs.k8s.io/kueue/pkg/features"
3433
utilqueue "sigs.k8s.io/kueue/pkg/util/queue"
3534
)
3635

@@ -79,7 +78,7 @@ func WorkloadShouldBeSuspended(ctx context.Context, jobObj client.Object, k8sCli
7978
}
8079

8180
func ApplyDefaultLocalQueue(jobObj client.Object, defaultQueueExist func(string) bool) {
82-
if !features.Enabled(features.LocalQueueDefaulting) || !defaultQueueExist(jobObj.GetNamespace()) {
81+
if !defaultQueueExist(jobObj.GetNamespace()) {
8382
return
8483
}
8584
if QueueNameForObject(jobObj) == "" {

‎pkg/controller/jobframework/validation.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import (
3838
jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2"
3939

4040
"sigs.k8s.io/kueue/pkg/controller/constants"
41-
"sigs.k8s.io/kueue/pkg/features"
4241
utilpod "sigs.k8s.io/kueue/pkg/util/pod"
4342
"sigs.k8s.io/kueue/pkg/workloadslicing"
4443
)
@@ -117,10 +116,8 @@ func validateUpdateForQueueName(oldJob, newJob GenericJob, defaultQueueExist fun
117116
if !newJob.IsSuspended() {
118117
allErrs = append(allErrs, apivalidation.ValidateImmutableField(QueueName(newJob), QueueName(oldJob), queueNameLabelPath)...)
119118
}
120-
if features.Enabled(features.LocalQueueDefaulting) {
121-
if QueueName(newJob) == "" && QueueName(oldJob) != "" && defaultQueueExist(oldJob.Object().GetNamespace()) {
122-
allErrs = append(allErrs, field.Invalid(queueNameLabelPath, "", "queue-name must not be empty in namespace with default queue"))
123-
}
119+
if QueueName(newJob) == "" && QueueName(oldJob) != "" && defaultQueueExist(oldJob.Object().GetNamespace()) {
120+
allErrs = append(allErrs, field.Invalid(queueNameLabelPath, "", "queue-name must not be empty in namespace with default queue"))
124121
}
125122

126123
return allErrs

‎pkg/controller/jobframework/validation_test.go‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,8 @@ func TestValidateJobOnUpdate(t *testing.T) {
262262
wantErr field.ErrorList
263263
}{
264264
"local queue cannot be changed if job is not suspended": {
265-
oldJob: utiltestingjob.MakeJob("test-job", "ns1").Queue("lq1").Suspend(false).Obj(),
266-
newJob: utiltestingjob.MakeJob("test-job", "ns1").Queue("lq2").Suspend(false).Obj(),
267-
featureGates: map[featuregate.Feature]bool{features.LocalQueueDefaulting: true},
265+
oldJob: utiltestingjob.MakeJob("test-job", "ns1").Queue("lq1").Suspend(false).Obj(),
266+
newJob: utiltestingjob.MakeJob("test-job", "ns1").Queue("lq2").Suspend(false).Obj(),
268267
wantErr: field.ErrorList{
269268
&field.Error{
270269
Type: field.ErrorTypeInvalid,
@@ -298,12 +297,6 @@ func TestValidateJobOnUpdate(t *testing.T) {
298297
newJob: utiltestingjob.MakeJob("test-job", "ns1").Suspend(true).Queue("").Obj(),
299298
nsHasDefaultQueue: false,
300299
},
301-
"local queue can be removed if feature is not enabled": {
302-
oldJob: utiltestingjob.MakeJob("test-job", "ns1").Suspend(true).Queue("lq1").Obj(),
303-
newJob: utiltestingjob.MakeJob("test-job", "ns1").Suspend(true).Queue("").Obj(),
304-
nsHasDefaultQueue: true,
305-
featureGates: map[featuregate.Feature]bool{features.LocalQueueDefaulting: false},
306-
},
307300
"elastic job enabled annotation cannot be removed on update": {
308301
oldJob: utiltestingjob.MakeJob("test-job", "ns1").SetAnnotation(workloadslicing.EnabledAnnotationKey, workloadslicing.EnabledAnnotationValue).Obj(),
309302
newJob: utiltestingjob.MakeJob("test-job", "ns1").Obj(),

‎pkg/controller/jobs/deployment/deployment_webhook_test.go‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,16 @@ import (
3030
"sigs.k8s.io/kueue/pkg/controller/constants"
3131
"sigs.k8s.io/kueue/pkg/controller/jobframework"
3232
podconstants "sigs.k8s.io/kueue/pkg/controller/jobs/pod/constants"
33-
"sigs.k8s.io/kueue/pkg/features"
3433
utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
3534
utiltestingapi "sigs.k8s.io/kueue/pkg/util/testing/v1beta2"
3635
testingdeployment "sigs.k8s.io/kueue/pkg/util/testingjobs/deployment"
3736
)
3837

3938
func TestDefault(t *testing.T) {
4039
testCases := map[string]struct {
41-
deployment *appsv1.Deployment
42-
localQueueDefaulting bool
43-
defaultLqExist bool
44-
want *appsv1.Deployment
40+
deployment *appsv1.Deployment
41+
defaultLqExist bool
42+
want *appsv1.Deployment
4543
}{
4644
"deployment without queue": {
4745
deployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
@@ -74,32 +72,29 @@ func TestDefault(t *testing.T) {
7472
deployment: testingdeployment.MakeDeployment("test-pod", "").PodTemplateSpecQueue("test-queue").Obj(),
7573
want: testingdeployment.MakeDeployment("test-pod", "").PodTemplateSpecQueue("test-queue").Obj(),
7674
},
77-
"LocalQueueDefaulting enabled, default lq is created, job doesn't have queue label": {
78-
localQueueDefaulting: true,
79-
defaultLqExist: true,
80-
deployment: testingdeployment.MakeDeployment("test-pod", "default").Obj(),
75+
"default lq is created, job doesn't have queue label": {
76+
defaultLqExist: true,
77+
deployment: testingdeployment.MakeDeployment("test-pod", "default").Obj(),
8178
want: testingdeployment.MakeDeployment("test-pod", "default").
8279
PodTemplateSpecManagedByKueue().
8380
Queue("default").
8481
PodTemplateSpecQueue("default").
8582
PodTemplateAnnotation(podconstants.SuspendedByParentAnnotation, FrameworkName).
8683
Obj(),
8784
},
88-
"LocalQueueDefaulting enabled, default lq is created, job has queue label": {
89-
localQueueDefaulting: true,
90-
defaultLqExist: true,
91-
deployment: testingdeployment.MakeDeployment("test-pod", "").Queue("test-queue").Obj(),
85+
"default lq is created, job has queue label": {
86+
defaultLqExist: true,
87+
deployment: testingdeployment.MakeDeployment("test-pod", "").Queue("test-queue").Obj(),
9288
want: testingdeployment.MakeDeployment("test-pod", "").
9389
PodTemplateSpecManagedByKueue().
9490
Queue("test-queue").
9591
PodTemplateSpecQueue("test-queue").
9692
PodTemplateAnnotation(podconstants.SuspendedByParentAnnotation, FrameworkName).
9793
Obj(),
9894
},
99-
"LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label": {
100-
localQueueDefaulting: true,
101-
defaultLqExist: false,
102-
deployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
95+
"default lq isn't created, job doesn't have queue label": {
96+
defaultLqExist: false,
97+
deployment: testingdeployment.MakeDeployment("test-pod", "").Obj(),
10398
want: testingdeployment.MakeDeployment("test-pod", "").
10499
Obj(),
105100
},
@@ -148,7 +143,6 @@ func TestDefault(t *testing.T) {
148143
for name, tc := range testCases {
149144
t.Run(name, func(t *testing.T) {
150145
ctx, _ := utiltesting.ContextWithLog(t)
151-
features.SetFeatureGateDuringTest(t, features.LocalQueueDefaulting, tc.localQueueDefaulting)
152146
t.Cleanup(jobframework.EnableIntegrationsForTest(t, "pod"))
153147
builder := utiltesting.NewClientBuilder()
154148
client := builder.Build()

‎pkg/controller/jobs/job/job_webhook_test.go‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,6 @@ func TestDefault(t *testing.T) {
724724
admissionCheck *kueue.AdmissionCheck
725725
manageJobsWithoutQueueName bool
726726
multiKueueEnabled bool
727-
localQueueDefaulting bool
728727
defaultLqExist bool
729728
enableIntegrations []string
730729
want *batchv1.Job
@@ -805,32 +804,28 @@ func TestDefault(t *testing.T) {
805804
Obj(),
806805
multiKueueEnabled: true,
807806
},
808-
"LocalQueueDefaulting enabled, default lq is created, job doesn't have queue label": {
809-
localQueueDefaulting: true,
810-
defaultLqExist: true,
811-
job: testingutil.MakeJob("test-job", "default").Obj(),
807+
"default lq is created, job doesn't have queue label": {
808+
defaultLqExist: true,
809+
job: testingutil.MakeJob("test-job", "default").Obj(),
812810
want: testingutil.MakeJob("test-job", "default").
813811
Queue("default").
814812
Obj(),
815813
},
816-
"LocalQueueDefaulting enabled, default lq is created, job has queue label": {
817-
localQueueDefaulting: true,
818-
defaultLqExist: true,
819-
job: testingutil.MakeJob("test-job", "").Queue("test-queue").Obj(),
814+
"default lq is created, job has queue label": {
815+
defaultLqExist: true,
816+
job: testingutil.MakeJob("test-job", "").Queue("test-queue").Obj(),
820817
want: testingutil.MakeJob("test-job", "").
821818
Queue("test-queue").
822819
Obj(),
823820
},
824-
"LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label": {
825-
localQueueDefaulting: true,
826-
defaultLqExist: false,
827-
job: testingutil.MakeJob("test-job", "").Obj(),
821+
"default lq isn't created, job doesn't have queue label": {
822+
defaultLqExist: false,
823+
job: testingutil.MakeJob("test-job", "").Obj(),
828824
want: testingutil.MakeJob("test-job", "").
829825
Obj(),
830826
},
831-
"LocalQueueDefaulting enabled, job is managed by Kueue managed owner, job doesn't have queue label": {
832-
localQueueDefaulting: true,
833-
defaultLqExist: true,
827+
"job is managed by Kueue managed owner, job doesn't have queue label": {
828+
defaultLqExist: true,
834829
// MPIJob callBackFunction is registered as integrations since we initialize MPIJob integration package.
835830
enableIntegrations: []string{"kubeflow.org/mpijob"},
836831
job: testingutil.MakeJob("test-job", metav1.NamespaceDefault).
@@ -843,9 +838,8 @@ func TestDefault(t *testing.T) {
843838
OwnerReference("owner", kfmpi.SchemeGroupVersionKind).
844839
Obj(),
845840
},
846-
"LocalQueueDefaulting enabled, job is managed by non Kueue managed owner, job has queue label": {
847-
localQueueDefaulting: true,
848-
defaultLqExist: true,
841+
"job is managed by non Kueue managed owner, job has queue label": {
842+
defaultLqExist: true,
849843
job: testingutil.MakeJob("test-job", metav1.NamespaceDefault).
850844
OwnerReference("owner", jobsetapi.SchemeGroupVersion.WithKind("JobSet")).
851845
Obj(),
@@ -858,7 +852,6 @@ func TestDefault(t *testing.T) {
858852
for name, tc := range testcases {
859853
t.Run(name, func(t *testing.T) {
860854
features.SetFeatureGateDuringTest(t, features.MultiKueue, tc.multiKueueEnabled)
861-
features.SetFeatureGateDuringTest(t, features.LocalQueueDefaulting, tc.localQueueDefaulting)
862855

863856
ctx, log := utiltesting.ContextWithLog(t)
864857

‎pkg/controller/jobs/jobset/jobset_webhook_test.go‎

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -353,17 +353,16 @@ func TestValidateUpdate(t *testing.T) {
353353

354354
func TestDefault(t *testing.T) {
355355
testCases := []struct {
356-
name string
357-
jobSet *jobset.JobSet
358-
queues []kueue.LocalQueue
359-
clusterQueues []kueue.ClusterQueue
360-
admissionCheck *kueue.AdmissionCheck
361-
multiKueueEnabled bool
362-
localQueueDefaulting bool
363-
defaultLqExist bool
364-
want *jobset.JobSet
365-
wantManagedBy *string
366-
wantErr error
356+
name string
357+
jobSet *jobset.JobSet
358+
queues []kueue.LocalQueue
359+
clusterQueues []kueue.ClusterQueue
360+
admissionCheck *kueue.AdmissionCheck
361+
multiKueueEnabled bool
362+
defaultLqExist bool
363+
want *jobset.JobSet
364+
wantManagedBy *string
365+
wantErr error
367366
}{
368367
{
369368
name: "TestDefault_JobSetManagedBy_jobsetapi.JobSetControllerName",
@@ -555,32 +554,28 @@ func TestDefault(t *testing.T) {
555554
wantManagedBy: nil,
556555
},
557556
{
558-
name: "LocalQueueDefaulting enabled, default lq is created, job doesn't have queue label",
559-
localQueueDefaulting: true,
560-
defaultLqExist: true,
561-
jobSet: testingutil.MakeJobSet("test-js", "default").Obj(),
562-
want: testingutil.MakeJobSet("test-js", "default").Queue("default").Obj(),
557+
name: "default lq is created, job doesn't have queue label",
558+
defaultLqExist: true,
559+
jobSet: testingutil.MakeJobSet("test-js", "default").Obj(),
560+
want: testingutil.MakeJobSet("test-js", "default").Queue("default").Obj(),
563561
},
564562
{
565-
name: "LocalQueueDefaulting enabled, default lq is created, job has queue label",
566-
localQueueDefaulting: true,
567-
defaultLqExist: true,
568-
jobSet: testingutil.MakeJobSet("test-js", "default").Queue("queue").Obj(),
569-
want: testingutil.MakeJobSet("test-js", "default").Queue("queue").Obj(),
563+
name: "default lq is created, job has queue label",
564+
defaultLqExist: true,
565+
jobSet: testingutil.MakeJobSet("test-js", "default").Queue("queue").Obj(),
566+
want: testingutil.MakeJobSet("test-js", "default").Queue("queue").Obj(),
570567
},
571568
{
572-
name: "LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label",
573-
localQueueDefaulting: true,
574-
defaultLqExist: false,
575-
jobSet: testingutil.MakeJobSet("test-js", "default").Obj(),
576-
want: testingutil.MakeJobSet("test-js", "default").Obj(),
569+
name: "default lq isn't created, job doesn't have queue label",
570+
defaultLqExist: false,
571+
jobSet: testingutil.MakeJobSet("test-js", "default").Obj(),
572+
want: testingutil.MakeJobSet("test-js", "default").Obj(),
577573
},
578574
}
579575

580576
for _, tc := range testCases {
581577
t.Run(tc.name, func(t *testing.T) {
582578
features.SetFeatureGateDuringTest(t, features.MultiKueue, tc.multiKueueEnabled)
583-
features.SetFeatureGateDuringTest(t, features.LocalQueueDefaulting, tc.localQueueDefaulting)
584579

585580
ctx, log := utiltesting.ContextWithLog(t)
586581

‎pkg/controller/jobs/leaderworkerset/leaderworkerset_webhook_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestDefault(t *testing.T) {
6565
WorkerTemplateSpecAnnotation(podconstants.GroupServingAnnotationKey, podconstants.GroupServingAnnotationValue).
6666
Obj(),
6767
},
68-
"LocalQueueDefaulting enabled, default lq is created, job doesn't have queue label": {
68+
"default lq is created, job doesn't have queue label": {
6969
defaultLqExist: true,
7070
lws: testingleaderworkerset.MakeLeaderWorkerSet("test-lws", "default").
7171
LeaderTemplate(corev1.PodTemplateSpec{}).
@@ -79,7 +79,7 @@ func TestDefault(t *testing.T) {
7979
WorkerTemplateSpecAnnotation(podconstants.GroupServingAnnotationKey, podconstants.GroupServingAnnotationValue).
8080
Obj(),
8181
},
82-
"LocalQueueDefaulting enabled, default lq is created, job has queue label": {
82+
"default lq is created, job has queue label": {
8383
defaultLqExist: true,
8484
lws: testingleaderworkerset.MakeLeaderWorkerSet("test-lws", "").
8585
LeaderTemplate(corev1.PodTemplateSpec{}).
@@ -94,7 +94,7 @@ func TestDefault(t *testing.T) {
9494
WorkerTemplateSpecAnnotation(podconstants.GroupServingAnnotationKey, podconstants.GroupServingAnnotationValue).
9595
Obj(),
9696
},
97-
"LocalQueueDefaulting enabled, default lq isn't created, job doesn't have queue label": {
97+
"default lq isn't created, job doesn't have queue label": {
9898
defaultLqExist: false,
9999
lws: testingleaderworkerset.MakeLeaderWorkerSet("test-lws", "").
100100
LeaderTemplate(corev1.PodTemplateSpec{}).

0 commit comments

Comments
 (0)