Skip succeeded by libesz · Pull Request #842 · istio-ecosystem/sail-operator

libesz · 2025-05-22T15:07:47Z

What type of PR is this?

What this PR does / why we need it:

It skips Succeeded pods (e.g. from Completed Jobs) when determining if a particular revision is used. Current code does not respect the actual state of the pod. Succeeded pods seems can safely ignored.

Which issue(s) this PR fixes:

Fixes #839

Related Issue/PR #

Additional information:

istio-testing · 2025-05-22T15:08:04Z

Hi @libesz. Thanks for your PR.

I'm waiting for a istio-ecosystem or istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com>

fjglira · 2025-05-22T15:10:44Z

/ok-to-test

istio-testing · 2025-05-22T15:19:09Z

@libesz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
docs-test_sail-operator_main	`6228a62`	link	false	`/test docs-test`

Details

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. I understand the commands that are listed here.

dgn

Thank you!

dgn

sorry, too quick to approve. This will also need to be patched in the IstioRevisionTag controller code. Sadly, we're still duplicating much of the logic there :/

libesz · 2025-05-22T15:33:03Z

sorry, too quick to approve. This will also need to be patched in the IstioRevisionTag controller code. Sadly, we're still duplicating much of the logic there :/

Didn't I add the same condition properly to the tag controller? https://github.com/istio-ecosystem/sail-operator/pull/842/files#diff-1815355208fa74b842698b65259042e322752e47f5dd6bea9cbb4f2256781403R371
I admit the test is not yet duplicated 😄

dgn · 2025-05-22T17:56:58Z

sorry, too quick to approve. This will also need to be patched in the IstioRevisionTag controller code. Sadly, we're still duplicating much of the logic there :/

Didn't I add the same condition properly to the tag controller? https://github.com/istio-ecosystem/sail-operator/pull/842/files#diff-1815355208fa74b842698b65259042e322752e47f5dd6bea9cbb4f2256781403R371 I admit the test is not yet duplicated 😄

ah- I saw only one test added, didn't look right. This works 🙂

