Skip to content

Commit edc9eac

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #214 from kerthcet/cleanup/add-none-policy
Deprecated DefaultRestartPolicy with NoneRestartPolicy
2 parents 39f4dd3 + b3a8977 commit edc9eac

File tree

8 files changed

+47
-24
lines changed

8 files changed

+47
-24
lines changed

‎api/leaderworkerset/v1/leaderworkerset_types.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,10 @@ type LeaderWorkerTemplate struct {
152152
Size *int32 `json:"size,omitempty"`
153153

154154
// RestartPolicy defines the restart policy when pod failures happen.
155-
// +kubebuilder:default=Default
156-
// +kubebuilder:validation:Enum={Default,RecreateGroupOnPodRestart}
155+
// The former named Default policy is deprecated, will be removed in the future,
156+
// replace with None policy for the same behavior.
157+
// +kubebuilder:default=RecreateGroupOnPodRestart
158+
// +kubebuilder:validation:Enum={Default,RecreateGroupOnPodRestart,None}
157159
// +optional
158160
RestartPolicy RestartPolicyType `json:"restartPolicy,omitempty"`
159161

@@ -263,7 +265,13 @@ const (
263265

264266
// Default will follow the same behavior as the StatefulSet where only the failed pod
265267
// will be restarted on failure and other pods in the group will not be impacted.
266-
DefaultRestartPolicy RestartPolicyType = "Default"
268+
//
269+
// Note: deprecated, use NoneRestartPolicy instead.
270+
DeprecatedDefaultRestartPolicy RestartPolicyType = "Default"
271+
272+
// None will follow the same behavior as the StatefulSet where only the failed pod
273+
// will be restarted on failure and other pods in the group will not be impacted.
274+
NoneRestartPolicy RestartPolicyType = "None"
267275
)
268276

269277
type StartupPolicyType string

‎config/crd/bases/leaderworkerset.x-k8s.io_leaderworkersets.yaml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8063,12 +8063,15 @@ spec:
80638063
type: object
80648064
type: object
80658065
restartPolicy:
8066-
default: Default
8067-
description: RestartPolicy defines the restart policy when pod
8068-
failures happen.
8066+
default: RecreateGroupOnPodRestart
8067+
description: |-
8068+
RestartPolicy defines the restart policy when pod failures happen.
8069+
The former named Default policy is deprecated, will be removed in the future,
8070+
replace with None policy for the same behavior.
80698071
enum:
80708072
- Default
80718073
- RecreateGroupOnPodRestart
8074+
- None
80728075
type: string
80738076
size:
80748077
default: 1

‎pkg/controllers/leaderworkerset_controller_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
125125
}).
126126
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
127127
Size(2).
128-
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
128+
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
129129
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
130130
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
131131
Kind: ptr.To[string]("StatefulSet"),
@@ -193,7 +193,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
193193
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
194194
LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).
195195
Size(2).
196-
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
196+
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
197197
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
198198
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
199199
Kind: ptr.To[string]("StatefulSet"),
@@ -326,7 +326,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
326326
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
327327
LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).
328328
Size(2).
329-
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
329+
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
330330
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
331331
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
332332
Kind: ptr.To[string]("StatefulSet"),

