Skip to content

Commit 328bb66

Browse files
authored
Merge pull request #1053 from k8s-infra-cherrypick-robot/cherry-pick-1035-to-release-0.4
[release-0.4] Missing create verb for job webhook
2 parents 1c326de + f42e37a commit 328bb66

File tree

14 files changed

+363
-24
lines changed

14 files changed

+363
-24
lines changed

charts/kueue/templates/webhook/webhook.yaml

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,31 @@ webhooks:
207207
apiVersions:
208208
- v1
209209
operations:
210+
- CREATE
210211
- UPDATE
211212
resources:
212213
- jobs
213214
sideEffects: None
215+
- admissionReviewVersions:
216+
- v1
217+
clientConfig:
218+
service:
219+
name: '{{ include "kueue.fullname" . }}-webhook-service'
220+
namespace: '{{ .Release.Namespace }}'
221+
path: /validate-jobset-x-k8s-io-v1alpha2-jobset
222+
failurePolicy: Fail
223+
name: vjobset.kb.io
224+
rules:
225+
- apiGroups:
226+
- jobset.x-k8s.io
227+
apiVersions:
228+
- v1alpha2
229+
operations:
230+
- CREATE
231+
- UPDATE
232+
resources:
233+
- jobsets
234+
sideEffects: None
214235
- admissionReviewVersions:
215236
- v1
216237
clientConfig:
@@ -226,7 +247,28 @@ webhooks:
226247
apiVersions:
227248
- v2beta1
228249
operations:
250+
- CREATE
229251
- UPDATE
230252
resources:
231253
- mpijobs
232254
sideEffects: None
255+
- admissionReviewVersions:
256+
- v1
257+
clientConfig:
258+
service:
259+
name: '{{ include "kueue.fullname" . }}-webhook-service'
260+
namespace: '{{ .Release.Namespace }}'
261+
path: /validate-ray-io-v1alpha1-rayjob
262+
failurePolicy: Fail
263+
name: vrayjob.kb.io
264+
rules:
265+
- apiGroups:
266+
- ray.io
267+
apiVersions:
268+
- v1alpha1
269+
operations:
270+
- CREATE
271+
- UPDATE
272+
resources:
273+
- rayjobs
274+
sideEffects: None

config/components/webhook/manifests.yaml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ webhooks:
2929
service:
3030
name: webhook-service
3131
namespace: system
32-
path: /mutate-jobset-x-k8s-io-v1alpha1-jobset
32+
path: /mutate-jobset-x-k8s-io-v1alpha2-jobset
3333
failurePolicy: Fail
3434
name: mjobset.kb.io
3535
rules:
3636
- apiGroups:
3737
- jobset.x-k8s.io
3838
apiVersions:
39-
- v1alpha1
39+
- v1alpha2
4040
operations:
4141
- CREATE
4242
resources:
@@ -159,6 +159,7 @@ webhooks:
159159
apiVersions:
160160
- v1
161161
operations:
162+
- CREATE
162163
- UPDATE
163164
resources:
164165
- jobs
@@ -169,15 +170,16 @@ webhooks:
169170
service:
170171
name: webhook-service
171172
namespace: system
172-
path: /validate-jobset-x-k8s-io-v1alpha1-jobset
173+
path: /validate-jobset-x-k8s-io-v1alpha2-jobset
173174
failurePolicy: Fail
174175
name: vjobset.kb.io
175176
rules:
176177
- apiGroups:
177178
- jobset.x-k8s.io
178179
apiVersions:
179-
- v1alpha1
180+
- v1alpha2
180181
operations:
182+
- CREATE
181183
- UPDATE
182184
resources:
183185
- jobsets
@@ -197,6 +199,7 @@ webhooks:
197199
apiVersions:
198200
- v2beta1
199201
operations:
202+
- CREATE
200203
- UPDATE
201204
resources:
202205
- mpijobs
@@ -216,6 +219,7 @@ webhooks:
216219
apiVersions:
217220
- v1alpha1
218221
operations:
222+
- CREATE
219223
- UPDATE
220224
resources:
221225
- rayjobs

