From 151a06e125ae855e79f7a12d25555cb464137758 Mon Sep 17 00:00:00 2001 From: Wei-Xiang Sun Date: Mon, 5 Dec 2022 14:21:57 +0800 Subject: [PATCH] allow to muate PVCTemplate of asts & cloneset (#1118) Co-authored-by: mingzhou.swx --- pkg/webhook/cloneset/validating/validation.go | 3 +- .../cloneset/validating/validation_test.go | 22 +- .../validating/statefulset_validation.go | 6 +- .../validating/statefulset_validation_test.go | 227 ++++++++++++++++++ 4 files changed, 254 insertions(+), 4 deletions(-) diff --git a/pkg/webhook/cloneset/validating/validation.go b/pkg/webhook/cloneset/validating/validation.go index 9a58078275..a4851669be 100644 --- a/pkg/webhook/cloneset/validating/validation.go +++ b/pkg/webhook/cloneset/validating/validation.go @@ -194,8 +194,9 @@ func (h *CloneSetCreateUpdateHandler) validateCloneSetUpdate(cloneSet, oldCloneS clone.Spec.MinReadySeconds = oldCloneSet.Spec.MinReadySeconds clone.Spec.Lifecycle = oldCloneSet.Spec.Lifecycle clone.Spec.RevisionHistoryLimit = oldCloneSet.Spec.RevisionHistoryLimit + clone.Spec.VolumeClaimTemplates = oldCloneSet.Spec.VolumeClaimTemplates if !apiequality.Semantic.DeepEqual(clone.Spec, oldCloneSet.Spec) { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to cloneset spec for fields other than 'replicas', 'template', 'lifecycle', 'scaleStrategy', 'updateStrategy', 'minReadySeconds' and 'revisionHistoryLimit' are forbidden")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to cloneset spec for fields other than 'replicas', 'template', 'lifecycle', 'scaleStrategy', 'updateStrategy', 'minReadySeconds', 'volumeClaimTemplates' and 'revisionHistoryLimit' are forbidden")) } coreControl := clonesetcore.New(cloneSet) diff --git a/pkg/webhook/cloneset/validating/validation_test.go b/pkg/webhook/cloneset/validating/validation_test.go index 0f4ef64b9e..27b9e82f93 100644 --- a/pkg/webhook/cloneset/validating/validation_test.go +++ b/pkg/webhook/cloneset/validating/validation_test.go @@ -10,9 +10,11 @@ import ( appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" "github.com/openkruise/kruise/pkg/util" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" + utilpointer "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -73,6 +75,21 @@ func TestValidate(t *testing.T) { }, } + validVolumeClaimTemplate := func(size string) v1.PersistentVolumeClaim { + return v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: v1.PersistentVolumeClaimSpec{ + StorageClassName: utilpointer.String("foo/bar"), + AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, + Resources: v1.ResourceRequirements{Requests: map[v1.ResourceName]resource.Quantity{ + v1.ResourceStorage: resource.MustParse(size), + }}, + }, + } + } + var valTrue = true var val1 int32 = 1 var val2 int32 = 2 @@ -171,6 +188,7 @@ func TestValidate(t *testing.T) { ScaleStrategy: appsv1alpha1.CloneSetScaleStrategy{ PodsToDelete: []string{"p0"}, }, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{validVolumeClaimTemplate("30Gi")}, UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ Type: appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType, Partition: util.GetIntOrStrPointer(intstr.FromInt(2)), @@ -185,7 +203,7 @@ func TestValidate(t *testing.T) { ScaleStrategy: appsv1alpha1.CloneSetScaleStrategy{ PodsToDelete: []string{"p1"}, }, - + VolumeClaimTemplates: []v1.PersistentVolumeClaim{validVolumeClaimTemplate("60Gi")}, UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ Type: appsv1alpha1.RecreateCloneSetUpdateStrategyType, Partition: util.GetIntOrStrPointer(intstr.FromInt(2)), @@ -203,6 +221,7 @@ func TestValidate(t *testing.T) { ScaleStrategy: appsv1alpha1.CloneSetScaleStrategy{ PodsToDelete: []string{"p0"}, }, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{validVolumeClaimTemplate("30Gi")}, UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ Type: appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType, Partition: util.GetIntOrStrPointer(intstr.FromInt(2)), @@ -217,7 +236,6 @@ func TestValidate(t *testing.T) { ScaleStrategy: appsv1alpha1.CloneSetScaleStrategy{ PodsToDelete: []string{}, }, - UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ Type: appsv1alpha1.RecreateCloneSetUpdateStrategyType, Partition: util.GetIntOrStrPointer(intstr.FromInt(2)), diff --git a/pkg/webhook/statefulset/validating/statefulset_validation.go b/pkg/webhook/statefulset/validating/statefulset_validation.go index 80403d1920..2e7b5c4cf5 100644 --- a/pkg/webhook/statefulset/validating/statefulset_validation.go +++ b/pkg/webhook/statefulset/validating/statefulset_validation.go @@ -318,19 +318,23 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *appsv1beta1.Stateful restoreScaleStrategy := statefulSet.Spec.ScaleStrategy statefulSet.Spec.ScaleStrategy = oldStatefulSet.Spec.ScaleStrategy + restorePVCTemplate := statefulSet.Spec.VolumeClaimTemplates + statefulSet.Spec.VolumeClaimTemplates = oldStatefulSet.Spec.VolumeClaimTemplates + restoreReserveOrdinals := statefulSet.Spec.ReserveOrdinals statefulSet.Spec.ReserveOrdinals = oldStatefulSet.Spec.ReserveOrdinals statefulSet.Spec.Lifecycle = oldStatefulSet.Spec.Lifecycle statefulSet.Spec.RevisionHistoryLimit = oldStatefulSet.Spec.RevisionHistoryLimit if !apiequality.Semantic.DeepEqual(statefulSet.Spec, oldStatefulSet.Spec) { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'reserveOrdinals', 'lifecycle', 'revisionHistoryLimit', 'persistentVolumeClaimRetentionPolicy' and 'updateStrategy' are forbidden")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'reserveOrdinals', 'lifecycle', 'revisionHistoryLimit', 'persistentVolumeClaimRetentionPolicy', `volumeClaimTemplates` and 'updateStrategy' are forbidden")) } statefulSet.Spec.Replicas = restoreReplicas statefulSet.Spec.Template = restoreTemplate statefulSet.Spec.UpdateStrategy = restoreStrategy statefulSet.Spec.ScaleStrategy = restoreScaleStrategy statefulSet.Spec.ReserveOrdinals = restoreReserveOrdinals + statefulSet.Spec.VolumeClaimTemplates = restorePVCTemplate statefulSet.Spec.PersistentVolumeClaimRetentionPolicy = restorePersistentVolumeClaimRetentionPolicy allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*statefulSet.Spec.Replicas), field.NewPath("spec", "replicas"))...) diff --git a/pkg/webhook/statefulset/validating/statefulset_validation_test.go b/pkg/webhook/statefulset/validating/statefulset_validation_test.go index e022d6981b..b6ed2deeee 100644 --- a/pkg/webhook/statefulset/validating/statefulset_validation_test.go +++ b/pkg/webhook/statefulset/validating/statefulset_validation_test.go @@ -21,9 +21,11 @@ import ( "strings" "testing" + appspub "github.com/openkruise/kruise/apis/apps/pub" appsv1beta1 "github.com/openkruise/kruise/apis/apps/v1beta1" apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" utilpointer "k8s.io/utils/pointer" @@ -477,6 +479,231 @@ func TestValidateStatefulSet(t *testing.T) { } } +func TestValidateStatefulSetUpdate(t *testing.T) { + validLabels := map[string]string{"a": "b"} + validPodTemplate1 := v1.PodTemplate{ + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: validLabels, + }, + Spec: v1.PodSpec{ + RestartPolicy: v1.RestartPolicyAlways, + DNSPolicy: v1.DNSClusterFirst, + Containers: []v1.Container{{Name: "abc", Image: "image:v1", ImagePullPolicy: "IfNotPresent"}}, + }, + }, + } + validPodTemplate2 := v1.PodTemplate{ + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: validLabels, + }, + Spec: v1.PodSpec{ + RestartPolicy: v1.RestartPolicyAlways, + DNSPolicy: v1.DNSClusterFirst, + Containers: []v1.Container{{Name: "abc", Image: "image:v2", ImagePullPolicy: "IfNotPresent"}}, + }, + }, + } + + validVolumeClaimTemplate := func(size string) v1.PersistentVolumeClaim { + return v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: v1.PersistentVolumeClaimSpec{ + StorageClassName: utilpointer.String("foo/bar"), + AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, + Resources: v1.ResourceRequirements{Requests: map[v1.ResourceName]resource.Quantity{ + v1.ResourceStorage: resource.MustParse(size), + }}, + }, + } + } + + successCases := []struct { + old *appsv1beta1.StatefulSet + new *appsv1beta1.StatefulSet + }{ + { + old: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + Replicas: utilpointer.Int32Ptr(5), + RevisionHistoryLimit: utilpointer.Int32Ptr(5), + ReserveOrdinals: []int{1}, + Lifecycle: &appspub.Lifecycle{PreDelete: &appspub.LifecycleHook{FinalizersHandler: []string{"foo/bar"}}}, + Template: validPodTemplate1.Template, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{validVolumeClaimTemplate("30Gi")}, + ScaleStrategy: &appsv1beta1.StatefulSetScaleStrategy{MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 1}}, + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{ + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: &appsv1beta1.RollingUpdateStatefulSetStrategy{Partition: utilpointer.Int32Ptr(5)}, + }, + PersistentVolumeClaimRetentionPolicy: &appsv1beta1.StatefulSetPersistentVolumeClaimRetentionPolicy{ + WhenScaled: appsv1beta1.RetainPersistentVolumeClaimRetentionPolicyType, + WhenDeleted: appsv1beta1.RetainPersistentVolumeClaimRetentionPolicyType, + }, + }, + }, + new: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + Replicas: utilpointer.Int32Ptr(10), + RevisionHistoryLimit: utilpointer.Int32Ptr(10), + ReserveOrdinals: []int{2}, + Lifecycle: &appspub.Lifecycle{PreDelete: &appspub.LifecycleHook{FinalizersHandler: []string{"foo/hello"}}}, + Template: validPodTemplate2.Template, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{validVolumeClaimTemplate("60Gi")}, + ScaleStrategy: &appsv1beta1.StatefulSetScaleStrategy{MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 2}}, + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{ + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: &appsv1beta1.RollingUpdateStatefulSetStrategy{Partition: utilpointer.Int32Ptr(10)}, + }, + PersistentVolumeClaimRetentionPolicy: &appsv1beta1.StatefulSetPersistentVolumeClaimRetentionPolicy{ + WhenScaled: appsv1beta1.RetainPersistentVolumeClaimRetentionPolicyType, + WhenDeleted: appsv1beta1.RetainPersistentVolumeClaimRetentionPolicyType, + }, + }, + }, + }, + } + + for i, successCase := range successCases { + t.Run("success case "+strconv.Itoa(i), func(t *testing.T) { + if errs := ValidateStatefulSetUpdate(successCase.new, successCase.old); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + }) + } + + errorCases := map[string]struct { + old *appsv1beta1.StatefulSet + new *appsv1beta1.StatefulSet + }{ + "selector changed": { + old: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: "", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + new: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: "", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "bar"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + }, + "serviceName changed": { + old: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: "", + ServiceName: "foo", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + new: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: "", + ServiceName: "bar", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + }, + "podManagementPolicy changed": { + old: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: apps.OrderedReadyPodManagement, + ServiceName: "bar", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + new: &appsv1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + ResourceVersion: "1", + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: apps.ParallelPodManagement, + ServiceName: "bar", + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Template: validPodTemplate1.Template, + Replicas: utilpointer.Int32Ptr(1), + UpdateStrategy: appsv1beta1.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + }, + } + + for k, v := range errorCases { + t.Run(k, func(t *testing.T) { + setTestDefault(v.old) + setTestDefault(v.new) + errs := ValidateStatefulSetUpdate(v.new, v.old) + if len(errs) == 0 { + t.Errorf("expected failure for %s", k) + } + + for i := range errs { + field := errs[i].Field + if field != "spec" { + t.Errorf("%s: missing prefix for: %v", k, errs[i]) + } + } + }) + } +} + func setTestDefault(obj *appsv1beta1.StatefulSet) { if obj.Spec.Replicas == nil { obj.Spec.Replicas = new(int32)