Skip to content

Commit

Permalink
ignore vct hash changes when inplace-only update strategy type
Browse files Browse the repository at this point in the history 
Signed-off-by: Abner-1 <Abner199709@gmail.com>
  • Loading branch information
@ABNER-1
ABNER-1 committed Apr 15, 2024
1 parent b472924 commit 8aaaf72
Show file tree
Hide file tree
Showing 6 changed files with 299 additions and 3 deletions.
5 changes: 5 additions & 0 deletions pkg/controller/cloneset/core/cloneset_core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
74 changes: 74 additions & 0 deletions pkg/controller/cloneset/core/cloneset_core_test.go
View file
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
2 changes: 2 additions & 0 deletions pkg/util/inplaceupdate/inplace_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type UpdateResult struct {
}

type UpdateOptions struct {
IgnoreVolumeClaimTemplatesHashDiff bool

GracePeriodSeconds int32
AdditionalFuncs []func(*v1.Pod)

Expand Down
8 changes: 5 additions & 3 deletions pkg/util/inplaceupdate/inplace_update_defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
191 changes: 191 additions & 0 deletions pkg/util/inplaceupdate/inplace_update_defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
}
}
}`

ignoreVCTHashUpdateOpts := &UpdateOptions{IgnoreVolumeClaimTemplatesHashDiff: true}
tests := []struct {
name string
args args
Expand All @@ -588,6 +590,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
Raw: []byte(newData),
},
},
opts: &UpdateOptions{},
},
want: &UpdateSpec{
Revision: "new-revision",
Expand Down Expand Up @@ -617,6 +620,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
Raw: []byte(newData),
},
},
opts: &UpdateOptions{},
},
want: &UpdateSpec{
Revision: "new-revision",
Expand Down Expand Up @@ -646,6 +650,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
Raw: []byte(newData),
},
},
opts: &UpdateOptions{},
},
want: &UpdateSpec{
Revision: "new-revision",
Expand Down Expand Up @@ -675,6 +680,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
Raw: []byte(newData),
},
},
opts: &UpdateOptions{},
},
want: nil,
},
Expand All @@ -699,6 +705,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
Raw: []byte(newData),
},
},
opts: &UpdateOptions{},
},
want: &UpdateSpec{
Revision: "new-revision",
Expand Down Expand Up @@ -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",
Expand All @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions test/e2e/apps/cloneset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 8aaaf72

Please sign in to comment.