Skip to content

Commit 7fe64cc

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#84984 from cofyc/fix84942
apps/StatefulSets: Garbage collector should be able to orphan ControllerRevisions too
2 parents ad68c4a + bb2b50f commit 7fe64cc

File tree

2 files changed

+68
-38
lines changed

2 files changed

+68
-38
lines changed

‎pkg/controller/statefulset/stateful_set.go

+12-13
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"time"
2424

2525
apps "k8s.io/api/apps/v1"
26-
"k8s.io/api/core/v1"
26+
v1 "k8s.io/api/core/v1"
2727
"k8s.io/apimachinery/pkg/api/errors"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/apimachinery/pkg/labels"
@@ -289,9 +289,14 @@ func (ssc *StatefulSetController) getPodsForStatefulSet(set *apps.StatefulSet, s
289289
return isMemberOf(set, pod)
290290
}
291291

292-
// If any adoptions are attempted, we should first recheck for deletion with
293-
// an uncached quorum read sometime after listing Pods (see #42639).
294-
canAdoptFunc := controller.RecheckDeletionTimestamp(func() (metav1.Object, error) {
292+
cm := controller.NewPodControllerRefManager(ssc.podControl, set, selector, controllerKind, ssc.canAdoptFunc(set))
293+
return cm.ClaimPods(pods, filter)
294+
}
295+
296+
// If any adoptions are attempted, we should first recheck for deletion with
297+
// an uncached quorum read sometime after listing Pods/ControllerRevisions (see #42639).
298+
func (ssc *StatefulSetController) canAdoptFunc(set *apps.StatefulSet) func() error {
299+
return controller.RecheckDeletionTimestamp(func() (metav1.Object, error) {
295300
fresh, err := ssc.kubeClient.AppsV1().StatefulSets(set.Namespace).Get(context.TODO(), set.Name, metav1.GetOptions{})
296301
if err != nil {
297302
return nil, err
@@ -301,9 +306,6 @@ func (ssc *StatefulSetController) getPodsForStatefulSet(set *apps.StatefulSet, s
301306
}
302307
return fresh, nil
303308
})
304-
305-
cm := controller.NewPodControllerRefManager(ssc.podControl, set, selector, controllerKind, canAdoptFunc)
306-
return cm.ClaimPods(pods, filter)
307309
}
308310

309311
// adoptOrphanRevisions adopts any orphaned ControllerRevisions matched by set's Selector.
@@ -319,12 +321,9 @@ func (ssc *StatefulSetController) adoptOrphanRevisions(set *apps.StatefulSet) er
319321
}
320322
}
321323
if len(orphanRevisions) > 0 {
322-
fresh, err := ssc.kubeClient.AppsV1().StatefulSets(set.Namespace).Get(context.TODO(), set.Name, metav1.GetOptions{})
323-
if err != nil {
324-
return err
325-
}
326-
if fresh.UID != set.UID {
327-
return fmt.Errorf("original StatefulSet %v/%v is gone: got uid %v, wanted %v", set.Namespace, set.Name, fresh.UID, set.UID)
324+
canAdoptErr := ssc.canAdoptFunc(set)()
325+
if canAdoptErr != nil {
326+
return fmt.Errorf("can't adopt ControllerRevisions: %v", canAdoptErr)
328327
}
329328
return ssc.control.AdoptOrphanRevisions(set, orphanRevisions)
330329
}

‎pkg/controller/statefulset/stateful_set_test.go

+56-25
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"testing"
2424

2525
apps "k8s.io/api/apps/v1"
26-
"k8s.io/api/core/v1"
26+
v1 "k8s.io/api/core/v1"
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/apimachinery/pkg/runtime"
2929
"k8s.io/apimachinery/pkg/types"
@@ -42,7 +42,7 @@ func alwaysReady() bool { return true }
4242

4343
func TestStatefulSetControllerCreates(t *testing.T) {
4444
set := newStatefulSet(3)
45-
ssc, spc := newFakeStatefulSetController(set)
45+
ssc, spc, _ := newFakeStatefulSetController(set)
4646
if err := scaleUpStatefulSetController(set, ssc, spc); err != nil {
4747
t.Errorf("Failed to turn up StatefulSet : %s", err)
4848
}
@@ -58,7 +58,7 @@ func TestStatefulSetControllerCreates(t *testing.T) {
5858

5959
func TestStatefulSetControllerDeletes(t *testing.T) {
6060
set := newStatefulSet(3)
61-
ssc, spc := newFakeStatefulSetController(set)
61+
ssc, spc, _ := newFakeStatefulSetController(set)
6262
if err := scaleUpStatefulSetController(set, ssc, spc); err != nil {
6363
t.Errorf("Failed to turn up StatefulSet : %s", err)
6464
}
@@ -86,7 +86,7 @@ func TestStatefulSetControllerDeletes(t *testing.T) {
8686

8787
func TestStatefulSetControllerRespectsTermination(t *testing.T) {
8888
set := newStatefulSet(3)
89-
ssc, spc := newFakeStatefulSetController(set)
89+
ssc, spc, _ := newFakeStatefulSetController(set)
9090
if err := scaleUpStatefulSetController(set, ssc, spc); err != nil {
9191
t.Errorf("Failed to turn up StatefulSet : %s", err)
9292
}
@@ -137,7 +137,7 @@ func TestStatefulSetControllerRespectsTermination(t *testing.T) {
137137

138138
func TestStatefulSetControllerBlocksScaling(t *testing.T) {
139139
set := newStatefulSet(3)
140-
ssc, spc := newFakeStatefulSetController(set)
140+
ssc, spc, _ := newFakeStatefulSetController(set)
141141
if err := scaleUpStatefulSetController(set, ssc, spc); err != nil {
142142
t.Errorf("Failed to turn up StatefulSet : %s", err)
143143
}
@@ -185,7 +185,7 @@ func TestStatefulSetControllerBlocksScaling(t *testing.T) {
185185
func TestStatefulSetControllerDeletionTimestamp(t *testing.T) {
186186
set := newStatefulSet(3)
187187
set.DeletionTimestamp = new(metav1.Time)
188-
ssc, spc := newFakeStatefulSetController(set)
188+
ssc, spc, _ := newFakeStatefulSetController(set)
189189

190190
spc.setsIndexer.Add(set)
191191

@@ -210,7 +210,7 @@ func TestStatefulSetControllerDeletionTimestampRace(t *testing.T) {
210210
set := newStatefulSet(3)
211211
// The bare client says it IS deleted.
212212
set.DeletionTimestamp = new(metav1.Time)
213-
ssc, spc := newFakeStatefulSetController(set)
213+
ssc, spc, ssh := newFakeStatefulSetController(set)
214214

215215
// The lister (cache) says it's NOT deleted.
216216
set2 := *set
@@ -221,6 +221,16 @@ func TestStatefulSetControllerDeletionTimestampRace(t *testing.T) {
221221
pod := newStatefulSetPod(set, 1)
222222
pod.OwnerReferences = nil
223223
spc.podsIndexer.Add(pod)
224+
set.Status.CollisionCount = new(int32)
225+
revision, err := newRevision(set, 1, set.Status.CollisionCount)
226+
if err != nil {
227+
t.Fatal(err)
228+
}
229+
revision.OwnerReferences = nil
230+
revision, err = ssh.CreateControllerRevision(set, revision, set.Status.CollisionCount)
231+
if err != nil {
232+
t.Fatal(err)
233+
}
224234

225235
// Force a sync. It should not try to create any Pods.
226236
ssc.enqueueStatefulSet(set)
@@ -237,10 +247,31 @@ func TestStatefulSetControllerDeletionTimestampRace(t *testing.T) {
237247
if got, want := len(pods), 1; got != want {
238248
t.Errorf("len(pods) = %v, want %v", got, want)
239249
}
250+
251+
// It should not adopt pods.
252+
for _, pod := range pods {
253+
if len(pod.OwnerReferences) > 0 {
254+
t.Errorf("unexpect pod owner references: %v", pod.OwnerReferences)
255+
}
256+
}
257+
258+
// It should not adopt revisions.
259+
revisions, err := ssh.ListControllerRevisions(set, selector)
260+
if err != nil {
261+
t.Fatal(err)
262+
}
263+
if got, want := len(revisions), 1; got != want {
264+
t.Errorf("len(revisions) = %v, want %v", got, want)
265+
}
266+
for _, revision := range revisions {
267+
if len(revision.OwnerReferences) > 0 {
268+
t.Errorf("unexpect revision owner references: %v", revision.OwnerReferences)
269+
}
270+
}
240271
}
241272

242273
func TestStatefulSetControllerAddPod(t *testing.T) {
243-
ssc, spc := newFakeStatefulSetController()
274+
ssc, spc, _ := newFakeStatefulSetController()
244275
set1 := newStatefulSet(3)
245276
set2 := newStatefulSet(3)
246277
pod1 := newStatefulSetPod(set1, 0)
@@ -272,7 +303,7 @@ func TestStatefulSetControllerAddPod(t *testing.T) {
272303
}
273304

274305
func TestStatefulSetControllerAddPodOrphan(t *testing.T) {
275-
ssc, spc := newFakeStatefulSetController()
306+
ssc, spc, _ := newFakeStatefulSetController()
276307
set1 := newStatefulSet(3)
277308
set2 := newStatefulSet(3)
278309
set2.Name = "foo2"
@@ -293,7 +324,7 @@ func TestStatefulSetControllerAddPodOrphan(t *testing.T) {
293324
}
294325

295326
func TestStatefulSetControllerAddPodNoSet(t *testing.T) {
296-
ssc, _ := newFakeStatefulSetController()
327+
ssc, _, _ := newFakeStatefulSetController()
297328
set := newStatefulSet(3)
298329
pod := newStatefulSetPod(set, 0)
299330
ssc.addPod(pod)
@@ -305,7 +336,7 @@ func TestStatefulSetControllerAddPodNoSet(t *testing.T) {
305336
}
306337

307338
func TestStatefulSetControllerUpdatePod(t *testing.T) {
308-
ssc, spc := newFakeStatefulSetController()
339+
ssc, spc, _ := newFakeStatefulSetController()
309340
set1 := newStatefulSet(3)
310341
set2 := newStatefulSet(3)
311342
set2.Name = "foo2"
@@ -340,7 +371,7 @@ func TestStatefulSetControllerUpdatePod(t *testing.T) {
340371
}
341372

342373
func TestStatefulSetControllerUpdatePodWithNoSet(t *testing.T) {
343-
ssc, _ := newFakeStatefulSetController()
374+
ssc, _, _ := newFakeStatefulSetController()
344375
set := newStatefulSet(3)
345376
pod := newStatefulSetPod(set, 0)
346377
prev := *pod
@@ -354,7 +385,7 @@ func TestStatefulSetControllerUpdatePodWithNoSet(t *testing.T) {
354385
}
355386

356387
func TestStatefulSetControllerUpdatePodWithSameVersion(t *testing.T) {
357-
ssc, spc := newFakeStatefulSetController()
388+
ssc, spc, _ := newFakeStatefulSetController()
358389
set := newStatefulSet(3)
359390
pod := newStatefulSetPod(set, 0)
360391
spc.setsIndexer.Add(set)
@@ -367,7 +398,7 @@ func TestStatefulSetControllerUpdatePodWithSameVersion(t *testing.T) {
367398
}
368399

369400
func TestStatefulSetControllerUpdatePodOrphanWithNewLabels(t *testing.T) {
370-
ssc, spc := newFakeStatefulSetController()
401+
ssc, spc, _ := newFakeStatefulSetController()
371402
set := newStatefulSet(3)
372403
pod := newStatefulSetPod(set, 0)
373404
pod.OwnerReferences = nil
@@ -385,7 +416,7 @@ func TestStatefulSetControllerUpdatePodOrphanWithNewLabels(t *testing.T) {
385416
}
386417

387418
func TestStatefulSetControllerUpdatePodChangeControllerRef(t *testing.T) {
388-
ssc, spc := newFakeStatefulSetController()
419+
ssc, spc, _ := newFakeStatefulSetController()
389420
set := newStatefulSet(3)
390421
set2 := newStatefulSet(3)
391422
set2.Name = "foo2"
@@ -403,7 +434,7 @@ func TestStatefulSetControllerUpdatePodChangeControllerRef(t *testing.T) {
403434
}
404435

405436
func TestStatefulSetControllerUpdatePodRelease(t *testing.T) {
406-
ssc, spc := newFakeStatefulSetController()
437+
ssc, spc, _ := newFakeStatefulSetController()
407438
set := newStatefulSet(3)
408439
set2 := newStatefulSet(3)
409440
set2.Name = "foo2"
@@ -420,7 +451,7 @@ func TestStatefulSetControllerUpdatePodRelease(t *testing.T) {
420451
}
421452

422453
func TestStatefulSetControllerDeletePod(t *testing.T) {
423-
ssc, spc := newFakeStatefulSetController()
454+
ssc, spc, _ := newFakeStatefulSetController()
424455
set1 := newStatefulSet(3)
425456
set2 := newStatefulSet(3)
426457
set2.Name = "foo2"
@@ -451,7 +482,7 @@ func TestStatefulSetControllerDeletePod(t *testing.T) {
451482
}
452483

453484
func TestStatefulSetControllerDeletePodOrphan(t *testing.T) {
454-
ssc, spc := newFakeStatefulSetController()
485+
ssc, spc, _ := newFakeStatefulSetController()
455486
set1 := newStatefulSet(3)
456487
set2 := newStatefulSet(3)
457488
set2.Name = "foo2"
@@ -467,7 +498,7 @@ func TestStatefulSetControllerDeletePodOrphan(t *testing.T) {
467498
}
468499

469500
func TestStatefulSetControllerDeletePodTombstone(t *testing.T) {
470-
ssc, spc := newFakeStatefulSetController()
501+
ssc, spc, _ := newFakeStatefulSetController()
471502
set := newStatefulSet(3)
472503
pod := newStatefulSetPod(set, 0)
473504
spc.setsIndexer.Add(set)
@@ -485,7 +516,7 @@ func TestStatefulSetControllerDeletePodTombstone(t *testing.T) {
485516
}
486517

487518
func TestStatefulSetControllerGetStatefulSetsForPod(t *testing.T) {
488-
ssc, spc := newFakeStatefulSetController()
519+
ssc, spc, _ := newFakeStatefulSetController()
489520
set1 := newStatefulSet(3)
490521
set2 := newStatefulSet(3)
491522
set2.Name = "foo2"
@@ -514,7 +545,7 @@ func TestGetPodsForStatefulSetAdopt(t *testing.T) {
514545
pod4.OwnerReferences = nil
515546
pod4.Name = "x" + pod4.Name
516547

517-
ssc, spc := newFakeStatefulSetController(set, pod1, pod2, pod3, pod4)
548+
ssc, spc, _ := newFakeStatefulSetController(set, pod1, pod2, pod3, pod4)
518549

519550
spc.podsIndexer.Add(pod1)
520551
spc.podsIndexer.Add(pod2)
@@ -556,7 +587,7 @@ func TestAdoptOrphanRevisions(t *testing.T) {
556587
ss1Rev2.Namespace = ss1.Namespace
557588
ss1Rev2.OwnerReferences = []metav1.OwnerReference{}
558589

559-
ssc, spc := newFakeStatefulSetController(ss1, ss1Rev1, ss1Rev2)
590+
ssc, spc, _ := newFakeStatefulSetController(ss1, ss1Rev1, ss1Rev2)
560591

561592
spc.revisionsIndexer.Add(ss1Rev1)
562593
spc.revisionsIndexer.Add(ss1Rev2)
@@ -583,7 +614,7 @@ func TestAdoptOrphanRevisions(t *testing.T) {
583614

584615
func TestGetPodsForStatefulSetRelease(t *testing.T) {
585616
set := newStatefulSet(3)
586-
ssc, spc := newFakeStatefulSetController(set)
617+
ssc, spc, _ := newFakeStatefulSetController(set)
587618
pod1 := newStatefulSetPod(set, 1)
588619
// pod2 is owned but has wrong name.
589620
pod2 := newStatefulSetPod(set, 2)
@@ -619,7 +650,7 @@ func TestGetPodsForStatefulSetRelease(t *testing.T) {
619650
}
620651
}
621652

622-
func newFakeStatefulSetController(initialObjects ...runtime.Object) (*StatefulSetController, *fakeStatefulPodControl) {
653+
func newFakeStatefulSetController(initialObjects ...runtime.Object) (*StatefulSetController, *fakeStatefulPodControl, history.Interface) {
623654
client := fake.NewSimpleClientset(initialObjects...)
624655
informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc())
625656
fpc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets(), informerFactory.Apps().V1().ControllerRevisions())
@@ -637,7 +668,7 @@ func newFakeStatefulSetController(initialObjects ...runtime.Object) (*StatefulSe
637668
recorder := record.NewFakeRecorder(10)
638669
ssc.control = NewDefaultStatefulSetControl(fpc, ssu, ssh, recorder)
639670

640-
return ssc, fpc
671+
return ssc, fpc, ssh
641672
}
642673

643674
func fakeWorker(ssc *StatefulSetController) {

0 commit comments

Comments
 (0)