pkg/controller/jobs/job/job_webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func (w *JobWebhook) Default(ctx context.Context, obj runtime.Object) error {
8484
return nil
8585
}
8686

87-
// +kubebuilder:webhook:path=/validate-batch-v1-job,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch,resources=jobs,verbs=update,versions=v1,name=vjob.kb.io,admissionReviewVersions=v1
87+
// +kubebuilder:webhook:path=/validate-batch-v1-job,mutating=false,failurePolicy=fail,sideEffects=None,groups=batch,resources=jobs,verbs=create;update,versions=v1,name=vjob.kb.io,admissionReviewVersions=v1
8888

8989
var _ webhook.CustomValidator = &JobWebhook{}
9090

pkg/controller/jobs/jobset/jobset_webhook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func SetupJobSetWebhook(mgr ctrl.Manager, opts ...jobframework.Option) error {
4949
Complete()
5050
}
5151

52-
// +kubebuilder:webhook:path=/mutate-jobset-x-k8s-io-v1alpha1-jobset,mutating=true,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=create,versions=v1alpha1,name=mjobset.kb.io,admissionReviewVersions=v1
52+
// +kubebuilder:webhook:path=/mutate-jobset-x-k8s-io-v1alpha2-jobset,mutating=true,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=create,versions=v1alpha2,name=mjobset.kb.io,admissionReviewVersions=v1
5353

5454
var _ webhook.CustomDefaulter = &JobSetWebhook{}
5555

@@ -62,7 +62,7 @@ func (w *JobSetWebhook) Default(ctx context.Context, obj runtime.Object) error {
6262
return nil
6363
}
6464

65-
// +kubebuilder:webhook:path=/validate-jobset-x-k8s-io-v1alpha1-jobset,mutating=false,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=update,versions=v1alpha1,name=vjobset.kb.io,admissionReviewVersions=v1
65+
// +kubebuilder:webhook:path=/validate-jobset-x-k8s-io-v1alpha2-jobset,mutating=false,failurePolicy=fail,sideEffects=None,groups=jobset.x-k8s.io,resources=jobsets,verbs=create;update,versions=v1alpha2,name=vjobset.kb.io,admissionReviewVersions=v1
6666

6767
var _ webhook.CustomValidator = &JobSetWebhook{}
6868

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package jobset
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
24+
"k8s.io/apimachinery/pkg/util/validation/field"
25+
jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2"
26+
27+
"sigs.k8s.io/kueue/pkg/controller/constants"
28+
testingutil "sigs.k8s.io/kueue/pkg/util/testingjobs/jobset"
29+
)
30+
31+
const (
32+
invalidRFC1123Message = `a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`
33+
)
34+
35+
var (
36+
labelsPath = field.NewPath("metadata", "labels")
37+
queueNameLabelPath = labelsPath.Key(constants.QueueLabel)
38+
)
39+
40+
func TestValidateCreate(t *testing.T) {
41+
testcases := []struct {
42+
name string
43+
job *jobset.JobSet
44+
wantErr error
45+
}{
46+
{
47+
name: "simple",
48+
job: testingutil.MakeJobSet("job", "default").Queue("queue").Obj(),
49+
wantErr: nil,
50+
},
51+
{
52+
name: "invalid queue-name label",
53+
job: testingutil.MakeJobSet("job", "default").Queue("queue_name").Obj(),
54+
wantErr: field.ErrorList{field.Invalid(queueNameLabelPath, "queue_name", invalidRFC1123Message)}.ToAggregate(),
55+
},
56+
}
57+
58+
for _, tc := range testcases {
59+
t.Run(tc.name, func(t *testing.T) {
60+
jsw := &JobSetWebhook{}
61+
_, gotErr := jsw.ValidateCreate(context.Background(), tc.job)
62+
63+
if diff := cmp.Diff(tc.wantErr, gotErr); diff != "" {
64+
t.Errorf("validateCreate() mismatch (-want +got):\n%s", diff)
65+
}
66+
})
67+
}
68+
}

pkg/controller/jobs/mpijob/mpijob_webhook.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,21 +57,21 @@ var _ webhook.CustomDefaulter = &MPIJobWebhook{}
5757
// Default implements webhook.CustomDefaulter so a webhook will be registered for the type
5858
func (w *MPIJobWebhook) Default(ctx context.Context, obj runtime.Object) error {
5959
job := fromObject(obj)
60-
log := ctrl.LoggerFrom(ctx).WithName("job-webhook")
60+
log := ctrl.LoggerFrom(ctx).WithName("mpijob-webhook")
6161
log.V(5).Info("Applying defaults", "job", klog.KObj(job))
6262

6363
jobframework.ApplyDefaultForSuspend(job, w.manageJobsWithoutQueueName)
6464
return nil
6565
}
6666

67-
// +kubebuilder:webhook:path=/validate-kubeflow-org-v2beta1-mpijob,mutating=false,failurePolicy=fail,sideEffects=None,groups=kubeflow.org,resources=mpijobs,verbs=update,versions=v2beta1,name=vmpijob.kb.io,admissionReviewVersions=v1
67+
// +kubebuilder:webhook:path=/validate-kubeflow-org-v2beta1-mpijob,mutating=false,failurePolicy=fail,sideEffects=None,groups=kubeflow.org,resources=mpijobs,verbs=create;update,versions=v2beta1,name=vmpijob.kb.io,admissionReviewVersions=v1
6868

6969
var _ webhook.CustomValidator = &MPIJobWebhook{}
7070

