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 more paths to crun runtime detection #9086

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

rlex
Copy link
Contributor

@rlex rlex commented Dec 22, 2023

Proposed Changes

Add /usr/local/bin and /usr/local/sbin to crun detection. Crun can also be installed manually, without packages, and /usr/local is preferred path when dealing with non-packaged software.

Types of Changes

New feature

Verification

Drop crun binary to /usr/local/bin or /usr/local/sbin, k3s should detect it and add to containerd config

Testing

This is just new path to detection, so i hasn't touched tests

Linked Issues

#9112

User-Facing Change

NONE

Further Comments

@rlex rlex requested a review from a team as a code owner December 22, 2023 02:42
rlex added 2 commits December 22, 2023 05:42
Signed-off-by: Lex Rivera <[email protected]>
@rlex rlex force-pushed the crun_additional_path branch from bc330c3 to 0ccabab Compare December 22, 2023 02:43
@brandond
Copy link
Member

We're picking up a lot of different hardcoded paths here, I'm kinda starting to feel like we should just search $PATH for the bins with os.LookPath and be done with it.

@rlex
Copy link
Contributor Author

rlex commented Dec 22, 2023

I checked crun packages and it seems like it can be trimmed down to just /usr/bin and /usr/local/bin:

https://rpmfind.net/linux/RPM/fedora/updates/39/aarch64/Packages/c/crun-1.12-1.fc39.aarch64.html
https://ubuntu.pkgs.org/23.10/ubuntu-universe-amd64/crun_1.8.5-1_amd64.deb.html
https://centos.pkgs.org/9-stream/centos-appstream-aarch64/crun-1.11.2-1.el9.aarch64.rpm.html

As crun doesn't require any special permissions to just run, there is no need to put that to sbin

@brandond
Copy link
Member

brandond commented Jan 2, 2024

This looks good for now but I think I may rework the entire runtime detection code to reduce duplication at some point.

Copy link

codecov bot commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3190a5f) 46.35% compared to head (0ccabab) 41.65%.
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9086      +/-   ##
==========================================
- Coverage   46.35%   41.65%   -4.71%     
==========================================
  Files         148      148              
  Lines       15900    15902       +2     
==========================================
- Hits         7371     6624     -747     
- Misses       7354     8149     +795     
+ Partials     1175     1129      -46     
Flag Coverage Δ
e2etests ?
inttests 38.86% <100.00%> (-0.04%) ⬇️
unittests 18.37% <100.00%> (+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.

@brandond brandond merged commit 5fe074b into k3s-io:master Jan 5, 2024
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jan 10, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jan 12, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jan 12, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jan 12, 2024
* add usr/local paths for crun detection

Signed-off-by: Lex Rivera <[email protected]>
(cherry picked from commit 5fe074b)
Signed-off-by: Brad Davidson <[email protected]>
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.

3 participants