Skip to content

docker-images: explicitly create storage folders in Dockerfile…

Warren Gifford requested to merge volumes into master

Created by: ggilmore

Part of #7829 (closed)

See also https://github.com/sourcegraph/infrastructure/pull/1792

Many of our docker images write to external volumes for caches, data storage, etc. The file system permissions are identical from both the container's and host's POV.

For example, let's say you mount a folder (/data) into a container and on the host, only uid 11 has r/w access to it. If the container is running as some other unprivileged user (say uid 22) then the container process can't access the folder. This is the source of all the permission issues/container crashes in #7829 (closed).

There are multiple ways to mount volumes in Docker:

  • When using bind mounts, what we currently tell people to do by default, if the host folder that's being mounted has mismatched file permissions you're forced to run chmod commands as a workaround. This is hacky, brittle, and might not be possible to run in every environment.

  • When using 'named' volumes, the method that's recommended in the official documentation, the volume's initial contents and permissions are propagated from the container. However, this only applies if the path in the container that the volume is being attached to existed beforehand (otherwise, the volume will be attached with root permissions).

This PR ensures that all of the storage paths in our docker images are created ahead of time in their respective Dockerfiles with permissions that are accessible to our "sourcegraph" user.

There was one complication with https://github.com/sourcegraph/sourcegraph/tree/master/docker-images/prometheus. The upstream image that we inherit from explicitly declares a VOLUME in the Dockerfile with permissions that are only writeable via their default "nobody" user. It is not possible to change the permissions on this volume with a Dockerfile that inherits from this image. (Note that using explicit VOLUME's in Dockerfiles has been a long-standing complaint of many base images: https://github.com/docker-library/postgres/issues/404, https://github.com/moby/moby/issues/3465, https://github.com/moby/moby/issues/3465). The workaround used here is to effectively copy the upstream Dockerfile's assets into our own Dockerfile that deliberately omits that VOLUME directive. We'll need to ensure that our Dockerfile is kept in sync whenever we need to upgrade the Prometheus version, but https://github.com/prometheus/prometheus/commits/master/Dockerfile doesn't change all that often in practice.

--- Test plan

I'm pretty confident our users won't need to do any manual migration due to this change. (Our k8s configuration still runs all our images as root, and anyone still using bind mounts shouldn't have to do anything since the existing host folder's permissions still override anything that's set in the container).

However, just to be sure, we'll need to test a fresh kubernetes deployment, the kubernetes upgrade path, and a fresh docker-compose deployment( note that https://github.com/sourcegraph/deploy-sourcegraph-docker/tree/permission depends on changes from this PR). This change doesn't affect sourcegraph/server.

Merge request reports