Skip to content

vendor: github.com/moby/moby/api v1.52.0, moby/client v0.1.0 - #3326

Merged
crazy-max merged 1 commit into
docker:masterfrom
thaJeztah:migrate_moby_modules
Nov 12, 2025
Merged

vendor: github.com/moby/moby/api v1.52.0, moby/client v0.1.0#3326
crazy-max merged 1 commit into
docker:masterfrom
thaJeztah:migrate_moby_modules

Conversation

@thaJeztah

@thaJeztahthaJeztah commented Jul 21, 2025

Copy link
Copy Markdown
Member

@thaJeztah
thaJeztah marked this pull request as draft July 21, 2025 18:32
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 4 times, most recently from 2a8cb2e to d95a6e4CompareJuly 29, 2025 18:24
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 2 times, most recently from b38ce36 to b2b1838CompareAugust 5, 2025 21:31
@thaJeztahthaJeztah changed the title WIP: migrate to moby modulesWIP: migrate to moby/moby/api v1.52.0-alpha.1, moby/moby/client v0.1.0-alpha.0Aug 5, 2025
@thaJeztahthaJeztah changed the title WIP: migrate to moby/moby/api v1.52.0-alpha.1, moby/moby/client v0.1.0-alpha.0migrate to moby/moby/api v1.52.0-alpha.1, moby/moby/client v0.1.0-alpha.0Aug 5, 2025
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 2 times, most recently from af0516f to ac1ad8eCompareAugust 6, 2025 21:35
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 2 times, most recently from 82ff223 to 62592fbCompareAugust 28, 2025 15:33
@thaJeztahthaJeztah changed the title migrate to moby/moby/api v1.52.0-alpha.1, moby/moby/client v0.1.0-alpha.0migrate to moby/moby/api v1.52.0-beta.1, moby/moby/client v0.1.0-beta.0Sep 6, 2025
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 4 times, most recently from af171bc to d7eb935CompareSeptember 25, 2025 17:54
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 5 times, most recently from 53275e1 to 79a35cdCompareOctober 14, 2025 13:05
@thaJeztahthaJeztah changed the title migrate to moby/moby/api v1.52.0-beta.1, moby/moby/client v0.1.0-beta.0migrate to github.com/moby/moby/api v1.52-beta.2, moby/client v0.1.0-beta.2Oct 14, 2025
@thaJeztah
thaJeztahforce-pushed the migrate_moby_modules branch 4 times, most recently from 3c00d0e to f52e45dCompareNovember 7, 2025 01:45
@thaJeztahthaJeztah changed the title migrate to github.com/moby/moby/api v1.52-beta.2, moby/client v0.1.0-beta.2vendor: github.com/moby/moby/api v1.52.0, moby/client v0.1.0Nov 10, 2025
@thaJeztah
thaJeztah marked this pull request as ready for review November 11, 2025 20:01
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment threadgo.mod
github.com/docker/cli v29.0.0+incompatible
github.com/docker/cli-docs-tool v0.10.0
github.com/docker/docker v28.5.1+incompatible
github.com/docker/docker v28.5.2+incompatible

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like we keep this module only for the namesgenerator package:

"github.com/docker/docker/pkg/namesgenerator"

Seems it has moved to an internal package since v29: https://github.com/moby/moby/tree/master/internal/namesgenerator which relates to this opened PR #3354

@thaJeztah I wonder if this package could be moved to client/pkg like you have done for pkg/stringid in moby/moby#50504 so we avoid multiple implementation and fragmentation across projects?

@crazy-max
crazy-max merged commit 5234ee9 into docker:masterNov 12, 2025
201 of 202 checks passed
@thaJeztah
thaJeztah deleted the migrate_moby_modules branch November 12, 2025 11:23
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

I wonder if this package could be moved to client/pkg like you have done for pkg/stringid in moby/moby#50504 so we avoid multiple implementation and fragmentation across projects?

No, this package was never meant to be used by any client; it's a fallback for the daemon if a name is omitted. There's been various conversations to consider removing it entirely (possibly using plain GUIDs or PERHAPS adding an option for users to provide a prefix, which could work for buildx)

I think we should look if buildx needs to generate a name at all, but if it does, I think it should actually use its own naming scheme instead of the same scheme as the docker daemon uses for generated names; I think the current approach is to some extend even more confusing, because these names don't directly correlate with container names, only builder names;

If I see it correctly, the current approach is for buildx to generate a name, and as part of that check if there's no builders that have that name;

docker buildx create
charming_nash

However, it (by default) does not create an actual container at that point, it only defines a builder "template" for container(s) to be created, but it does show an "inactive" instance;

docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
charming_nash docker-container
\_ charming_nash0 \_ desktop-linux inactive
...

But that instance doesn't exist yet, so .. it's not really "inactive"; it doesn't exist yet;

docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES

This part is already slightly confusing, because it shows the name of a builder instance that doesn't exist (and buildx doesn't allow picking an instance, or is that the --builder option on docker buildx use?);

