api: Add a field MacAddress to EndpointSettings #45905

akerouanton · 2023-07-07T17:08:17Z

Fix:

fixes [Feature Request] Set mac address for multiple network interfaces #34720

Related to:

- What I did

Add a new MacAddress field to EndpointSettings ;
Deprecate the old MacAddress in container's Config ;

- Description for the changelog

A MAC address can now be specified per-endpoint when creating a container.

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

thaJeztah

Yikes... looks like I didn't submit my review 🙈 😂

daemon/inspect.go

daemon/inspect_linux.go

api/server/router/container/container_routes.go

thaJeztah · 2023-07-29T13:58:08Z

api/server/router/container/container_routes.go

@@ -608,6 +609,19 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 		}
 	}

+	if config != nil && versions.LessThan(version, "1.44") {


Do we need to make changes in the swarm (service create, service update, service inspect) endpoints as well?

Yep, as well as the Compose specification. I have WIP branches for that, but other PRs I created in the past few weeks will also need to update the swarmkit protobuf. So, I'll do that in follow-up PRs.

daemon/network.go

integration-cli/docker_cli_netmode_test.go

thaJeztah · 2023-08-01T14:55:00Z

Ah, right, so some commits are also part of Add missing opts to --network advanced syntax docker/cli#4419, which is where I submitted my review on.. and GitHub showed them here as "pending"

thaJeztah · 2023-08-01T15:02:06Z

OOHH.... no it was more fun... I posted my review comments here (but didn't submit) and accidentally submitted "a review" on the CLI PR related to this docker/cli#4419 (review) 😂

Guess I had the wrong tab open when I did.

OHMAN this one is complicated I tried to make changes what I (think) is needed while reviewing, and opened a PR;

review suggestions akerouanton/docker#3

However, it may need checking, and there's some other comments I left (including "migrating containers during restore"), > which I didn't look into yet.

Following flags are silently ignored when they're passed with no `--network` specified (ie. when the default network is used): - `--network-alias` - `--ip` - `--ip6` - `--link-local-ip` This is not really an issue right now since the first 3 parameters are not allowed on the default bridge network. However, with [moby/moby#45905][1], the container-wide MacAddress parameter will be deprecated and dismissed. Because of that, with [docker#4419][2], it's currently not possible to use the `--mac-address` flag for the default network. Morever, `docker network connect --link-local-ip ...` works properly, so it should also work on `docker container create`. This also lay the ground for making the default bridge network just a "normal" network. [1]: moby/moby#45905 [2]: docker#4419 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Following flags are silently ignored when they're passed with no `--network` specified (ie. when the default network is used): - `--network-alias` - `--ip` - `--ip6` - `--link-local-ip` This is not really an issue right now since the first 3 parameters are not allowed on the default bridge network. However, with [moby/moby#45905][1], the container-wide MacAddress parameter will be deprecated and dismissed. Because of that, with [docker#4419][2], it's currently not possible to use the `--mac-address` flag with no default network specified. Morever, `docker network connect --link-local-ip ...` works properly, so it should also work on `docker container create`. This also lay the ground for making the default bridge network just a "normal" network. [1]: moby/moby#45905 [2]: docker#4419 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Following flags are silently ignored when they're passed with no `--network` specified (ie. when the default network is used): - `--network-alias` - `--ip` - `--ip6` - `--link-local-ip` This is not really an issue right now since the first 3 parameters are not allowed on the default bridge network. However, with [moby/moby#45905][1], the container-wide MacAddress parameter will be deprecated and dismissed. Because of that, with [docker#4419][2], it's currently not possible to use the `--mac-address` flag with no default network specified. Morever, `docker network connect --link-local-ip ...` works properly, so it should also work on `docker container create`. This also lay the ground for making the default bridge network just a "normal" network. Since the 3 parameters in the list above aren't ignored anymore, if users provide them, moby's ContainerStart endpoint will complain about those. To provide better UX, [moby/moby#46183][3] make sure these invalid parameters lead to a proper error message on `docker container create` / `docker run`. [1]: moby/moby#45905 [2]: docker#4419 [3]: moby/moby#46183 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

thaJeztah · 2023-11-07T15:37:23Z

Let me bring this one in

Since moby/moby#45905 has been merged (will be part of the upcoming v25 release), it's now possible to specify an endpoint-specific MAC address. Moreover, setting the container-wide MAC address will return a warning. Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Related to: - moby/moby#45905 - compose-spec/compose-spec#435 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Since moby/moby#45905 has been merged (will be part of the upcoming v25 release), it's now possible to specify an endpoint-specific MAC address. Moreover, setting the container-wide MAC address will return a warning. Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Related to: - moby/moby#45905 - compose-spec/compose-spec#435 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Since moby/moby#45905 has been merged (will be part of the upcoming v25 release), it's now possible to specify an endpoint-specific MAC address. Moreover, setting the container-wide MAC address will return a warning. Signed-off-by: Albin Kerouanton <albinker@gmail.com>

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>

akerouanton added area/api API status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/networking Networking impact/api impact/changelog labels Jul 7, 2023

akerouanton requested review from corhere, neersighted and thaJeztah July 7, 2023 17:08

akerouanton added this to the 25.0.0 milestone Jul 7, 2023

akerouanton force-pushed the endpoint-specific-mac-address branch from ffa33a0 to 258958a Compare July 7, 2023 17:11

akerouanton marked this pull request as draft July 7, 2023 17:16

akerouanton force-pushed the endpoint-specific-mac-address branch from 258958a to 58bf4e5 Compare July 7, 2023 17:22

akerouanton marked this pull request as ready for review July 7, 2023 18:01

akerouanton force-pushed the endpoint-specific-mac-address branch 3 times, most recently from 739a26a to b9df778 Compare July 13, 2023 12:18

akerouanton mentioned this pull request Jul 13, 2023

Add missing opts to --network advanced syntax docker/cli#4419

Merged

akerouanton mentioned this pull request Jul 27, 2023

25.0 Release Tracking #45926

Closed

26 tasks

thaJeztah reviewed Aug 1, 2023

View reviewed changes

thaJeztah mentioned this pull request Aug 1, 2023

review suggestions akerouanton/docker#3

Closed

akerouanton mentioned this pull request Aug 1, 2023

integration: Add RunAttach helper #46138

Merged

akerouanton mentioned this pull request Aug 10, 2023

Create default EndpointSettings if no --network provided docker/cli#4493

Merged

thaJeztah merged commit 49cea49 into moby:master Nov 7, 2023

thaJeztah mentioned this pull request Nov 7, 2023

vendor: github.com/docker/docker c14694a424ab (v25.0.0-dev) docker/cli#4644

Merged

akerouanton deleted the endpoint-specific-mac-address branch November 7, 2023 17:00

akerouanton mentioned this pull request Nov 8, 2023

Add a network-scoped mac_address field compose-spec/compose-spec#435

Merged

akerouanton added a commit to akerouanton/compose-go that referenced this pull request Nov 20, 2023

Add a network-scoped mac_address field

973331b

Related to: - moby/moby#45905 - compose-spec/compose-spec#435 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

akerouanton mentioned this pull request Nov 20, 2023

Add a network-scoped mac_address field compose-spec/compose-go#491

Merged

akerouanton added a commit to akerouanton/compose-go that referenced this pull request Nov 20, 2023

Add a network-scoped mac_address field

7c47367

Related to: - moby/moby#45905 - compose-spec/compose-spec#435 Signed-off-by: Albin Kerouanton <albinker@gmail.com>

akerouanton mentioned this pull request Nov 20, 2023

Add support for endpoint-specific MAC address docker/compose#11208

Merged

robmry mentioned this pull request Jan 22, 2024

Remove generated MAC addresses on restart. #47168

Merged

thaJeztah mentioned this pull request Jan 22, 2024

[25.0 backport] Remove generated MAC addresses on restart. #47171

Merged

robmry mentioned this pull request Jan 26, 2024

Networking between containers in custom network breaks after time #47211

Open

robmry mentioned this pull request Feb 29, 2024

Don't create endpoint config for MAC addr config migration #47474

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

api: Add a field MacAddress to EndpointSettings #45905

api: Add a field MacAddress to EndpointSettings #45905

Uh oh!

akerouanton commented Jul 7, 2023 •

edited by thaJeztah

Loading

Uh oh!

thaJeztah left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thaJeztah Jul 29, 2023

Uh oh!

akerouanton Aug 4, 2023

Uh oh!

Uh oh!

Uh oh!

thaJeztah commented Aug 1, 2023

Uh oh!

thaJeztah commented Aug 1, 2023

Uh oh!

thaJeztah commented Nov 7, 2023

Uh oh!

Uh oh!

api: Add a field MacAddress to EndpointSettings #45905

api: Add a field MacAddress to EndpointSettings #45905

Uh oh!

Conversation

akerouanton commented Jul 7, 2023 • edited by thaJeztah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thaJeztah Jul 29, 2023

Choose a reason for hiding this comment

Uh oh!

akerouanton Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thaJeztah commented Aug 1, 2023

Uh oh!

thaJeztah commented Aug 1, 2023

Uh oh!

thaJeztah commented Nov 7, 2023

Uh oh!

Uh oh!

akerouanton commented Jul 7, 2023 •

edited by thaJeztah

Loading