Add KEEP_ON_FAILURE flag to e2e kind tests by mkolesnik · Pull Request #832 · istio-ecosystem/sail-operator

mkolesnik · 2025-05-20T14:25:22Z

Using this flag allows debugging the failed test on the environment, instead of destroying it.
The default behavior is still to clean up the cluster when failing.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #675

Related Issue/PR #

Additional information:

codecov · 2025-05-20T14:29:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.51%. Comparing base (ac14e1d) to head (feb3eb8).
Report is 21 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
+ Coverage   76.36%   76.51%   +0.15%     
==========================================
  Files          44       44              
  Lines        2636     2636              
==========================================
+ Hits         2013     2017       +4     
+ Misses        535      532       -3     
+ Partials       88       87       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tests/e2e/integ-suite-kind.sh

MaxBab · 2025-05-22T10:56:10Z

Mike, keep attention also on the skipDeploy flag within the e2e framework.
It's used when the sail operator has been deployed externally.
So, in this case, all the control plane deployment skipped.

It means that there are two possible flows when you're applying the KEEP_ON_FAILURE flag.
When the test resources kept for debugging, they needs to be cleaned on the next execution.
So,

When the e2e framework deploys the operator, it's resources needs to be cleaned alongside with the test resources.
When the operator has been deployed externally, only test resources needs to be cleaned, but not the operator ones.

Using this flag allows debugging the failed test on the environment, instead of destroying it. The default behavior is still to clean up the cluster when failing. Fixes: istio-ecosystem#675 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com>

mkolesnik · 2025-05-22T14:39:11Z

/retest

mkolesnik · 2025-05-26T12:29:31Z

/retest

fjglira

Some small comments

tests/e2e/README.md

tests/e2e/integ-suite-kind.sh

fjglira · 2025-05-27T12:37:32Z

tests/e2e/ambient/ambient_test.go

+						if CurrentSpecReport().Failed() && keepOnFailure {
+							return
+						}
+


I like this overall, but it will be better to centralize the logic in one place. I mean, it will be harder to maintain if we need to make a change, and we have the same logic for each test case. Maybe centralizing everything in one place, for example, util.cleanup() or something similar, and adding the logic to skip cleanup, log the failures, and all the cleanup for the resources could be a better way to keep it more simple and avoid duplication of code in several tc. wdyt?

The improvement of the cleanup already has a different issue: #833. So, no need to be addressed on this PR. Centralizing the deletion of the resources, I think, will be good to avoid the kind of errors described in the issue

fjglira · 2025-05-29T08:21:21Z

/test e2e-kind-multicluster

