Dockerfile: Use separate cli for shell and integration-cli by vvoland · Pull Request #45358 · moby/moby

vvoland · 2023-04-19T13:24:36Z

Closes: hack/install: Update docker-cli source, add missing platforms #44214

Use separate cli for integration-cli to allow use newer CLI for interactive dev shell usage.

Both versions can be overriden with DOCKERCLI_VERSION or DOCKERCLI_INTEGRATION_VERSION. Binary is downloaded from download.docker.com if it's available, otherwise it's built from the source.

For backwards compatibility DOCKER_CLI_PATH overrides BOTH clis.

- What I did

- How I did it

- How to verify it

$ make DOCKERCLI_VERSION=v24.0.0-beta.2 \
       shell
...
root@c380d24cb22b:/go/src/github.com/docker/docker# docker version
Client:
 Version:           24.0.0-beta.2
 API version:       1.43
 Go version:        go1.20.3
 Git commit:        67c4570f
 Built:             Wed Apr 19 13:18:12 2023
 OS/Arch:           linux/arm64
 Context:           default

$ make DOCKERCLI_INTEGRATION_VERSION=v18.06.3-ce \
       DOCKER_GRAPHDRIVER=vfs TEST_FILTER=TestRunEchoStdout \
       test-integration
...
INFO: Testing against a local daemon
INFO: Testing with docker cli version:
Client:
 Version:           18.06.3-ce
 API version:       1.38
 Go version:        go1.10.4
 Git commit:        d7080c1
 Built:             Wed Feb 20 02:24:33 2019
 OS/Arch:           linux/arm64
 Experimental:      false

$ make DOCKERCLI_REPOSITORY="https://github.com/vvoland/cli" \
       DOCKERCLI_VERSION=ff7f76af7a092ca9aeb689850d4ad0a85746bc72 \
       shell
...
root@85181e8c0ea9:/go/src/github.com/docker/docker# docker version
Client:
 Version:           ff7f76af
 API version:       1.43
 Go version:        go1.20.3
 Git commit:        ff7f76af
 Built:             Wed Apr 19 13:21:20 2023
 OS/Arch:           linux/arm64
 Context:           default

- Description for the changelog

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

thaJeztah · 2023-04-21T13:26:38Z

Thanks! I like this (think we discussed this approach at some point). Haven't had the time yet to check the implementation, but "+1" in principle

neersighted · 2023-04-21T13:32:56Z

Big conceptual 👍, but I'll need some time to look at the actual implementation.

Dockerfile

thaJeztah

ah, crap. Didn't submit my comments 🙈😂 here they go

Dockerfile

neersighted

It came to my attention that we won't have the ability to build with BuildKit if we take this PR as-is -- can we consider adding buildx to the dev container as well?

vvoland · 2023-05-30T21:39:20Z

I had a separate PR for adding buildx: #44957

Was thinking about just rebasing it once this one is merged, but I'm fine either way.

neersighted · 2023-05-30T21:42:08Z

I forgot about that one! I'm fine with rebasing that based on this PR, though I think I'd prefer if we did it now, just so it's possible to test the desired end state.

thaJeztah

Left some thoughts / comments ("code suggestions" are just a quick "these probably need to be changed", but may be incomplete)

Dockerfile

hack/dockerfile/cli.sh

Dockerfile

Makefile

Use separate cli for integration-cli to allow use newer CLI for interactive dev shell usage. Both versions can be overriden with DOCKERCLI_VERSION or DOCKERCLI_INTEGRATION_VERSION. Binary is downloaded from download.docker.com if it's available, otherwise it's built from the source. For backwards compatibility DOCKER_CLI_PATH overrides BOTH clis. Signed-off-by: Paweł Gronowski <[email protected]>

Installs the buildx cli plugin in the container shell by default. Previously user had to manually download the buildx binary to use buildkit. Signed-off-by: Paweł Gronowski <[email protected]>

neersighted · 2023-06-05T14:17:19Z

