Skip to content

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Jun 19, 2023

related to docker/buildx#1832

- What I did

We missed a case when parsing extra hosts from the dockerfile frontend so the build fails.

- How I did it

To handle this case we need to set a dedicated worker label that contains the host gateway IP so clients like Buildx can just set the proper host:ip when parsing extra hosts that contain the special string "host-gateway". See docker/buildx#1894

- How to verify it

- Description for the changelog

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

@crazy-max crazy-max marked this pull request as ready for review June 20, 2023 09:44
@crazy-max
Copy link
Member Author

I put this one back in draft as worker label might not be ideal. Best would be to expose the host gateway IP to the API in /info.

@crazy-max crazy-max marked this pull request as draft June 20, 2023 12:41
@crazy-max
Copy link
Member Author

Using worker label looks to be the best alternative to fix this issue. PTAL @tonistiigi.

@crazy-max crazy-max marked this pull request as ready for review June 20, 2023 21:51
We missed a case when parsing extra hosts from the dockerfile
frontend so the build fails.

To handle this case we need to set a dedicated worker label
that contains the host gateway IP so clients like Buildx
can just set the proper host:ip when parsing extra hosts
that contain the special string "host-gateway".

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@neersighted
Copy link
Member

I assume we'll need changes on the buildkit side, and then we'll need to vendor those changes in?

@crazy-max
Copy link
Member Author

I assume we'll need changes on the buildkit side, and then we'll need to vendor those changes in?

No changes needed in BuildKit.

@neersighted
Copy link
Member

Oh, I see -- the call is coming from inside the building (we're using the label in the Moby code)?

@crazy-max
Copy link
Member Author

Oh, I see -- the call is coming from inside the building (we're using the label in the Moby code)?

This label will be used client side: https://github.com/docker/buildx/pull/1894/files#diff-25074304352810d97f912548941e8eded43dbc708066d4074cf5765a8b42ecd8R203-R204 for the sole purpose of host-gateway handling with moby workers (graphdriver or containerd snap).

@neersighted neersighted requested a review from thaJeztah June 21, 2023 13:25
@neersighted
Copy link
Member

I see; that PR connects the dots for me (I thought, likely because of the existing code passing on the magic string, that some machinery was needed in BuildKit itself).

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

Asked @tonistiigi to give his blessing, but I guess we can already prepare a cherry-pick @crazy-max

@thaJeztah thaJeztah added this to the 25.0.0 milestone Jun 22, 2023
@thaJeztah
Copy link
Member

I'm gonna take @tonistiigi 's "thumbs up" on slack as an "LGTM", so lets bring this one in;
Screenshot 2023-06-22 at 22 55 45

@thaJeztah thaJeztah merged commit eb76c93 into moby:master Jun 22, 2023
@thaJeztah
Copy link
Member

thaJeztah commented Jun 22, 2023

@crazy-max crazy-max deleted the fix-host-gateway branch June 22, 2023 21:13
woodpecker-bot pushed a commit to woodpecker-ci/plugin-docker-buildx that referenced this pull request Oct 17, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [docker/buildx-bin](https://github.com/docker/buildx) | stage | patch | `0.11.0` -> `0.11.2` |

---

### Release Notes

<details>
<summary>docker/buildx (docker/buildx-bin)</summary>

### [`v0.11.2`](https://github.com/docker/buildx/releases/tag/v0.11.2)

[Compare Source](docker/buildx@v0.11.1...v0.11.2)

Welcome to the v0.11.2 release of buildx!

Please try out the release binaries and report any issues at https://github.com/docker/buildx/issues.

##### Contributors

-   [Justin Chadwell](https://github.com/jedevc)
-   [CrazyMax](https://github.com/crazy-max)
-   [Sebastiaan van Stijn](https://github.com/thaJeztah)

##### Changes

-   Fix a regression that caused buildx to not read the `KUBECONFIG` path from the instance store [#&#8203;1941](docker/buildx#1941)
-   Fix a regression with result handle builds showing up in the build history incorrectly [#&#8203;1954](docker/buildx#1954)

##### Dependency Changes

-   **github.com/docker/docker**          v24.0.2 -> [`36e9e79`](docker/buildx@36e9e796c6fc)
-   **github.com/moby/buildkit**          [`67a0862`](docker/buildx@67a08623b95a) -> [`faa0cc7`](docker/buildx@faa0cc7da353)
-   **github.com/tonistiigi/fsutil**      [`9e7a6df`](docker/buildx@9e7a6df48576) -> [`36ef4d8`](docker/buildx@36ef4d8c0dbb)
-   **github.com/xeipuuv/gojsonpointer**  [`4e3ac27`](docker/buildx@4e3ac2762d5f) -> [`02993c4`](docker/buildx@02993c407bfb)

Previous release can be found at [v0.11.1](https://github.com/docker/buildx/releases/tag/v0.11.1)

### [`v0.11.1`](https://github.com/docker/buildx/releases/tag/v0.11.1)

[Compare Source](docker/buildx@v0.11.0...v0.11.1)

Welcome to the v0.11.1 release of buildx!

Please try out the release binaries and report any issues at https://github.com/docker/buildx/issues.

##### Contributors

-   [CrazyMax](https://github.com/crazy-max)
-   [Justin Chadwell](https://github.com/jedevc)
-   [David Karlsson](https://github.com/dvdksn)
-   [Jhan S. Álvarez](https://github.com/yastanotheruser)

##### Changes

-   Fix a regression for bake where services in profiles would not be loaded. [#&#8203;1903](docker/buildx#1903)

-   Fix a regression where `--cgroup-parent` option had no effect during build. [#&#8203;1913](docker/buildx#1913)

-   Fix a regression where valid docker contexts could fail buildx builder name validation. [#&#8203;1879](docker/buildx#1879)

-   Fix an issue where the `host-gateway` special address could not be used as an argument to `--add-host`. [#&#8203;1894](docker/buildx#1894) (also requires moby/moby#45767)

-   Fix a possible panic when terminal is resized during the build. [#&#8203;1929](docker/buildx#1929)

##### Dependency Changes

-   **github.com/docker/cli-docs-tool**  v0.5.1 -> v0.6.0

Previous release can be found at [v0.11.0](https://github.com/docker/buildx/releases/tag/v0.11.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

��� **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMy4wIiwidXBkYXRlZEluVmVyIjoiMzcuMjQuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Reviewed-on: https://codeberg.org/woodpecker-plugins/docker-buildx/pulls/85
Co-authored-by: Patrick Schratz <pat-s@mailbox.org>
Co-committed-by: Patrick Schratz <pat-s@mailbox.org>
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.

3 participants