DeleteCollection should include uninitialized resources #47138
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
approved
|
@kubernetes/sig-api-machinery-pr-reviews this is necessary for 1.7 to prevent resources from being leaked during deletion of a namespace. |
k8s-ci-robot
added
the
sig/api-machinery
smarterclayton
added
release-note-none
|
/assign @liggitt |
|
the e2e test that detects remaining content in namespaces should do this as well to make sure it detects pending uninitialized content |
Users who delete a collection expect all resources to be deleted, and users can also delete an uninitialized resource. To preserve this expectation, DeleteCollection selects all resources regardless of initialization. The namespace controller should list uninitialized resources in order to gate cleanup of a namespace.
smarterclayton
force-pushed
the
delete_collection
branch
from
35d62e2
to
9ad1f80
Compare
k8s-github-robot
added
size/L
| @@ -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) | |||
There was a problem hiding this comment.
The recreate isn't necessary to get
There was a problem hiding this comment.
Not necessary today, id prefer to future proof. Ie someone changes how namespace protections work. This test shouldn't depend on us allowing you to access things outside a namespace
| @@ -176,6 +220,10 @@ func ensureServicesAreRemovedWhenNamespaceIsDeleted(f *framework.Framework) { | |||
| return false, nil | |||
| })) | |||
|
|
|||
| By("Recreating the namespace") | |||
| namespace, err = f.CreateNamespace("nsdeletetest", nil) | |||
|
A couple nits on tests, the main change LGTM, though I don't see how this affected the e2e test |
|
The e2e test wasn't broken by this. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, smarterclayton Associated issue: 47137 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 46979, 47078, 47138, 46916) |
Users who delete a collection expect all resources to be deleted, and
users can also delete an uninitialized resource. To preserve this
expectation, DeleteCollection selects all resources regardless of
initialization.
The namespace controller should list uninitialized resources in order to
gate cleanup of a namespace.
Fixes #47137