-
Notifications
You must be signed in to change notification settings - Fork 96
🌈 Feature: Add support for Flatcar Container Linux #2209
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
🌈 Feature: Add support for Flatcar Container Linux #2209
Conversation
This change adds support for Flatcar to the blobfuse installer. Changes are minimal; we're re-using the rhcos installer script and the gardenlinux paths. As Flatcar currently does not ship libfuse, we install it alongside blobfuse2. Signed-off-by: Thilo Fromm <[email protected]>
The installer script wants to update the PATH variable to include $BIN_PATH in blobfuse-proxs's systemd unit by setting: Environment="PATH=<BIN_PATH>:$PATH" BIN_PATH is evaluated in the installer script, and $PATH is escaped, to be evaluated when the service unit runs. However, environment variables like "$PATH" are NOT evaluated by systemd in Environment="...". Quoting the documentation: " Variable expansion is not performed inside the strings and the "$" character has no special meaning. " https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#Environment This effectively removes all other paths and sets PATH to ONLY <BIN_PATH>. The resulting environment variable PATH contains the actual contents of BIN_PATH and a literal '$PATH'. This commit fixes that by wrapping ExecStart in bash -c '...' and updating PATH there.
Signed-off-by: Thilo Fromm <[email protected]>
|
Welcome @t-lo! |
|
Hi @t-lo. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
andyzhangx
left a comment
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.
/ok-to-test
andyzhangx
left a comment
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.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, t-lo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@t-lo could you also cherrypick this PR to release-1.33? thx |
|
@andyzhangx I'd be happy to help! Could you please elaborate on |
|
@andyzhangx I was browsing closed PRs and saw that everything gets backported to 1.26 and 1.27, so I did the same:
Is this what you're looking for? |
What type of PR is this?
/kind bug
/kind feature
What this PR does / why we need it:
This change adds support for Flatcar to the blobfuse installer. Changes are minimal; we're re-using the rhcos installer script and the gardenlinux paths.
Changes are minimal; we're re-using the rhcos installer script and the gardenlinux paths.
As Flatcar currently does not ship libfuse, we install it alongside blobfuse2, similar to what #2208 does for RHCOS.
Lastly, this change contains a fix for updating
PATHin theblobfuse-proxy.servicesystemd service unit. Previously, the wayPATHwas modified led to the variable contents being overwritten instead of added to. See 0f30780 for details.Special notes for your reviewer:
This change was verified by a test cluster deployment by @invidian . Pre-built container images for testing are available from https://github.com/flatcar-hub/blob-csi-driver/pkgs/container/blob-csi .
Release note: