New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure invalid cluster-scoped children do not block cleanup of valid namespaced children #98068
Conversation
…tly cluster-scoped nodes
/priority important-soon |
LGTM but needs a review from someone more familiar with the GC controller. |
/retest /cc @jpbetz |
}, | ||
{ | ||
// https://github.com/kubernetes/kubernetes/issues/98040 | ||
name: "namespaced good child, cluster-scoped bad child, missing parent", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is like "cluster-scoped bad child, namespaced good child, missing parent" except the order the children are observed/processed is swapped. I didn't realize when first reading the test names that ordering was important.
assertState(state{ | ||
clientActions: []string{ | ||
"get /v1, Resource=pods ns=ns1 name=podname2", // get good child, returns 200 | ||
"get /v1, Resource=pods ns=ns1 name=podname1", // get missing parent, returns 404 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these gets informational or are they required for the test? I'm curious because we don't check the response codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks right to me. I spent most of my time reviewing the tests to convince myself they cover the cases described in #98040.
Added two comments, which are non-blocking and primarily for my understanding.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpbetz, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…8-upstream-release-1.20 Automated cherry pick of #98068: Ensure invalid cluster-scoped children do not block cleanup of valid namespaced children
What type of PR is this?
/kind bug
Which issue(s) this PR fixes:
Fixes #98040
bad child -> good child
orderDoes this PR introduce a user-facing change?:
/cc @enj
/sig api-machinery