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

Add data-dir to uninstall and killall scripts #10473

Merged

Conversation

jakefhyde
Copy link
Contributor

Proposed Changes

Add K3S_DATA_DIR as an optional environment variable to the k3s-killall and k3s-uninstall script generation within install.sh.

Types of Changes

Bugfix

Verification

Installing K3s with a custom data-dir (not /var/lib/rancher/k3s), and running the killall and uninstall scripts, verifying assets in /var/lib/rancher/k3s are properly removed.

Testing

This is not currently covered by testing, though I would be happy to write an integration test if need be.

Linked Issues

#10471

User-Facing Change

NONE

Further Comments

@jakefhyde jakefhyde requested a review from a team as a code owner July 9, 2024 20:59
@jakefhyde jakefhyde changed the title Add data-dir to uninstall and killall scripts Add data-dir to uninstall and killall scripts Jul 9, 2024
@jakefhyde jakefhyde force-pushed the 10741-add-data-dir-to-uninstall-and-killall branch from b615aba to e66e0e3 Compare July 9, 2024 21:00
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

don't forget to update the install script sha256sum file

@jakefhyde jakefhyde force-pushed the 10741-add-data-dir-to-uninstall-and-killall branch from e66e0e3 to 6e6fd3d Compare July 9, 2024 21:57
@jakefhyde jakefhyde requested a review from brandond July 9, 2024 21:57
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.53%. Comparing base (d1709d6) to head (0cdc30f).

❗ There is a different number of reports uploaded between BASE (d1709d6) and HEAD (0cdc30f). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d1709d6) HEAD (0cdc30f)
e2etests 7 6
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10473      +/-   ##
==========================================
- Coverage   47.87%   41.53%   -6.35%     
==========================================
  Files         177      177              
  Lines       14830    14830              
==========================================
- Hits         7100     6159     -941     
- Misses       6384     7492    +1108     
+ Partials     1346     1179     -167     
Flag Coverage Δ
e2etests 36.23% <ø> (-10.20%) ⬇️
inttests 37.02% <ø> (+0.01%) ⬆️
unittests 11.34% <ø> (ø)

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.

brandond
brandond previously approved these changes Jul 10, 2024
vitorsavian
vitorsavian previously approved these changes Jul 10, 2024
@brandond
Copy link
Member

I think we're good to go, unfortunately I just merged another change to the install script so you'll need to rebase to fix the checksum conflict :(

Signed-off-by: Jake Hyde <jakefhyde@gmail.com>
@jakefhyde jakefhyde dismissed stale reviews from vitorsavian and brandond via 0cdc30f July 10, 2024 17:09
@jakefhyde jakefhyde force-pushed the 10741-add-data-dir-to-uninstall-and-killall branch from 6e6fd3d to 0cdc30f Compare July 10, 2024 17:09
@brandond
Copy link
Member

No backports necessary, as install script is served off master

@brandond
Copy link
Member

centos7 is dead; merging

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.

4 participants