From 8aaaf72a0233b42057844bf5b92428a539e374be Mon Sep 17 00:00:00 2001 From: Abner-1 Date: Mon, 15 Apr 2024 11:18:02 +0800 Subject: [PATCH] ignore vct hash changes when inplace-only update strategy type Signed-off-by: Abner-1 --- pkg/controller/cloneset/core/cloneset_core.go | 5 + .../cloneset/core/cloneset_core_test.go | 74 +++++++ pkg/util/inplaceupdate/inplace_update.go | 2 + .../inplaceupdate/inplace_update_defaults.go | 8 +- .../inplace_update_defaults_test.go | 191 ++++++++++++++++++ test/e2e/apps/cloneset.go | 22 ++ 6 files changed, 299 insertions(+), 3 deletions(-) create mode 100644 pkg/controller/cloneset/core/cloneset_core_test.go diff --git a/pkg/controller/cloneset/core/cloneset_core.go b/pkg/controller/cloneset/core/cloneset_core.go index c8a7788526..b4f6c3a09f 100644 --- a/pkg/controller/cloneset/core/cloneset_core.go +++ b/pkg/controller/cloneset/core/cloneset_core.go @@ -142,6 +142,11 @@ func (c *commonControl) GetUpdateOptions() *inplaceupdate.UpdateOptions { if c.Spec.UpdateStrategy.InPlaceUpdateStrategy != nil { opts.GracePeriodSeconds = c.Spec.UpdateStrategy.InPlaceUpdateStrategy.GracePeriodSeconds } + // For the InPlaceOnly strategy, ignore the hash comparison of VolumeClaimTemplates. + // Consider making changes through a feature gate. + if c.Spec.UpdateStrategy.Type == appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType { + opts.IgnoreVolumeClaimTemplatesHashDiff = true + } return opts } diff --git a/pkg/controller/cloneset/core/cloneset_core_test.go b/pkg/controller/cloneset/core/cloneset_core_test.go new file mode 100644 index 0000000000..75da20c915 --- /dev/null +++ b/pkg/controller/cloneset/core/cloneset_core_test.go @@ -0,0 +1,74 @@ +package core + +import ( + "reflect" + "testing" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/util/inplaceupdate" +) + +func Test_CommonControl_GetUpdateOptions(t *testing.T) { + type fields struct { + CloneSet *appsv1alpha1.CloneSet + } + + defaultOps := &inplaceupdate.UpdateOptions{} + ignoreVCTHashOps := &inplaceupdate.UpdateOptions{IgnoreVolumeClaimTemplatesHashDiff: true} + tests := []struct { + name string + fields fields + want *inplaceupdate.UpdateOptions + }{ + { + name: "inplace only update type", + fields: fields{ + &appsv1alpha1.CloneSet{ + Spec: appsv1alpha1.CloneSetSpec{ + UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ + Type: appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType, + }, + }, + }, + }, + want: ignoreVCTHashOps, + }, + { + name: "inplace if possible update type", + fields: fields{ + &appsv1alpha1.CloneSet{ + Spec: appsv1alpha1.CloneSetSpec{ + UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ + Type: appsv1alpha1.InPlaceIfPossibleCloneSetUpdateStrategyType, + }, + }, + }, + }, + want: defaultOps, + }, + { + // unexpected case: the method should not be called with recreate update strategy type. + name: "recreate update type", + fields: fields{ + &appsv1alpha1.CloneSet{ + Spec: appsv1alpha1.CloneSetSpec{ + UpdateStrategy: appsv1alpha1.CloneSetUpdateStrategy{ + Type: appsv1alpha1.InPlaceIfPossibleCloneSetUpdateStrategyType, + }, + }, + }, + }, + want: defaultOps, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &commonControl{ + CloneSet: tt.fields.CloneSet, + } + if got := c.GetUpdateOptions(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetUpdateOptions() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/util/inplaceupdate/inplace_update.go b/pkg/util/inplaceupdate/inplace_update.go index 5f31d8b88a..48e7a15bc7 100644 --- a/pkg/util/inplaceupdate/inplace_update.go +++ b/pkg/util/inplaceupdate/inplace_update.go @@ -59,6 +59,8 @@ type UpdateResult struct { } type UpdateOptions struct { + IgnoreVolumeClaimTemplatesHashDiff bool + GracePeriodSeconds int32 AdditionalFuncs []func(*v1.Pod) diff --git a/pkg/util/inplaceupdate/inplace_update_defaults.go b/pkg/util/inplaceupdate/inplace_update_defaults.go index 185051779d..65b9849c05 100644 --- a/pkg/util/inplaceupdate/inplace_update_defaults.go +++ b/pkg/util/inplaceupdate/inplace_update_defaults.go @@ -236,9 +236,11 @@ func defaultCalculateInPlaceUpdateSpec(oldRevision, newRevision *apps.Controller return nil } - canInPlace := volumeclaimtemplate.CanVCTemplateInplaceUpdate(oldRevision, newRevision) - if !canInPlace { - return nil + if !opts.IgnoreVolumeClaimTemplatesHashDiff { + canInPlace := volumeclaimtemplate.CanVCTemplateInplaceUpdate(oldRevision, newRevision) + if !canInPlace { + return nil + } } oldTemp, err := GetTemplateFromRevision(oldRevision) diff --git a/pkg/util/inplaceupdate/inplace_update_defaults_test.go b/pkg/util/inplaceupdate/inplace_update_defaults_test.go index 4973ff9621..5bd833356e 100644 --- a/pkg/util/inplaceupdate/inplace_update_defaults_test.go +++ b/pkg/util/inplaceupdate/inplace_update_defaults_test.go @@ -562,6 +562,8 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { } } }` + + ignoreVCTHashUpdateOpts := &UpdateOptions{IgnoreVolumeClaimTemplatesHashDiff: true} tests := []struct { name string args args @@ -588,6 +590,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, }, want: &UpdateSpec{ Revision: "new-revision", @@ -617,6 +620,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, }, want: &UpdateSpec{ Revision: "new-revision", @@ -646,6 +650,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, }, want: &UpdateSpec{ Revision: "new-revision", @@ -675,6 +680,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, }, want: nil, }, @@ -699,6 +705,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, }, want: &UpdateSpec{ Revision: "new-revision", @@ -728,6 +735,189 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { Raw: []byte(newData), }, }, + opts: &UpdateOptions{}, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + + // IgnoreVolumeClaimTemplatesHashDiff is true + { + name: "IgnoreVolumeClaimTemplatesHashDiff&both revision annotation is nil=> ignore", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: nil, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: map[string]string{}, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + { + name: "IgnoreVolumeClaimTemplatesHashDiff&old revision annotation is nil=> ignore", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: nil, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: "balala"}, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + { + name: "IgnoreVolumeClaimTemplatesHashDiff&new revision annotation is nil => ignore", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: "balala"}, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: nil, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + { + name: "IgnoreVolumeClaimTemplatesHashDiff&revision annotation changes => in-place update", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: ""}, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: "balala"}, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + { + name: "IgnoreVolumeClaimTemplatesHashDiff&the same revision annotation => in-place update", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: "balala"}, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: "balala"}, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, + }, + want: &UpdateSpec{ + Revision: "new-revision", + ContainerImages: map[string]string{ + "nginx": "nginx:stable-alpine", + }, + }, + }, + { + name: "IgnoreVolumeClaimTemplatesHashDiff&both empty revision annotation => in-place update", + args: args{ + oldRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: ""}, + }, + Data: runtime.RawExtension{ + Raw: []byte(oldData), + }, + }, + newRevision: &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-revision", + Annotations: map[string]string{volumeclaimtemplate.HashAnnotation: ""}, + }, + Data: runtime.RawExtension{ + Raw: []byte(newData), + }, + }, + opts: ignoreVCTHashUpdateOpts, }, want: &UpdateSpec{ Revision: "new-revision", @@ -737,6 +927,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) { }, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := defaultCalculateInPlaceUpdateSpec(tt.args.oldRevision, tt.args.newRevision, tt.args.opts) diff --git a/test/e2e/apps/cloneset.go b/test/e2e/apps/cloneset.go index 0f5429936e..2f71e4f41c 100644 --- a/test/e2e/apps/cloneset.go +++ b/test/e2e/apps/cloneset.go @@ -1279,4 +1279,26 @@ func testUpdateVolumeClaimTemplates(tester *framework.CloneSetTester, randStr st instanceIds, pvcIds = changeCloneSetAndWaitReady(tester, cs, updateImageOnly, instanceIds, pvcIds, checkPodsDoRecreate(replicas, true), checkPVCsDoRecreate(replicas*2, true), checkPVCSize2) + + // inplace-only update strategy with image change + inplaceOnlyWithImage := func(cs *appsv1alpha1.CloneSet) { + imageConfig = imageutils.GetConfig(imageutils.Httpd) + cs.Spec.Template.Spec.Containers[0].Image = imageConfig.GetE2EImage() + cs.Spec.UpdateStrategy = appsv1alpha1.CloneSetUpdateStrategy{Type: appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType} + } + instanceIds, pvcIds = changeCloneSetAndWaitReady(tester, cs, inplaceOnlyWithImage, + instanceIds, pvcIds, checkPodsDoRecreate(replicas, false), + checkPVCsDoRecreate(replicas*2, false), checkPVCSize2) + + // inplace-only update strategy with image and vct changes -> in-place update with pvc no changes + inplaceOnlyWithImageAndVCT := func(cs *appsv1alpha1.CloneSet) { + imageConfig = imageutils.GetConfig(imageutils.Redis) + cs.Spec.Template.Spec.Containers[0].Image = imageConfig.GetE2EImage() + cs.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests = v1.ResourceList{v1.ResourceStorage: resource.MustParse("3Gi")} + cs.Spec.VolumeClaimTemplates[1].Spec.Resources.Requests = v1.ResourceList{v1.ResourceStorage: resource.MustParse("3Gi")} + cs.Spec.UpdateStrategy = appsv1alpha1.CloneSetUpdateStrategy{Type: appsv1alpha1.InPlaceOnlyCloneSetUpdateStrategyType} + } + instanceIds, pvcIds = changeCloneSetAndWaitReady(tester, cs, inplaceOnlyWithImageAndVCT, + instanceIds, pvcIds, checkPodsDoRecreate(replicas, false), + checkPVCsDoRecreate(replicas*2, false), checkPVCSize2) }