Skip to content

Commit a458e9b

Browse files
zhouhaibing089zhouhaibing089
committed
namespace: remove gc finalizers based on delete options
This makes the behavior being consistent with generic store, The orphan finalizer should be removed if the delete options does not specify propagarionPolicy as orphan.
1 parent aa52140 commit a458e9b

File tree

3 files changed

+259
-9
lines changed

3 files changed

+259
-9
lines changed

‎pkg/registry/core/namespace/storage/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ go_test(
1616
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
1717
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
1818
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
19+
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
1920
"//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
2021
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
2122
"//staging/src/k8s.io/apiserver/pkg/registry/generic/testing:go_default_library",

‎pkg/registry/core/namespace/storage/storage.go‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,33 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp
185185
existingNamespace.Status.Phase = api.NamespaceTerminating
186186
}
187187

188-
// Remove orphan finalizer if options.OrphanDependents = false.
189-
if options.OrphanDependents != nil && *options.OrphanDependents == false {
190-
// remove Orphan finalizer.
188+
// the current finalizers which are on namespace
189+
currentFinalizers := map[string]bool{}
190+
for _, f := range existingNamespace.Finalizers {
191+
currentFinalizers[f] = true
192+
}
193+
// the finalizers we should ensure on namespace
194+
shouldHaveFinalizers := map[string]bool{
195+
metav1.FinalizerOrphanDependents: shouldHaveOrphanFinalizer(options, currentFinalizers[metav1.FinalizerOrphanDependents]),
196+
metav1.FinalizerDeleteDependents: shouldHaveDeleteDependentsFinalizer(options, currentFinalizers[metav1.FinalizerDeleteDependents]),
197+
}
198+
// determine whether there are changes
199+
changeNeeded := false
200+
for finalizer, shouldHave := range shouldHaveFinalizers {
201+
changeNeeded = currentFinalizers[finalizer] != shouldHave || changeNeeded
202+
if shouldHave {
203+
currentFinalizers[finalizer] = true
204+
} else {
205+
delete(currentFinalizers, finalizer)
206+
}
207+
}
208+
// make the changes if needed
209+
if changeNeeded {
191210
newFinalizers := []string{}
192-
for i := range existingNamespace.ObjectMeta.Finalizers {
193-
finalizer := existingNamespace.ObjectMeta.Finalizers[i]
194-
if string(finalizer) != metav1.FinalizerOrphanDependents {
195-
newFinalizers = append(newFinalizers, finalizer)
196-
}
211+
for f := range currentFinalizers {
212+
newFinalizers = append(newFinalizers, f)
197213
}
198-
existingNamespace.ObjectMeta.Finalizers = newFinalizers
214+
existingNamespace.Finalizers = newFinalizers
199215
}
200216
return existingNamespace, nil
201217
}),
@@ -222,6 +238,26 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp
222238
return r.store.Delete(ctx, name, options)
223239
}
224240

241+
func shouldHaveOrphanFinalizer(options *metav1.DeleteOptions, haveOrphanFinalizer bool) bool {
242+
if options.OrphanDependents != nil {
243+
return *options.OrphanDependents
244+
}
245+
if options.PropagationPolicy != nil {
246+
return *options.PropagationPolicy == metav1.DeletePropagationOrphan
247+
}
248+
return haveOrphanFinalizer
249+
}
250+
251+
func shouldHaveDeleteDependentsFinalizer(options *metav1.DeleteOptions, haveDeleteDependentsFinalizer bool) bool {
252+
if options.OrphanDependents != nil {
253+
return *options.OrphanDependents == false
254+
}
255+
if options.PropagationPolicy != nil {
256+
return *options.PropagationPolicy == metav1.DeletePropagationForeground
257+
}
258+
return haveDeleteDependentsFinalizer
259+
}
260+
225261
func (e *REST) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1beta1.Table, error) {
226262
return e.store.ConvertToTable(ctx, object, tableOptions)
227263
}

