Skip to content

fix "host-gateway-ip" label not set for builder workers#47187

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:fix_gateway_ip
Jan 23, 2024
Merged

fix "host-gateway-ip" label not set for builder workers#47187
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:fix_gateway_ip

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Jan 23, 2024 •
edited
Loading

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

- What I did

- How I did it

- How to verify it

- Description for the changelog

Fix `host-gateway-ip` not working during build when not set through configuration

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

@thaJeztah thaJeztah self-assigned this Jan 23, 2024
@thaJeztah thaJeztah added this to the 26.0.0 milestone Jan 23, 2024
@thaJeztah thaJeztah mentioned this pull request Jan 23, 2024
Merged
@thaJeztah
fix "host-gateway-ip" label not set for builder workers
00c9785 
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 <[email protected]>
thaJeztah
thaJeztah commented Jan 23, 2024
View reviewed changes
Comment thread daemon/daemon.go
Comment on lines +183 to +184
// Config returns daemon's config.
func (daemon *Daemon) Config() config.Config {
Copy link
Copy Markdown
Member Author

@thaJeztah thaJeztah Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

FWIW, not "stoked" by this solution, but it should get us going.

@akerouanton akerouanton mentioned this pull request Jan 23, 2024
Closed
@thaJeztah thaJeztah mentioned this pull request Jan 23, 2024
Merged
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jan 23, 2024

Failure is unrelated; and tracked in #47119

=== FAIL: amd64.integration.system TestDiskUsage/empty (0.00s)
    disk_usage_test.go:41: assertion failed: 
        --- du
        +++ →
          types.DiskUsage{
        - 	LayersSize: 4096,
        + 	LayersSize: 0,
          	Images:     {},
          	Containers: {},
          	... // 3 identical fields
          }
        
    --- FAIL: TestDiskUsage/empty (0.00s)

@thaJeztah thaJeztah marked this pull request as ready for review January 23, 2024 15:27
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jan 23, 2024

New failure is also unrelated (other flaky test); #46508

--- FAIL: TestLiveRestore (0.00s)
    --- SKIP: TestLiveRestore/autoremove (0.00s)
    --- FAIL: TestLiveRestore/volume_references (17.09s)
        --- PASS: TestLiveRestore/volume_references/restartPolicy (10.17s)
            --- PASS: TestLiveRestore/volume_references/restartPolicy/always (2.68s)
            --- PASS: TestLiveRestore/volume_references/restartPolicy/unless-stopped (2.62s)
            --- PASS: TestLiveRestore/volume_references/restartPolicy/on-failure (2.54s)
            --- PASS: TestLiveRestore/volume_references/restartPolicy/no (2.32s)
        --- FAIL: TestLiveRestore/volume_references/local_volume_with_mount_options (2.33s)
            --- SKIP: TestLiveRestore/volume_references/local_volume_with_mount_options/volume_still_mounted (0.00s)
        --- PASS: TestLiveRestore/volume_references/container_with_bind-mounts (2.34s)

@thaJeztah thaJeztah merged commit f19f233 into moby:master Jan 23, 2024
@thaJeztah thaJeztah deleted the fix_gateway_ip branch January 23, 2024 15:52
laurazard
laurazard approved these changes Jan 23, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM (but yeah, this is a messy area and we should track this somewhere to improve)

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Jan 23, 2024

@laurazard yeah, agreed. I did an ever-so-slightly cleanup after I worked on this here; #47188

(nothing for the "global picture" though.

This was referenced Jan 24, 2024
Merged
Merged
@thaJeztah thaJeztah mentioned this pull request Nov 26, 2024
Closed
FelixLinn added a commit to FelixLinn/IGinX that referenced this pull request Feb 28, 2026
@FelixLinn
fix(docker): fix host-gateway error on Windows CI
edc913c 
Remove --add-host=host.docker.internal:host-gateway from run_docker.bat.
Docker 25+ on GitHub Actions Windows fails to resolve host-gateway
(docker/buildx#2201, moby/moby#47187). On Windows, host.docker.internal
is provided by Docker Desktop, so the flag is unnecessary and causes
"unable to derive the IP value for host-gateway".
zhuyuqing pushed a commit to IGinX-THU/IGinX that referenced this pull request Apr 28, 2026
@FelixLinn
fix(storage): tighten ALTER STORAGEENGINE rules and option key grammar (
9a81d06 
#674)

* feat(storage): tighten ALTER STORAGEENGINE rules and option key grammar

- Add ALTER_ENGINE_IMMUTABLE_PARAMS in Constants; reject altering immutable
  params (ip, port, data_prefix, schema_prefix, has_data, is_read_only,
  engine) with explicit error message
- Reject adding new extra params via ALTER (modify existing options only)
- Change storageEngineOptionKey to ID (DOT ID)* (plain identifiers only)
- Add ALTER STORAGEENGINE integration tests in BaseCapacityExpansionIT

* fix(storage): fix the option param

* fix(test): fix expPort storageengine remove

* fix(test): change the alter storageengine order

* fix(test): change the alter storageengine order

* fix(docker): fix host-gateway error on Windows CI

Remove --add-host=host.docker.internal:host-gateway from run_docker.bat.
Docker 25+ on GitHub Actions Windows fails to resolve host-gateway
(docker/buildx#2201, moby/moby#47187). On Windows, host.docker.internal
is provided by Docker Desktop, so the flag is unnecessary and causes
"unable to derive the IP value for host-gateway".

* fix(test): recover test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crazy-max crazy-max crazy-max approved these changes

@vvoland vvoland vvoland approved these changes

@laurazard laurazard laurazard approved these changes

@akerouanton akerouanton Awaiting requested review from akerouanton

Assignees

@thaJeztah thaJeztah

Labels

area/builder/buildkit Build area/builder Build area/daemon Core Engine area/networking Networking kind/bugfix PR's that fix bugs process/cherry-picked status/2-code-review

Projects

None yet

Milestone

26.0.0

Development

Successfully merging this pull request may close these issues.

After Update to Docker 25.0.0 (buildx v0.12.1), host-gateway in build does not work anymore

4 participants

@thaJeztah @crazy-max @vvoland @laurazard