container: move volume chown after spec generation

[giuseppe]

move the chown for newly created volumes after the spec generation so the correct UID/GID are known.

Closes: #5698

Alternative to: github.com//pull/6730

Signed-off-by: Giuseppe Scrivano [email protected]

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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:

• OWNERS [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

If a second container uses the same volume, does it get chowned?

[TomSweeneyRedHat]

If a second container uses the same volume, does it get chowned?

might be good to add a test for this.

[vrothberg]

make validate and repushing should make gating happy

[giuseppe]

If a second container uses the same volume, does it get chowned?

no, it is chowned only the first time. And it should be race free (a second container that accesses the volume between its creation and being chowned doesn't affect it)

[vrothberg]

Code LGTM, thanks a lot @giuseppe!

@mheon PTAL

[mheon]

I'm still convinced this makes more sense as part of the volume mount operation - keeping the separate bool makes sense, but we should put it in the same place that copy-up currently happens.

[giuseppe]

I'm still convinced this makes more sense as part of the volume mount operation - keeping the separate bool makes sense, but we should put it in the same place that copy-up currently happens.

the mount currently happens before the spec is generated (the UID/GID are not known)

[mheon]

That shouldn't be true - mount + copy up of volumes only happens as part of prepare() which is called as part of Start() (or Init()) once the container is created

[giuseppe]

That shouldn't be true - mount + copy up of volumes only happens as part of prepare() which is called as part of Start() (or Init()) once the container is created

yes volumes are mounted with mountStorage as part of prepare() while generateSpec is called later as part of init()

[mheon]

Ah, I think I see now - you want to do the chown later, so we don't need a duplicate lookup up UID/GID. OK.

[giuseppe]

Ah, I think I see now - you want to do the chown later, so we don't need a duplicate lookup up UID/GID. OK.

does it look good for you?

[mheon]

Sorry for the delay. Few issues with the volume functions, otherwise LGTM.

[disaster123]

Does this one also affects v1.9 ? I'm always getting "permission denied" messages for containers running in rootless mode using volumes.

[mheon]

Yes, this affects every Podman version, I believe.

[disaster123]

OK i'll try to backport your branch to v1.9 and check whether it fixes my problems.

[giuseppe]

/retest

[disaster123]

I backported this to v1.9 (which was trivial just two missing imports). But it still fails:

/home/podman/.local/share/containers/storage/volumes/maria_cs19-mariadb-data/_data is the source path for the volume which is owned by user podman shouldn't this be owned by the user running mysqld inside the container?

Output / log from Container:

2020-06-30 14:21:14 0 [Note] mysqld (mysqld 10.3.22-MariaDB-1:10.3.22+maria~bionic) starting as process 32 ...
2020-06-30 14:21:15 0 [ERROR] mysqld: Can't create/write to file '/var/lib/mysql/aria_log_control' (Errcode: 13 "Permission denied")
2020-06-30 14:21:15 0 [ERROR] mysqld: Got error 'Can't create file' when trying to use aria control file '/var/lib/mysql/aria_log_control'

The volume was created by podman-compose

[giuseppe]

tests are green again

[disaster123]

The output of podman-compose looks like this:

podman pod create --name=maria --share net
a5ccf3e089fb2cac498d807982b5e21c4b54f153c9ff4270d318c2f187a65f98
0
podman volume inspect maria_cs19-mariadb-data || podman volume create maria_cs19-mariadb-data
Error: no volume with name "maria_cs19-mariadb-data" found: no such volume
podman volume inspect maria_cs19-mariadb-logs || podman volume create maria_cs19-mariadb-logs
Error: no volume with name "maria_cs19-mariadb-logs" found: no such volume
podman run --name=cs20-mariadb -d --pod=maria --label io.podman.compose.config-hash=123 --label io.podman.compose.project=maria --label io.podman.compose.version=0.0.1 --label com.docker.compose.container-number=1 --label com.docker.compose.service=mariadb -e zabbixServer=172.24.0.7 -e enableZabbixMonitoring=yes -e innodb_read_io_threads=4 -e innodb_write_io_threads=8 -e innodb_buffer_pool_size=8192M --mount type=bind,source=/home/podman/.local/share/containers/storage/volumes/maria_cs19-mariadb-data/_data,destination=/var/lib/mysql,bind-propagation=z --mount type=bind,source=/home/podman/.local/share/containers/storage/volumes/maria_cs19-mariadb-logs/_data,destination=/var/log/mysql,bind-propagation=z --add-host mariadb:127.0.0.1 --add-host cs20-mariadb:127.0.0.1 --hostname cs20-mariadb --tty docker.contentserv.com/cs-saas-prod/mariadbprod:stable

[giuseppe]

I don't think the --mount type=bind is translated to a volume. That looks like a plain bind mount

[mheon]

It's a bind mount of a Libpod-managed named volume. That's honestly bizarre, and not a good idea.

Also, there's no --user directive, so I question whether this patch would help - it looks like mysql is started as root, then drops caps and becomes an unprivileged user

[rhatdan]

/lgtm