‎pkg/registry/core/namespace/storage/storage_test.go‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ import (
2222
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2323
"k8s.io/apimachinery/pkg/fields"
2424
"k8s.io/apimachinery/pkg/labels"
25+
"k8s.io/apimachinery/pkg/runtime"
2526
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
2627
"k8s.io/apiserver/pkg/registry/generic"
2728
genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
2829
"k8s.io/apiserver/pkg/registry/rest"
2930
etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing"
31+
3032
api "k8s.io/kubernetes/pkg/apis/core"
3133
"k8s.io/kubernetes/pkg/registry/registrytest"
3234
)
@@ -189,6 +191,217 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) {
189191
}
190192
}
191193

194+
func TestDeleteWithGCFinalizers(t *testing.T) {
195+
storage, server := newStorage(t)
196+
defer server.Terminate(t)
197+
defer storage.store.DestroyFunc()
198+
199+
propagationBackground := metav1.DeletePropagationBackground
200+
propagationForeground := metav1.DeletePropagationForeground
201+
propagationOrphan := metav1.DeletePropagationOrphan
202+
trueVar := true
203+
204+
var tests = []struct {
205+
name string
206+
deleteOptions *metav1.DeleteOptions
207+
208+
existingFinalizers []string
209+
remainingFinalizers map[string]bool
210+
}{
211+
{
212+
name: "nil-with-orphan",
213+
deleteOptions: nil,
214+
existingFinalizers: []string{
215+
metav1.FinalizerOrphanDependents,
216+
},
217+
remainingFinalizers: map[string]bool{
218+
metav1.FinalizerOrphanDependents: true,
219+
},
220+
},
221+
{
222+
name: "nil-with-delete",
223+
deleteOptions: nil,
224+
existingFinalizers: []string{
225+
metav1.FinalizerDeleteDependents,
226+
},
227+
remainingFinalizers: map[string]bool{
228+
metav1.FinalizerDeleteDependents: true,
229+
},
230+
},
231+
{
232+
name: "nil-without-finalizers",
233+
deleteOptions: nil,
234+
existingFinalizers: []string{},
235+
remainingFinalizers: map[string]bool{},
236+
},
237+
{
238+
name: "propagation-background-with-orphan",
239+
deleteOptions: &metav1.DeleteOptions{
240+
PropagationPolicy: &propagationBackground,
241+
},
242+
existingFinalizers: []string{
243+
metav1.FinalizerOrphanDependents,
244+
},
245+
remainingFinalizers: map[string]bool{},
246+
},
247+
{
248+
name: "propagation-background-with-delete",
249+
deleteOptions: &metav1.DeleteOptions{
250+
PropagationPolicy: &propagationBackground,
251+
},
252+
existingFinalizers: []string{
253+
metav1.FinalizerDeleteDependents,
254+
},
255+
remainingFinalizers: map[string]bool{},
256+
},
257+
{
258+
name: "propagation-background-without-finalizers",
259+
deleteOptions: &metav1.DeleteOptions{
260+
PropagationPolicy: &propagationBackground,
261+
},
262+
existingFinalizers: []string{},
263+
remainingFinalizers: map[string]bool{},
264+
},
265+
{
266+
name: "propagation-foreground-with-orphan",
267+
deleteOptions: &metav1.DeleteOptions{
268+
PropagationPolicy: &propagationForeground,
269+
},
270+
existingFinalizers: []string{
271+
metav1.FinalizerOrphanDependents,
272+
},
273+
remainingFinalizers: map[string]bool{
274+
metav1.FinalizerDeleteDependents: true,
275+
},
276+
},
277+
{
278+
name: "propagation-foreground-with-delete",
279+
deleteOptions: &metav1.DeleteOptions{
280+
PropagationPolicy: &propagationForeground,
281+
},
282+
existingFinalizers: []string{
283+
metav1.FinalizerDeleteDependents,
284+
},
285+
remainingFinalizers: map[string]bool{
286+
metav1.FinalizerDeleteDependents: true,
287+
},
288+
},
289+
{
290+
name: "propagation-foreground-without-finalizers",
291+
deleteOptions: &metav1.DeleteOptions{
292+
PropagationPolicy: &propagationForeground,
293+
},
294+
existingFinalizers: []string{},
295+
remainingFinalizers: map[string]bool{
296+
metav1.FinalizerDeleteDependents: true,
297+
},
298+
},
299+
{
300+
name: "propagation-orphan-with-orphan",
301+
deleteOptions: &metav1.DeleteOptions{
302+
PropagationPolicy: &propagationOrphan,
303+
},
304+
existingFinalizers: []string{
305+
metav1.FinalizerOrphanDependents,
306+
},
307+
remainingFinalizers: map[string]bool{
308+
metav1.FinalizerOrphanDependents: true,
309+
},
310+
},
311+
{
312+
name: "propagation-orphan-with-delete",
313+
deleteOptions: &metav1.DeleteOptions{
314+
PropagationPolicy: &propagationOrphan,
315+
},
316+
existingFinalizers: []string{
317+
metav1.FinalizerDeleteDependents,
318+
},
319+
remainingFinalizers: map[string]bool{
320+
metav1.FinalizerOrphanDependents: true,
321+
},
322+
},
323+
{
324+
name: "propagation-orphan-without-finalizers",
325+
deleteOptions: &metav1.DeleteOptions{
326+
PropagationPolicy: &propagationOrphan,
327+
},
328+
existingFinalizers: []string{},
329+
remainingFinalizers: map[string]bool{
330+
metav1.FinalizerOrphanDependents: true,
331+
},
332+
},
333+
{
334+
name: "orphan-dependents-with-orphan",
335+
deleteOptions: &metav1.DeleteOptions{
336+
OrphanDependents: &trueVar,
337+
},
338+
existingFinalizers: []string{
339+
metav1.FinalizerOrphanDependents,
340+
},
341+
remainingFinalizers: map[string]bool{
342+
metav1.FinalizerOrphanDependents: true,
343+
},
344+
},
345+
{
346+
name: "orphan-dependents-with-delete",
347+
deleteOptions: &metav1.DeleteOptions{
348+
OrphanDependents: &trueVar,
349+
},
350+
existingFinalizers: []string{
351+
metav1.FinalizerDeleteDependents,
352+
},
353+
remainingFinalizers: map[string]bool{
354+
metav1.FinalizerOrphanDependents: true,
355+
},
356+
},
357+
{
358+
name: "orphan-dependents-without-finalizers",
359+
deleteOptions: &metav1.DeleteOptions{
360+
OrphanDependents: &trueVar,
361+
},
362+
existingFinalizers: []string{},
363+
remainingFinalizers: map[string]bool{
364+
metav1.FinalizerOrphanDependents: true,
365+
},
366+
},
367+
}
368+
369+
for _, test := range tests {
370+
key := "namespaces/" + test.name
371+
ctx := genericapirequest.NewContext()
372+
namespace := &api.Namespace{
373+
ObjectMeta: metav1.ObjectMeta{
374+
Name: test.name,
375+
Finalizers: test.existingFinalizers,
376+
},
377+
Spec: api.NamespaceSpec{
378+
Finalizers: []api.FinalizerName{},
379+
},
380+
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
381+
}
382+
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
383+
t.Fatalf("unexpected error: %v", err)
384+
}
385+
var obj runtime.Object
386+
var err error
387+
if obj, _, err = storage.Delete(ctx, test.name, test.deleteOptions); err != nil {
388+
t.Fatalf("unexpected error: %v", err)
389+
}
390+
ns, ok := obj.(*api.Namespace)
391+
if !ok {
392+
t.Errorf("unexpected object kind: %+v", obj)
393+
}
394+
if len(ns.Finalizers) != len(test.remainingFinalizers) {
395+
t.Errorf("%s: unexpected remaining finalizers: %v", test.name, ns.Finalizers)
396+
}
397+
for _, f := range ns.Finalizers {
398+
if test.remainingFinalizers[f] != true {
399+
t.Errorf("%s: unexpected finalizer %s", test.name, f)
400+
}
401+
}
402+
}
403+
}
404+
192405
func TestShortNames(t *testing.T) {
193406
storage, server := newStorage(t)
194407
defer server.Terminate(t)

0 commit comments

Comments
 (0)