docker buildx use charming_nash0
ERROR: failed to find instance "charming_nash0": open /Users/thajeztah/.docker/buildx/instances/charming_nash0: no such file or directory

Using the builder;

docker buildx use --builder charming_nash0 charming_nash

or is that the --builder option on docker buildx use?

Setting the --builder option doesn't show the charming_nash0 to be the one I picked, so .. maybe not?

docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
charming_nash* docker-container
\_ charming_nash0 \_ desktop-linux inactive
...

No container created up until this point;

docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES

When building, the container is bootstrapped;

echo'FROM alpine'| docker buildx build --quiet -
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load

And now a container IS created, but it doesn't use the naming scheme as-is, but an additional prefix;

docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
e816a30f4fc8 moby/buildkit:buildx-stable-1 "buildkitd --allow-i…" 21 seconds ago Up 21 seconds buildx_buildkit_charming_nash0

This also means that there's a TOCTOU issue here as well; with these names being predictable, I can create a container with the expected to be used name;

docker buildx create
lucid_hypatia
docker run -d --name buildx_buildkit_lucid_hypatia0 nginx:alpine
37084e0cf8c4cdf5272699274db4fe2b8d1e3ff76796c99f544f8a3240e8e51e
docker buildx use --builder lucid_hypatia0 lucid_hypatia
docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
lucid_hypatia* docker-container
\_ lucid_hypatia0 \_ desktop-linux error
...
Failed to get status for lucid_hypatia (lucid_hypatia0): listing workers: failed to list workers: Unavailable: connection error: desc = "error reading server preface: http2: frame too large"echo'FROM alpine'| docker buildx build --quiet -
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: failed to build: listing workers for Build: failed to list workers: Unavailable: connection error: desc = "error reading server preface: http2: frame too large"

Based on the above;

  • The names-generator (which was designed for the daemon to generate container names) is used by buildx for builder names
  • ☝️ normally this generator would be used by the daemon if no container-name was provided, so that it can generate a name on the daemon side (and make sure there's no conflicts)
  • 👉 but for buildx, a name is reserved client side ahead of time (with the expectation that the name will be available the moment it will create one)
  • ❓ should it? Or should docker buildx ls show that no instances exist (yet)?
  • ☝️ at the time it does create a container, it uses its own naming scheme (using the buildx_buildkit_ prefix, and a <number> suffix)

Some possible alternatives;

Use labels and/or annotations instead of depending on the name.

I think in general, the reason for these names are for buildx to be able to correlate containers with its builders. This is similar to docker compose, which also needs to do this, and historically used the container name for this purpose. Instead of using the name, buildx can use labels or annotations (although we currently lack many options for filtering on annotations).

The intent for these containers is to be managed by buildx, and not manually by the user, so for users, the name shouldn't really matter; ideally we should even put these in their own (containerd) namespace, so that they don't show up among user-created containers, but we don't currently expose containerd namespaces (which we should work on).

For fun, I checked what would happen with a container that happened to be named following buildx's naming scheme, and it looks like 💥 buildx happily removes my container;

docker buildx create
silly_chatelet
docker run -d --name buildx_buildkit_silly_chatelet0 nginx:alpine
90290ff3b5c85ed2724769384facc628b46404c36e246e8f26dd3b1d10fe4f03
docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
90290ff3b5c8 nginx:alpine "/docker-entrypoint.…" 13 seconds ago Up 12 seconds 80/tcp buildx_buildkit_silly_chatelet0
docker buildx rm silly_chatelet
silly_chatelet removed
docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES

Using labels and/or annotations to identify if a container is a container managed by / created by buildx would also avoid the above.

Alternative ideas

docker buildx create without a name could base the name of the builder on the context used for it; this could even mean showing instances underneath the context, so instead of this;

docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
lucid_hypatia* docker-container
\_ lucid_hypatia0 \_ desktop-linux error
silly_chatelet docker-container
\_ silly_chatelet0 \_ desktop-linux inactive
strange_bardeen docker-container
\_ strange_bardeen0 \_ desktop-linux inactive
default docker
\_ default \_ default running v0.25.1 linux/amd64 (+2), linux/arm64, linux/ppc64le, linux/s390x, (2 more)
desktop-linux docker
\_ desktop-linux \_ desktop-linux running v0.25.1 linux/amd64 (+2), linux/arm64, linux/ppc64le, linux/s390x, (2 more)

We may have something like this (just a really quick write-up without giving it much thought);

docker buildx ls
CONTEXT/INSTANCE DRIVER STATUS BUILDKIT PLATFORMS
desktop-linux *\_ desktop-linux docker running v0.25.1 linux/amd64 (+2), linux/arm64, linux/ppc64le, linux/s390x, (2 more)
\_ desktop-linux0 docker-container error
\_ desktop-linux1 docker-container inactive

I'm not 100% sure what the exact use-cases are for multiple builders per context; I could see this being to namespace builds, but (see my earlier comment), perhaps that's something we should look into in a wider scope to namespace objects (containers, volumes, builders, networks).

Sign up for freeto 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.

3 participants

@thaJeztah@tonistiigi@crazy-max