‎pkg/webhooks/leaderworkerset_webhook.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ var _ webhook.CustomDefaulter = &LeaderWorkerSetWebhook{}
5454
func (r *LeaderWorkerSetWebhook) Default(ctx context.Context, obj runtime.Object) error {
5555
lws := obj.(*v1.LeaderWorkerSet)
5656
if lws.Spec.LeaderWorkerTemplate.RestartPolicy == "" {
57-
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.DefaultRestartPolicy
57+
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.RecreateGroupOnPodRestart
58+
}
59+
60+
if lws.Spec.LeaderWorkerTemplate.RestartPolicy == v1.DeprecatedDefaultRestartPolicy {
61+
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.NoneRestartPolicy
5862
}
5963

6064
if lws.Spec.RolloutStrategy.Type == "" {

‎test/e2e/e2e_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ var _ = ginkgo.Describe("leaderWorkerSet e2e tests", func() {
5757
})
5858

5959
ginkgo.It("Can deploy lws with 'replicas', 'size', and 'restart policy' set", func() {
60-
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(4).Size(5).RestartPolicy(v1.RecreateGroupOnPodRestart).Obj()
60+
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(4).Size(5).RestartPolicy(v1.NoneRestartPolicy).Obj()
6161
testing.MustCreateLws(ctx, k8sClient, lws)
6262
testing.ExpectLeaderWorkerSetAvailable(ctx, k8sClient, lws, "All replicas are ready")
6363

@@ -69,7 +69,7 @@ var _ = ginkgo.Describe("leaderWorkerSet e2e tests", func() {
6969

7070
gomega.Expect(*lws.Spec.Replicas).To(gomega.Equal(int32(4)))
7171
gomega.Expect(*lws.Spec.LeaderWorkerTemplate.Size).To(gomega.Equal(int32(5)))
72-
gomega.Expect(lws.Spec.LeaderWorkerTemplate.RestartPolicy).To(gomega.Equal(v1.RecreateGroupOnPodRestart))
72+
gomega.Expect(lws.Spec.LeaderWorkerTemplate.RestartPolicy).To(gomega.Equal(v1.NoneRestartPolicy))
7373

7474
expectedLabels := []string{v1.SetNameLabelKey, v1.GroupIndexLabelKey, v1.WorkerIndexLabelKey, v1.TemplateRevisionHashKey}
7575
expectedAnnotations := []string{v1.LeaderPodNameAnnotationKey, v1.SizeAnnotationKey}

‎test/integration/controllers/leaderworkerset_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,9 +391,9 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
391391
},
392392
},
393393
}),
394-
ginkgo.Entry("Pod restart will not recreate the pod group when restart policy is Default", &testCase{
394+
ginkgo.Entry("Pod restart will not recreate the pod group when restart policy is None", &testCase{
395395
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
396-
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.DefaultRestartPolicy).Replica(1).Size(3)
396+
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.NoneRestartPolicy).Replica(1).Size(3)
397397
},
398398
updates: []*update{
399399
{

‎test/integration/webhooks/leaderworkerset_test.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
7171
return lwsWrapper
7272
},
7373
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
74-
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(1).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
74+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(1).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
7575
},
7676
}),
7777
ginkgo.Entry("apply defaulting logic for size", &testDefaultingCase{
@@ -81,31 +81,39 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
8181
return lwsWrapper
8282
},
8383
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
84-
return testutils.BuildLeaderWorkerSet(ns.Name).Size(1).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
84+
return testutils.BuildLeaderWorkerSet(ns.Name).Size(1).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
8585
},
8686
}),
8787
ginkgo.Entry("defaulting logic won't apply when shouldn't", &testDefaultingCase{
8888
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
8989
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2)
9090
},
9191
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
92-
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
92+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
9393
},
9494
}),
9595
ginkgo.Entry("defaulting logic applies when leaderworkertemplate.restartpolicy is not set", &testDefaultingCase{
9696
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
9797
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy("")
9898
},
9999
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
100-
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
100+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
101101
},
102102
}),
103103
ginkgo.Entry("defaulting logic won't apply when leaderworkertemplate.restartpolicy is set", &testDefaultingCase{
104104
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
105-
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
105+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
106106
},
107107
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
108-
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
108+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
109+
},
110+
}),
111+
ginkgo.Entry("DeprecatedDefaultRestartPolicy will be shift to NoneRestartPolicy", &testDefaultingCase{
112+
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
113+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.DeprecatedDefaultRestartPolicy)
114+
},
115+
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
116+
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
109117
},
110118
}),
111119
ginkgo.Entry("defaulting logic applies when spec.startpolicy is not set", &testDefaultingCase{
@@ -139,7 +147,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
139147
return testutils.BuildLeaderWorkerSet(ns.Name).RolloutStrategy(leaderworkerset.RolloutStrategy{}) // unset rollout strategy
140148
},
141149
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
142-
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.DefaultRestartPolicy).RolloutStrategy(leaderworkerset.RolloutStrategy{
150+
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).RolloutStrategy(leaderworkerset.RolloutStrategy{
143151
Type: leaderworkerset.RollingUpdateStrategyType,
144152
RollingUpdateConfiguration: &leaderworkerset.RollingUpdateConfiguration{
145153
MaxUnavailable: intstr.FromInt32(1),
@@ -159,7 +167,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
159167
},
160168
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
161169
return testutils.BuildLeaderWorkerSet(ns.Name).
162-
RestartPolicy(leaderworkerset.DefaultRestartPolicy).
170+
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).
163171
RolloutStrategy(leaderworkerset.RolloutStrategy{
164172
Type: leaderworkerset.RollingUpdateStrategyType,
165173
RollingUpdateConfiguration: &leaderworkerset.RollingUpdateConfiguration{
@@ -364,7 +372,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
364372
}),
365373
ginkgo.Entry("set restart policy should succeed", &testValidationCase{
366374
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
367-
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
375+
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.NoneRestartPolicy)
368376
},
369377
lwsCreationShouldFail: false,
370378
}),

‎test/testutils/wrappers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func BuildLeaderWorkerSet(nsName string) *LeaderWorkerSetWrapper {
136136
lws.Namespace = nsName
137137
lws.Spec = leaderworkerset.LeaderWorkerSetSpec{}
138138
lws.Spec.Replicas = ptr.To[int32](2)
139-
lws.Spec.LeaderWorkerTemplate = leaderworkerset.LeaderWorkerTemplate{RestartPolicy: leaderworkerset.DefaultRestartPolicy}
139+
lws.Spec.LeaderWorkerTemplate = leaderworkerset.LeaderWorkerTemplate{RestartPolicy: leaderworkerset.RecreateGroupOnPodRestart}
140140
lws.Spec.LeaderWorkerTemplate.Size = ptr.To[int32](2)
141141
lws.Spec.LeaderWorkerTemplate.LeaderTemplate = &corev1.PodTemplateSpec{}
142142
lws.Spec.LeaderWorkerTemplate.LeaderTemplate.Spec = MakeLeaderPodSpec()

0 commit comments

Comments
 (0)