Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: allow variables in the RUN --mount=type=bind values #815

Closed
hakanurhan opened this issue Feb 5, 2019 · 22 comments · Fixed by #2089
Closed

feature request: allow variables in the RUN --mount=type=bind values #815

hakanurhan opened this issue Feb 5, 2019 · 22 comments · Fixed by #2089

Comments

@hakanurhan
Copy link

hakanurhan commented Feb 5, 2019

When I define an argument (ARG) or an environment variable (ENV) and then try to use it in the RUN --mount=type=bind,... command, I don't get the mount working.

How to reproduce:
Docker version:

$ docker version
Client:
 Version:           18.09.1
 API version:       1.39
 Go version:        go1.10.6
 Git commit:        4c52b90
 Built:             Wed Jan  9 19:35:01 2019
 OS/Arch:           linux/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          18.09.1
  API version:      1.39 (minimum version 1.12)
  Go version:       go1.10.6
  Git commit:       4c52b90
  Built:            Wed Jan  9 19:06:30 2019
  OS/Arch:          linux/amd64
  Experimental:     false

I used the following Dockerfile with BUILDKIT=1 and it failed. If I only change ...target=${TEMPORARY_MOUNT_POINT}... to ...target=/tmp-mount..., it works

# syntax = docker/dockerfile:experimental
FROM ubuntu:14.04.5
ARG TEMPORARY_MOUNT_POINT=/tmp-mount
RUN --mount=type=bind,source=./,target=${TEMPORARY_MOUNT_POINT} \
        ls ${TEMPORARY_MOUNT_POINT}

I also tried using ENV instead of the ARG command, but I had the same results.

The failing and non-failing Dockerfile's are attached (I had to add .txt extension, they are ordinary Dockerfiles)
Dockerfile.fail.txt
Dockerfile.success.txt

@hakanurhan hakanurhan changed the title Using variables in the RUN --mount=type=bind command Using variables fails in the RUN --mount=type=bind command Feb 5, 2019
@hakanurhan hakanurhan changed the title Using variables fails in the RUN --mount=type=bind command Using variables in the RUN --mount=type=bind command fails Feb 5, 2019
@hakanurhan hakanurhan changed the title Using variables in the RUN --mount=type=bind command fails Using variables in the RUN --mount=type=bind command fails Feb 5, 2019
@tonistiigi tonistiigi changed the title Using variables in the RUN --mount=type=bind command fails feature request: allow variables in the RUN --mount=type=bind values Feb 5, 2019
@simon-weber
Copy link

I just ran into this. It's interesting that my --progress=plain ... output shows each command identically -- it made me think this was actually supported.

@matshch
Copy link

matshch commented Mar 18, 2019

The same for me, wasted some time on exploring that variables just doesn't work in mount options, showing nonsense not found: not found error with all variables expanded in command name.

I think there could be two improvements:

  1. Show commands that really executed. So, if variables isn't expanded by buildkit itself, they shouldn't be shown as expanded.
  2. Document that variables can't be used in mount options or implement variable expansion.

@petr-motejlek
Copy link

+1

I too hate the fact that the full command line is printed out expanded, while it would appear that internally it's really not expanded, or perhaps expanded at a different stage of the build. It is really confusing to users.

I would like the expansions to be supported, but if they cannot, at least make the printed out commands show the variable names to make it easier for debug :).

@tonistiigi
Copy link
Member

Marking the invalid expansion in output as a bug.

Technically we can do the expansion quite easily for the values of the csv, so --mount=type=$type but not so easily for the full strings, eg --mount=$mount because they are evaluated on parse time. Wondering if doing it only for the values might be confusing.

@petr-motejlek
Copy link

