Skip to content

Commit 04f73b9

Browse files
author
Nina van der Linden
committed
feat: add feature to recreate pod when the volumeClaimTemplate of a StatefulSet with OnPodRollingUpdate changes
Signed-off-by: Nina van der Linden <[email protected]>
1 parent 4025f61 commit 04f73b9

File tree

12 files changed

+355
-84
lines changed

12 files changed

+355
-84
lines changed

‎pkg/controller/cloneset/core/cloneset_core.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,9 @@ func (c *commonControl) GetPodsSortFunc(pods []*v1.Pod, waitUpdateIndexes []int)
141141
}
142142

143143
func (c *commonControl) GetUpdateOptions() *inplaceupdate.UpdateOptions {
144-
opts := &inplaceupdate.UpdateOptions{}
144+
opts := &inplaceupdate.UpdateOptions{
145+
RecreatePodWhenChangedVolumeClaimTemplate: utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInCloneSetGate),
146+
}
145147
if c.Spec.UpdateStrategy.InPlaceUpdateStrategy != nil {
146148
opts.GracePeriodSeconds = c.Spec.UpdateStrategy.InPlaceUpdateStrategy.GracePeriodSeconds
147149
}

‎pkg/controller/statefulset/stateful_pod_control_test.go

Lines changed: 82 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -250,58 +250,94 @@ func TestStatefulPodControlCreatePodFailed(t *testing.T) {
250250
}
251251

252252
func TestStatefulPodControlNoOpUpdate(t *testing.T) {
253-
recorder := record.NewFakeRecorder(10)
254-
set := newStatefulSet(3)
255-
pod := newStatefulSetPod(set, 0)
256-
fakeClient := &fake.Clientset{}
257-
claims := getPersistentVolumeClaims(set, pod)
258-
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
259-
for k := range claims {
260-
claim := claims[k]
261-
indexer.Add(&claim)
253+
testFn := func(t *testing.T) {
254+
recorder := record.NewFakeRecorder(10)
255+
set := newStatefulSet(3)
256+
pod := newStatefulSetPod(set, 0)
257+
fakeClient := &fake.Clientset{}
258+
claims := getPersistentVolumeClaims(set, pod)
259+
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
260+
for k := range claims {
261+
claim := claims[k]
262+
indexer.Add(&claim)
263+
}
264+
claimLister := corelisters.NewPersistentVolumeClaimLister(indexer)
265+
control := NewStatefulPodControl(fakeClient, nil, claimLister, nil, recorder)
266+
fakeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
267+
t.Error("no-op update should not make any client invocation")
268+
return true, nil, apierrors.NewInternalError(errors.New("If we are here we have a problem"))
269+
})
270+
if err := control.UpdateStatefulPod(set, pod); err != nil {
271+
t.Errorf("Error returned on no-op update error: %s", err)
272+
}
273+
events := collectEvents(recorder.Events)
274+
if eventCount := len(events); eventCount != 0 {
275+
t.Errorf("no-op update: got %d events, but want 0", eventCount)
276+
}
262277
}
263-
claimLister := corelisters.NewPersistentVolumeClaimLister(indexer)
264-
control := NewStatefulPodControl(fakeClient, nil, claimLister, nil, recorder)
265-
fakeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
266-
t.Error("no-op update should not make any client invocation")
267-
return true, nil, apierrors.NewInternalError(errors.New("If we are here we have a problem"))
278+
279+
t.Run("RecreatePodWhenChangeVCTInStatefulSetGate enabled", func(t *testing.T) {
280+
defer utilfeature.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecreatePodWhenChangeVCTInStatefulSetGate, true)()
281+
testFn(t)
268282
})
269-
if err := control.UpdateStatefulPod(set, pod); err != nil {
270-
t.Errorf("Error returned on no-op update error: %s", err)
271-
}
272-
events := collectEvents(recorder.Events)
273-
if eventCount := len(events); eventCount != 0 {
274-
t.Errorf("no-op update: got %d events, but want 0", eventCount)
275-
}
283+
t.Run("RecreatePodWhenChangeVCTInStatefulSetGate disabled", func(t *testing.T) {
284+
defer utilfeature.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecreatePodWhenChangeVCTInStatefulSetGate, false)()
285+
testFn(t)
286+
})
287+
276288
}
277289

