-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
🌱 chore: cover uninstall in e2e tests #474
🌱 chore: cover uninstall in e2e tests #474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good but I have same concerns as @salasberryfin about merging it before solving uninstall issue upstream in CAPI Operator.
+1 |
Thanks for the reviews. Since we agree on waiting for the issue to be solved before merging this, I moved the original ticket to blocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM now, can you please rebase @salasberryfin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also LGTM from my side. After the rebase lets merge.
I’ll take care of rebase as Carlos is on PTO. |
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
0387e89
72a684d
to
0387e89
Compare
What this PR does / why we need it:
After running the
import-gitops
test suite, we verify thathelm uninstall
works on Turtles. This also upgrades Helm tov3.14.0
, asv3.8.1
was failing to detect the--cascade
flag.Which issue(s) this PR fixes:
Fixes #410
Special notes for your reviewer:
The uninstall of Turtles may fail unexpectedly due to a problem with the CAPI Operator Helm chart and CRDs being deleted in such a way that doesn't guarantee the order and may end up in a deadlock caused by a finalizer that is never removed.
Note: perhaps we should wait to have this sorted out before merging this change and having the uninstall step as part of the E2E tests.
Checklist: