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 support for endpoint-specific MAC address #11208
Add support for endpoint-specific MAC address #11208
Conversation
71513d4
to
6db3b46
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, please rebase so we can merge
pkg/compose/create.go
Outdated
|
||
// TODO(aker): maybe show a warning and reset all mac_address fields except for the main endpoint? | ||
if len(withMacAddress) > 1 { | ||
return createConfigs{}, fmt.Errorf("a MAC address is specified for multiple networks (%s), but the target Docker Engine is too old to support that", strings.Join(withMacAddress, ", ")) |
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.
better let user know the exact version needed, see https://github.com/docker/compose/pull/10939/files#diff-afca7d69550488d990f7db87d3d979a6980f6029c8c86e6b24e26858e87aa70cR77
Related to: - compose-spec/compose-spec#435 - moby/moby#45905 Since API v1.44, Moby supports a per-endpoint MAC address and returns a warning when the container-wide mac_address field is set. A corresponding field has been added to compose-spec and compose-go, so we need to leverage it to set the right API field. This commit is backward-compatible with compose files that still set the container-wide mac_address field, and older API versions that don't know about the endpoint-specific MAC address field. Signed-off-by: Albin Kerouanton <albinker@gmail.com>
6db3b46
to
05c047d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #11208 +/- ##
==========================================
+ Coverage 56.20% 56.34% +0.14%
==========================================
Files 134 134
Lines 11564 11604 +40
==========================================
+ Hits 6499 6538 +39
+ Misses 4421 4419 -2
- Partials 644 647 +3 ☔ View full report in Codecov by Sentry. |
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
Related to:
Since API v1.44, Moby supports a per-endpoint MAC address and returns a warning when the container-wide mac_address field is set.
A corresponding field has been added to compose-spec and compose-go, so we need to leverage it to set the right API field.
This commit is backward-compatible with compose files that still set the container-wide mac_address field, and older API versions that don't know about the endpoint-specific MAC address field.
What I did
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did