278290
func TestStatefulPodControlUpdatesIdentity(t *testing.T) {
279-
recorder := record.NewFakeRecorder(10)
280-
set := newStatefulSet(3)
281-
pod := newStatefulSetPod(set, 0)
282-
fakeClient := fake.NewSimpleClientset(pod)
283-
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
284-
claimLister := corelisters.NewPersistentVolumeClaimLister(indexer)
285-
control := NewStatefulPodControl(fakeClient, nil, claimLister, nil, recorder)
286-
var updated *v1.Pod
287-
fakeClient.PrependReactor("update", "pods", func(action core.Action) (bool, runtime.Object, error) {
288-
update := action.(core.UpdateAction)
289-
updated = update.GetObject().(*v1.Pod)
290-
return true, update.GetObject(), nil
291-
})
292-
pod.Name = "goo-0"
293-
if err := control.UpdateStatefulPod(set, pod); err != nil {
294-
t.Errorf("Successful update returned an error: %s", err)
295-
}
296-
events := collectEvents(recorder.Events)
297-
if eventCount := len(events); eventCount != 1 {
298-
t.Errorf("Pod update successful:got %d events,but want 1", eventCount)
299-
} else if !strings.Contains(events[0], v1.EventTypeNormal) {
300-
t.Errorf("Found unexpected non-normal event %s", events[0])
301-
}
302-
if !identityMatches(set, updated) {
303-
t.Error("Name update failed identity does not match")
291+
testFn := func(t *testing.T, expectAnnotationUpdate bool) {
292+
recorder := record.NewFakeRecorder(10)
293+
set := newStatefulSet(3)
294+
pod := newStatefulSetPod(set, 0)
295+
296+
vctAnnotationBefore := pod.Annotations[PodVolumeClaimTemplatesKey]
297+
298+
fakeClient := fake.NewSimpleClientset(pod)
299+
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
300+
claimLister := corelisters.NewPersistentVolumeClaimLister(indexer)
301+
control := NewStatefulPodControl(fakeClient, nil, claimLister, nil, recorder)
302+
var updated *v1.Pod
303+
fakeClient.PrependReactor("update", "pods", func(action core.Action) (bool, runtime.Object, error) {
304+
update := action.(core.UpdateAction)
305+
updated = update.GetObject().(*v1.Pod)
306+
return true, update.GetObject(), nil
307+
})
308+
pod.Name = "goo-0"
309+
set.Spec.VolumeClaimTemplates = append(set.Spec.VolumeClaimTemplates, v1.PersistentVolumeClaim{
310+
ObjectMeta: metav1.ObjectMeta{
311+
Name: "test",
312+
},
313+
})
314+
315+
if err := control.UpdateStatefulPod(set, pod); err != nil {
316+
t.Errorf("Successful update returned an error: %s", err)
317+
}
318+
events := collectEvents(recorder.Events)
319+
if eventCount := len(events); eventCount != 3 {
320+
t.Errorf("Pod update successful:got %d events,but want 3", eventCount)
321+
t.Log(events)
322+
} else if !strings.Contains(events[0], v1.EventTypeNormal) {
323+
t.Errorf("Found unexpected non-normal event %s", events[0])
324+
}
325+
if !identityMatches(set, updated) {
326+
t.Error("Name update failed identity does not match")
327+
}
328+
if expectAnnotationUpdate != (vctAnnotationBefore != updated.Annotations[PodVolumeClaimTemplatesKey]) {
329+
t.Error("Expected pod-vct-names to be updated")
330+
}
304331
}
332+
333+
t.Run("RecreatePodWhenChangeVCTInStatefulSetGate enabled", func(t *testing.T) {
334+
defer utilfeature.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecreatePodWhenChangeVCTInStatefulSetGate, true)()
335+
testFn(t, true)
336+
})
337+
t.Run("RecreatePodWhenChangeVCTInStatefulSetGate disabled", func(t *testing.T) {
338+
defer utilfeature.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecreatePodWhenChangeVCTInStatefulSetGate, false)()
339+
testFn(t, false)
340+
})
305341
}
306342

307343
func TestStatefulPodControlUpdateIdentityFailure(t *testing.T) {

‎pkg/controller/statefulset/stateful_set_control.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,9 @@ func (ssc *defaultStatefulSetControl) inPlaceUpdatePod(
854854
}
855855
}
856856

