-
Notifications
You must be signed in to change notification settings - Fork 18.8k
builder: handle host-gateway with extra hosts #44537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9b8423a
to
e07e26a
Compare
Ah, yes, see docker/cli#2337 Although, I'm wondering if we should perform validation at all on the client side (basically; perhaps it should always be delegated to the API server to consider if options are valid or not) |
Yes I think it would be better, we already validate extra hosts on BuildKit: https://github.com/moby/buildkit/blob/7804e2c7fc09b4a536934f5607698bc3255cf0ed/frontend/dockerfile/builder/build.go#L751-L775 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but didn't test and kind of unfortunate to have two copies of toBuildkitExtraHosts
(here and in buildx). Not sure if they can factored since there is currently no way for buildx to retrieve the host-gateway-ip setting from the daemon (not even exposed in /info).
I think this feature would currently only apply to the docker builder, unless BuildKit itself (running as container-builder) would implement a configuration for this, in which case it would (should?) also do the conversion on the daemon side. As mentioned in an earlier comment, I think we should remove validation from the client side altogether (other than splitting |
I discussed about this with @tonistiigi yesterday and the validation client-side avoid sending extra build request. Edit: opened follow-up on buildx repo: docker/buildx#1446 |
3537d0f
to
f885882
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
f885882
to
521b8c0
Compare
fixes #44525
- What I did
If the IP Address is a string called
host-gateway
, replace this value with the IP address stored in the daemon levelHostGatewayIP
config variable.This is the current behavior with the classic builder.
Keeping in draft for now as it's still subject to discussion.Need changes in Buildx client: https://github.com/docker/buildx/blob/23cabd67fb1c5e7ba2e3765e91bd5cc0f0311337/build/utils.go#L41-L56 cc @tonistiigi @jedevc
- How I did it
- How to verify it
- Description for the changelog
handle
host-gateway
with extra hosts.- A picture of a cute animal (not mandatory but encouraged)
Signed-off-by: CrazyMax crazy-max@users.noreply.github.com