Skip to content

Commit

Permalink
Deprecated DefaultRestartPolicy with NoneRestartPolicy
Browse files Browse the repository at this point in the history
Signed-off-by: kerthcet <[email protected]>
  • Loading branch information
kerthcet committed Sep 13, 2024
1 parent 39f4dd3 commit 3892afa
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 23 deletions.
18 changes: 13 additions & 5 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 @@ -255,15 +257,21 @@ const (
type RestartPolicyType string

const (
// RecreateGroupOnPodRestart will recreate all the pods in the group if
// DefaultRestartPolicy will recreate all the pods in the group if
// 1. Any individual pod in the group is recreated; 2. Any containers/init-containers
// in a pod is restarted. This is to ensure all pods/containers in the group will be
// started in the same time.
RecreateGroupOnPodRestart RestartPolicyType = "RecreateGroupOnPodRestart"
DefaultRestartPolicy RestartPolicyType = "RecreateGroupOnPodRestart"

// 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 Default policy is deprecated, will be removed in the future, replace
with None for the same behavior.
enum:
- Default
- RecreateGroupOnPodRestart
- None
type: string
size:
default: 1
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/leaderworkerset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
}).
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
Size(1).
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
Kind: ptr.To[string]("StatefulSet"),
Expand Down Expand Up @@ -259,7 +259,7 @@ func TestLeaderStatefulSetApplyConfig(t *testing.T) {
}).
WorkerTemplateSpec(testutils.MakeWorkerPodSpec()).
Size(1).
RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Obj(),
RestartPolicy(leaderworkerset.DefaultRestartPolicy).Obj(),
wantApplyConfig: &appsapplyv1.StatefulSetApplyConfiguration{
TypeMetaApplyConfiguration: metaapplyv1.TypeMetaApplyConfiguration{
Kind: ptr.To[string]("StatefulSet"),
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/pod_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
}

func (r *PodReconciler) handleRestartPolicy(ctx context.Context, pod corev1.Pod, leaderWorkerSet leaderworkerset.LeaderWorkerSet) (bool, error) {
if leaderWorkerSet.Spec.LeaderWorkerTemplate.RestartPolicy != leaderworkerset.RecreateGroupOnPodRestart {
if leaderWorkerSet.Spec.LeaderWorkerTemplate.RestartPolicy != leaderworkerset.DefaultRestartPolicy {
return false, nil
}
// the leader pod will be deleted if the worker pod is deleted or any containes were restarted
Expand Down
4 changes: 4 additions & 0 deletions pkg/webhooks/leaderworkerset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func (r *LeaderWorkerSetWebhook) Default(ctx context.Context, obj runtime.Object
lws.Spec.LeaderWorkerTemplate.RestartPolicy = v1.DefaultRestartPolicy
}

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

if lws.Spec.RolloutStrategy.Type == "" {
lws.Spec.RolloutStrategy.Type = v1.RollingUpdateStrategyType
}
Expand Down
10 changes: 5 additions & 5 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 All @@ -92,7 +92,7 @@ var _ = ginkgo.Describe("leaderWorkerSet e2e tests", func() {
})

ginkgo.It("Can create/update a lws with size=1", func() {
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(4).MaxSurge(1).Size(1).RestartPolicy(v1.RecreateGroupOnPodRestart).Obj()
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(4).MaxSurge(1).Size(1).RestartPolicy(v1.DefaultRestartPolicy).Obj()
testing.MustCreateLws(ctx, k8sClient, lws)

testing.ExpectValidLeaderStatefulSet(ctx, k8sClient, lws, 4)
Expand Down Expand Up @@ -262,8 +262,8 @@ var _ = ginkgo.Describe("leaderWorkerSet e2e tests", func() {
}
})

ginkgo.It("Pod restart will delete the pod group when restart policy is RecreateGroupOnPodRestart", func() {
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(1).Size(3).RestartPolicy(v1.RecreateGroupOnPodRestart).Obj()
ginkgo.It("Pod restart will delete the pod group when restart policy is DefaultRestartPolicy", func() {
lws = testing.BuildLeaderWorkerSet(ns.Name).Replica(1).Size(3).RestartPolicy(v1.DefaultRestartPolicy).Obj()
testing.MustCreateLws(ctx, k8sClient, lws)
testing.ExpectLeaderWorkerSetAvailable(ctx, k8sClient, lws, "All replicas are ready")

Expand Down
8 changes: 4 additions & 4 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 All @@ -420,9 +420,9 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
},
},
}),
ginkgo.Entry("Pod restart will delete the pod group when restart policy is RecreateGroupOnPodRestart", &testCase{
ginkgo.Entry("Pod restart will delete the pod group when restart policy is DefaultRestartPolicy", &testCase{
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.RecreateGroupOnPodRestart).Replica(1).Size(3)
return testing.BuildLeaderWorkerSet(nsName).RestartPolicy(leaderworkerset.DefaultRestartPolicy).Replica(1).Size(3)
},
updates: []*update{
{
Expand Down
14 changes: 11 additions & 3 deletions test/integration/webhooks/leaderworkerset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,18 @@ var _ = ginkgo.Describe("leaderworkerset defaulting, creation and update", func(
}),
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 @@ -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

0 comments on commit 3892afa

Please sign in to comment.