update default build args, make pull use them

[lastephey]

Addresses

https://github.com/NERSC/podman-hpc/issues/81

https://github.com/NERSC/podman-hpc/issues/80

https://github.com/NERSC/podman-hpc/issues/76

Adjusts the base set of default args. Forces pull to use the default args (it wasn't before, and was ignoring log-level) and append extra args as needed. More in line with how shared_run was already handling args.

Have completed some preliminary testing on Muller- looks ok so far.

For example, from build:

DEBU[0037] Called build.PersistentPostRunE(/usr/bin/podman build --root /tmp/75313_hpc/storage --runroot /tmp/75313_hpc --storage-opt mount_program=/usr/bin/fuse-overlayfs-wrap --storage-opt ignore_chown_errors=true --cgroup-manager cgroupfs --hooks-dir /usr/share/containers/oci/hooks.d --env PODMANHPC_MODULES_DIR=/etc/podman_hpc/modules.d --annotation podman_hpc.hook_tool=true --log-level debug -t test:test .)

From pull:

DEBU[0001] Called pull.PersistentPostRunE(/usr/bin/podman pull --root /tmp/75313_hpc/storage --runroot /tmp/75313_hpc --storage-opt mount_program=/usr/bin/fuse-overlayfs-wrap --storage-opt ignore_chown_errors=true --cgroup-manager cgroupfs --log-level debug ubuntu:latest) INFO: Migrating image to /mscratch/sd/s/stephey/storage

From run:

time="2023-07-28T15:02:20-07:00" level=debug msg="Called run.PersistentPostRunE(/usr/bin/podman run --root /tmp/75313_hpc/storage --runroot /tmp/75313_hpc --storage-opt mount_program=/usr/bin/fuse-overlayfs-wrap --storage-opt ignore_chown_errors=true --cgroup-manager cgroupfs --storage-opt additionalimagestore=/mscratch/sd/s/stephey/storage --hooks-dir /usr/share/containers/oci/hooks.d --env PODMANHPC_MODULES_DIR=/etc/podman_hpc/modules.d --annotation podman_hpc.hook_tool=true --security-opt seccomp=unconfined --log-level debug --rm registry.nersc.gov/library/nersc/mpi4py:3.1.3 date)"

[danfulton]

This looks like it removes ignore_chown_errors which is the opposite of what #81 says? But after reading more about ignore_chown_errors I'm not sure we should always use it, rather than just include it in the documentation.

Wouldn't squashing the file ownership during the build/pull result in losing the separation in the image if you push it up and want to use it somewhere else? This particularly might be an issue if users are building images here and pushing them to use elsewhere. It would be better for users to specify a more appropriate id mapping at build time.

[lastephey]

It does add ignore_chown_errors- you can see it in the output commands I pasted above from testing with this code on muller. Do you see it there?

Since we've seen errors without ignore_chown_errors, my feeling is that we should turn it on. We can leave it to more expert users to override that setting if they need to (and should provide some corresponding directions).

[danfulton]

What I'm afraid of is how the man page describes the effect of ignore_chown_errors here https://www.redhat.com/sysadmin/controlling-access-rootless-podman-users

ignore_chown_errors can be set to allow a non privileged user running with a single UID within a user namespace to run containers. The user can pull and use any image, even those with multiple uids. Note multiple UIDs will be squashed down to the default uid in the container. These images will have no separation between the users in the container. (default: false)

So when the image gets pulled, it no longer has ownership separation between files in the image. Dan also says (emphasis mine):

HPC does not want users to have more than one UID, so this allows their users to run standard OCI images but not have to loosen their security settings at all.

... which to me suggests this makes sense for cases where users are not receiving a range of subordinate ids in /etc/subuid.

If I understand the the errors correctly, the cases where we still need it is if users want to pull or build a container with an id above 65536. In those cases, if the user is doing a pull to run at NERSC, I guess squashing ids is okay in most cases. We might run into issues if we start allowing more sophisticated group mappings, but I think a it would be reasonable to enable it all the time for pull. In cases where the user is doing a pull to get a base image for a build, or building an image that might be used elsewhere, then I think this might be very undesirable behaviour for most users.

Or am I misunderstanding the permanence of the id squashing effect of ignore_chown_errors?

[lastephey]

As far as I understand the squashing to a single user from using ignore_chown_errors is permanent.

I think it would be better to leave it on as default for our novice and intermediate users, and leave a note in our docs advising expert users how to turn it off if desired via specifying their own default_args. I think we'll need to do some work though to make that straightforward. Do you think that work makes sense in this PR or in another PR?

[lastephey]

After some thought I think it makes sense in this PR, so I'll work on it.

[lastephey]

Ok, what do you think about this:

stephey@nid001006:/mscratch/sd/s/stephey/containerfiles/test> PODMANHPC_USE_DEFAULT_ARGS=False podman-hpc --log-level=debug pull ubuntu:latest
INFO[0000] /usr/bin/podman filtering at log level debug 
DEBU[0000] Called pull.PersistentPreRunE(/usr/bin/podman pull --log-level debug ubuntu:latest) 

This gives users the ability to turn off the defaults and would require that they specify their own. I think this is reasonable for expert users.

This is pretty straightforward for pull since there aren't any additional pull args, but for build it gets a little more complex since the build args inherit from the default args.

To disable ignore_chown_errors and pull successfully, a user would have to do something like:

stephey@nid001006:/mscratch/sd/s/stephey/containerfiles/test> PODMANHPC_USE_DEFAULT_ARGS=False podman-hpc --log-level=debug pull --root /tmp/75313_hpc/storage --runroot /tmp/75313_hpc --storage-opt mount_program=/usr/bin/fuse-overlayfs-wrap --cgroup-manager cgroupfs ubuntu:latest

The resulting config looks like:

DEBU[0001] Called pull.PersistentPostRunE(/usr/bin/podman pull --root /tmp/75313_hpc/storage --runroot /tmp/75313_hpc --storage-opt mount_program=/usr/bin/fuse-overlayfs-wrap --cgroup-manager cgroupfs --log-level debug ubuntu:latest)

Note this will also require some changes in the podman_hpc.yaml config file structure (both here in the repo and in the NERSC ansible config) and changes to the README.

@danfulton and @scanon what do you think of this proposed structure?

[danfulton]

Yeah, agree it makes sense to include here. I think I'm confusing myself about the diff though. Maybe we can discuss at containers meeting today.

[lastephey]

Ok, this makes the changes that @danfulton and I discussed yesterday:

1) We only turn on ignore_chown_errors for pull and run. It is off by default for build to enable users to build multiuser containers for other systems. It's not clear if we need it in run, but we thought we'd leave it there for now to be safe. 2) We use only one use_default_args setting to enable all defaults to be turned on or off. 3) An expert user can set PODMANHPC_USE_DEFAULT_ARGS to turn off all our defaults and specify their own commands.

What do you think?

[danfulton]

Looks good to me! Thanks for the fixes.