857-
opts := &inplaceupdate.UpdateOptions{}
857+
opts := &inplaceupdate.UpdateOptions{
858+
RecreatePodWhenChangedVolumeClaimTemplate: utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInStatefulSetGate),
859+
}
858860
if set.Spec.UpdateStrategy.RollingUpdate.InPlaceUpdateStrategy != nil {
859861
opts.GracePeriodSeconds = set.Spec.UpdateStrategy.RollingUpdate.InPlaceUpdateStrategy.GracePeriodSeconds
860862
}
@@ -1217,6 +1219,11 @@ func (ssc *defaultStatefulSetControl) processReplica(
12171219
}
12181220
}
12191221

1222+
if utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInStatefulSetGate) &&
1223+
!volumeClaimTemplatesMatchPod(set, replicas[i]) {
1224+
return false, false, nil
1225+
}
1226+
12201227
if identityMatches(set, replicas[i]) && storageMatches(set, replicas[i]) && retentionMatch {
12211228
return false, false, nil
12221229
}

‎pkg/controller/statefulset/stateful_set_utils.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ import (
2222
"encoding/json"
2323
"fmt"
2424
"regexp"
25+
"slices"
2526
"sort"
2627
"strconv"
28+
"strings"
2729
"time"
2830

2931
apps "k8s.io/api/apps/v1"
@@ -124,7 +126,8 @@ func identityMatches(set *appsv1beta1.StatefulSet, pod *v1.Pod) bool {
124126
set.Name == parent &&
125127
pod.Name == getPodName(set, ordinal) &&
126128
pod.Namespace == set.Namespace &&
127-
pod.Labels[apps.StatefulSetPodNameLabel] == pod.Name
129+
pod.Labels[apps.StatefulSetPodNameLabel] == pod.Name &&
130+
volumeClaimTemplatesMatchPod(set, pod)
128131
}
129132

130133
// storageMatches returns true if pod's Volumes cover the set of PersistentVolumeClaims
@@ -372,6 +375,31 @@ func initIdentity(set *appsv1beta1.StatefulSet, pod *v1.Pod) {
372375
pod.Spec.Subdomain = set.Spec.ServiceName
373376
}
374377

378+
const (
379+
PodVolumeClaimTemplatesKey = "statefulset.apps.kruise.io/pod-vct-names"
380+
)
381+
382+
func getPodVolumeClaimTemplateValue(set *appsv1beta1.StatefulSet) string {
383+
names := make([]string, len(set.Spec.VolumeClaimTemplates))
384+
for i, vct := range set.Spec.VolumeClaimTemplates {
385+
names[i] = vct.Name
386+
}
387+
slices.Sort(names)
388+
return strings.Join(names, ",")
389+
}
390+
391+
func volumeClaimTemplatesMatchPod(set *appsv1beta1.StatefulSet, pod *v1.Pod) bool {
392+
if !utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInStatefulSetGate) {
393+
return true
394+
}
395+
targetValue := getPodVolumeClaimTemplateValue(set)
396+
actualValue := ""
397+
if pod.Annotations != nil {
398+
actualValue = pod.Annotations[PodVolumeClaimTemplatesKey]
399+
}
400+
return targetValue == actualValue
401+
}
402+
375403
// updateIdentity updates pod's name, hostname, and subdomain, and StatefulSetPodNameLabel to conform to set's name
376404
// and headless service.
377405
func updateIdentity(set *appsv1beta1.StatefulSet, pod *v1.Pod) {
@@ -385,6 +413,13 @@ func updateIdentity(set *appsv1beta1.StatefulSet, pod *v1.Pod) {
385413
if utilfeature.DefaultFeatureGate.Enabled(features.PodIndexLabel) {
386414
pod.Labels[apps.PodIndexLabel] = strconv.Itoa(ordinal)
387415
}
416+
417+
if pod.Annotations == nil {
418+
pod.Annotations = make(map[string]string)
419+
}
420+
if utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInStatefulSetGate) {
421+
pod.Annotations[PodVolumeClaimTemplatesKey] = getPodVolumeClaimTemplateValue(set)
422+
}
388423
}
389424

390425
// isRunningAndAvailable returns true if pod is in the PodRunning Phase,