* upstream/main: Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Update github action for broken links (istio-ecosystem#888) Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Enchance debug information when e2e fails: add istioctl proxy status (istio-ecosystem#891) Update github action for broken links (istio-ecosystem#888) Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* upstream/main: Improve e2e test cleanup by recording & cleaning up (istio-ecosystem#889) Enchance debug information when e2e fails: add istioctl proxy status (istio-ecosystem#891) Update github action for broken links (istio-ecosystem#888) Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

Using this flag allows debugging the failed test on the environment, instead of destroying it. The default behavior is still to clean up the cluster when failing. Fixes: istio-ecosystem#675 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com>

* upstream/main: Improve e2e test cleanup by recording & cleaning up (istio-ecosystem#889) Enchance debug information when e2e fails: add istioctl proxy status (istio-ecosystem#891) Update github action for broken links (istio-ecosystem#888) Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

* Add KEEP_ON_FAILURE flag to e2e kind tests (#832) Using this flag allows debugging the failed test on the environment, instead of destroying it. The default behavior is still to clean up the cluster when failing. Fixes: #675 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Simplify logic when using `KEEP_ON_FAILURE` (#858) When using `KEEP_ON_FAILURE=true` also set `SKIP_CLEANUP=true`. This keeps the cluster running after the tests, without a need to defined our own trap logic. The downside is that the cluster will keep running even if the tests passed, but seeing as using the `KEEP_ON_FAILURE` flag is excpected mainly when there are failures, it seems that it's resonable to do. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Speed up docs tests by running on a single cluster (#855) Refactor the tests to run all tags on a single cluster without redeploying it. After each tag the cluster is cleaned back to its original state. Dual stack tests are still being run in their own cluster, as it needs to be deployed in dual stack mode to support it. Fixes: #827 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Improve e2e test cleanup by recording & cleaning up (#889) * Consolidate namespace cleanup in a new package The new package tracks namespaces and cleans up any namespaces it doesn't know about. The new `Cleaner` type can record and clean up the resources, given it's called with `Record()` before the tests and `Cleanup()` after. This is an initial change and more resources will be added. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Add CRD/CR cleanup logic for tests Now the `Cleaner` can also record and clean up any rogue CRDs/CRs that were created during the tests. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Replace operator uninstall with Cleanup Now that Cleanup takes care of all resources that we care about, replace the operator uninstall with it. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Add docs how to use `Cleaner` Add docs to E2E tests about the `Cleaner` and how it should be used. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Add `make cluster` command (#705) Add the ability to create a cluster for testing by using `make cluster`. Multiple clusters can be created by utilizing the `MULTICLUSTER=true` option. This allows developers to easily create a cluster to run manual testing on. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com>

* upstream/main: Improve e2e test cleanup by recording & cleaning up (istio-ecosystem#889) Enchance debug information when e2e fails: add istioctl proxy status (istio-ecosystem#891) Update github action for broken links (istio-ecosystem#888) Fix "Check broken link" workflow (istio-ecosystem#887) Split Documentation: splitting docs, adding table of content and fixing links (istio-ecosystem#881) Do not block the workflow on errors when publishing to operatorhub.io (istio-ecosystem#877) chore(docs): cleans up IstioCNI section (istio-ecosystem#867) Force push to operator hub fork (istio-ecosystem#873) Make a yaml linter in the community-operators-prod happy (istio-ecosystem#869) Adding description for release workflow run (istio-ecosystem#868) Docs: adding istiod HA documentations (istio-ecosystem#865) Add to vendors_default the possibility to add values for IstioCNI (istio-ecosystem#845) Updates release and nightly-images GH workflows to work with new FBC used in community-operators-prod (istio-ecosystem#854) Simplify logic when using `KEEP_ON_FAILURE` (istio-ecosystem#858) Add KEEP_ON_FAILURE flag to e2e kind tests (istio-ecosystem#832) switch to ubi-micro (istio-ecosystem#856) Speed up docs tests by running on a single cluster (istio-ecosystem#855) Add misspell to do basic spell validation in our docs (istio-ecosystem#853)

dgn · 2025-07-03T14:13:27Z

/cherry-pick release-1.26

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

@dgn: #832 failed to apply on top of branch "release-1.26":

Applying: Add KEEP_ON_FAILURE flag to e2e kind tests
Using index info to reconstruct a base tree...
M	Makefile.core.mk
M	tests/e2e/README.md
M	tests/e2e/ambient/ambient_suite_test.go
M	tests/e2e/ambient/ambient_test.go
M	tests/e2e/controlplane/control_plane_suite_test.go
M	tests/e2e/controlplane/control_plane_test.go
M	tests/e2e/controlplane/control_plane_update_test.go
M	tests/e2e/dualstack/dualstack_suite_test.go
M	tests/e2e/dualstack/dualstack_test.go
M	tests/e2e/multicluster/multicluster_externalcontrolplane_test.go
M	tests/e2e/multicluster/multicluster_multiprimary_test.go
M	tests/e2e/multicluster/multicluster_primaryremote_test.go
M	tests/e2e/multicluster/multicluster_suite_test.go
M	tests/e2e/multicontrolplane/multi_control_plane_suite_test.go
M	tests/e2e/multicontrolplane/multi_control_plane_test.go
M	tests/e2e/operator/operator_install_test.go
M	tests/e2e/operator/operator_suite_test.go
Falling back to patching base and 3-way merge...
Auto-merging tests/e2e/operator/operator_install_test.go
CONFLICT (content): Merge conflict in tests/e2e/operator/operator_install_test.go
Auto-merging tests/e2e/multicontrolplane/multi_control_plane_test.go
CONFLICT (content): Merge conflict in tests/e2e/multicontrolplane/multi_control_plane_test.go
Auto-merging tests/e2e/multicluster/multicluster_suite_test.go
Auto-merging tests/e2e/multicluster/multicluster_primaryremote_test.go
CONFLICT (content): Merge conflict in tests/e2e/multicluster/multicluster_primaryremote_test.go
Auto-merging tests/e2e/multicluster/multicluster_multiprimary_test.go
CONFLICT (content): Merge conflict in tests/e2e/multicluster/multicluster_multiprimary_test.go
Auto-merging tests/e2e/multicluster/multicluster_externalcontrolplane_test.go
CONFLICT (content): Merge conflict in tests/e2e/multicluster/multicluster_externalcontrolplane_test.go
Auto-merging tests/e2e/dualstack/dualstack_test.go
CONFLICT (content): Merge conflict in tests/e2e/dualstack/dualstack_test.go
Auto-merging tests/e2e/controlplane/control_plane_update_test.go
Auto-merging tests/e2e/controlplane/control_plane_test.go
CONFLICT (content): Merge conflict in tests/e2e/controlplane/control_plane_test.go
Auto-merging tests/e2e/controlplane/control_plane_suite_test.go
Auto-merging tests/e2e/ambient/ambient_test.go
CONFLICT (content): Merge conflict in tests/e2e/ambient/ambient_test.go
Auto-merging tests/e2e/README.md
Auto-merging Makefile.core.mk
CONFLICT (content): Merge conflict in Makefile.core.mk
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Add KEEP_ON_FAILURE flag to e2e kind tests

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.

istio-testing · 2025-07-03T14:14:04Z

@dgn: new issue created for failed cherrypick: #976

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.

mkolesnik requested a review from a team as a code owner May 20, 2025 14:25

mkolesnik added the enhancement New feature or request label May 20, 2025

istio-testing added the size/S label May 20, 2025

fjglira reviewed May 20, 2025

View reviewed changes

tests/e2e/integ-suite-kind.sh Show resolved Hide resolved

nareshrajkumar866 approved these changes May 22, 2025

View reviewed changes

Add KEEP_ON_FAILURE flag to e2e kind tests

feb3eb8

Using this flag allows debugging the failed test on the environment, instead of destroying it. The default behavior is still to clean up the cluster when failing. Fixes: istio-ecosystem#675 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com>

mkolesnik force-pushed the keep-on-fail branch from e8ef415 to feb3eb8 Compare May 22, 2025 13:50

istio-testing added size/L and removed size/S labels May 22, 2025

mkolesnik requested a review from fjglira May 26, 2025 12:51

fjglira reviewed May 27, 2025

View reviewed changes

fjglira approved these changes May 29, 2025

View reviewed changes

istio-testing merged commit dee0d23 into istio-ecosystem:main May 29, 2025
17 checks passed

mkolesnik mentioned this pull request May 29, 2025

Simplify logic when using KEEP_ON_FAILURE #858

Merged

6 tasks

fjglira added cherrypick/release-1.26 and removed cherrypick/release-1.26 labels Jun 11, 2025

istio-testing mentioned this pull request Jul 3, 2025

[release-1.26] Add KEEP_ON_FAILURE flag to e2e kind tests #976

Closed

Conversation

mkolesnik commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Additional information:

Uh oh!

codecov bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

MaxBab commented May 22, 2025

Uh oh!

mkolesnik commented May 22, 2025

Uh oh!

mkolesnik commented May 26, 2025

Uh oh!

fjglira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fjglira May 27, 2025

Choose a reason for hiding this comment

Uh oh!

fjglira May 27, 2025

Choose a reason for hiding this comment

Uh oh!

fjglira commented May 29, 2025

Uh oh!

Uh oh!

dgn commented Jul 3, 2025

Uh oh!

istio-testing commented Jul 3, 2025

Uh oh!

istio-testing commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mkolesnik commented May 20, 2025 •

edited

Loading

codecov bot commented May 20, 2025 •

edited

Loading