host-ctr: add '/etc/bottlerocket-release' to host containers #1883
Conversation
| // Bottlerocket release information for the container | ||
| { | ||
| Options: []string{"rbind", "r"}, | ||
| Destination: fmt.Sprintf("/etc/bottlerocket-release"), | ||
| Source: fmt.Sprintf("/etc/os-release"), | ||
| }, |
There was a problem hiding this comment.
Is this required if it can be accessed from /.bottlerocket/rootfs/etc/release? Is this mount just for better developer ergonomics?
I couldn't find the r option here, I think the options should be:
| // Bottlerocket release information for the container | |
| { | |
| Options: []string{"rbind", "r"}, | |
| Destination: fmt.Sprintf("/etc/bottlerocket-release"), | |
| Source: fmt.Sprintf("/etc/os-release"), | |
| }, | |
| // Bottlerocket release information for the container | |
| { | |
| Destination: fmt.Sprintf("/etc/bottlerocket-release"), | |
| Source: fmt.Sprintf("/etc/os-release"), | |
| Options: []string{"bind", "ro"} | |
| }, |
bind: you don't need rbind, since nothing underneath /etc/os-release is mounted
ro: set the mount as read-only
There was a problem hiding this comment.
Agree with @arnaldo2792 - unless we have a need/ask for better ergonomics around this file we probably don't need to add this since it's already available! :)
There was a problem hiding this comment.
We only mount up /.bottlerocket/rootfs/ on superpowered containers, but this file would be potentially useful on less privileged containers as well. Mounting to /etc/bottlerocket-release is also a bit more user friendly.
I was a little silly with the mounting options though, so I will definitely make that change!
This grants host containers context to the underlying OS by mounting Bottlerocket's '/etc/os-release' as '/etc/bottlerocket-release'.
jpculp
force-pushed
the
br-release-in-host-container
branch
from
cc270dd
to
47f22aa
Compare
|
Fixed mount options. |
| // Bottlerocket release information for the container | ||
| { | ||
| Options: []string{"bind", "ro"}, | ||
| Destination: fmt.Sprintf("/etc/bottlerocket-release"), |
There was a problem hiding this comment.
nit: I do like the simplicity of just having /etc instead of /.bottlerocket/bottlerocket-release, but I still think we should have the prefix just to be consistent with the container's persistent storage mount: /.bottlerocket/<host-containers>/<bootstrap-containers>/
There was a problem hiding this comment.
I went back and forth on this as well. As a general rule, we try to be hands-off with the container's root filesystem, but we've bent that rule to put things in the "expected" places - apiclient in /usr/local/bin, and the API socket in /run.
Having the release file in /etc seems like a reasonable exception by that standard. And if we're touching /etc in any case, making it a symlink to /.bottlerocket/<blah>/etc just seems overly complicated.
Description of changes:
This grants host containers context to the underlying OS by mounting
Bottlerocket's
/etc/os-releaseas/etc/bottlerocket-release.Testing done:
aws-k8s-1.21instance.cat /etc/bottlerocket-releasein both the control and admin host containers.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.