From b1ce1ffc5585a604a2df795722ccbf23f491e666 Mon Sep 17 00:00:00 2001 From: Kenneth Owens Date: Mon, 12 Jun 2017 10:06:09 -0700 Subject: [PATCH] Removes PartitionStatefulSetStrategyType and Partition from UpdateStrategy and replaces them with a parameterized RollingUpdate strategy. --- pkg/apis/apps/types.go | 24 +++------- pkg/apis/apps/v1beta1/conversion.go | 17 +++---- pkg/apis/apps/v1beta1/defaults.go | 6 +++ pkg/apis/apps/v1beta1/types.go | 24 +++------- pkg/apis/apps/validation/validation.go | 34 ++++++-------- pkg/apis/apps/validation/validation_test.go | 28 ++++-------- .../statefulset/stateful_set_control.go | 4 +- .../statefulset/stateful_set_control_test.go | 44 +++++++++---------- .../statefulset/stateful_set_utils.go | 7 ++- 9 files changed, 76 insertions(+), 112 deletions(-) diff --git a/pkg/apis/apps/types.go b/pkg/apis/apps/types.go index 47c621941fd..403a88a608e 100644 --- a/pkg/apis/apps/types.go +++ b/pkg/apis/apps/types.go @@ -66,11 +66,8 @@ const ( type StatefulSetUpdateStrategy struct { // Type indicates the type of the StatefulSetUpdateStrategy. Type StatefulSetUpdateStrategyType - // Partition is used to communicate the ordinal at which to partition - // the StatefulSet when Type is PartitionStatefulSetStrategyType. This - // value must be set when Type is PartitionStatefulSetStrategyType, - // and it must be nil otherwise. - Partition *PartitionStatefulSetStrategy + // RollingUpdate is used to communicate parameters when Type is RollingUpdateStatefulSetStrategyType. + RollingUpdate *RollingUpdateStatefulSetStrategy } // StatefulSetUpdateStrategyType is a string enumeration type that enumerates @@ -78,14 +75,6 @@ type StatefulSetUpdateStrategy struct { type StatefulSetUpdateStrategyType string const ( - // PartitionStatefulSetStrategyType indicates that updates will only be - // applied to a partition of the StatefulSet. This is useful for canaries - // and phased roll outs. When a scale operation is performed with this - // strategy, new Pods will be created from the specification version indicated - // by the StatefulSet's currentRevision if there ordinal is less than the supplied - // Partition's ordinal. Otherwise, they will be created from the specification - // version indicated by the StatefulSet's updateRevision. - PartitionStatefulSetStrategyType StatefulSetUpdateStrategyType = "Partition" // RollingUpdateStatefulSetStrategyType indicates that update will be // applied to all Pods in the StatefulSet with respect to the StatefulSet // ordering constraints. When a scale operation is performed with this @@ -100,12 +89,11 @@ const ( OnDeleteStatefulSetStrategyType = "OnDelete" ) -// PartitionStatefulSetStrategy contains the parameters used with the -// PartitionStatefulSetStrategyType. -type PartitionStatefulSetStrategy struct { - // Ordinal indicates the ordinal at which the StatefulSet should be +// RollingUpdateStatefulSetStrategy is used to communicate parameter for RollingUpdateStatefulSetStrategyType. +type RollingUpdateStatefulSetStrategy struct { + // Partition indicates the ordinal at which the StatefulSet should be // partitioned. - Ordinal int32 + Partition int32 } // A StatefulSetSpec is the specification of a StatefulSet. diff --git a/pkg/apis/apps/v1beta1/conversion.go b/pkg/apis/apps/v1beta1/conversion.go index e11b8623bb7..a21c37db062 100644 --- a/pkg/apis/apps/v1beta1/conversion.go +++ b/pkg/apis/apps/v1beta1/conversion.go @@ -167,22 +167,23 @@ func Convert_apps_StatefulSetSpec_To_v1beta1_StatefulSetSpec(in *apps.StatefulSe func Convert_v1beta1_StatefulSetUpdateStrategy_To_apps_StatefulSetUpdateStrategy(in *StatefulSetUpdateStrategy, out *apps.StatefulSetUpdateStrategy, s conversion.Scope) error { out.Type = apps.StatefulSetUpdateStrategyType(in.Type) - if in.Partition != nil { - out.Partition = new(apps.PartitionStatefulSetStrategy) - out.Partition.Ordinal = in.Partition.Ordinal + if in.RollingUpdate != nil { + out.RollingUpdate = new(apps.RollingUpdateStatefulSetStrategy) + out.RollingUpdate.Partition = *in.RollingUpdate.Partition } else { - out.Partition = nil + out.RollingUpdate = nil } return nil } func Convert_apps_StatefulSetUpdateStrategy_To_v1beta1_StatefulSetUpdateStrategy(in *apps.StatefulSetUpdateStrategy, out *StatefulSetUpdateStrategy, s conversion.Scope) error { out.Type = StatefulSetUpdateStrategyType(in.Type) - if in.Partition != nil { - out.Partition = new(PartitionStatefulSetStrategy) - out.Partition.Ordinal = in.Partition.Ordinal + if in.RollingUpdate != nil { + out.RollingUpdate = new(RollingUpdateStatefulSetStrategy) + out.RollingUpdate.Partition = new(int32) + *out.RollingUpdate.Partition = in.RollingUpdate.Partition } else { - out.Partition = nil + out.RollingUpdate = nil } return nil } diff --git a/pkg/apis/apps/v1beta1/defaults.go b/pkg/apis/apps/v1beta1/defaults.go index 895c8b90eb7..af133d25fdc 100644 --- a/pkg/apis/apps/v1beta1/defaults.go +++ b/pkg/apis/apps/v1beta1/defaults.go @@ -53,6 +53,12 @@ func SetDefaults_StatefulSet(obj *StatefulSet) { obj.Spec.RevisionHistoryLimit = new(int32) *obj.Spec.RevisionHistoryLimit = 10 } + if obj.Spec.UpdateStrategy.Type == RollingUpdateStatefulSetStrategyType && + obj.Spec.UpdateStrategy.RollingUpdate != nil && + obj.Spec.UpdateStrategy.RollingUpdate.Partition == nil { + obj.Spec.UpdateStrategy.RollingUpdate.Partition = new(int32) + *obj.Spec.UpdateStrategy.RollingUpdate.Partition = 0 + } } diff --git a/pkg/apis/apps/v1beta1/types.go b/pkg/apis/apps/v1beta1/types.go index a8a7319310c..c0ad09aeb25 100644 --- a/pkg/apis/apps/v1beta1/types.go +++ b/pkg/apis/apps/v1beta1/types.go @@ -118,11 +118,8 @@ const ( type StatefulSetUpdateStrategy struct { // Type indicates the type of the StatefulSetUpdateStrategy. Type StatefulSetUpdateStrategyType `json:"type,omitempty" protobuf:"bytes,1,opt,name=type,casttype=StatefulSetStrategyType"` - // Partition is used to communicate the ordinal at which to partition - // the StatefulSet when Type is PartitionStatefulSetStrategyType. This - // value must be set when Type is PartitionStatefulSetStrategyType, - // and it must be nil otherwise. - Partition *PartitionStatefulSetStrategy `json:"partition,omitempty" protobuf:"bytes,2,opt,name=partition"` + // RollingUpdate is used to communicate parameters when Type is RollingUpdateStatefulSetStrategyType. + RollingUpdate *RollingUpdateStatefulSetStrategy `json:"rollingUpdate,omitempty" protobuf:"bytes,2,opt,name=rollingUpdate"` } // StatefulSetUpdateStrategyType is a string enumeration type that enumerates @@ -130,14 +127,6 @@ type StatefulSetUpdateStrategy struct { type StatefulSetUpdateStrategyType string const ( - // PartitionStatefulSetStrategyType indicates that updates will only be - // applied to a partition of the StatefulSet. This is useful for canaries - // and phased roll outs. When a scale operation is performed with this - // strategy, new Pods will be created from the specification version indicated - // by the StatefulSet's currentRevision if there ordinal is less than the supplied - // Partition's ordinal. Otherwise, they will be created from the specification - // version indicated by the StatefulSet's updateRevision. - PartitionStatefulSetStrategyType StatefulSetUpdateStrategyType = "Partition" // RollingUpdateStatefulSetStrategyType indicates that update will be // applied to all Pods in the StatefulSet with respect to the StatefulSet // ordering constraints. When a scale operation is performed with this @@ -152,12 +141,11 @@ const ( OnDeleteStatefulSetStrategyType = "OnDelete" ) -// PartitionStatefulSetStrategy contains the parameters used with the -// PartitionStatefulSetStrategyType. -type PartitionStatefulSetStrategy struct { - // Ordinal indicates the ordinal at which the StatefulSet should be +// RollingUpdateStatefulSetStrategy is used to communicate parameter for RollingUpdateStatefulSetStrategyType. +type RollingUpdateStatefulSetStrategy struct { + // Partition indicates the ordinal at which the StatefulSet should be // partitioned. - Ordinal int32 `json:"ordinal" protobuf:"varint,1,opt,name=ordinal"` + Partition *int32 `json:"partition,omitempty" protobuf:"varint,1,opt,name=partition"` } // A StatefulSetSpec is the specification of a StatefulSet. diff --git a/pkg/apis/apps/validation/validation.go b/pkg/apis/apps/validation/validation.go index a51c3634f8b..d4a02bc7434 100644 --- a/pkg/apis/apps/validation/validation.go +++ b/pkg/apis/apps/validation/validation.go @@ -78,35 +78,29 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path) fi switch spec.UpdateStrategy.Type { case "": allErrs = append(allErrs, field.Required(fldPath.Child("updateStrategy"), "")) - case apps.OnDeleteStatefulSetStrategyType, apps.RollingUpdateStatefulSetStrategyType: - if spec.UpdateStrategy.Partition != nil { + case apps.OnDeleteStatefulSetStrategyType: + if spec.UpdateStrategy.RollingUpdate != nil { allErrs = append( allErrs, field.Invalid( - fldPath.Child("updateStrategy").Child("partition"), - spec.UpdateStrategy.Partition.Ordinal, - fmt.Sprintf("only allowed for updateStrategy '%s'", apps.PartitionStatefulSetStrategyType))) + fldPath.Child("updateStrategy").Child("rollingUpdate"), + spec.UpdateStrategy.RollingUpdate, + fmt.Sprintf("only allowed for updateStrategy '%s'", apps.RollingUpdateStatefulSetStrategyType))) } - case apps.PartitionStatefulSetStrategyType: - if spec.UpdateStrategy.Partition == nil { - allErrs = append( - allErrs, - field.Required( - fldPath.Child("updateStrategy").Child("partition"), - fmt.Sprintf("required for updateStrategy '%s'", apps.PartitionStatefulSetStrategyType))) - break + case apps.RollingUpdateStatefulSetStrategyType: + if spec.UpdateStrategy.RollingUpdate != nil { + allErrs = append(allErrs, + apivalidation.ValidateNonnegativeField( + int64(spec.UpdateStrategy.RollingUpdate.Partition), + fldPath.Child("updateStrategy").Child("rollingUpdate").Child("partition"))...) } - allErrs = append(allErrs, - apivalidation.ValidateNonnegativeField( - int64(spec.UpdateStrategy.Partition.Ordinal), - fldPath.Child("updateStrategy").Child("partition").Child("ordinal"))...) + default: allErrs = append(allErrs, field.Invalid(fldPath.Child("updateStrategy"), spec.UpdateStrategy, - fmt.Sprintf("must be '%s', '%s', or '%s'", + fmt.Sprintf("must be '%s' or '%s'", apps.RollingUpdateStatefulSetStrategyType, - apps.OnDeleteStatefulSetStrategyType, - apps.PartitionStatefulSetStrategyType))) + apps.OnDeleteStatefulSetStrategyType))) } allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...) diff --git a/pkg/apis/apps/validation/validation_test.go b/pkg/apis/apps/validation/validation_test.go index b53d2fc888a..f968fdc83cb 100644 --- a/pkg/apis/apps/validation/validation_test.go +++ b/pkg/apis/apps/validation/validation_test.go @@ -97,9 +97,9 @@ func TestValidateStatefulSet(t *testing.T) { Template: validPodTemplate.Template, Replicas: 3, UpdateStrategy: apps.StatefulSetUpdateStrategy{ - Type: apps.PartitionStatefulSetStrategyType, - Partition: func() *apps.PartitionStatefulSetStrategy { - return &apps.PartitionStatefulSetStrategy{Ordinal: 2} + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: func() *apps.RollingUpdateStatefulSetStrategy { + return &apps.RollingUpdateStatefulSetStrategy{Partition: 2} }()}, }, }, @@ -259,7 +259,7 @@ func TestValidateStatefulSet(t *testing.T) { UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: "foo"}, }, }, - "partitioned rolling update": { + "negative parition": { ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, Spec: apps.StatefulSetSpec{ PodManagementPolicy: apps.OrderedReadyPodManagement, @@ -267,23 +267,11 @@ func TestValidateStatefulSet(t *testing.T) { Template: validPodTemplate.Template, Replicas: 3, UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType, - Partition: func() *apps.PartitionStatefulSetStrategy { - return &apps.PartitionStatefulSetStrategy{Ordinal: 2} + RollingUpdate: func() *apps.RollingUpdateStatefulSetStrategy { + return &apps.RollingUpdateStatefulSetStrategy{Partition: -1} }()}, }, }, - "empty partition": { - ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, - Spec: apps.StatefulSetSpec{ - PodManagementPolicy: apps.OrderedReadyPodManagement, - Selector: &metav1.LabelSelector{MatchLabels: validLabels}, - Template: validPodTemplate.Template, - Replicas: 3, - UpdateStrategy: apps.StatefulSetUpdateStrategy{ - Type: apps.PartitionStatefulSetStrategyType, - Partition: nil}, - }, - }, } for k, v := range errorCases { errs := ValidateStatefulSet(&v) @@ -304,8 +292,8 @@ func TestValidateStatefulSet(t *testing.T) { field != "metadata.labels" && field != "status.replicas" && field != "spec.updateStrategy" && - field != "spec.updateStrategy.partition" && - field != "spec.updateStrategy.partition.ordinal" { + field != "spec.updateStrategy.rollingUpate" && + field != "spec.updateStrategy.rollingUpdate.partition" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) } } diff --git a/pkg/controller/statefulset/stateful_set_control.go b/pkg/controller/statefulset/stateful_set_control.go index b70b943e398..82580d4e729 100644 --- a/pkg/controller/statefulset/stateful_set_control.go +++ b/pkg/controller/statefulset/stateful_set_control.go @@ -483,8 +483,8 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( // we compute the minimum ordinal of the target sequence for a destructive update based on the strategy. updateMin := 0 - if set.Spec.UpdateStrategy.Type == apps.PartitionStatefulSetStrategyType { - updateMin = int(set.Spec.UpdateStrategy.Partition.Ordinal) + if set.Spec.UpdateStrategy.RollingUpdate != nil { + updateMin = int(*set.Spec.UpdateStrategy.RollingUpdate.Partition) } // we terminate the Pod with the largest ordinal that does not match the update revision. for target := len(replicas) - 1; target >= updateMin; target-- { diff --git a/pkg/controller/statefulset/stateful_set_control_test.go b/pkg/controller/statefulset/stateful_set_control_test.go index 5ac8ae84a68..3af840dc6f7 100644 --- a/pkg/controller/statefulset/stateful_set_control_test.go +++ b/pkg/controller/statefulset/stateful_set_control_test.go @@ -1053,7 +1053,7 @@ func TestStatefulSetControlOnDeleteUpdate(t *testing.T) { } } -func TestStatefulSetControlPartitionUpdate(t *testing.T) { +func TestStatefulSetControlRollingUpdateWithPartition(t *testing.T) { type testcase struct { name string partition int32 @@ -1066,9 +1066,9 @@ func TestStatefulSetControlPartitionUpdate(t *testing.T) { testFn := func(test *testcase, t *testing.T) { set := test.initial() set.Spec.UpdateStrategy = apps.StatefulSetUpdateStrategy{ - Type: apps.PartitionStatefulSetStrategyType, - Partition: func() *apps.PartitionStatefulSetStrategy { - return &apps.PartitionStatefulSetStrategy{Ordinal: test.partition} + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: func() *apps.RollingUpdateStatefulSetStrategy { + return &apps.RollingUpdateStatefulSetStrategy{Partition: &test.partition} }(), } client := fake.NewSimpleClientset(set) @@ -2068,28 +2068,28 @@ func updateComplete(set *apps.StatefulSet, pods []*v1.Pod) bool { return false } - if set.Spec.UpdateStrategy.Type == apps.RollingUpdateStatefulSetStrategyType { - if set.Status.CurrentReplicas < *set.Spec.Replicas { - return false - } - for i := range pods { - if getPodRevision(pods[i]) != set.Status.CurrentRevision { + switch set.Spec.UpdateStrategy.Type { + case apps.OnDeleteStatefulSetStrategyType: + return true + case apps.RollingUpdateStatefulSetStrategyType: + if set.Spec.UpdateStrategy.RollingUpdate == nil || *set.Spec.UpdateStrategy.RollingUpdate.Partition <= 0 { + if set.Status.CurrentReplicas < *set.Spec.Replicas { return false } - } - } - if set.Spec.UpdateStrategy.Type == apps.PartitionStatefulSetStrategyType { - if set.Status.UpdatedReplicas < (*set.Spec.Replicas - set.Spec.UpdateStrategy.Partition.Ordinal) { - return false - } - for i := 0; i < int(set.Spec.UpdateStrategy.Partition.Ordinal); i++ { - if getPodRevision(pods[i]) != set.Status.CurrentRevision { + for i := range pods { + if getPodRevision(pods[i]) != set.Status.CurrentRevision { + return false + } + } + } else { + partition := int(*set.Spec.UpdateStrategy.RollingUpdate.Partition) + if len(pods) < partition { return false } - } - for i := int(set.Spec.UpdateStrategy.Partition.Ordinal); i < int(*set.Spec.Replicas); i++ { - if getPodRevision(pods[i]) != set.Status.UpdateRevision { - return false + for i := partition; i < len(pods); i++ { + if getPodRevision(pods[i]) != set.Status.UpdateRevision { + return false + } } } } diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index 2fa09349fea..c55a1e0455f 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -286,10 +286,9 @@ func newStatefulSetPod(set *apps.StatefulSet, ordinal int) *v1.Pod { // the current revision. updateRevision is the name of the update revision. ordinal is the ordinal of the Pod. If the // returned error is nil, the returned Pod is valid. func newVersionedStatefulSetPod(currentSet, updateSet *apps.StatefulSet, currentRevision, updateRevision string, ordinal int) *v1.Pod { - if (currentSet.Spec.UpdateStrategy.Type == apps.RollingUpdateStatefulSetStrategyType && - ordinal < int(currentSet.Status.CurrentReplicas)) || - (currentSet.Spec.UpdateStrategy.Type == apps.PartitionStatefulSetStrategyType && - ordinal < int(currentSet.Spec.UpdateStrategy.Partition.Ordinal)) { + if currentSet.Spec.UpdateStrategy.Type == apps.RollingUpdateStatefulSetStrategyType && + (currentSet.Spec.UpdateStrategy.RollingUpdate == nil && ordinal < int(currentSet.Status.CurrentReplicas)) || + (currentSet.Spec.UpdateStrategy.RollingUpdate != nil && ordinal < int(*currentSet.Spec.UpdateStrategy.RollingUpdate.Partition)) { pod := newStatefulSetPod(currentSet, ordinal) setPodRevision(pod, currentRevision) return pod