diff --git a/pkg/controller/namespace/deletion/BUILD b/pkg/controller/namespace/deletion/BUILD index a39952ec6af..15b0c8123f4 100644 --- a/pkg/controller/namespace/deletion/BUILD +++ b/pkg/controller/namespace/deletion/BUILD @@ -46,7 +46,9 @@ go_test( "//staging/src/k8s.io/client-go/discovery:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library", "//staging/src/k8s.io/client-go/metadata:go_default_library", + "//staging/src/k8s.io/client-go/metadata/fake:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", ], diff --git a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go index 6d0cf2586b1..7f97f34fdb9 100644 --- a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go +++ b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go @@ -265,11 +265,9 @@ func (d *namespacedResourcesDeleter) updateNamespaceStatusFunc(namespace *v1.Nam if namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == v1.NamespaceTerminating { return namespace, nil } - newNamespace := v1.Namespace{} - newNamespace.ObjectMeta = namespace.ObjectMeta - newNamespace.Status = *namespace.Status.DeepCopy() + newNamespace := namespace.DeepCopy() newNamespace.Status.Phase = v1.NamespaceTerminating - return d.nsClient.UpdateStatus(context.TODO(), &newNamespace, metav1.UpdateOptions{}) + return d.nsClient.UpdateStatus(context.TODO(), newNamespace, metav1.UpdateOptions{}) } // finalized returns true if the namespace.Spec.Finalizers is an empty list @@ -330,10 +328,8 @@ func (d *namespacedResourcesDeleter) deleteCollection(gvr schema.GroupVersionRes // we have a resource returned in the discovery API that supports no top-level verbs: // /apis/extensions/v1beta1/namespaces/default/replicationcontrollers // when working with this resource type, we will get a literal not found error rather than expected method not supported - // remember next time that this resource does not support delete collection... if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) { klog.V(5).Infof("namespace controller - deleteCollection not supported - namespace: %s, gvr: %v", namespace, gvr) - d.opCache.setNotSupported(key) return false, nil } @@ -365,10 +361,8 @@ func (d *namespacedResourcesDeleter) listCollection(gvr schema.GroupVersionResou // we have a resource returned in the discovery API that supports no top-level verbs: // /apis/extensions/v1beta1/namespaces/default/replicationcontrollers // when working with this resource type, we will get a literal not found error rather than expected method not supported - // remember next time that this resource does not support delete collection... if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) { klog.V(5).Infof("namespace controller - listCollection not supported - namespace: %s, gvr: %v", namespace, gvr) - d.opCache.setNotSupported(key) return nil, false, nil } diff --git a/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go b/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go index 1762c536c33..5bb8b94c718 100644 --- a/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go +++ b/pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go @@ -34,7 +34,9 @@ import ( "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes/fake" + scheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/metadata" + metadatafake "k8s.io/client-go/metadata/fake" restclient "k8s.io/client-go/rest" core "k8s.io/client-go/testing" api "k8s.io/kubernetes/pkg/apis/core" @@ -396,3 +398,67 @@ func testResources() []*metav1.APIResourceList { } return results } + +func TestDeleteEncounters404(t *testing.T) { + now := metav1.Now() + ns1 := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "ns1", ResourceVersion: "1", DeletionTimestamp: &now}, + Spec: v1.NamespaceSpec{Finalizers: []v1.FinalizerName{"kubernetes"}}, + Status: v1.NamespaceStatus{Phase: v1.NamespaceActive}, + } + ns2 := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "ns2", ResourceVersion: "1", DeletionTimestamp: &now}, + Spec: v1.NamespaceSpec{Finalizers: []v1.FinalizerName{"kubernetes"}}, + Status: v1.NamespaceStatus{Phase: v1.NamespaceActive}, + } + mockClient := fake.NewSimpleClientset(ns1, ns2) + + ns1FlakesNotFound := func(action core.Action) (handled bool, ret runtime.Object, err error) { + if action.GetNamespace() == "ns1" { + // simulate the flakes resource not existing when ns1 is processed + return true, nil, errors.NewNotFound(schema.GroupResource{}, "") + } + return false, nil, nil + } + mockMetadataClient := metadatafake.NewSimpleMetadataClient(scheme.Scheme) + mockMetadataClient.PrependReactor("delete-collection", "flakes", ns1FlakesNotFound) + mockMetadataClient.PrependReactor("list", "flakes", ns1FlakesNotFound) + + resourcesFn := func() ([]*metav1.APIResourceList, error) { + return []*metav1.APIResourceList{{ + GroupVersion: "example.com/v1", + APIResources: []metav1.APIResource{{Name: "flakes", Namespaced: true, Kind: "Flake", Verbs: []string{"get", "list", "delete", "deletecollection", "create", "update"}}}, + }}, nil + } + + d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), mockMetadataClient, mockClient.CoreV1(), resourcesFn, v1.FinalizerKubernetes) + + // Delete ns1 and get NotFound errors for the flakes resource + mockMetadataClient.ClearActions() + if err := d.Delete(ns1.Name); err != nil { + t.Fatal(err) + } + if len(mockMetadataClient.Actions()) != 3 || + !mockMetadataClient.Actions()[0].Matches("delete-collection", "flakes") || + !mockMetadataClient.Actions()[1].Matches("list", "flakes") || + !mockMetadataClient.Actions()[2].Matches("list", "flakes") { + for _, action := range mockMetadataClient.Actions() { + t.Log("ns1", action) + } + t.Error("ns1: expected delete-collection -> fallback to list -> list to verify 0 items") + } + + // Delete ns2 + mockMetadataClient.ClearActions() + if err := d.Delete(ns2.Name); err != nil { + t.Fatal(err) + } + if len(mockMetadataClient.Actions()) != 2 || + !mockMetadataClient.Actions()[0].Matches("delete-collection", "flakes") || + !mockMetadataClient.Actions()[1].Matches("list", "flakes") { + for _, action := range mockMetadataClient.Actions() { + t.Log("ns2", action) + } + t.Error("ns2: expected delete-collection -> list to verify 0 items") + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 0631bc7248d..78b700bd99a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2045,6 +2045,7 @@ k8s.io/client-go/listers/storage/v1 k8s.io/client-go/listers/storage/v1alpha1 k8s.io/client-go/listers/storage/v1beta1 k8s.io/client-go/metadata +k8s.io/client-go/metadata/fake k8s.io/client-go/metadata/metadatainformer k8s.io/client-go/metadata/metadatalister k8s.io/client-go/pkg/apis/clientauthentication