@tonistiigi , thank you. For my current use case, expanding only the leaves (--mount=type=$type,src=$src,dst=$dst would be enough. Not sure about others, though.

Cheers

@matshch
Copy link

matshch commented Apr 9, 2019

I think expanding only values will be enough for almost everything, and it will be OK if this behavior will be documented.

bcressey added a commit to bottlerocket-os/bottlerocket that referenced this issue Sep 13, 2019
The cache `id` was not set correctly, leading to various undefined
behaviors as the overlay volume was reused across parallel builds.

Adding it back is gated on the ability to use variables this way:
  moby/buildkit#815

Signed-off-by: Ben Cressey <bcressey@amazon.com>
@mowings
Copy link

mowings commented Sep 20, 2019

Expanding values would cover almost all use cases, I think. In my case, I have multiple projects that need separate caches -- and because these builds are automated, I need to be able to pass in a couple of strings to determine which cache is used at build time.

I can work around my particular issue by creating subdirectories off of the cache target based on the passed-in build args, but this is far from ideal.

@alice-sawatzky
Copy link

while we're at it, lets make sure when we implement this that we implement it for all flag values. i just had to workaround --mount=type=ssh,uid=${UID} not working

@tonistiigi
Copy link
Member

Can't make it work for the from option in the --mount as well. Same issue as with COPY --from. At least without a significantly complicated change and a slower builder algorithm. Global scope args could still be used in them like it is possible to use them now by defining a new single-line stage.

@Jim-Bar
Copy link

Jim-Bar commented Mar 12, 2020

Can agree that expanding only the values would cover our use case.

What's the status of this?

@petr-motejlek
Copy link

@tonistiigi ,

Can you provide the "single-line" stage as an example? Not that I need to do that, but I wonder what you meant by it :).

@staticfloat
Copy link

I'd like to express my support for allowing variables in uid and gid, as otherwise it's very difficult to parameterize UID's in large multi-container setups that require coordinated user IDs.

@omermizr
Copy link

omermizr commented Apr 3, 2021

Hey @tonistiigi, can I take a stab at adding support for this?
I'm thinking:

  • Support for expansion in the values.
  • Support for expansion in "from" (for COPY as well?)
  • Raise error when variable is expanded to an empty string.

WDYT?

@tonistiigi
Copy link
Member

@omermizr Sure. For from it is quite tricky to add it, so probably a separate task. Moving Dockerfile's LLB generation over to #1426 should be a prerequisite for it.

I'm not sure if the empty value should be an automatic error. There a probably valid cases for that.

I'd add another task: Make sure progress output shows replaced variables correctly. Eg. var in csv key should not show up as replaced in progress. Instead of just replacing variables on the String() output of the command, it should be done per command logic so that only parts that support variables perform the replacement.

@tonistiigi
Copy link
Member

@omermizr Did you get anywhere with this?

@omermizr
Copy link

@tonistiigi Actually yeah, I got it working locally. I still need to do some cleanups and add a few tests and I'll make a PR :)

@ayk33
Copy link

ayk33 commented Jun 10, 2021

Can you put an example of this working? I'm still getting an error when I try to set uid to an arg. Using # syntax=docker/dockerfile:1.2.1

@omermizr
Copy link

@ayk33 I don't think a new version was released since my contribution. Looks like there's a staging build you can use, but it's probably not a good idea: https://hub.docker.com/r/docker/dockerfile-upstream.
@tonistiigi Any idea when a new version is planned (or what the cadence is)?

@tonistiigi
Copy link
Member

@omermizr v0.9-rc targeted within a week

Mahoney added a commit to Mahoney-playground/goos that referenced this issue Sep 10, 2021
Previously it wasn't possible to use docker ARG values in the `--mount`
argument: moby/buildkit#815

This has been fixed, so we can stop the duplication.
@raphaelpreston
Copy link

What is the status on this? Have to agree that it's frustrating that the command line shows it expanded and the failing is silent...

@findepi
Copy link

findepi commented Apr 5, 2023

@raphaelpreston it looks like you need to start your Dockerfile with a line like

# syntax=docker/dockerfile:1.3.0-labs

to enable ARG support in RUN --mount=type=bind,...

@ar13pit
Copy link

ar13pit commented May 29, 2023

@findepi The latest is a couple of minor versions ahead and has these changes, so it is safe to say that starting the Dockerfile with

# syntax = docker/dockerfile:latest

also supports ARG in the RUN --mount=type=bind,...

I have verified this as well.

webern pushed a commit to webern/twoliter that referenced this issue Aug 24, 2023
The cache `id` was not set correctly, leading to various undefined
behaviors as the overlay volume was reused across parallel builds.

Adding it back is gated on the ability to use variables this way:
  moby/buildkit#815

Signed-off-by: Ben Cressey <bcressey@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.