Skip to content

Commit 65879f9

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#86097 from nan-yu/statefulset_fix
Honor the RevisionHistoryLimit in StatefulSetSpec
2 parents 72b04ef + 89bb7d8 commit 65879f9

File tree

3 files changed

+75
-15
lines changed

3 files changed

+75
-15
lines changed

‎pkg/controller/statefulset/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ go_test(
6969
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
7070
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
7171
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
72+
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
7273
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
7374
"//staging/src/k8s.io/client-go/informers:go_default_library",
7475
"//staging/src/k8s.io/client-go/informers/apps/v1:go_default_library",

‎pkg/controller/statefulset/stateful_set_control.go‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ import (
2020
"math"
2121
"sort"
2222

23-
"k8s.io/klog"
24-
2523
apps "k8s.io/api/apps/v1"
26-
"k8s.io/api/core/v1"
24+
v1 "k8s.io/api/core/v1"
2725
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
2827
"k8s.io/client-go/tools/record"
28+
"k8s.io/klog"
2929
"k8s.io/kubernetes/pkg/controller/history"
3030
)
3131

@@ -81,22 +81,34 @@ func (ssc *defaultStatefulSetControl) UpdateStatefulSet(set *apps.StatefulSet, p
8181
}
8282
history.SortControllerRevisions(revisions)
8383

84+
currentRevision, updateRevision, err := ssc.performUpdate(set, pods, revisions)
85+
if err != nil {
86+
return utilerrors.NewAggregate([]error{err, ssc.truncateHistory(set, pods, revisions, currentRevision, updateRevision)})
87+
}
88+
89+
// maintain the set's revision history limit
90+
return ssc.truncateHistory(set, pods, revisions, currentRevision, updateRevision)
91+
}
92+
93+
func (ssc *defaultStatefulSetControl) performUpdate(
94+
set *apps.StatefulSet, pods []*v1.Pod, revisions []*apps.ControllerRevision) (*apps.ControllerRevision, *apps.ControllerRevision, error) {
95+
8496
// get the current, and update revisions
8597
currentRevision, updateRevision, collisionCount, err := ssc.getStatefulSetRevisions(set, revisions)
8698
if err != nil {
87-
return err
99+
return currentRevision, updateRevision, err
88100
}
89101

90102
// perform the main update function and get the status
91103
status, err := ssc.updateStatefulSet(set, currentRevision, updateRevision, collisionCount, pods)
92104
if err != nil {
93-
return err
105+
return currentRevision, updateRevision, err
94106
}
95107

96108
// update the set's status
97109
err = ssc.updateStatefulSetStatus(set, status)
98110
if err != nil {
99-
return err
111+
return currentRevision, updateRevision, err
100112
}
101113

102114
klog.V(4).Infof("StatefulSet %s/%s pod status replicas=%d ready=%d current=%d updated=%d",
@@ -113,8 +125,7 @@ func (ssc *defaultStatefulSetControl) UpdateStatefulSet(set *apps.StatefulSet, p
113125
status.CurrentRevision,
114126
status.UpdateRevision)
115127

116-
// maintain the set's revision history limit
117-
return ssc.truncateHistory(set, pods, revisions, currentRevision, updateRevision)
128+
return currentRevision, updateRevision, nil
118129
}
119130

120131
func (ssc *defaultStatefulSetControl) ListRevisions(set *apps.StatefulSet) ([]*apps.ControllerRevision, error) {
@@ -151,7 +162,13 @@ func (ssc *defaultStatefulSetControl) truncateHistory(
151162
update *apps.ControllerRevision) error {
152163
history := make([]*apps.ControllerRevision, 0, len(revisions))
153164
// mark all live revisions
154-
live := map[string]bool{current.Name: true, update.Name: true}
165+
live := map[string]bool{}
166+
if current != nil {
167+
live[current.Name] = true
168+
}
169+
if update != nil {
170+
live[update.Name] = true
171+
}
155172
for i := range pods {
156173
live[getPodRevision(pods[i])] = true
157174
}

‎pkg/controller/statefulset/stateful_set_control_test.go‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ import (
2929
"time"
3030

3131
apps "k8s.io/api/apps/v1"
32-
"k8s.io/api/core/v1"
32+
v1 "k8s.io/api/core/v1"
3333
apierrors "k8s.io/apimachinery/pkg/api/errors"
3434
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3535
"k8s.io/apimachinery/pkg/labels"
36+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
3637
"k8s.io/client-go/informers"
3738
appsinformers "k8s.io/client-go/informers/apps/v1"
3839
coreinformers "k8s.io/client-go/informers/core/v1"
@@ -302,7 +303,7 @@ func CreatePodFailure(t *testing.T, set *apps.StatefulSet, invariants invariantF
302303
defer close(stop)
303304
spc.SetCreateStatefulPodError(apierrors.NewInternalError(errors.New("API server failed")), 2)
304305

305-
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); !apierrors.IsInternalError(err) {
306+
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); err != nil && isOrHasInternalError(err) {
306307
t.Errorf("StatefulSetControl did not return InternalError found %s", err)
307308
}
308309
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); err != nil {
@@ -362,7 +363,7 @@ func UpdatePodFailure(t *testing.T, set *apps.StatefulSet, invariants invariantF
362363
spc.podsIndexer.Update(pods[0])
363364

364365
// now it should fail
365-
if err := ssc.UpdateStatefulSet(set, pods); !apierrors.IsInternalError(err) {
366+
if err := ssc.UpdateStatefulSet(set, pods); err != nil && isOrHasInternalError(err) {
366367
t.Errorf("StatefulSetControl did not return InternalError found %s", err)
367368
}
368369
}
@@ -373,7 +374,7 @@ func UpdateSetStatusFailure(t *testing.T, set *apps.StatefulSet, invariants inva
373374
defer close(stop)
374375
ssu.SetUpdateStatefulSetStatusError(apierrors.NewInternalError(errors.New("API server failed")), 2)
375376

376-
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); !apierrors.IsInternalError(err) {
377+
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); err != nil && isOrHasInternalError(err) {
377378
t.Errorf("StatefulSetControl did not return InternalError found %s", err)
378379
}
379380
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); err != nil {
@@ -421,7 +422,7 @@ func PodRecreateDeleteFailure(t *testing.T, set *apps.StatefulSet, invariants in
421422
pods[0].Status.Phase = v1.PodFailed
422423
spc.podsIndexer.Update(pods[0])
423424
spc.SetDeleteStatefulPodError(apierrors.NewInternalError(errors.New("API server failed")), 0)
424-
if err := ssc.UpdateStatefulSet(set, pods); !apierrors.IsInternalError(err) {
425+
if err := ssc.UpdateStatefulSet(set, pods); err != nil && isOrHasInternalError(err) {
425426
t.Errorf("StatefulSet failed to %s", err)
426427
}
427428
if err := invariants(set, spc); err != nil {
@@ -459,7 +460,7 @@ func TestStatefulSetControlScaleDownDeleteError(t *testing.T) {
459460
}
460461
*set.Spec.Replicas = 0
461462
spc.SetDeleteStatefulPodError(apierrors.NewInternalError(errors.New("API server failed")), 2)
462-
if err := scaleDownStatefulSetControl(set, ssc, spc, invariants); !apierrors.IsInternalError(err) {
463+
if err := scaleDownStatefulSetControl(set, ssc, spc, invariants); err != nil && isOrHasInternalError(err) {
463464
t.Errorf("StatefulSetControl failed to throw error on delete %s", err)
464465
}
465466
if err := scaleDownStatefulSetControl(set, ssc, spc, invariants); err != nil {
@@ -1201,6 +1202,42 @@ func TestStatefulSetControlRollingUpdateWithPartition(t *testing.T) {
12011202
}
12021203
}
12031204

1205+
func TestStatefulSetHonorRevisionHistoryLimit(t *testing.T) {
1206+
invariants := assertMonotonicInvariants
1207+
set := newStatefulSet(3)
1208+
client := fake.NewSimpleClientset(set)
1209+
spc, ssu, ssc, stop := setupController(client)
1210+
defer close(stop)
1211+
1212+
if err := scaleUpStatefulSetControl(set, ssc, spc, invariants); err != nil {
1213+
t.Errorf("Failed to turn up StatefulSet : %s", err)
1214+
}
1215+
var err error
1216+
set, err = spc.setsLister.StatefulSets(set.Namespace).Get(set.Name)
1217+
if err != nil {
1218+
t.Fatalf("Error getting updated StatefulSet: %v", err)
1219+
}
1220+
1221+
for i := 0; i < int(*set.Spec.RevisionHistoryLimit)+5; i++ {
1222+
set.Spec.Template.Spec.Containers[0].Image = fmt.Sprintf("foo-%d", i)
1223+
ssu.SetUpdateStatefulSetStatusError(apierrors.NewInternalError(errors.New("API server failed")), 2)
1224+
updateStatefulSetControl(set, ssc, spc, assertUpdateInvariants)
1225+
set, err = spc.setsLister.StatefulSets(set.Namespace).Get(set.Name)
1226+
if err != nil {
1227+
t.Fatalf("Error getting updated StatefulSet: %v", err)
1228+
}
1229+
revisions, err := ssc.ListRevisions(set)
1230+
if err != nil {
1231+
t.Fatalf("Error listing revisions: %v", err)
1232+
}
1233+
// the extra 2 revisions are `currentRevision` and `updateRevision`
1234+
// They're considered as `live`, and truncateHistory only cleans up non-live revisions
1235+
if len(revisions) > int(*set.Spec.RevisionHistoryLimit)+2 {
1236+
t.Fatalf("%s: %d greater than limit %d", "", len(revisions), *set.Spec.RevisionHistoryLimit)
1237+
}
1238+
}
1239+
}
1240+
12041241
func TestStatefulSetControlLimitsHistory(t *testing.T) {
12051242
type testcase struct {
12061243
name string
@@ -2135,3 +2172,8 @@ func newRevisionOrDie(set *apps.StatefulSet, revision int64) *apps.ControllerRev
21352172
}
21362173
return rev
21372174
}
2175+
2176+
func isOrHasInternalError(err error) bool {
2177+
agg, ok := err.(utilerrors.Aggregate)
2178+
return !ok && !apierrors.IsInternalError(err) || ok && len(agg.Errors()) > 0 && !apierrors.IsInternalError(agg.Errors()[0])
2179+
}

0 commit comments

Comments
 (0)