Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecated DefaultRestartPolicy with NoneRestartPolicy #214

Merged
merged 2 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions api/leaderworkerset/v1/leaderworkerset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,10 @@ type LeaderWorkerTemplate struct {
Size *int32 `json:"size,omitempty"`

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

Expand Down Expand Up @@ -263,7 +265,13 @@ const (

// Default will follow the same behavior as the StatefulSet where only the failed pod
// will be restarted on failure and other pods in the group will not be impacted.
DefaultRestartPolicy RestartPolicyType = "Default"
//
// Note: deprecated, use NoneRestartPolicy instead.
DeprecatedDefaultRestartPolicy RestartPolicyType = "Default"

// None will follow the same behavior as the StatefulSet where only the failed pod
// will be restarted on failure and other pods in the group will not be impacted.
NoneRestartPolicy RestartPolicyType = "None"
)

type StartupPolicyType string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8063,12 +8063,15 @@ spec:
type: object
type: object
restartPolicy:
default: Default
description: RestartPolicy defines the restart policy when pod
failures happen.
default: RecreateGroupOnPodRestart
description: |-
RestartPolicy defines the restart policy when pod failures happen.
The former named Default policy is deprecated, will be removed in the future,
replace with None policy for the same behavior.
enum:
- Default
- RecreateGroupOnPodRestart
- None
type: string
size:
default: 1
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/leaderworkerset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
}).
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
Size(2).
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
Kind: ptr.To[string]("StatefulSet"),
Expand Down Expand Up @@ -193,7 +193,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).
Size(2).
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
Kind: ptr.To[string]("StatefulSet"),
Expand Down Expand Up @@ -326,7 +326,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).
Size(2).
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
Kind: ptr.To[string]("StatefulSet"),
Expand Down
6 changes: 5 additions & 1 deletion pkg/webhooks/leaderworkerset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ var _ webhook.CustomDefaulter = &LeaderWorkerSetWebhook{}
func (r *LeaderWorkerSetWebhook) Default(ctx context.Context, obj runtime.Object) error {
lws := obj.(*v1.LeaderWorkerSet)
if lws.Spec.LeaderWorkerTemplate.RestartPolicy == "" {
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.DefaultRestartPolicy
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.RecreateGroupOnPodRestart
}

if lws.Spec.LeaderWorkerTemplate.RestartPolicy == v1.DeprecatedDefaultRestartPolicy {
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.NoneRestartPolicy
}

if lws.Spec.RolloutStrategy.Type == "" {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var _ = ginkgo.Describe("leaderWorkerSet e2e tests", func() {
})

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

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

gomega.Expect(*lws.Spec.Replicas).To(gomega.Equal(int32(4)))
gomega.Expect(*lws.Spec.LeaderWorkerTemplate.Size).To(gomega.Equal(int32(5)))
gomega.Expect(lws.Spec.LeaderWorkerTemplate.RestartPolicy).To(gomega.Equal(v1.RecreateGroupOnPodRestart))
gomega.Expect(lws.Spec.LeaderWorkerTemplate.RestartPolicy).To(gomega.Equal(v1.NoneRestartPolicy))

expectedLabels := []string{v1.SetNameLabelKey, v1.GroupIndexLabelKey, v1.WorkerIndexLabelKey, v1.TemplateRevisionHashKey}
expectedAnnotations := []string{v1.LeaderPodNameAnnotationKey, v1.SizeAnnotationKey}
Expand Down
4 changes: 2 additions & 2 deletions test/integration/controllers/leaderworkerset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,9 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
},
},
}),
ginkgo.Entry("Pod restart will not recreate the pod group when restart policy is Default", &testCase{
ginkgo.Entry("Pod restart will not recreate the pod group when restart policy is None", &testCase{
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.DefaultRestartPolicy).Replica(1).Size(3)
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.NoneRestartPolicy).Replica(1).Size(3)
},
updates: []*update{
{
Expand Down
26 changes: 17 additions & 9 deletions test/integration/webhooks/leaderworkerset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
return lwsWrapper
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(1).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(1).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
},
}),
ginkgo.Entry("apply defaulting logic for size", &testDefaultingCase{
Expand All @@ -81,31 +81,39 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
return lwsWrapper
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Size(1).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
return testutils.BuildLeaderWorkerSet(ns.Name).Size(1).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
},
}),
ginkgo.Entry("defaulting logic won't apply when shouldn't", &testDefaultingCase{
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2)
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).LeaderTemplateSpec(testutils.MakeLeaderPodSpec()).WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
},
}),
ginkgo.Entry("defaulting logic applies when leaderworkertemplate.restartpolicy is not set", &testDefaultingCase{
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy("")
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.DefaultRestartPolicy)
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
},
}),
ginkgo.Entry("defaulting logic won't apply when leaderworkertemplate.restartpolicy is set", &testDefaultingCase{
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
},
}),
ginkgo.Entry("DeprecatedDefaultRestartPolicy will be shift to NoneRestartPolicy", &testDefaultingCase{
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.DeprecatedDefaultRestartPolicy)
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).Replica(2).Size(2).RestartPolicy(leaderworkerset.NoneRestartPolicy)
},
}),
ginkgo.Entry("defaulting logic applies when spec.startpolicy is not set", &testDefaultingCase{
Expand Down Expand Up @@ -139,7 +147,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
return testutils.BuildLeaderWorkerSet(ns.Name).RolloutStrategy(leaderworkerset.RolloutStrategy{}) // unset rollout strategy
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.DefaultRestartPolicy).RolloutStrategy(leaderworkerset.RolloutStrategy{
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).RolloutStrategy(leaderworkerset.RolloutStrategy{
Type: leaderworkerset.RollingUpdateStrategyType,
RollingUpdateConfiguration: &leaderworkerset.RollingUpdateConfiguration{
MaxUnavailable: intstr.FromInt32(1),
Expand All @@ -159,7 +167,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
},
getExpectedLWS: func(lws *leaderworkerset.LeaderWorkerSet) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).
RestartPolicy(leaderworkerset.DefaultRestartPolicy).
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).
RolloutStrategy(leaderworkerset.RolloutStrategy{
Type: leaderworkerset.RollingUpdateStrategyType,
RollingUpdateConfiguration: &leaderworkerset.RollingUpdateConfiguration{
Expand Down Expand Up @@ -364,7 +372,7 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
}),
ginkgo.Entry("set restart policy should succeed", &testValidationCase{
makeLeaderWorkerSet: func(ns *corev1.Namespace) *testutils.LeaderWorkerSetWrapper {
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart)
return testutils.BuildLeaderWorkerSet(ns.Name).RestartPolicy(leaderworkerset.NoneRestartPolicy)
},
lwsCreationShouldFail: false,
}),
Expand Down
2 changes: 1 addition & 1 deletion test/testutils/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func BuildLeaderWorkerSet(nsName string) *LeaderWorkerSetWrapper {
lws.Namespace = nsName
lws.Spec = leaderworkerset.LeaderWorkerSetSpec{}
lws.Spec.Replicas = ptr.To[int32](2)
lws.Spec.LeaderWorkerTemplate = leaderworkerset.LeaderWorkerTemplate{RestartPolicy: leaderworkerset.DefaultRestartPolicy}
lws.Spec.LeaderWorkerTemplate = leaderworkerset.LeaderWorkerTemplate{RestartPolicy: leaderworkerset.RecreateGroupOnPodRestart}
lws.Spec.LeaderWorkerTemplate.Size = ptr.To[int32](2)
lws.Spec.LeaderWorkerTemplate.LeaderTemplate = &corev1.PodTemplateSpec{}
lws.Spec.LeaderWorkerTemplate.LeaderTemplate.Spec = MakeLeaderPodSpec()
Expand Down