Failures are unrelated; do we want to consider taking this for backport to (at least) 24.0; or should we just keep it in master because this is fairly invasive?

vvoland · 2023-06-05T14:40:00Z

I thought we won't be backporting it as it only enhances the development workflow (which mostly happens on master anyway).
On the other hand, it shouldn't be actually harmful and could help with keeping the Dockerfile consistent across branches.

Btw, wanted to push some small enhancements, but you merged this one faster 😅 I'll open a separate PR, and then can open a chery-pick.

vvoland · 2023-06-05T14:46:11Z

Opened #45697

vvoland added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/testing labels Apr 19, 2023

vvoland requested a review from tianon as a code owner April 19, 2023 13:24

vvoland force-pushed the dockerfile-separate-cli branch from a44078d to eb364b7 Compare April 19, 2023 13:59

vvoland added the impact/dockerfile label Apr 19, 2023

vvoland requested review from corhere, cpuguy83, crazy-max, neersighted and thaJeztah April 21, 2023 12:55

vvoland force-pushed the dockerfile-separate-cli branch 2 times, most recently from b2946b5 to 527028a Compare April 21, 2023 14:55

corhere reviewed Apr 27, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Dockerfile Outdated Show resolved Hide resolved

vvoland force-pushed the dockerfile-separate-cli branch from 527028a to 43cf20b Compare April 27, 2023 14:45

corhere approved these changes Apr 27, 2023

View reviewed changes

thaJeztah reviewed Apr 27, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Dockerfile Outdated Show resolved Hide resolved

vvoland force-pushed the dockerfile-separate-cli branch from 43cf20b to d5d8a4c Compare April 27, 2023 17:01

vvoland requested review from crazy-max and removed request for crazy-max May 11, 2023 13:53

neersighted approved these changes May 18, 2023

View reviewed changes

neersighted requested changes May 30, 2023

View reviewed changes

vvoland force-pushed the dockerfile-separate-cli branch from d5d8a4c to cbae543 Compare May 31, 2023 07:51

thaJeztah reviewed May 31, 2023

View reviewed changes

vvoland force-pushed the dockerfile-separate-cli branch 2 times, most recently from e9993e4 to cd5a115 Compare June 5, 2023 12:53

vvoland force-pushed the dockerfile-separate-cli branch 2 times, most recently from 12b8d09 to c75f3d2 Compare June 5, 2023 13:19

vvoland added 2 commits June 5, 2023 15:25

Dockerfile/shell: Install buildx cli plugin

49f76a3

Installs the buildx cli plugin in the container shell by default. Previously user had to manually download the buildx binary to use buildkit. Signed-off-by: Paweł Gronowski <[email protected]>

vvoland force-pushed the dockerfile-separate-cli branch from c75f3d2 to 49f76a3 Compare June 5, 2023 13:25

neersighted approved these changes Jun 5, 2023

View reviewed changes

neersighted merged commit 9aecb90 into moby:master Jun 5, 2023

vvoland added this to the 25.0.0 milestone Jun 5, 2023

vvoland mentioned this pull request Jun 5, 2023

Dockerfile: Separate cli follow-up #45697

Merged

vvoland mentioned this pull request Jun 5, 2023

[24.0 backport] Dockerfile: Use separate cli for shell and integration-cli #45699

Merged

vvoland added the process/cherry-picked label Jun 5, 2023

thaJeztah mentioned this pull request Jun 20, 2023

testing: update docker-py 6.1.3 #43998

Merged

2 tasks

Conversation

vvoland commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Apr 21, 2023

Uh oh!

neersighted commented Apr 21, 2023

Uh oh!

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

neersighted left a comment

Choose a reason for hiding this comment

Uh oh!

vvoland commented May 30, 2023

Uh oh!

neersighted commented May 30, 2023

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neersighted commented Jun 5, 2023

Uh oh!

vvoland commented Jun 5, 2023

Uh oh!

vvoland commented Jun 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vvoland commented Apr 19, 2023 •

edited

Loading