Improve rke2-uninstall.ps1 script #5779
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HarrisonWAffel
approved these changes
Apr 15, 2024
brandond
reviewed
Apr 15, 2024
brandond
reviewed
Apr 15, 2024
manuelbuil
force-pushed
the
rke2uninstallwindows
branch
from
4f9b86b to
6622d4a
Compare
brandond
reviewed
Apr 18, 2024
manuelbuil
force-pushed
the
rke2uninstallwindows
branch
from
6622d4a to
0075531
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5779 +/- ##
=======================================
Coverage 26.46% 26.46%
=======================================
Files 30 30
Lines 2649 2649
=======================================
Hits 701 701
Misses 1903 1903
Partials 45 45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
brandond
requested changes
Apr 19, 2024
manuelbuil
force-pushed
the
rke2uninstallwindows
branch
2 times, most recently
from
0e6961a to
6324be7
Compare
brandond
approved these changes
Apr 30, 2024
brandond
reviewed
Apr 30, 2024
bundle/bin/rke2-uninstall.ps1
Outdated
| New-Item -ItemType File -Path $lockFilePath -Force | ||
| } else { | ||
| # ctr should exist at this point but just in case we add this log | ||
| Write-Host "Command ctr not found. Aumatica uninstallation might fail" |
There was a problem hiding this comment.
nit on spelling/grammar:
Suggested change
| Write-Host "Command ctr not found. Aumatica uninstallation might fail" | |
| Write-Host "ctr.exe not found, container cleanup and RKE2 uninstallation is unlikely to succeed." |
There was a problem hiding this comment.
ugh! Thanks, I missed that
manuelbuil
force-pushed
the
rke2uninstallwindows
branch
from
6324be7 to
e04c655
Compare
HarrisonWAffel
requested changes
May 9, 2024
bundle/bin/rke2-uninstall.ps1
Outdated
| if ($command) { | ||
| $executablePath = $command.Source | ||
| $dataBinDirDirectory = Split-Path -Parent $executablePath | ||
| $lockFilePath = Join-Path -Path $directory -ChildPath "rke2-uninstall.lock" |
There was a problem hiding this comment.
Looking back over this with the new changes, I don't see $directory being set anywhere, looks like it should be $dataBinDirDirectory
There was a problem hiding this comment.
good catch! Let me retest this carefully again
Signed-off-by: Manuel Buil <[email protected]>
manuelbuil
force-pushed
the
rke2uninstallwindows
branch
from
e04c655 to
d58842e
Compare
|
restested and works :) |
HarrisonWAffel
approved these changes
May 14, 2024
This was referenced May 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes
This PR tries to fix the current implementation of the rke2-uninstall.ps1 script. Often this script fails to remove everything from the node because by the time it tries to remove the containerd namespace, there are still resources in that namespace. As a consequence, we end up with files in /var/lib/rancher/agent/ which are impossible to remove because according to Windows, they are locked. To remove them, a restart normally works but not always.
This PR does two things:
1 - The rke2-uninstall.ps1 would create a lock file in the dataDir. Then it will kill kubelet. In the pebinary code, we won't restart kubelet if that lock file exists
2 - It retries removing the namespace for a few times, in case the previous trial failed. There might be situations where the script can't succeed (e.g. user forgot to drain). For those cases, it gives up after 30 seconds and continues deleting other stuff, as it does today
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
rancher/rancher#45016
User-Facing Change
Further Comments