7171
// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type
7272
func (w *MPIJobWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
7373
job := fromObject(obj)
74-
log := ctrl.LoggerFrom(ctx).WithName("job-webhook")
74+
log := ctrl.LoggerFrom(ctx).WithName("mpijob-webhook")
7575
log.Info("Validating create", "job", klog.KObj(job))
7676
return nil, validateCreate(job).ToAggregate()
7777
}
@@ -84,7 +84,7 @@ func validateCreate(job jobframework.GenericJob) field.ErrorList {
8484
func (w *MPIJobWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
8585
oldJob := fromObject(oldObj)
8686
newJob := fromObject(newObj)
87-
log := ctrl.LoggerFrom(ctx).WithName("job-webhook")
87+
log := ctrl.LoggerFrom(ctx).WithName("mpijob-webhook")
8888
log.Info("Validating update", "job", klog.KObj(newJob))
8989
allErrs := jobframework.ValidateUpdateForQueueName(oldJob, newJob)
9090
return nil, allErrs.ToAggregate()

pkg/controller/jobs/rayjob/rayjob_webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (w *RayJobWebhook) Default(ctx context.Context, obj runtime.Object) error {
6565
return nil
6666
}
6767

68-
// +kubebuilder:webhook:path=/validate-ray-io-v1alpha1-rayjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayjobs,verbs=update,versions=v1alpha1,name=vrayjob.kb.io,admissionReviewVersions=v1
68+
// +kubebuilder:webhook:path=/validate-ray-io-v1alpha1-rayjob,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayjobs,verbs=create;update,versions=v1alpha1,name=vrayjob.kb.io,admissionReviewVersions=v1
6969

7070
var _ webhook.CustomValidator = &RayJobWebhook{}
7171

test/integration/controller/job/job_webhook_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ import (
2121
"github.com/onsi/gomega"
2222
batchv1 "k8s.io/api/batch/v1"
2323
corev1 "k8s.io/api/core/v1"
24+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2526
"k8s.io/apimachinery/pkg/types"
2627

2728
"sigs.k8s.io/kueue/pkg/controller/constants"
2829
"sigs.k8s.io/kueue/pkg/controller/jobframework"
30+
"sigs.k8s.io/kueue/pkg/controller/jobs/job"
2931
"sigs.k8s.io/kueue/pkg/util/pointer"
3032
testingjob "sigs.k8s.io/kueue/pkg/util/testingjobs/job"
3133
"sigs.k8s.io/kueue/test/integration/framework"
@@ -61,6 +63,13 @@ var _ = ginkgo.Describe("Job Webhook", func() {
6163
fwk.Teardown()
6264
})
6365

66+
ginkgo.It("checking the workload is not created when JobMinParallelismAnnotation is invalid", func() {
67+
job := testingjob.MakeJob("job-with-queue-name", ns.Name).Queue("foo").SetAnnotation(job.JobMinParallelismAnnotation, "a").Obj()
68+
err := k8sClient.Create(ctx, job)
69+
gomega.Expect(err).Should(gomega.HaveOccurred())
70+
gomega.Expect(apierrors.IsForbidden(err)).Should(gomega.BeTrue(), "error: %v", err)
71+
})
72+
6473
ginkgo.It("Should suspend a Job even no queue name specified", func() {
6574
job := testingjob.MakeJob("job-without-queue-name", ns.Name).Suspend(false).Obj()
6675
gomega.Expect(k8sClient.Create(ctx, job)).Should(gomega.Succeed())
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package jobset
18+
19+
import (
20+
"github.com/onsi/ginkgo/v2"
21+
"github.com/onsi/gomega"
22+
corev1 "k8s.io/api/core/v1"
23+
apierrors "k8s.io/apimachinery/pkg/api/errors"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
26+
testingjob "sigs.k8s.io/kueue/pkg/util/testingjobs/jobset"
27+
"sigs.k8s.io/kueue/test/integration/framework"
28+
"sigs.k8s.io/kueue/test/util"
29+
)
30+
31+
var _ = ginkgo.Describe("JobSet Webhook", func() {
32+
var ns *corev1.Namespace
33+
34+
ginkgo.When("with manageJobsWithoutQueueName disabled", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
35+
ginkgo.BeforeAll(func() {
36+
fwk = &framework.Framework{
37+
ManagerSetup: managerSetup(),
38+
CRDPath: crdPath,
39+
DepCRDPaths: []string{jobsetCrdPath},
40+
WebhookPath: webhookPath,
41+
}
42+
ctx, cfg, k8sClient = fwk.Setup()
43+
})
44+
ginkgo.BeforeEach(func() {
45+
ns = &corev1.Namespace{
46+
ObjectMeta: metav1.ObjectMeta{
47+
GenerateName: "jobset-",
48+
},
49+
}
50+
gomega.Expect(k8sClient.Create(ctx, ns)).To(gomega.Succeed())
51+
})
52+
ginkgo.AfterEach(func() {
53+
gomega.Expect(util.DeleteNamespace(ctx, k8sClient, ns)).To(gomega.Succeed())
54+
})
55+
ginkgo.AfterAll(func() {
56+
fwk.Teardown()
57+
})
58+
59+
ginkgo.It("the creation doesn't succeed if the queue name is invalid", func() {
60+
job := testingjob.MakeJobSet(jobSetName, ns.Name).Queue("indexed_job").Obj()
61+
err := k8sClient.Create(ctx, job)
62+
gomega.Expect(err).Should(gomega.HaveOccurred())
63+
gomega.Expect(apierrors.IsForbidden(err)).Should(gomega.BeTrue(), "error: %v", err)
64+
})
65+
})
66+
67+
})

test/integration/controller/jobset/suite_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ var (
4747
fwk *framework.Framework
4848
crdPath = filepath.Join("..", "..", "..", "..", "config", "components", "crd", "bases")
4949
jobsetCrdPath = filepath.Join("..", "..", "..", "..", "dep-crds", "jobset-operator")
50+
webhookPath = filepath.Join("..", "..", "..", "..", "config", "components", "webhook")
5051
)
5152

5253
func TestAPIs(t *testing.T) {

0 commit comments

Comments
 (0)