Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Pod controller: "when Using pod group Should keep the running pod group with the queue name if workload is evicted" flakes #2252

Merged
merged 2 commits into from
May 22, 2024
Merged

Pod controller: "when Using pod group Should keep the running pod group with the queue name if workload is evicted" flakes #2252

merged 2 commits into from
May 22, 2024

Conversation

vladikkuzn
Copy link
Contributor

@vladikkuzn vladikkuzn commented May 21, 2024 •
edited
Loading

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

Fixes failing test caused by changes in isPodRunnableOrSucceeded of #2212

-	if p.DeletionTimestamp != nil && len(p.Spec.SchedulingGates) > 0 {
+	if p.DeletionTimestamp != nil && len(p.Spec.NodeName) == 0 {

It's solely tests issue, since node name was not set, the workload reconciliation was stuck in a reclaimable pods reset attempt

Which issue(s) this PR fixes:

Fixes #2243

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels May 21, 2024
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 21, 2024
@vladikkuzn
Copy link
Contributor Author

/assign @vladikkuzn @trasc

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 21, 2024
@netlify Netlify
Copy link

netlify bot commented May 21, 2024 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 516862e
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/664e41d24ea6e9000850c0bb

@vladikkuzn
Copy link
Contributor Author

/cc @trasc

@k8s-ci-robot k8s-ci-robot requested a review from trasc May 21, 2024 13:33
@vladikkuzn
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 21, 2024
@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

1 similar comment
@trasc
Copy link
Contributor

trasc commented May 21, 2024

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

5 similar comments
@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

1 similar comment
@vladikkuzn
Copy link
Contributor Author

/test pull-kueue-test-integration-main

@alculquicondor
Copy link
Contributor

/cherry-pick release-0.6

@k8s-infra-cherrypick-robot
Copy link
Contributor

@alculquicondor: once the present PR merges, I will cherry-pick it on top of release-0.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-0.6

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.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2024
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 22, 2024
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 22, 2024
@alculquicondor
Copy link
Contributor

/lgtm
/approve
/hold
in case you want to do the name change.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 22, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4e6b6194a0e44b102d31a9753a50a56405cbb708

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, vladikkuzn

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2024
@vladikkuzn
Pod controller: "when Using pod group Should keep the running pod gro…
516862e 
…up with the queue name if workload is evicted" flakes

* Rename SetNodeName -> BindPodWithNode
* Reorder to match production env
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2024
@vladikkuzn
Copy link
Contributor Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024
trasc
trasc reviewed May 22, 2024
View reviewed changes
Copy link
Contributor

@trasc trasc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 611c1828fa9774ee8b6474374eb99c96218277c3

@k8s-ci-robot k8s-ci-robot merged commit 05cc80f into kubernetes-sigs:main May 22, 2024
15 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.7 milestone May 22, 2024
@vladikkuzn vladikkuzn deleted the flaky-pod-replacement branch May 22, 2024 19:19
@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request May 22, 2024
Merged
@k8s-infra-cherrypick-robot
Copy link
Contributor

@alculquicondor: new pull request created: #2263

In response to this:

/cherry-pick release-0.6

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.

kannon92 pushed a commit to openshift-kannon92/kubernetes-sigs-kueue that referenced this pull request Nov 19, 2024
@vladikkuzn @kannon92
Pod controller: "when Using pod group Should keep the running pod gro…
bd57745 
…up with the queue name if workload is evicted" flakes (kubernetes-sigs#2252)

* Pod controller: "when Using pod group Should keep the running pod group with the queue name if workload is evicted" flakes

* Pod controller: "when Using pod group Should keep the running pod group with the queue name if workload is evicted" flakes

* Rename SetNodeName -> BindPodWithNode
* Reorder to match production env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trasc trasc trasc left review comments

@mimowo mimowo Awaiting requested review from mimowo

@tenzen-y tenzen-y Awaiting requested review from tenzen-y

@alculquicondor alculquicondor Awaiting requested review from alculquicondor

Assignees

@alculquicondor alculquicondor

@vladikkuzn vladikkuzn

@trasc trasc

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v0.7
5 participants
@vladikkuzn @k8s-ci-robot @trasc @alculquicondor @k8s-infra-cherrypick-robot