-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
add killall as a subcommand #8670
base: master
Are you sure you want to change the base?
Conversation
We need to figure out what's going on with this before updating the PR again; somehow it managed to fork-bomb on the s390x runner and take it completely out of service. |
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.
We should also clean up unnecessary/inconsistent whitespace in the script.
If we're going to bundle this here, do we want to remove the version that's embedded in the install script? I guess that might negatively affect folks that are still installing older releases. I just hate maintaining parallel copies. At the very least, it feels confusing for new releases that include this as a subcommand to include two different copies that may drift. Is there some way we can skip creating the standalone killall script when installing newer k3s releases? |
I have an idea for that, but it will need to wait for a future release. I can't test a PR commit (because #9185 is not merged), so I can't modify the |
But yeah the idea is that the
Basically we need to have some comments in the install.sh FUTURE changes to the killall script should not be placed in |
Signed-off-by: Ian Cardoso <[email protected]>
Signed-off-by: Ian Cardoso <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8670 +/- ##
==========================================
+ Coverage 43.14% 43.16% +0.01%
==========================================
Files 151 155 +4
Lines 13342 13389 +47
==========================================
+ Hits 5757 5779 +22
+ Misses 6498 6447 -51
- Partials 1087 1163 +76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
What is the status of this? The kill all script is no where to be found in this repository and it's not clear where I should get it. I can't have k3s leaving tons of container shims and stuff laying around on my machine. |
It's built into the install script. Run the install script to install K3s and you will also get the killall and uninstall scripts. |
I figured that out about an hour after posting that comment. I ended up just copying it out. Unfortunately with In the mean time I solved my issues with that script. |
We don't support the Arch package. If they're going to build and package it their own way, they should also include the utility scripts. |
Yes, and that is apparent since it is in the AUR not the official repo. It's just sad that the killall script that is referenced in the help docs is not easy to find. |
Proposed Changes
#4181
Types of Changes
Verification
Testing
Linked Issues
User-Facing Change
Further Comments