Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

fix(e2e): cleanup helm releases in e2e #1886

Merged
merged 2 commits into from
Oct 23, 2020
Merged

fix(e2e): cleanup helm releases in e2e #1886

merged 2 commits into from
Oct 23, 2020

Conversation

nojnhuh
Copy link
Contributor

@nojnhuh nojnhuh commented Oct 21, 2020

Description:
This change will uninstall all Helm releases in the namespaces being
cleaned up before deleting the namespaces to avoid the namespaces
getting stuck in a Terminating state.

Fixes #1880

Affected area:

  • New Functionality [ ]
  • Documentation [ ]
  • Install [ ]
  • Control Plane [ ]
  • CLI Tool [ ]
  • Certificate Management [ ]
  • Networking [ ]
  • Metrics [ ]
  • SMI Policy [ ]
  • Security [ ]
  • Tests [X]
  • CI System [ ]
  • Performance [ ]
  • Other [ ]

Please answer the following questions with yes/no.

  • Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
    No

This change will uninstall all Helm releases in the namespaces being
cleaned up before deleting the namespaces to avoid the namespaces
getting stuck in a Terminating state.

Fixes #1880
@nojnhuh nojnhuh requested a review from a team as a code owner October 21, 2020 22:48
@@ -820,7 +820,26 @@ func (td *OsmTestData) Cleanup(ct CleanupType) {
}

for _, ns := range testNs.Items {
err := td.DeleteNs(ns.Name)
// Delete Helm releases created in the namespace
Copy link
Contributor

@eduser25 eduser25 Oct 22, 2020

Choose a reason for hiding this comment

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

Question. Would it be help if this was run defacto on DeleteNs code? Just in case someone decided to manually run a DeleteNs individually (as part of the test) on an NS that had helm resources.

I'm ok eitherway though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I'll make that change.

eduser25
eduser25 previously approved these changes Oct 22, 2020
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@2cd45c4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1886   +/-   ##
=======================================
  Coverage        ?   58.34%           
=======================================
  Files           ?      129           
  Lines           ?     5267           
  Branches        ?        0           
=======================================
  Hits            ?     3073           
  Misses          ?     2191           
  Partials        ?        3           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cd45c4...630f06c. Read the comment docs.

Copy link
Contributor

@draychev draychev left a comment

Choose a reason for hiding this comment

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

lgtm

@nojnhuh nojnhuh merged commit 202121c into openservicemesh:main Oct 23, 2020
@nojnhuh nojnhuh deleted the e2e-cleanup branch October 23, 2020 16:46
draychev pushed a commit to draychev/osm that referenced this pull request Oct 28, 2020
This change will uninstall all Helm releases in the namespaces being
cleaned up before deleting the namespaces to avoid the namespaces
getting stuck in a Terminating state.

Fixes openservicemesh#1880
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(e2e, CI) Namespace in termination state indefinitely
5 participants