[25.0 backport] fix "host-gateway-ip" label not set for builder workers #47192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit 21e50b8 added a label on the buildkit worker to advertise the host-gateway-ip. This option can be either set by the user in the daemon config, or otherwise defaults to the gateway-ip. If no value is set by the user, discovery of the gateway-ip happens when initializing the network-controller (`NewDaemon`, `daemon.restore()`). However d222bf0 changed how we handle the daemon config. As a result, the `cli.Config` used when initializing the builder only holds configuration information form the daemon config (user-specified or defaults), but is not updated with information set by `NewDaemon`. This patch adds an accessor on the daemon to get the current daemon config. An alternative could be to return the config by `NewDaemon` (which should likely be a _copy_ of the config). Before this patch: docker buildx inspect default Name: default Driver: docker Nodes: Name: default Endpoint: default Status: running Buildkit: v0.12.4+3b6880d2a00f Platforms: linux/arm64, linux/amd64, linux/amd64/v2, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/mips64le, linux/mips64, linux/arm/v7, linux/arm/v6 Labels: org.mobyproject.buildkit.worker.moby.host-gateway-ip: <nil> After this patch: docker buildx inspect default Name: default Driver: docker Nodes: Name: default Endpoint: default Status: running Buildkit: v0.12.4+3b6880d2a00f Platforms: linux/arm64, linux/amd64, linux/amd64/v2, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/mips64le, linux/mips64, linux/arm/v7, linux/arm/v6 Labels: org.mobyproject.buildkit.worker.moby.host-gateway-ip: 172.18.0.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 00c9785) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added
area/builder
status/2-code-review
area/networking
impact/changelog
kind/bugfix
vvoland
approved these changes
Jan 23, 2024
crazy-max
approved these changes
Jan 23, 2024
laurazard
approved these changes
Jan 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit 21e50b8 added a label on the buildkit worker to advertise the host-gateway-ip. This option can be either set by the user in the daemon config, or otherwise defaults to the gateway-ip.
If no value is set by the user, discovery of the gateway-ip happens when initializing the network-controller (
NewDaemon,daemon.restore()).However d222bf0 changed how we handle the daemon config. As a result, the
cli.Configused when initializing the builder only holds configuration information form the daemon config (user-specified or defaults), but is not updated with information set byNewDaemon.This patch adds an accessor on the daemon to get the current daemon config. An alternative could be to return the config by
NewDaemon(which should likely be a copy of the config).Before this patch:
After this patch:
(cherry picked from commit 00c9785)
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)