makefile/docker.inc.mk: Expose docker volume mount args

[MrKevinWeiss]

Contribution description

Only for use when using BUILD_IN_DOCKER=1 and using a different docker container.

Add DOCKER_VOLUME_NAME to override the default volume name.
Add DOCKER_VOLUME_MOUNT_PATH to override default mounting path.
Allow DOCKER_RIOTPROJECT to be overridden when changing defaults and being outside RIOT.

In order to run BUILD_IN_DOCKER within a docker instance there parameters must be exposed.
Since true docker in docker is not always straight forward the recommend way is just exposing the hosts docker command.

host
  - volume_for_persistant_data
  - docker_container_with_riot_repo
    - uses volume_for_persistant_data
    - calls make BUILD_IN_DOCKER from RIOT repo
  - riotbuild docker containter
    - requires volume_for_persistant_data and relative path to the RIOT repo

Note that all defaults remain the same, this doesn't change how BUILD_IN_DOCKER can be used. It only adds optional env vars to allow for BUILD_IN_DOCKER from a docker with a socket to the host docker... I know how silly that statement sounds.

Testing procedure

written for those with docker experience

• Start a docker container (with build essentials for make) that has a socket to the host docker
• clone the RIOT into a working directory
• try to BUILD_IN_DOCKER=1 make (it should fail since it is assuming docker is running in the container when really it is using a socket to the hosts docker)

expected results for BUILD_IN_DOCKER=1 -C examples/hello-world

docker run --rm --tty --user $(id -u) \
-v '/usr/share/zoneinfo/UCT:/etc/localtime:ro' \
-v '<volume_path_to_riot>:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' \
-e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' \
-e 'BUILD_DIR=/data/riotbuild/riotbase/build' \
-e 'RIOTPROJECT=/data/riotbuild/riotbase' \
-e 'RIOTCPU=/data/riotbuild/riotbase/cpu' \
-e 'RIOTBOARD=/data/riotbuild/riotbase/boards' \
-e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles' \
-w '/data/riotbuild/riotbase/examples/hello-world/' 'riot/riotbuild:latest' make

• now try

DOCKER_VOLUME_HOME=<generic_riotbase_name> \
DOCKER_RIOTBASE=<generic_riotbase_name>/<path_to_riotbase_relative_to_volume_home> \
DOCKER_VOLUME_NAME=<volume_name_used_in_current_container> \
BUILD_IN_DOCKER=1 make -C examples/hello-world/

expect it to succeed.

For bonus points try to build with riotbase outside the directory calling make, if that is the case then the -w arg changes and fails. This is why the DOCKER_RIOTPROJECT should be overwritten.

Issues/PRs references

[MrKevinWeiss]

There might be a better way of doing this but so far this is the only way I could make it work without changing the system.

Since @cladmi is not longer here it seems @fjmolinas or @maribu would be next in line.

[MrKevinWeiss]

I also don't know if this stuff should be documented somewhere such as vars.inc.mk or in a readme. This really is just fine tuning for cornercases when using BUILD_IN_DOCKER.

[cladmi]

Suggested change

	# Name of the docker volue contianing RIOT.
	# Name of the docker volume containing RIOT.

[MrKevinWeiss]

Many thanks for taking a look @cladmi, fixed!

[maribu]

I do not fully understand the goal. However, this PR does not change any configuration, but only allows users to do so if they want to. There is certainly no harm in that.

I wonder if it might make sense to have a RIOT somewhere in the name of the new variables. Those names are currently pretty generic and it would be shame if some users end up with BUILD_IN_DOCKER=1 being broken just because by chance they already had an environment variable with that name in use. Other than that, I'd ACK.

[MrKevinWeiss]

@maribu Thanks for the feedback, I will namespace better.

The goal is to be able to run a CI (in this case Jenkins) in docker while using the hosts version of docker for the BUILD_IN_DOCKER.

The real docker in docker approach seems to be not recommend for this case and merging the two docker images is not exactly trival.

With finer grain control over the volume I can expose the RIOT that would exist in the CI docker container to the BUILD_IN_RIOT docker container.

[cladmi]

In that case, not sure why you need the DOCKER_VOLUME_MOUNT_PATH as there is already the DOCKER_BUILD_ROOT and DOCKER_RIOTBASE. All mounted directories are based on that one.

[MrKevinWeiss]

It is my workaround because DOCKER_RIOTBASE must contain the relative path to RIOT from the volume root. DOCKER_VOLUME_MOUNT_PATH is just a way to say keep this fixed while everything else gets changed.

The ideal way would be to add a relative_path_to_riot_from_volume_base to each of the other arguments but it seems would have to change more.

I would like to get something like this:

docker run --rm --tty --user $(id -u) \
-v '/usr/share/zoneinfo/UCT:/etc/localtime:ro' \
-v '<ci_volume>:<ci_volume_base_path>' \
-e 'RIOTBASE=<ci_volume_base_path>/<relative_riot_path>' \
-e 'CCACHE_BASEDIR=<ci_volume_base_path>/<relative_riot_path>' \
-e 'BUILD_DIR=<ci_volume_base_path>/<relative_riot_path>/build' \
-e 'RIOTPROJECT=<ci_volume_base_path>/<relative_riot_path>' \
-e 'RIOTCPU=<ci_volume_base_path>/<relative_riot_path>/cpu' \
-e 'RIOTBOARD=<ci_volume_base_path>/<relative_riot_path>/boards' \
-e 'RIOTMAKE=<ci_volume_base_path>/<relative_riot_path>/makefiles' \
-w '<ci_volume_base_path>/<relative_riot_path>/examples/hello-world/' 'riot/riotbuild:latest' make

[cladmi]

So the goal for you, is to trick/configure to mount not the riot directory but another one which has RIOT inside. The logic is indeed quite based on the fact that the base directory mounted is RIOT.
It could be adapted and not be a too big change.

I did not execute to see how what you would want is different from what you currently have.
However by playing with the other variables you may be able to get what you want.

You should check the variables used in path_in_docker and dir_is_outside_riotbase.
It is using DOCKER_BUILD_ROOT, RIOTBASE, DOCKER_RIOTBASE, you may be able to tweak it from outside, or adapt them to be based on the mount directory without being too complicated.

[MrKevinWeiss]

I will try that, I did play around that for a bit but not exhaustively. The also tried with the dir_is_outside_riotbase which is why I needed to overwrite the DOCKER_RIOTPROJECT.

I agree that it could be more simple.

[MrKevinWeiss]

Closing for now as I am avoiding socket docker in docker.

I may reopen if it comes up later, but I still think there is a better solution.