Share the additional container storage #766
Conversation
| skip_mount_home = "" | ||
| mount_program = "/usr/bin/fuse-overlayfs" | ||
| mountopt = "" | ||
| additionalimagestores = [ "/usr/lib/containers/storage",] |
There was a problem hiding this comment.
We will need to allow non-root users access (at lest read) to this path.
Probably need to sort out selinux as well.
There was a problem hiding this comment.
The read access is set in the Containerfile. We could limit to a specific group, too.
training/nvidia-bootc/Containerfile
Outdated
| @@ -191,6 +191,9 @@ RUN --mount=type=secret,id=${INSTRUCTLAB_IMAGE_PULL_SECRET}/.dockerconfigjson \ | |||
| else \ | |||
| IID=$(sudo podman --root /usr/lib/containers/storage pull ${INSTRUCTLAB_IMAGE}); \ | |||
| fi | |||
| chmod -R a+rx /usr/lib/containers/storage | |||
There was a problem hiding this comment.
This was our original approach but we abandoned it because this makes all the files even inside the container executable
There was a problem hiding this comment.
We could refine with find to put the x only on the folders. Would that be better?
There was a problem hiding this comment.
There was this suggestion but I never tried it, IDK
There was a problem hiding this comment.
@cgwalters's suggestion to use chmod a+rX is nice. It will avoid adding x permission to config files or example scripts.
fabiendupont
force-pushed
the
nvidia-bootc-shared-container-storage
branch
2 times, most recently
from
92e156a to
90216f7
Compare
fabiendupont
force-pushed
the
nvidia-bootc-shared-container-storage
branch
2 times, most recently
from
aa66e5d to
857b513
Compare
fabiendupont
requested review from
MichaelClifford,
rhatdan,
sallyom,
lmilbaum,
cgwalters and
Gregory-Pereira
as code owners
|
@giuseppe PTAL |
| @@ -190,7 +190,10 @@ RUN --mount=type=secret,id=${INSTRUCTLAB_IMAGE_PULL_SECRET}/.dockerconfigjson \ | |||
| IID=$(sudo podman --root /usr/lib/containers/storage pull --authfile /run/secrets/${INSTRUCTLAB_IMAGE_PULL_SECRET}/.dockerconfigjson ${INSTRUCTLAB_IMAGE}); \ | |||
| else \ | |||
| IID=$(sudo podman --root /usr/lib/containers/storage pull ${INSTRUCTLAB_IMAGE}); \ | |||
| fi | |||
| fi && \ | |||
| chmod -R a+rX /usr/lib/containers | |||
There was a problem hiding this comment.
are you also setting force_mask=shared in the storage.conf file?
If you do so, then podman will store the original permissions in an extended attribute, that it is used by fuse-overlays
There was a problem hiding this comment.
Currently, it is not set. Should it be in the global storage.conf or it could be in the user config?
There was a problem hiding this comment.
sorry for the delay.
It must be in the configuration file for the user that is creating the storage (i.e. runs podman pull). Also, when using --root, it tells Podman to ignore any other setting from the storage.conf, so in this case, it must be provided on the command line as a --storage-opt overlay.force_mask=shared
There was a problem hiding this comment.
Thanks @giuseppe. I updated the user storage.conf template and the Containerfile. Would you mind reviewing again, please?
There was a problem hiding this comment.
Note that bootc does not preserve these extended arguments, you will lose them
We need to share container image storage between rootless users, so that we don't need `sudo` and we don't duplicate the `instructlab` image. This change follows the Red Hat solution to [create additional image store for rootless users](https://access.redhat.com/solutions/6206192). The `/usr/lib/containers/storage` folder can be read by anyone and new users will inherit a default configuration via `/etc/skel` that configures the additional storage. The `ilab` wrapper is also modified to remove the impersonation code and not use `sudo` anymore. Signed-off-by: Fabien Dupont <[email protected]>
fabiendupont
force-pushed
the
nvidia-bootc-shared-container-storage
branch
from
857b513 to
fe98baf
Compare
|
@giuseppe PTANL |
| @@ -190,7 +190,10 @@ RUN --mount=type=secret,id=${INSTRUCTLAB_IMAGE_PULL_SECRET}/.dockerconfigjson \ | |||
| IID=$(sudo podman --root /usr/lib/containers/storage pull --authfile /run/secrets/${INSTRUCTLAB_IMAGE_PULL_SECRET}/.dockerconfigjson ${INSTRUCTLAB_IMAGE}); \ | |||
| else \ | |||
| IID=$(sudo podman --root /usr/lib/containers/storage pull ${INSTRUCTLAB_IMAGE}); \ | |||
| fi | |||
| fi && \ | |||
| chmod -R a+rX /usr/lib/containers | |||
|
@fabiendupont still something we want to merge? Will this work well with rootful containers as well, without using fuse-overlay? |
|
@rhatdan, yes, that's still something we want to merge. It removes the requirement to use |
|
LGTM |
We need to share container image storage between rootless users, so that we don't need `sudo` and we don't duplicate the `instructlab` image. This change follows the Red Hat solution to [create additional image store for rootless users](https://access.redhat.com/solutions/6206192). The `/usr/lib/containers/storage` folder can be read by anyone and new users will inherit a default configuration via `/etc/skel` that configures the additional storage. The `ilab` wrapper is also modified to remove the impersonation code and not use `sudo` anymore. Follow-up on containers#766 Signed-off-by: Fabien Dupont <[email protected]>
Follow-up containers#766 Signed-off-by: Fabien Dupont <[email protected]>
We need to share container image storage between rootless users, so that
we don't need
sudoand we don't duplicate theinstructlabimage.This change follows the Red Hat solution to
create additional image store for rootless users.
The
/usr/lib/containers/storagefolder can be read by anyone and newusers will inherit a default configuration via
/etc/skelthatconfigures the additional storage.
The
ilabwrapper is also modified to remove the impersonation code andnot use
sudoanymore.