‎pkg/controller/statefulset/statefulset_predownload_image.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ import (
3232

3333
appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
3434
appsv1beta1 "github.com/openkruise/kruise/apis/apps/v1beta1"
35+
"github.com/openkruise/kruise/pkg/features"
3536
"github.com/openkruise/kruise/pkg/util/expectations"
37+
utilfeature "github.com/openkruise/kruise/pkg/util/feature"
3638
imagejobutilfunc "github.com/openkruise/kruise/pkg/util/imagejob/utilfunction"
3739
"github.com/openkruise/kruise/pkg/util/inplaceupdate"
3840
"github.com/openkruise/kruise/pkg/util/revisionadapter"
@@ -75,7 +77,9 @@ func (dss *defaultStatefulSetControl) createImagePullJobsForInPlaceUpdate(sts *a
7577
}
7678

7779
// opt is update option, this section is to get update option
78-
opts := &inplaceupdate.UpdateOptions{}
80+
opts := &inplaceupdate.UpdateOptions{
81+
RecreatePodWhenChangedVolumeClaimTemplate: utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInStatefulSetGate),
82+
}
7983
if sts.Spec.UpdateStrategy.RollingUpdate.InPlaceUpdateStrategy != nil {
8084
opts.GracePeriodSeconds = sts.Spec.UpdateStrategy.RollingUpdate.InPlaceUpdateStrategy.GracePeriodSeconds
8185
}

‎pkg/features/kruise_features.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ const (
120120
// RecreatePodWhenChangeVCTInCloneSetGate recreate the pod upon changing volume claim templates in a clone set to ensure PVC consistency.
121121
RecreatePodWhenChangeVCTInCloneSetGate featuregate.Feature = "RecreatePodWhenChangeVCTInCloneSetGate"
122122

123+
// RecreatePodWhenChangeVCTInStatefulSetGate enables Advances StatefulSetController to recreate Pods if volumeClaimTemplates
124+
// are added or removed
125+
RecreatePodWhenChangeVCTInStatefulSetGate featuregate.Feature = "RecreatePodWhenChangeVCTInStatefulSetGate"
126+
123127
// Enables a StatefulSet to start from an arbitrary non zero ordinal
124128
StatefulSetStartOrdinal featuregate.Feature = "StatefulSetStartOrdinal"
125129

@@ -173,16 +177,17 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
173177
ResourceDistributionGate: {Default: false, PreRelease: featuregate.Alpha},
174178
DeletionProtectionForCRDCascadingGate: {Default: false, PreRelease: featuregate.Alpha},
175179

176-
EnhancedLivenessProbeGate: {Default: false, PreRelease: featuregate.Alpha},
177-
RecreatePodWhenChangeVCTInCloneSetGate: {Default: false, PreRelease: featuregate.Alpha},
178-
StatefulSetStartOrdinal: {Default: false, PreRelease: featuregate.Alpha},
179-
PodIndexLabel: {Default: true, PreRelease: featuregate.Beta},
180-
EnableExternalCerts: {Default: false, PreRelease: featuregate.Alpha},
181-
StatefulSetAutoResizePVCGate: {Default: false, PreRelease: featuregate.Alpha},
182-
ForceDeleteTimeoutExpectationFeatureGate: {Default: false, PreRelease: featuregate.Alpha},
183-
InPlaceWorkloadVerticalScaling: {Default: false, PreRelease: featuregate.Alpha},
184-
EnablePodProbeMarkerOnServerless: {Default: false, PreRelease: featuregate.Alpha},
185-
EnableSortSidecarContainerByName: {Default: false, PreRelease: featuregate.Alpha},
180+
EnhancedLivenessProbeGate: {Default: false, PreRelease: featuregate.Alpha},
181+
RecreatePodWhenChangeVCTInCloneSetGate: {Default: false, PreRelease: featuregate.Alpha},
182+
RecreatePodWhenChangeVCTInStatefulSetGate: {Default: false, PreRelease: featuregate.Alpha},
183+
StatefulSetStartOrdinal: {Default: false, PreRelease: featuregate.Alpha},
184+
PodIndexLabel: {Default: true, PreRelease: featuregate.Beta},
185+
EnableExternalCerts: {Default: false, PreRelease: featuregate.Alpha},
186+
StatefulSetAutoResizePVCGate: {Default: false, PreRelease: featuregate.Alpha},
187+
ForceDeleteTimeoutExpectationFeatureGate: {Default: false, PreRelease: featuregate.Alpha},
188+
InPlaceWorkloadVerticalScaling: {Default: false, PreRelease: featuregate.Alpha},
189+
EnablePodProbeMarkerOnServerless: {Default: false, PreRelease: featuregate.Alpha},
190+
EnableSortSidecarContainerByName: {Default: false, PreRelease: featuregate.Alpha},
186191
}
187192

