Skip to content

Commit 77b989b

Browse files
danielvegamyhredanielvegamyhre
authored
Validate job spec is immutable (#49)
* validate job spec is immutable * address comments * flip updateShouldFail to updateShouldSucceed * remove update prefix
1 parent c762106 commit 77b989b

File tree

3 files changed

+133
-30
lines changed

3 files changed

+133
-30
lines changed

‎api/v1alpha1/jobset_types.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ type JobSetStatus struct {
6767
type JobSet struct {
6868
metav1.TypeMeta `json:",inline"`
6969
metav1.ObjectMeta `json:"metadata,omitempty"`
70-
Spec JobSetSpec `json:"spec,omitempty"`
71-
Status JobSetStatus `json:"status,omitempty"`
70+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
71+
Spec JobSetSpec `json:"spec,omitempty"`
72+
Status JobSetStatus `json:"status,omitempty"`
7273
}
7374

7475
// +kubebuilder:object:root=true

‎config/crd/bases/batch.x-k8s.io_jobsets.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -9515,6 +9515,9 @@ spec:
95159515
required:
95169516
- jobs
95179517
type: object
9518+
x-kubernetes-validations:
9519+
- message: Value is immutable
9520+
rule: self == oldSelf
95189521
status:
95199522
description: JobSetStatus defines the observed state of JobSet
95209523
properties:

‎test/integration/jobset_controller_test.go

+127-28
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ const (
3939
interval = time.Millisecond * 250
4040
)
4141

42-
var _ = ginkgo.Describe("JobSet controller", func() {
43-
42+
var _ = ginkgo.Describe("JobSet validation", func() {
4443
// Each test runs in a separate namespace.
4544
var ns *corev1.Namespace
4645

@@ -71,18 +70,17 @@ var _ = ginkgo.Describe("JobSet controller", func() {
7170
// jobSetUpdate contains the mutations to perform on the jobset and the
7271
// checks to perform afterwards.
7372
type jobSetUpdate struct {
74-
jobUpdateFn func(*batchv1.JobList)
75-
checkJobSetState func(*jobset.JobSet)
76-
expectedJobSetCondition jobset.JobSetConditionType
73+
fn func(*jobset.JobSet)
74+
shouldSucceed bool
7775
}
7876

7977
type testCase struct {
80-
makeJobSet func(*corev1.Namespace) *testing.JobSetWrapper
81-
jobSetCreationShouldFail bool
82-
updates []*jobSetUpdate
78+
makeJobSet func(*corev1.Namespace) *testing.JobSetWrapper
79+
jobSetCreationShouldSucceed bool
80+
updates []*jobSetUpdate
8381
}
8482

85-
ginkgo.DescribeTable("jobset is created and its jobs go through a series of updates",
83+
ginkgo.DescribeTable("validation on jobset creation and updates",
8684
func(tc *testCase) {
8785
ctx := context.Background()
8886

@@ -91,7 +89,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
9189
js := tc.makeJobSet(ns).Obj()
9290

9391
// If we are expected a validation error creating the jobset, end the test early.
94-
if tc.jobSetCreationShouldFail {
92+
if !tc.jobSetCreationShouldSucceed {
9593
ginkgo.By("checking that jobset creation fails")
9694
gomega.Expect(k8sClient.Create(ctx, js)).Should(gomega.Not(gomega.Succeed()))
9795
return
@@ -104,6 +102,118 @@ var _ = ginkgo.Describe("JobSet controller", func() {
104102
// We'll need to retry getting this newly created jobset, given that creation may not immediately happen.
105103
gomega.Eventually(k8sClient.Get(ctx, types.NamespacedName{Name: js.Name, Namespace: js.Namespace}, &jobset.JobSet{}), timeout, interval).Should(gomega.Succeed())
106104

105+
// Perform updates to the jobset and verify the validation is working correctly.
106+
for _, update := range tc.updates {
107+
108+
// Update jobset if specified.
109+
if update.fn != nil {
110+
update.fn(js)
111+
112+
// If we are expecting a validation error, we can skip the rest of the checks.
113+
if !update.shouldSucceed {
114+
gomega.Expect(k8sClient.Update(ctx, js)).Should(gomega.Not(gomega.Succeed()))
115+
continue
116+
}
117+
// Verify a valid jobset update succeeded.
118+
gomega.Expect(k8sClient.Update(ctx, js)).Should(gomega.Succeed())
119+
}
120+
}
121+
},
122+
ginkgo.Entry("validate enableDNSHostnames can't be set if job is not Indexed", &testCase{
123+
makeJobSet: func(ns *corev1.Namespace) *testing.JobSetWrapper {
124+
return testing.MakeJobSet("js-hostnames-non-indexed", ns.Name).
125+
ReplicatedJob(testing.MakeReplicatedJob("test-job").
126+
Job(testing.MakeJobTemplate("test-job", ns.Name).PodSpec(testing.TestPodSpec).Obj()).
127+
EnableDNSHostnames(true).
128+
Obj())
129+
},
130+
}),
131+
ginkgo.Entry("validate jobset spec is immutable", &testCase{
132+
makeJobSet: testJobSet,
133+
jobSetCreationShouldSucceed: true,
134+
updates: []*jobSetUpdate{
135+
{
136+
fn: func(js *jobset.JobSet) {
137+
// Try mutating jobs list.
138+
js.Spec.Jobs = append(js.Spec.Jobs, testing.MakeReplicatedJob("test-job").
139+
Job(testing.MakeJobTemplate("test-job", ns.Name).PodSpec(testing.TestPodSpec).Obj()).
140+
EnableDNSHostnames(true).
141+
Obj())
142+
},
143+
},
144+
{
145+
fn: func(js *jobset.JobSet) {
146+
// Try mutating failure policy.
147+
js.Spec.FailurePolicy = &jobset.FailurePolicy{
148+
Operator: jobset.TerminationPolicyTargetAny,
149+
RestartPolicy: jobset.RestartPolicyRecreateAll,
150+
MaxRestarts: 3,
151+
}
152+
},
153+
},
154+
},
155+
}),
156+
) // end of DescribeTable
157+
}) // end of Describe
158+
159+
var _ = ginkgo.Describe("JobSet controller", func() {
160+
161+
// Each test runs in a separate namespace.
162+
var ns *corev1.Namespace
163+
164+
ginkgo.BeforeEach(func() {
165+
// Create test namespace before each test.
166+
ns = &corev1.Namespace{
167+
ObjectMeta: metav1.ObjectMeta{
168+
GenerateName: "test-ns-",
169+
},
170+
}
171+
gomega.Expect(k8sClient.Create(ctx, ns)).To(gomega.Succeed())
172+
173+
// Wait for namespace to exist before proceeding with test.
174+
gomega.Eventually(func() bool {
175+
err := k8sClient.Get(ctx, types.NamespacedName{Namespace: ns.Namespace, Name: ns.Name}, ns)
176+
if err != nil {
177+
return false
178+
}
179+
return true
180+
}, timeout, interval).Should(gomega.BeTrue())
181+
})
182+
183+
ginkgo.AfterEach(func() {
184+
// Delete test namespace after each test.
185+
gomega.Expect(k8sClient.Delete(ctx, ns)).To(gomega.Succeed())
186+
})
187+
188+
// jobUpdate contains the mutations to perform on the jobs and the
189+
// checks to perform afterwards.
190+
type jobUpdate struct {
191+
jobUpdateFn func(*batchv1.JobList)
192+
checkJobSetState func(*jobset.JobSet)
193+
expectedJobSetCondition jobset.JobSetConditionType
194+
}
195+
196+
type testCase struct {
197+
makeJobSet func(*corev1.Namespace) *testing.JobSetWrapper
198+
jobSetCreationShouldFail bool
199+
updates []*jobUpdate
200+
}
201+
202+
ginkgo.DescribeTable("jobset is created and its jobs go through a series of updates",
203+
func(tc *testCase) {
204+
ctx := context.Background()
205+
206+
// Create JobSet.
207+
ginkgo.By("creating jobset")
208+
js := tc.makeJobSet(ns).Obj()
209+
210+
// Verify jobset created successfully.
211+
ginkgo.By("checking that jobset creation succeeds")
212+
gomega.Expect(k8sClient.Create(ctx, js)).Should(gomega.Succeed())
213+
214+
// We'll need to retry getting this newly created jobset, given that creation may not immediately happen.
215+
gomega.Eventually(k8sClient.Get(ctx, types.NamespacedName{Name: js.Name, Namespace: js.Namespace}, &jobset.JobSet{}), timeout, interval).Should(gomega.Succeed())
216+
107217
ginkgo.By("checking all jobs were created successfully")
108218
gomega.Eventually(checkNumJobs, timeout, interval).WithArguments(ctx, js).Should(gomega.Equal(numExpectedJobs(js)))
109219

@@ -131,20 +241,9 @@ var _ = ginkgo.Describe("JobSet controller", func() {
131241
}
132242
}
133243
},
134-
// TODO: move validation tests to separate Describe + DescribeTable
135-
ginkgo.Entry("validate enableDNSHostnames can't be set if job is not Indexed", &testCase{
136-
makeJobSet: func(ns *corev1.Namespace) *testing.JobSetWrapper {
137-
return testing.MakeJobSet("js-hostnames-non-indexed", ns.Name).
138-
ReplicatedJob(testing.MakeReplicatedJob("test-job").
139-
Job(testing.MakeJobTemplate("test-job", ns.Name).Obj()).
140-
EnableDNSHostnames(true).
141-
Obj())
142-
},
143-
jobSetCreationShouldFail: true,
144-
}),
145244
ginkgo.Entry("jobset should succeed after all jobs succeed", &testCase{
146245
makeJobSet: testJobSet,
147-
updates: []*jobSetUpdate{
246+
updates: []*jobUpdate{
148247
{
149248
jobUpdateFn: completeAllJobs,
150249
expectedJobSetCondition: jobset.JobSetCompleted,
@@ -153,7 +252,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
153252
}),
154253
ginkgo.Entry("jobset with no failure policy should fail if any jobs fail", &testCase{
155254
makeJobSet: testJobSet,
156-
updates: []*jobSetUpdate{
255+
updates: []*jobUpdate{
157256
{
158257
jobUpdateFn: func(jobList *batchv1.JobList) {
159258
failJob(&jobList.Items[0])
@@ -164,7 +263,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
164263
}),
165264
ginkgo.Entry("jobset with DNS hostnames enabled should created 1 headless service per job and succeed when all jobs succeed", &testCase{
166265
makeJobSet: testJobSet,
167-
updates: []*jobSetUpdate{
266+
updates: []*jobUpdate{
168267
{
169268
checkJobSetState: checkExpectedServices,
170269
},
@@ -176,7 +275,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
176275
}),
177276
ginkgo.Entry("succeeds from first run", &testCase{
178277
makeJobSet: testJobSet,
179-
updates: []*jobSetUpdate{
278+
updates: []*jobUpdate{
180279
{
181280
checkJobSetState: checkExpectedServices,
182281
},
@@ -188,7 +287,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
188287
}),
189288
ginkgo.Entry("fails from first run, no restarts", &testCase{
190289
makeJobSet: testJobSet,
191-
updates: []*jobSetUpdate{
290+
updates: []*jobUpdate{
192291
{
193292
checkJobSetState: checkExpectedServices,
194293
},
@@ -209,7 +308,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
209308
MaxRestarts: 1,
210309
})
211310
},
212-
updates: []*jobSetUpdate{
311+
updates: []*jobUpdate{
213312
{
214313
jobUpdateFn: func(jobList *batchv1.JobList) {
215314
failJob(&jobList.Items[0])
@@ -236,7 +335,7 @@ var _ = ginkgo.Describe("JobSet controller", func() {
236335
MaxRestarts: 1,
237336
})
238337
},
239-
updates: []*jobSetUpdate{
338+
updates: []*jobUpdate{
240339
{
241340
jobUpdateFn: func(jobList *batchv1.JobList) {
242341
completeJob(&jobList.Items[0])

0 commit comments

Comments
 (0)