e2e: refactor FilterNonRestartablePods function #127071
Conversation
|
@bouaouda-achraf: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hi @bouaouda-achraf. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/sig node |
|
/ok-to-test |
|
/cc @tallclair As you posted the issue, can you check if this resolves your concerns? |
|
/retest-required |
|
/cc @bart0sh |
|
Release notes as written do not make sense. Relnotes needs to be useful for end user, not for k8s developer. Please rephrase. Also it seems that there will be a change in behavior, but I didn't dig into what behavior will change. If we change any behavior, it needs to be tested and explained. |
12d8cef to
fb2a88a
Compare
|
@SergeyKanzhelev , I have updated the release notes and the PR description. Additionally, I’ve added tests covering different cases. |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
/triage accepted |
|
/assign |
test/e2e/framework/pod/resource.go
Outdated
| // Check if the pod is a mirror pod | ||
| if _, ok := p.Annotations[v1.MirrorPodAnnotationKey]; !ok { | ||
| return false | ||
| func isPodNotRecreatedIfDeleted(p *v1.Pod) bool { |
There was a problem hiding this comment.
nit: avoid "not" in function names. Better to invert the logic and call this isPodRecreatedIfDeleted
test/e2e/framework/pod/resource.go
Outdated
| if !isMirrorPod { | ||
| // normal pod will be recreated if deleted (managed by the controller) | ||
| // naked pod will not be recreated | ||
| return len(p.OwnerReferences) == 0 |
There was a problem hiding this comment.
I don't think job pods are recreated if deleted?
There was a problem hiding this comment.
A condition has been added to check if the pod was created by a Job
test/e2e/framework/pod/resource.go
Outdated
| // Mirror pods with restart policy != v1.RestartPolicyAlways will not get | ||
| // recreated if they are deleted after the pods have | ||
| // terminated. For now, we discount such pods. | ||
| // https://github.com/kubernetes/kubernetes/issues/34003 |
There was a problem hiding this comment.
A static pod without RestartPolicyAlways doesn't make any sense to me. Maybe reference #130288 instead?
|
/lgtm cancel to follow up on @tallclair 's comments. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bouaouda-achraf The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fb2a88a to
cbd48a3
Compare
|
/retest-required |
|
/assign @tallclair |
/kind cleanup
/kind e2e
What this PR does / why we need it:
Which issue(s) this PR fixes:
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #109703.
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?