-
Notifications
You must be signed in to change notification settings - Fork 496
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
Add NetworkMode to bake target #863
Add NetworkMode to bake target #863
Conversation
Allows specification of network mode in a bake target. Fixes docker#848 Signed-off-by: Zachary Povey <[email protected]>
I wonder if we should leave it only for compose files for now. Using |
@tonistiigi - happy to change it to just compose files, should I do that? My use case for this is backwards compatibility with |
I think so. cc @crazy-max |
Yes sgtm |
Signed-off-by: Zachary Povey <[email protected]>
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 |
bake/bake.go
Outdated
case "network": | ||
t.NetworkMode = &o.Value |
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.
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.
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.
Ok that's done now 👍
Signed-off-by: Zachary Povey <[email protected]>
@tonistiigi @crazy-max - think this is good to go now? |
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
Allows specification of network mode in a bake target.
Fixes #848
Signed-off-by: Zachary Povey [email protected]