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

builder: fix detection of experimental --stream option (deprecated) #41648

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

thaJeztah
Copy link
Member

fixes #41647

Commit 6ca3ec8 deprecated (#39983) the experimental --stream option for the legacy builder, adding an error message is a client attempted to use this feature.

However, the detection used the session-ID (session=xxx query parameter), which happens to be set automatically by the CLI if it detects that the daemon has session support. Because of this, builds fail when trying to perform them on a daemon with the --experimental flag set.

This patch changes the detection to look for the remote query parameter, which is set to "client-session" when using the --stream option with the classic (non-Buildkit) builder.

Before this change, running docker build with an older (19.03 or older) cli against a daemon with --experimental enabled caused an error:

$ dockerd --experimental &
$ docker pull docker:18.09
$ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:18.09 sh -c 'echo "FROM scratch" | docker build -'

Sending build context to Docker daemon  2.048kB
Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

$ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock -w /foo docker:18.09 sh -c 'echo "FROM scratch" > Dockerfile && docker build --stream .'
Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

With this patch, the error only occurs when trying to use the experimental --stream option:

$ dockerd --experimental &
$ docker pull docker:18.09
$ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:18.09 sh -c 'echo "FROM scratch" | docker build -'

Step 1/1 : FROM scratch
 --->
No image was generated. Is your Dockerfile empty?

$ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock -w /foo docker:18.09 sh -c 'echo "FROM scratch" > Dockerfile && docker build --stream .'

Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Commit 6ca3ec8 deprecated the experimental
"--stream" option for the legacy builder, adding an error message is a client
attempted to use this feature.

However, the detection used the session-ID (`session=xxx` query parameter),
which happens to be set automatically by the CLI if it detects that the daemon
has session support. Because of this, builds fail when trying to perform them
on a daemon with the `--experimental` flag set.

This patch changes the detection to look for the `remote` query parameter, which
is set to "client-session" when using the `--stream` option with the classic
(non-Buildkit) builder.

Before this change, running `docker build` with an older (19.03 or older) cli
against a daemon with `--experimental` enabled caused an error:

    $ dockerd --experimental &
    $ docker pull docker:18.09
    $ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:18.09 sh -c 'echo "FROM scratch" | docker build -'

    Sending build context to Docker daemon  2.048kB
    Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

    docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock -w /foo docker:18.09 sh -c 'echo "FROM scratch" > Dockerfile && docker build --stream .'
    Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

With this patch, the error only occurs when trying to use the experimental
`--stream` option:

    $ dockerd --experimental &
    $ docker pull docker:18.09
    $ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:18.09 sh -c 'echo "FROM scratch" | docker build -'

    Step 1/1 : FROM scratch
     --->
    No image was generated. Is your Dockerfile empty?

    $ docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock -w /foo docker:18.09 sh -c 'echo "FROM scratch" > Dockerfile && docker build --stream .'

    Error response from daemon: experimental session with v1 builder is no longer supported, use builder version v2 (BuildKit) instead

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@tonistiigi @tiborvass @cpuguy83 PTAL

@@ -33,12 +33,7 @@ func Detect(config backend.BuildConfig) (remote builder.Source, dockerfile *pars
case remoteURL == "":
remote, dockerfile, err = newArchiveRemote(config.Source, dockerfilePath)
case remoteURL == ClientSessionRemote:
res, err := parser.Parse(config.Source)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW; this function (remotecontext.Detect()) was only used by the classic builder, and not used in the CLI, so I thought this would be a good spot to produce the error (initially had it in the old place, but thought this would be cleaner)

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[20.10] Regression: detection of experimental --stream is incorrect
3 participants