* upstream/main: Skip succeeded (istio-ecosystem#842)

* skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Francisco H <frherrer@redhat.com>

* Add to vendors_default the possibility to add values for istiocni Signed-off-by: Francisco H <frherrer@redhat.com> * Fix lint Signed-off-by: Francisco H <frherrer@redhat.com> * Update test for vendors default Signed-off-by: Francisco H <frherrer@redhat.com> * Adding code coverage: new unit test over istiocni controller Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me Signed-off-by: Francisco H <frherrer@redhat.com> * Skip succeeded (#842) * skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Francisco H <frherrer@redhat.com> * enhancements: use issue ID's as SEP numbers (#849) Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Add unit test Signed-off-by: Francisco H <frherrer@redhat.com> * Update tests/e2e/README.md Co-authored-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me from review Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Gergo Huszty <gergo.huszty@digitaltrip.hu> Co-authored-by: Daniel Grimm <dgrimm@redhat.com>

* skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Francisco H <frherrer@redhat.com>

…tio-ecosystem#845) * Add to vendors_default the possibility to add values for istiocni Signed-off-by: Francisco H <frherrer@redhat.com> * Fix lint Signed-off-by: Francisco H <frherrer@redhat.com> * Update test for vendors default Signed-off-by: Francisco H <frherrer@redhat.com> * Adding code coverage: new unit test over istiocni controller Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me Signed-off-by: Francisco H <frherrer@redhat.com> * Skip succeeded (istio-ecosystem#842) * skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Francisco H <frherrer@redhat.com> * enhancements: use issue ID's as SEP numbers (istio-ecosystem#849) Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Add unit test Signed-off-by: Francisco H <frherrer@redhat.com> * Update tests/e2e/README.md Co-authored-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me from review Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Gergo Huszty <gergo.huszty@digitaltrip.hu> Co-authored-by: Daniel Grimm <dgrimm@redhat.com>

…) (#897) * Add to vendors_default the possibility to add values for istiocni * Fix lint * Update test for vendors default * Adding code coverage: new unit test over istiocni controller * Update read me * Skip succeeded (#842) * skip succeeded pods * ut added to verify succeeded pods are skipped --------- * enhancements: use issue ID's as SEP numbers (#849) * Add unit test * Update tests/e2e/README.md * Update read me from review --------- Signed-off-by: Francisco H <frherrer@redhat.com> Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Gergo Huszty <gergo.huszty@digitaltrip.hu> Co-authored-by: Daniel Grimm <dgrimm@redhat.com>

dgn · 2025-07-03T14:09:28Z

/cherry-pick release-1.26

istio-testing · 2025-07-03T14:10:10Z

@dgn: new pull request created: #974

Details

In response to this:

/cherry-pick release-1.26

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.

* skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com>

…tio-ecosystem#845) * Add to vendors_default the possibility to add values for istiocni Signed-off-by: Francisco H <frherrer@redhat.com> * Fix lint Signed-off-by: Francisco H <frherrer@redhat.com> * Update test for vendors default Signed-off-by: Francisco H <frherrer@redhat.com> * Adding code coverage: new unit test over istiocni controller Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me Signed-off-by: Francisco H <frherrer@redhat.com> * Skip succeeded (istio-ecosystem#842) * skip succeeded pods Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> * ut added to verify succeeded pods are skipped Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> --------- Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Francisco H <frherrer@redhat.com> * enhancements: use issue ID's as SEP numbers (istio-ecosystem#849) Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Add unit test Signed-off-by: Francisco H <frherrer@redhat.com> * Update tests/e2e/README.md Co-authored-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Update read me from review Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Gergo Huszty <gergo.huszty@digitaltrip.hu> Co-authored-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com>

libesz requested a review from a team as a code owner May 22, 2025 15:07

istio-testing added size/S needs-ok-to-test labels May 22, 2025

libesz added 2 commits May 22, 2025 17:08

skip succeeded pods

46f92b6

Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com>

ut added to verify succeeded pods are skipped

6228a62

Signed-off-by: Gergo Huszty <gergo.huszty@ibm.com>

libesz force-pushed the skip-succeeded branch from 732faf5 to 6228a62 Compare May 22, 2025 15:08

istio-testing added ok-to-test and removed needs-ok-to-test labels May 22, 2025

dgn approved these changes May 22, 2025

View reviewed changes

dgn requested changes May 22, 2025

View reviewed changes

dgn approved these changes May 22, 2025

View reviewed changes

istio-testing merged commit 3037eff into istio-ecosystem:main May 22, 2025
12 of 13 checks passed

openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request May 22, 2025

Automated merge

47f4d5e

* upstream/main: Skip succeeded (istio-ecosystem#842)

openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request May 22, 2025

Automated merge

4f3c6dd

* upstream/main: Skip succeeded (istio-ecosystem#842)

dgn added the cherrypick/release-1.26 label Jul 3, 2025

istio-testing mentioned this pull request Jul 3, 2025

[release-1.26] Skip succeeded #974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip succeeded#842

Skip succeeded#842
istio-testing merged 2 commits intoistio-ecosystem:mainfrom
libesz:skip-succeeded

libesz commented May 22, 2025

Uh oh!

istio-testing commented May 22, 2025

Uh oh!

fjglira commented May 22, 2025

Uh oh!

istio-testing commented May 22, 2025

Uh oh!

dgn left a comment

Uh oh!

dgn left a comment

Uh oh!

libesz commented May 22, 2025

Uh oh!

dgn commented May 22, 2025

Uh oh!

Uh oh!

dgn commented Jul 3, 2025

Uh oh!

istio-testing commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

libesz commented May 22, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Additional information:

Uh oh!

istio-testing commented May 22, 2025

Uh oh!

fjglira commented May 22, 2025

Uh oh!

istio-testing commented May 22, 2025

Uh oh!

dgn left a comment

Choose a reason for hiding this comment

Uh oh!

dgn left a comment

Choose a reason for hiding this comment

Uh oh!

libesz commented May 22, 2025

Uh oh!

dgn commented May 22, 2025

Uh oh!

Uh oh!

dgn commented Jul 3, 2025

Uh oh!

istio-testing commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants