Skip to content

Commit b4cc2a5

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#76051 from zhouhaibing089/rm-orphan-by-default
namespace: remove gc finalizers based on delete options
2 parents 7461892 + a458e9b commit b4cc2a5

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)