Skip to content
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

Improve rke2-uninstall.ps1 #6098

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

manuelbuil
Copy link
Contributor

@manuelbuil manuelbuil commented Jun 4, 2024

Proposed Changes

After a QA iteration, we realized that the previous PR fixing the rke2 uninstallation is still not capable of removing all containerd resources in all environments. Main detected problems:

1 - Time out is too small for certain environments
2 - If we try removing the containerd namespace when there are resources there is an error message which could be confusing for some users
3 - Rarely snapshots get stuck and they require to be removed explicitly

This PR does the following:
1 - Increases the timeout to 3 minutes
2 - To avoid the confusing error message, it does not try to remove the containerd namespace until all the snapshots are gone. Note that the typical resources in a containerd namespace are tasks, containers, images and snapshots. We are removing the first three and once those three are removed, snapshots should be empty (except when snapshots get stuck)
3 - If timeout is reached, we try to remove the snapshots explicitly. If the namespace still can't be removed, we warn the user that the uninstall script might leave some files behind in /var/lib/rancher/rke2
4 - It adds some extra logs and comments to clarify what's going on

Types of Changes

Bugfix

Verification

Install rke2 on linux and windows. Deploy a pod in windows. Run the rke2-uninstall.ps1 script. Without this PR, half of the time the script will fail and it will be impossible to remove the directory /var/lib/rancher.

With this PR, the script works and /var/lib/rancher does not exist anymore

Testing

Linked Issues

#5778

User-Facing Change


Further Comments

@manuelbuil manuelbuil requested a review from a team as a code owner June 4, 2024 10:21
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.09%. Comparing base (71d00cc) to head (04dff5a).
Report is 155 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6098      +/-   ##
==========================================
- Coverage   26.10%   26.09%   -0.01%     
==========================================
  Files          32       32              
  Lines        2697     2698       +1     
==========================================
  Hits          704      704              
- Misses       1947     1948       +1     
  Partials       46       46              
Flag Coverage Δ
inttests 9.82% <ø> (-0.01%) ⬇️
unittests 18.57% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@manuelbuil manuelbuil force-pushed the improverke2uninstallwin branch from 4a65869 to 5dbe937 Compare June 4, 2024 16:58
@manuelbuil manuelbuil force-pushed the improverke2uninstallwin branch from 5dbe937 to 90604ab Compare June 4, 2024 18:33
@manuelbuil manuelbuil requested a review from mdrahman-suse June 4, 2024 18:33
@manuelbuil manuelbuil force-pushed the improverke2uninstallwin branch from 90604ab to ebd7970 Compare June 5, 2024 09:06
Signed-off-by: Manuel Buil <mbuil@suse.com>
@manuelbuil manuelbuil force-pushed the improverke2uninstallwin branch from ebd7970 to 04dff5a Compare June 5, 2024 20:27
@manuelbuil manuelbuil merged commit 9eae919 into rancher:master Jun 6, 2024
6 checks passed
@manuelbuil manuelbuil deleted the improverke2uninstallwin branch June 6, 2024 12:51
manuelbuil added a commit to manuelbuil/rke2 that referenced this pull request Jun 6, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
manuelbuil added a commit to manuelbuil/rke2 that referenced this pull request Jun 6, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
manuelbuil added a commit to manuelbuil/rke2 that referenced this pull request Jun 6, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
brandond pushed a commit that referenced this pull request Jun 7, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
brandond pushed a commit that referenced this pull request Jun 7, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
brandond pushed a commit that referenced this pull request Jun 7, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
iamsarthakk pushed a commit to iamsarthakk/rke2 that referenced this pull request Aug 19, 2024
Signed-off-by: Manuel Buil <mbuil@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants