moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
8.27k stars 1.17k forks source link

WCOW: support for `setx`, installers vs. `ENV`, a discussion #5445

Open profnandaa opened 1 month ago

profnandaa commented 1 month ago

Related to #3158 #4895 #4901

Background

Environment variables are sourced in two different ways on Windows containers:

  1. Registry: (primarily) -- this is the primary approach on how environment variables are stored and retrieved. On the shell, this done with the setx command. A rough equivalent of export on Unix systems.
  2. Image config -- An image's config file has a config.Env field, which containers entries in the format of VARNAME=VARVALUE; these values act as defaults and are merged with any specified when creating a container. Using dockerfile frontend, this is set using ENV.

For a mcr.microsoft.com/windows/nanoserver:ltsc2022 for example, and all the other official Windows base images, the config.Env field is null. Therefore all the environment variables come from the registry.

# quick way of inspecting the config:
PS> docker inspect mcr.microsoft.com/windows/nanoserver:ltsc2022
# <snip>
 "Config": {
            ...
            "StdinOnce": false,
            "Env": null,  # <---- this
            "Cmd": [
                "c:\\windows\\system32\\cmd.exe"
            ],
          ...
        },
# <snip>

PS>  docker run mcr.microsoft.com/windows/nanoserver:ltsc2022 cmd /c echo %PATH%
C:\Windows\system32;C:\Windows;

setx vs. ENV on buildkit and classic docker builder

When there is a name collision between what is set in the config and what is set in the registry with setx for example, the one in config takes precedence. For example:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS base
USER ContainerAdministrator

RUN setx /M PATH "C:\\my\\other\\path;%PATH%"
ENV PATH="C:\\my\\path;C:\\Windows\\system32;C:\\Windows;"

FROM base
RUN echo %PATH%

Buildkit:

#6 [stage-1 1/1] RUN echo %PATH%
#6 1.864 C:\my\path;C:\Windows\system32;C:\Windows;
#6 DONE 2.2s

Classic docker build:

Step 6/6 : RUN echo %PATH%
 ---> Running in 4e30c4c4d266
C:\my\path;C:\Windows\system32;C:\Windows;

Compare with this one:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS base
USER ContainerAdministrator

RUN setx /M PATH "C:\\my\\other\\path;%PATH%"

FROM base
RUN echo %PATH%

Buildkit:

#6 [stage-1 1/1] RUN echo %PATH%
#6 1.897 c:\Windows\System32;c:\Windows
        ^
        |  for buildkit, if PATH is not set in the config, this default one is set instead, see #3158
#6 DONE 2.3s

Classic docker build:

Step 5/5 : RUN echo %PATH%
 ---> Running in 4ef43f8a3bc0
C:\\my\\other\\path;C:\Windows\system32;C:\Windows;

At this point, there is no backward compatibility with classic docker build.

The difference between buildkit and classic docker build is the source for #3158 #4895. If the default path is left empty, Windows will handle getting the right path from registry.

Windows Installers

Installers on Windows also set environment variables (in the registry). A good example close home, is the Go installer. See example below:

FROM mcr.microsoft.com/windows/servercore:ltsc2022

# Set environment variables for Go
ENV GOLANG_VERSION 1.23.2
# before installation
RUN echo PATH=%PATH%
RUN echo GOPATH=%GOPATH%

# Download and Install Go
RUN curl.exe -Lo go.msi "https://go.dev/dl/go%GOLANG_VERSION%.windows-amd64.msi"

RUN  msiexec.exe /i go.msi /quiet /norestart 
RUN dir .

# Verify Go installation
RUN go version

# after installation
RUN echo PATH=%PATH%
RUN echo GOPATH=%GOPATH%

Classic docker build:

Step 3/10 : RUN echo PATH=%PATH%
 ---> Running in 1f89b3852891
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;
C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps

Step 4/10 : RUN echo GOPATH=%GOPATH%
 ---> Running in 9bfc156f0158
GOPATH=%GOPATH%

<snip>

Step 9/10 : RUN echo PATH=%PATH%
 ---> Running in 5c1d0d196931
PATH=C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;
C:\Windows\System32\OpenSSH\;C:\Program Files\Go\bin;
C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps;C:\Users\ContainerAdministrator\go\bin

Step 10/10 : RUN echo GOPATH=%GOPATH%
 ---> Running in ed75cae10192
GOPATH=C:\Users\ContainerAdministrator\go

For such case, you want to make sure to set the same environment variables declaratively in the dockerfile, for a consistent build experience across classic docker build and buildkit.

FROM mcr.microsoft.com/windows/servercore:ltsc2022

# Set environment variables for Go
ENV GOLANG_VERSION 1.23.2
ENV GOPATH "C:\\Users\\ContainerAdministrator\\go"
ENV PATH "$PATH;C:\\Program Files\\Go\\bin;C:\\Users\ContainerAdministrator\\go\\bin"

# <snip>

A case for building windows images from Linux

Buildkit can build windows images from Linux (cross-platform builds), however, with some limitations like you can't execute RUN statements. However, others like COPY, ENV, WORKDIR are supported. This is sort of an "offline build", since actual Windows containers are not spined up.

Actually, this is how Buildkit has been used for building Windows images by the early adopters, before WCOW support came about. See microsoft/Windows-Containers#493 for example.

Here is a simple example dockerfile that will build currently on both Window and Linux:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
ENV GOPATH="C:/go"

COPY ./sample.go $GOPATH/pkg/sample.go

Building from Linux with buildx:

$ docker buildx create --name buildkitd-dev \
  --driver=remote  unix:///run/buildkit/buildkitd.sock \
  --platform windows/amd64

$ docker buildx build \
  --platform windows/amd64 \
  --builder buildkitd-dev \
  --no-cache \
  --progress plain \
  --output type=image,name=profnandaa/hello-buildkit,push=false .

# <snip>
#5 [1/2] FROM mcr.microsoft.com/windows/nanoserver:ltsc2022@sha256:f59e2f21720e4d1192e0dde47e32c7dbf27144d52d79be14b2538fa07145c869
#5 CACHED

#6 [2/2] COPY ./sample.go C:/go/pkg/sample.go
#6 DONE 0.1s

#7 exporting to image
#7 exporting layers

Conclusion

A proper conclusion is yet to be made after the discussion. However, my preliminary suggestion is that we prioritize maintaining a uniform experience between building Windows images on Windows vs. building them on Linux. Therefore, I suggest the declarative way of setting environment variables using ENV, since they are treated similarly across board.

This means leaving everything as-is with a minimal change to fix #4901 (to open PR next, #5446 ) and a clear documentation on how to handle environment variables and advisory to prefer ENV over setx.

profnandaa commented 1 month ago

/cc. @thaJeztah

tonistiigi commented 1 month ago

I think Windows images should define the environment variables in the image config and not rely on the registry. Only variables in the config can be used for variable expansion in Dockerfile, they should take precedence over registry at runtime and ENV should remain metadata-only command.

What we could do is add some validation routines that would detect if RUN added a new global environment variable that was not set in image config or allow some functionality to "copy-up" value from registry to config, either at the end of the build or in RUN. Allowing this does disable lots of buildkit's lazy semantics though because even to just get a config and DefinitionOp (needed to link builds together for example) the whole build would need to run.

There is also a discussion in #3158 if some of the special cases only apply to PATH. Reading those responses makes me think if maybe hcsshim needs a special handling of PATH instead. E.g. something that would combine the subpaths from the image config with the ones inside the registry before running the container.

profnandaa commented 1 month ago

@tonistiigi -- thanks for taking a look!

I think Windows images should define the environment variables in the image config and not rely on the registry. Only variables in the config can be used for variable expansion in Dockerfile, they should take precedence over registry at runtime and ENV should remain metadata-only command.

We actually considered taking this request to the platform team, but if this is done, it will break the current/classic docker build experience for RUN setx. I had highlighted that in https://github.com/moby/buildkit/pull/5446

What we could do is add some validation routines that would detect if RUN added a new global environment variable that was not set in image config or allow some functionality to "copy-up" value from registry to config, either at the end of the build or in RUN. Allowing this does disable lots of buildkit's lazy semantics though because even to just get a config and DefinitionOp (needed to link builds together for example) the whole build would need to run.

I see, I don't know if this will be right price to pay...

There is also a discussion in #3158 if some of the special cases only apply to PATH. Reading those responses makes me think if maybe hcsshim needs a special handling of PATH instead. E.g. something that would combine the subpaths from the image config with the ones inside the registry before running the container.

Makes sense to handle $PATH a little different that the rest. I think taking the hcsshim route sounds like our best bet. I'll be taking up this discussion to the platform team and later bubble up to HCS folks for this consideration.

In the meantime, do you think we could land https://github.com/moby/buildkit/pull/5446 as a stop-gap measure for now?

slonopotamus commented 3 weeks ago

I believe that registry should not be ignored. If user installs a software that adds itself to PATH, it should Just Work in a container, without any extra steps.