Fix helm issue during e2e test run operator installation#914
Fix helm issue during e2e test run operator installation#914istio-testing merged 4 commits intoistio-ecosystem:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #914 +/- ##
=======================================
Coverage 76.93% 76.93%
=======================================
Files 44 44
Lines 2679 2679
=======================================
Hits 2061 2061
Misses 524 524
Partials 94 94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
76b736f to
e28405e
Compare
| # We create the operator namespace before the tests run | ||
| # To avoid any cleanup issues, after we build annd push the image we check if the namespace exists and delete it if it does. | ||
| # The test logic already handle the namespace creation and deletion during the test run. | ||
| # TODO: remove hardcoded kubectl command and use the COMMAND variable instead. To be changed during OCP enable doc test. |
There was a problem hiding this comment.
This is going to be pending until we make some changes to the run-docs-examples.sh script to enable the possibility to run the docs test over OCP clusters. To not mixing changes, meanwhile, I hardcoded kubectl.
mkolesnik
left a comment
There was a problem hiding this comment.
Couple of typos.
Also I'm not sure I understand why this needs to be specifically in the build-and-push-operator script and not, for example, the scripts that actually run the tests?
e.g. integ-suite-ocp.sh
I'll move it as a validation step inside to the script that actually run the test |
Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com>
Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com>
60b1b4e to
20a08fb
Compare
Signed-off-by: Francisco H <frherrer@redhat.com>
Hey, I did some changes:
|
|
|
||
| # Workaround for OCP helm operator installation issues: | ||
| # To avoid any cleanup issues, after we build and push the image we check if the namespace exists and delete it if it does. | ||
| # The test logic already handles the namespace creation and deletion during the test run. |
There was a problem hiding this comment.
If it's done by tests, why it's needed also here?
There was a problem hiding this comment.
A few weeks ago, we introduced a new cleanup method to ensure that all resources are cleaned up after each test run. This involves recording all resources before and after the test. because at the start of the first test, the sail operator namespace already exists from the setup, it's not being deleted at the end of the test, and this causes an error because the helm release still exists in the cluster and gives us a helm can't use any in-use name
| URL=$(${COMMAND} get route default-route -n openshift-image-registry --template='{{ .spec.host }}') | ||
| export HUB="${URL}/${NAMESPACE}" | ||
|
|
||
| # Create the istio-images namespace to store the operator image and avoid errors during test run. |
There was a problem hiding this comment.
Which errors is this avoiding?
There was a problem hiding this comment.
I'll rephrase this comment
Signed-off-by: Francisco H <frherrer@redhat.com>
|
/test e2e-kind-multicluster |
* upstream/main: (34 commits) Update VERSIONS_YAML_FILE e2e test information (istio-ecosystem#916) Improve automated docs test run (istio-ecosystem#894) Use `Eventually` in remote secret generation (istio-ecosystem#921) Docs followup link fix (istio-ecosystem#919) Fix helm issue during e2e test run operator installation (istio-ecosystem#914) e2e allow to build and push setting target arch (istio-ecosystem#913) Fix docs for "broken_links" workflow (istio-ecosystem#918) Revert "Disable imageDigest support in ztunnel controller (openshift-service-mesh#582)" (istio-ecosystem#905) Fix typo in the ReportAfterSuite text (istio-ecosystem#911) Fix cleanup in multicluster tests (istio-ecosystem#909) Adding Eventually to Helm operator install (istio-ecosystem#907) Skip cleanup if control plane tests fail (istio-ecosystem#910) Use debug logging in integration tests (istio-ecosystem#906) Annotate only our clusters when using kind (istio-ecosystem#904) Fix log message for cleaner when waiting (istio-ecosystem#903) Consolidate multicluster operator deploy/cleanup (istio-ecosystem#902) 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) ...
|
In response to a cherrypick label: #914 failed to apply on top of branch "release-1.26": |
|
In response to a cherrypick label: new issue created for failed cherrypick: #935 |
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com>
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com>
* Consolidate multicluster operator deploy/cleanup (#902) Now that we have `Cleaner` clean up after each test, it makes sense to run this logic once in the suite, rather than after each test (which cleans up its own resources). Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Fix typo in the ReportAfterSuite text (#911) Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> * Fix helm issue during e2e test run operator installation (#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> * Consolidate e2e resource creation (#931) * Consolidate IstioCNI creation in e2e tests This code is very repetitive and would better live in a common function. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Consolidate Istio creation in e2e tests This code is very repetitive and would better live in a common function. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Use `Eventually` in remote secret generation (#921) In this test the secret generation sometimes is attempted before everything is ready, leading to flaky, hard to debug, failures. Using `Eventually` to generate the secret avoids these failures. Fixes: #866 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>
* Stabilize e2e dual-stack test (istio-ecosystem#936) (istio-ecosystem#939) Signed-off-by: Mikhail Abramov <mabramov@redhat.com> * fix IstioRevisionTag uninstall when revision namespace changes (istio-ecosystem#945) Fixes istio-ecosystem#917. This is quite a complex edge case, I'll do my best to explain. If you look at the integration test I wrote (part of this commit), you can follow along. When a tag was created that referenced a revision in ns1, then patched to reference a revision in ns2, we failed to clean up the old helm install in ns1. That almost never causes problems - except in the case where afterwards, you remove all tags, and then create a new one with the same name that is never reconciled. If that tag is deleted, the operator will attempt to uninstall that lingering helm release, which fails, leading the operator to deadlock, and stopping the user from deleting the tag. The only way to get the operator back into a working state would be to manually delete that lingering helm release. That is a pretty significant bug in my opinion. Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Daniel Grimm <dgrimm@redhat.com> * Expose "Profile" parameter in UI for Istio and IstioCNI (istio-ecosystem#924) (istio-ecosystem#947) During deployment of Istio in ambient mode, the user should be able to set the "profile: ambient" for Istio and IstioCNI resources. Currently, this parameter is hidden from the UI. Turn on the visibility of this parameter in the UI. Signed-off-by: Maxim Babushkin <mbabushk@redhat.com> * [Manual Cherry pick] Pick e2e improve and solving helm issues (istio-ecosystem#948) * Consolidate multicluster operator deploy/cleanup (istio-ecosystem#902) Now that we have `Cleaner` clean up after each test, it makes sense to run this logic once in the suite, rather than after each test (which cleans up its own resources). Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Fix typo in the ReportAfterSuite text (istio-ecosystem#911) Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> * Fix helm issue during e2e test run operator installation (istio-ecosystem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> * Consolidate e2e resource creation (istio-ecosystem#931) * Consolidate IstioCNI creation in e2e tests This code is very repetitive and would better live in a common function. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Consolidate Istio creation in e2e tests This code is very repetitive and would better live in a common function. Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> * Use `Eventually` in remote secret generation (istio-ecosystem#921) In this test the secret generation sometimes is attempted before everything is ready, leading to flaky, hard to debug, failures. Using `Eventually` to generate the secret avoids these failures. Fixes: istio-ecosystem#866 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com> * Bump to 1.26.2, add Istio 1.26.2 (istio-ecosystem#950) Signed-off-by: Daniel Grimm <dgrimm@redhat.com> --------- Signed-off-by: Mikhail Abramov <mabramov@redhat.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com> Signed-off-by: Maxim Babushkin <mbabushk@redhat.com> Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mikhail Abramov <mabramov@redhat.com> Co-authored-by: Istio Automation <istio-testing-bot@google.com> Co-authored-by: Daniel Grimm <dgrimm@redhat.com> Co-authored-by: Maxim Babushkin <mbabushk@redhat.com> Co-authored-by: Francisco Herrera <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> (cherry picked from commit 5fc8365)
|
In response to a cherrypick label: #914 failed to apply on top of branch "release-1.26": |
|
In response to a cherrypick label: new issue created for failed cherrypick: #987 |
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- (cherry picked from commit 5fc8365) Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com>
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- (cherry picked from commit 5fc8365) Signed-off-by: Francisco Herrera <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com>
* Fix helm issue during e2e test run operator installation (#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- (cherry picked from commit 5fc8365) Signed-off-by: Francisco Herrera <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> * Deploy operator in the E2E scripts Given that we have the `Cleaner` and each test should be cleaning after itself, deploying the operator just once per test run will save some test run time. An added value is the tests are simplified and can focus on what they need to achieve, instead of bothering with the operator setup. Fixes: #666 (cherry picked from commit 4434ddf) Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --------- Signed-off-by: Francisco Herrera <frherrer@redhat.com> Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> Co-authored-by: Francisco Herrera <frherrer@redhat.com>
…stem#914) * Fix helm issue during e2e test run operator installation Signed-off-by: Francisco H <frherrer@redhat.com> Fix command error Signed-off-by: Francisco H <frherrer@redhat.com> * Apply suggestions from code review Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Francisco H <frherrer@redhat.com> * Improves from code review Signed-off-by: Francisco H <frherrer@redhat.com> * Update comment in build and push script Signed-off-by: Francisco H <frherrer@redhat.com> --------- Signed-off-by: Francisco H <frherrer@redhat.com> Co-authored-by: Mike Kolesnik <mkolesni@redhat.com> Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
What type of PR is this?
What this PR does / why we need it:
The operator namespace was being created during the image build/push only for OCP test runs, leading to a 'helm name error' in subsequent tests case. This happened because the cleaner record was incorrectly preventing the deletion of the sail-operator namespace.
Adding also the rollback of #907
Which issue(s) this PR fixes:
Fixes #
Related Issue/PR #
Additional information: