diff --git a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go index 7920173b43..e061a8ac51 100644 --- a/pkg/controller/namespace/deletion/namespaced_resources_deleter.go +++ b/pkg/controller/namespace/deletion/namespaced_resources_deleter.go @@ -379,7 +379,7 @@ func (d *namespacedResourcesDeleter) listCollection( } apiResource := metav1.APIResource{Name: gvr.Resource, Namespaced: true} - obj, err := dynamicClient.Resource(&apiResource, namespace).List(metav1.ListOptions{}) + obj, err := dynamicClient.Resource(&apiResource, namespace).List(metav1.ListOptions{IncludeUninitialized: true}) if err == nil { unstructuredList, ok := obj.(*unstructured.UnstructuredList) if !ok { @@ -553,7 +553,7 @@ func (d *namespacedResourcesDeleter) estimateGracefulTerminationForPods(ns strin if podsGetter == nil || reflect.ValueOf(podsGetter).IsNil() { return estimate, fmt.Errorf("unexpected: podsGetter is nil. Cannot estimate grace period seconds for pods") } - items, err := podsGetter.Pods(ns).List(metav1.ListOptions{}) + items, err := podsGetter.Pods(ns).List(metav1.ListOptions{IncludeUninitialized: true}) if err != nil { return estimate, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go index bf4f4cd6b9..38ddc80dfd 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion/register.go @@ -30,6 +30,9 @@ const GroupName = "meta.k8s.io" // Scheme is the registry for any type that adheres to the meta API spec. var scheme = runtime.NewScheme() +// Copier exposes copying on this scheme. +var Copier runtime.ObjectCopier = scheme + // Codecs provides access to encoding and decoding for the scheme. var Codecs = serializer.NewCodecFactory(scheme) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 001b7ba207..2dcea903a0 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -998,6 +998,18 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta return out, true, err } +// copyListOptions copies list options for mutation. +func copyListOptions(options *metainternalversion.ListOptions) *metainternalversion.ListOptions { + if options == nil { + return &metainternalversion.ListOptions{} + } + copied, err := metainternalversion.Copier.Copy(options) + if err != nil { + panic(err) + } + return copied.(*metainternalversion.ListOptions) +} + // DeleteCollection removes all items returned by List with a given ListOptions from storage. // // DeleteCollection is currently NOT atomic. It can happen that only subset of objects @@ -1009,6 +1021,12 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta // possibly with storage API, but watch is not delivered correctly then). // It will be possible to fix it with v3 etcd API. func (e *Store) DeleteCollection(ctx genericapirequest.Context, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { + // DeleteCollection must remain backwards compatible with old clients that expect it to + // remove all resources, initialized or not, within the type. It is also consistent with + // Delete which does not require IncludeUninitialized + listOptions = copyListOptions(listOptions) + listOptions.IncludeUninitialized = true + listObj, err := e.List(ctx, listOptions) if err != nil { return nil, err diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index d0443115ef..841483e7b6 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -839,6 +839,44 @@ func TestStoreDelete(t *testing.T) { } } +func TestStoreDeleteUninitialized(t *testing.T) { + podA := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Initializers: &metav1.Initializers{Pending: []metav1.Initializer{{Name: "Testing"}}}}, + Spec: example.PodSpec{NodeName: "machine"}, + } + + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") + destroyFunc, registry := NewTestGenericStoreRegistry(t) + defer destroyFunc() + + // test failure condition + _, _, err := registry.Delete(testContext, podA.Name, nil) + if !errors.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } + + // create pod + _, err = registry.Create(testContext, podA, true) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // delete object + _, wasDeleted, err := registry.Delete(testContext, podA.Name, nil) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !wasDeleted { + t.Errorf("unexpected, pod %s should have been deleted immediately", podA.Name) + } + + // try to get a item which should be deleted + _, err = registry.Get(testContext, podA.Name, &metav1.GetOptions{}) + if !errors.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } +} + // TestGracefulStoreCanDeleteIfExistingGracePeriodZero tests recovery from // race condition where the graceful delete is unable to complete // in prior operation, but the pod remains with deletion timestamp @@ -1546,6 +1584,14 @@ func TestStoreDeletionPropagation(t *testing.T) { func TestStoreDeleteCollection(t *testing.T) { podA := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} podB := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}} + podC := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz", + Initializers: &metav1.Initializers{ + Pending: []metav1.Initializer{{Name: "Test"}}, + }, + }, + } testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") destroyFunc, registry := NewTestGenericStoreRegistry(t) @@ -1557,6 +1603,9 @@ func TestStoreDeleteCollection(t *testing.T) { if _, err := registry.Create(testContext, podB, false); err != nil { t.Errorf("Unexpected error: %v", err) } + if _, err := registry.Create(testContext, podC, true); err != nil { + t.Errorf("Unexpected error: %v", err) + } // Delete all pods. deleted, err := registry.DeleteCollection(testContext, nil, &metainternalversion.ListOptions{}) @@ -1564,8 +1613,8 @@ func TestStoreDeleteCollection(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } deletedPods := deleted.(*example.PodList) - if len(deletedPods.Items) != 2 { - t.Errorf("Unexpected number of pods deleted: %d, expected: 2", len(deletedPods.Items)) + if len(deletedPods.Items) != 3 { + t.Errorf("Unexpected number of pods deleted: %d, expected: 3", len(deletedPods.Items)) } if _, err := registry.Get(testContext, podA.Name, &metav1.GetOptions{}); !errors.IsNotFound(err) { @@ -1574,6 +1623,9 @@ func TestStoreDeleteCollection(t *testing.T) { if _, err := registry.Get(testContext, podB.Name, &metav1.GetOptions{}); !errors.IsNotFound(err) { t.Errorf("Unexpected error: %v", err) } + if _, err := registry.Get(testContext, podC.Name, &metav1.GetOptions{}); !errors.IsNotFound(err) { + t.Errorf("Unexpected error: %v", err) + } } func TestStoreDeleteCollectionNotFound(t *testing.T) { diff --git a/test/e2e/namespace.go b/test/e2e/namespace.go index c63c6b375d..1dd9e827f4 100644 --- a/test/e2e/namespace.go +++ b/test/e2e/namespace.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/pkg/api/v1" + "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" "k8s.io/kubernetes/test/e2e/framework" . "github.com/onsi/ginkgo" @@ -78,9 +79,24 @@ func extinguish(f *framework.Framework, totalNS int, maxAllowedAfterDel int, max })) } -func ensurePodsAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { +func waitForPodInNamespace(c clientset.Interface, ns, podName string) *v1.Pod { + var pod *v1.Pod var err error + err = wait.PollImmediate(2*time.Second, 15*time.Second, func() (bool, error) { + pod, err = c.Core().Pods(ns).Get(podName, metav1.GetOptions{IncludeUninitialized: true}) + if errors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil + }) + Expect(err).NotTo(HaveOccurred()) + return pod +} +func ensurePodsAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { By("Creating a test namespace") namespace, err := f.CreateNamespace("nsdeletetest", nil) Expect(err).NotTo(HaveOccurred()) @@ -109,6 +125,28 @@ func ensurePodsAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { By("Waiting for the pod to have running status") framework.ExpectNoError(framework.WaitForPodRunningInNamespace(f.ClientSet, pod)) + By("Creating an uninitialized pod in the namespace") + podB := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod-uninitialized", + Initializers: &metav1.Initializers{Pending: []metav1.Initializer{{Name: "test.initializer.k8s.io"}}}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "nginx", + Image: framework.GetPauseImageName(f.ClientSet), + }, + }, + }, + } + go func() { + _, err = f.ClientSet.Core().Pods(namespace.Name).Create(podB) + // This error is ok, beacuse we will delete the pod before it completes initialization + framework.Logf("error from create uninitialized namespace: %v", err) + }() + podB = waitForPodInNamespace(f.ClientSet, podB.Namespace, podB.Name) + By("Deleting the namespace") err = f.ClientSet.Core().Namespaces().Delete(namespace.Name, nil) Expect(err).NotTo(HaveOccurred()) @@ -124,9 +162,15 @@ func ensurePodsAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { return false, nil })) - By("Verifying there is no pod in the namespace") + By("Recreating the namespace") + namespace, err = f.CreateNamespace("nsdeletetest", nil) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying there are no pods in the namespace") _, err = f.ClientSet.Core().Pods(namespace.Name).Get(pod.Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) + _, err = f.ClientSet.Core().Pods(namespace.Name).Get(podB.Name, metav1.GetOptions{IncludeUninitialized: true}) + Expect(err).To(HaveOccurred()) } func ensureServicesAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { @@ -176,6 +220,10 @@ func ensureServicesAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { return false, nil })) + By("Recreating the namespace") + namespace, err = f.CreateNamespace("nsdeletetest", nil) + Expect(err).NotTo(HaveOccurred()) + By("Verifying there is no service in the namespace") _, err = f.ClientSet.Core().Services(namespace.Name).Get(service.Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred())