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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update install script with option to skip reload #3248

Merged
merged 1 commit into from Sep 8, 2022

Conversation

dramich
Copy link
Contributor

@dramich dramich commented Aug 18, 2022

Proposed Changes

Add a new envvar INSTALL_RKE2_SKIP_RELOAD that skips the systemctl
daemon-reload call. This is useful when using the install script
to build images that contain rke2 but init has not run yet.

Types of Changes

Enhancement

Verification

Run the script with and without the new flag set. On a fresh box systemctl won't see the rke2 services until after the reload is called, so running the script with and without the flag should show the difference. Or run in a container before init is called and watch systemctl bomb 馃槃

Linked Issues

#3247

@brandond
Copy link
Contributor

Is the issue that systemd isn't installed yet when building the image? In the K3s installer we wrap things in a check to see if systemctl is available, perhaps we should do that here as well?

https://github.com/k3s-io/k3s/blob/master/install.sh#L694-L698

@dramich
Copy link
Contributor Author

dramich commented Aug 23, 2022

systemctl does exist at this point but systemd is not running since init has not run yet, so the check passes but still fails in the same way as before.

Copy link
Contributor

@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.

one nit; makes sense otherwise

install.sh Outdated Show resolved Hide resolved
Add a new envvar INSTALL_RKE2_SKIP_RELOAD that skips the systemctl
daemon-reload call. This is useful when using the install script
to build images that contain rke2 but init has not run yet.
@dramich
Copy link
Contributor Author

dramich commented Sep 6, 2022

@brandond Can you run the workflows?

@codecov-commenter
Copy link

Codecov Report

Merging #3248 (51ce86b) into master (f8f9e45) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3248      +/-   ##
==========================================
- Coverage   18.96%   18.88%   -0.08%     
==========================================
  Files          25       25              
  Lines        2289     2287       -2     
==========================================
- Hits          434      432       -2     
  Misses       1817     1817              
  Partials       38       38              
Flag Coverage 螖
unittests 18.88% <酶> (-0.08%) 猬囷笍

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

Impacted Files Coverage 螖
pkg/rke2/rke2_linux.go 84.08% <0.00%> (-0.15%) 猬囷笍
pkg/cli/defaults/defaults.go 93.54% <0.00%> (+0.21%) 猬嗭笍

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@galal-hussein galal-hussein merged commit 83fc168 into rancher:master Sep 8, 2022
@mdrahman-suse
Copy link
Contributor

mdrahman-suse commented Sep 13, 2022

@dramich can I get some assistance on how to test this? I tried installing rke2 on fresh node by running curl -sfL https://get.rke2.io | sudo INSTALL_RKE2_VERSION=v1.25.0-rc4+rke2r1 INSTALL_RKE2_SKIP_RELOAD=true sh - and when I try to check the status of the service using $ systemctl status rke2-server, I do see the service getting loaded... same happens if I dont use the var INSTALL_RKE2_SKIP_RELOAD=true

rke2-server.service - Rancher Kubernetes Engine v2 (server)
     Loaded: loaded (/usr/local/lib/systemd/system/rke2-server.service; disabled; vendor preset: enabled)
     Active: inactive (dead)
       Docs: https://github.com/rancher/rke2#readme

CC: @brandond

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.

None yet

5 participants