Skip to content

Commit

Permalink
allow to muate PVCTemplate of asts & cloneset (#1118)
Browse files Browse the repository at this point in the history 
Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com>

Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com>
Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com>
  • Loading branch information
@veophi
veophi and mingzhou.swx committed Dec 5, 2022
1 parent 0cfd676 commit 53fb7e1
Show file tree
Hide file tree
Showing 4 changed files with 254 additions and 4 deletions.
3 changes: 2 additions & 1 deletion pkg/webhook/cloneset/validating/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 20 additions & 2 deletions pkg/webhook/cloneset/validating/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand Down
6 changes: 5 additions & 1 deletion pkg/webhook/statefulset/validating/statefulset_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))...)
Expand Down
227 changes: 227 additions & 0 deletions pkg/webhook/statefulset/validating/statefulset_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 53fb7e1

Please sign in to comment.