Skip to content

Add NetworkMode to bake target#863

Merged
tonistiigi merged 3 commits intodocker:masterfrom
zachary-povey:support_host_network_in_compose
Dec 5, 2021
Merged

Add NetworkMode to bake target#863
tonistiigi merged 3 commits intodocker:masterfrom
zachary-povey:support_host_network_in_compose

Conversation

@zachary-povey
Copy link
Copy Markdown
Contributor

@zachary-povey zachary-povey commented Nov 25, 2021

Allows specification of network mode in a bake target.

Fixes #848

Signed-off-by: Zachary Povey [email protected]

Add NetworkMode to bake target
ae53101 
Allows specification of network mode in a bake target.

Fixes docker#848

Signed-off-by: Zachary Povey <[email protected]>
@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Nov 26, 2021

I wonder if we should leave it only for compose files for now. Using build --network is discouraged in buildx. Instead you are supposed to use RUN --network inside the dockerfile https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/syntax.md#network-modes-run---networknonehostdefault and allow network entitlement on build. Proposal for entitlements support in bake is in #179

@zachary-povey
Copy link
Copy Markdown
Contributor Author

zachary-povey commented Nov 29, 2021 •
edited
Loading

@tonistiigi - happy to change it to just compose files, should I do that?

My use case for this is backwards compatibility with docker-compose anyway, it just seemed (from a naive perspective) if something was available to build it should be available via a bake target.

@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Nov 29, 2021

happy to change it to just compose files, should I do that?

I think so. cc @crazy-max

@crazy-max
Copy link
Copy Markdown
Member

crazy-max commented Nov 30, 2021 •
edited
Loading

happy to change it to just compose files, should I do that?

I think so. cc @crazy-max

Yes sgtm

@zachary-povey
Copy link
Copy Markdown
Contributor Author

zachary-povey commented Nov 30, 2021

Ok, thanks both :)

@tonistiigi I've pushed a commit removing support from HCL/JSON files. Did you also want me to remove the ability to specify it via a cmd line override with --set?

crazy-max
crazy-max reviewed Dec 1, 2021
View reviewed changes
Comment thread bake/bake.go Outdated
Comment on lines +570 to +571
case "network":
t.NetworkMode = &o.Value
Copy link
Copy Markdown
Member

@crazy-max crazy-max Dec 1, 2021

Choose a reason for hiding this comment

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

Did you also want me to remove the ability to specify it via a cmd line override with --set?

Yes remove the override for this case.

Copy link
Copy Markdown
Contributor Author

@zachary-povey zachary-povey Dec 1, 2021

Choose a reason for hiding this comment

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

Ok that's done now 👍

@zachary-povey
Copy link
Copy Markdown
Contributor Author

zachary-povey commented Dec 2, 2021

@tonistiigi @crazy-max - think this is good to go now?

crazy-max
crazy-max approved these changes Dec 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM

@tonistiigi tonistiigi merged commit 7371dda into docker:master Dec 5, 2021
@crazy-max crazy-max mentioned this pull request Dec 10, 2021
Merged
@crazy-max crazy-max added this to the v0.8.0 milestone Jan 26, 2022
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0.8.0

Development

Successfully merging this pull request may close these issues.

buildx bake not honouring docker-compose build network mode host

3 participants

@zachary-povey @tonistiigi @crazy-max