Merge pull request #214 from kerthcet/cleanup/add-none-policy

Deprecated DefaultRestartPolicy with NoneRestartPolicy
kubernetes-sigs · Sep 13, 2024 · edc9eac · edc9eac
2 parents 39f4dd3 + b3a8977
commit edc9eac
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 24 deletions.
diff --git a/api/leaderworkerset/v1/leaderworkerset_types.go b/api/leaderworkerset/v1/leaderworkerset_types.go
@@ -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"`
 
@@ -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

diff --git a/config/crd/bases/leaderworkerset.x-k8s.io_leaderworkersets.yaml b/config/crd/bases/leaderworkerset.x-k8s.io_leaderworkersets.yaml
@@ -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

diff --git a/pkg/controllers/leaderworkerset_controller_test.go b/pkg/controllers/leaderworkerset_controller_test.go
@@ -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"),
@@ -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"),
@@ -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"),

diff --git a/pkg/webhooks/leaderworkerset_webhook.go b/pkg/webhooks/leaderworkerset_webhook.go
@@ -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 == "" {

diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
@@ -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")
 
@@ -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}

diff --git a/test/integration/controllers/leaderworkerset_test.go b/test/integration/controllers/leaderworkerset_test.go
@@ -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{
 				{

diff --git a/test/integration/webhooks/leaderworkerset_test.go b/test/integration/webhooks/leaderworkerset_test.go
@@ -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{
@@ -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{
@@ -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),
@@ -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{
@@ -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,
 		}),

diff --git a/test/testutils/wrappers.go b/test/testutils/wrappers.go
@@ -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()