188193
func init() {

‎pkg/util/inplaceupdate/inplace_update.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ type UpdateResult struct {
6060
}
6161

6262
type UpdateOptions struct {
63-
IgnoreVolumeClaimTemplatesHashDiff bool
63+
IgnoreVolumeClaimTemplatesHashDiff bool
64+
RecreatePodWhenChangedVolumeClaimTemplate bool
6465

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

‎pkg/util/inplaceupdate/inplace_update_defaults.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import (
2222
"strconv"
2323
"strings"
2424

25-
"github.com/appscode/jsonpatch"
26-
2725
appspub "github.com/openkruise/kruise/apis/apps/pub"
2826
"github.com/openkruise/kruise/pkg/features"
2927
"github.com/openkruise/kruise/pkg/util"
@@ -32,6 +30,7 @@ import (
3230
utilfeature "github.com/openkruise/kruise/pkg/util/feature"
3331
"github.com/openkruise/kruise/pkg/util/volumeclaimtemplate"
3432

33+
"github.com/appscode/jsonpatch"
3534
apps "k8s.io/api/apps/v1"
3635
v1 "k8s.io/api/core/v1"
3736
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -256,7 +255,7 @@ func addMetadataSharedContainersToUpdate(pod *v1.Pod, containersToUpdate sets.St
256255
}
257256

258257
// defaultCalculateInPlaceUpdateSpec calculates diff between old and update revisions.
259-
// If the diff just contains replace operation of spec.containers[x].image, it will returns an UpdateSpec.
258+
// If the diff just contains replace operation of spec.containers[x].image, it will return an UpdateSpec.
260259
// Otherwise, it returns nil which means can not use in-place update.
261260
func defaultCalculateInPlaceUpdateSpec(oldRevision, newRevision *apps.ControllerRevision, opts *UpdateOptions) *UpdateSpec {
262261
if oldRevision == nil || newRevision == nil {
@@ -269,13 +268,10 @@ func defaultCalculateInPlaceUpdateSpec(oldRevision, newRevision *apps.Controller
269268
return nil
270269
}
271270

272-
// RecreatePodWhenChangeVCTInCloneSetGate enabled
273-
if utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInCloneSetGate) {
274-
if !opts.IgnoreVolumeClaimTemplatesHashDiff {
275-
canInPlace := volumeclaimtemplate.CanVCTemplateInplaceUpdate(oldRevision, newRevision)
276-
if !canInPlace {
277-
return nil
278-
}
271+
if opts.RecreatePodWhenChangedVolumeClaimTemplate && !opts.IgnoreVolumeClaimTemplatesHashDiff {
272+
canInPlace := volumeclaimtemplate.CanVCTemplateInplaceUpdate(oldRevision, newRevision)
273+
if !canInPlace {
274+
return nil
279275
}
280276
}
281277

‎pkg/util/inplaceupdate/inplace_update_defaults_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,7 @@ func Test_defaultCalculateInPlaceUpdateSpec_VCTHash(t *testing.T) {
896896
defer utilfeature.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecreatePodWhenChangeVCTInCloneSetGate, enable)()
897897
for _, tt := range tests {
898898
t.Run(fmt.Sprintf("%v-%v", tt.name, enable), func(t *testing.T) {
899+
tt.args.opts.RecreatePodWhenChangedVolumeClaimTemplate = enable
899900
got := defaultCalculateInPlaceUpdateSpec(tt.args.oldRevision, tt.args.newRevision, tt.args.opts)
900901
wanted := tt.wantWhenDisable
901902
if utilfeature.DefaultFeatureGate.Enabled(features.RecreatePodWhenChangeVCTInCloneSetGate) {

0 commit comments

Comments
 (0)