Skip to content
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

Allow network with --config-from to be --internal #2414

Merged
merged 1 commit into from
Jul 15, 2019

Conversation

lemrouch
Copy link
Contributor

The --internal netlabel is discarded now.

Signed-off-by: Pavel Matěja [email protected]

Fix #2413

controller.go Outdated Show resolved Hide resolved
The --internal netlabel is discarded now.

Signed-off-by: Pavel Matěja <[email protected]>
@arkodg
Copy link
Contributor

arkodg commented Jul 11, 2019

@lemrouch I ran the following commands (without this patch)

~$ docker network create --config-only --subnet 192.168.22.0/24 --gateway 192.168.22.1 -o parent=eth0 --ip-range 192.168.22.32/27 dkrdev-config
7a6f0f6da870ef1cac9b1d5fb57bfa6e345c4234472c0eaa2f8e7fad3d58d855
~$ docker network create -d macvlan --scope swarm --config-from dkrdev-config --internal dkrdev
crsdyefvbtjnp5u4vhs7y1bm1
~$ docker network inspect dkrdev
[
    {
        "Name": "dkrdev",
        "Id": "crsdyefvbtjnp5u4vhs7y1bm1",
        "Created": "2019-07-11T20:59:03.817246Z",
        "Scope": "swarm",
        "Driver": "macvlan",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "",
            "Options": null,
            "Config": []
        },
        "Internal": true,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": "dkrdev-config"
        },
        "ConfigOnly": false,
        "Containers": null,
        "Options": null,
        "Labels": null
    }
]

and I can see the Internal field set to true, am I missing something ?

@arkodg
Copy link
Contributor

arkodg commented Jul 11, 2019

okay you're right :), I see the issue, we override the config in applyConfigurationTo, I think inside of adding code inside NewNetwork we should edit applyConfigurationTo to skip copying over netlabel.Internal

@lemrouch
Copy link
Contributor Author

lemrouch commented Jul 15, 2019

I'm not quite sure.
There is to.generic = options.Generic{} which means not copying the netlabel.Internal will end up in missing it completelly.
We will have to check if there is some netlabel.Internal original value in to network first, then save it and reapply in such case.
This kind code there won't make much sense without context. Why would we try to save one netlabel in network class?
But I'm willing to try if you insist.

@arkodg
Copy link
Contributor

arkodg commented Jul 15, 2019

applyConfigurationTo seems to be only used for ConfigFrom networks today, adding more code into the controller's NewNetwork will make it less readable .
Wouldn't something like this work in applyConfigurationTo

		for k, v := range n.generic {
                        // Skip network operator configurations
                        if k != netlabel.Internal {
			    to.generic[k] = v
                        }
		}

@lemrouch
Copy link
Contributor Author

I'm not really familiar with golang but I think because of network.go:450 to.generic = options.Generic{} you won't have the original value of to.generic[netlabel.Internal] anymore.

@arkodg
Copy link
Contributor

arkodg commented Jul 15, 2019

@lemrouch good catch, I missed that !

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM

@arkodg
Copy link
Contributor

arkodg commented Jul 15, 2019

PTAL @selansen

Copy link
Collaborator

@selansen selansen left a comment

Choose a reason for hiding this comment

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

LGTM

@selansen selansen merged commit 3fb133e into moby:master Jul 15, 2019
@lemrouch lemrouch deleted the 2413-fix branch July 15, 2019 22:46
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Jul 30, 2019
full diff: moby/libnetwork@83d30db...09cdcc8

changes included:

- moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong
  - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong
- moby/libnetwork#2414 Allow network with --config-from to be --internal
  - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal
- moby/libnetwork#2351 Use fewer modprobes
  - relates to moby#38930 Use fewer modprobes
- moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks
  - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle
  - fixes moby/libnetwork#1743 Phantom docker network

Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Jul 31, 2019
full diff: moby/libnetwork@83d30db...09cdcc8

changes included:

- moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong
  - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong
- moby/libnetwork#2414 Allow network with --config-from to be --internal
  - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal
- moby/libnetwork#2351 Use fewer modprobes
  - relates to moby/moby#38930 Use fewer modprobes
- moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks
  - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle
  - fixes moby/libnetwork#1743 Phantom docker network

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: 6f234db9fef23c591d8376f96db062e7107b658f
Component: engine
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 16, 2019
full diff: moby/libnetwork@83d30db...09cdcc8

changes included:

- moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong
  - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong
- moby/libnetwork#2414 Allow network with --config-from to be --internal
  - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal
- moby/libnetwork#2351 Use fewer modprobes
  - relates to moby#38930 Use fewer modprobes
- moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks
  - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle
  - fixes moby/libnetwork#1743 Phantom docker network

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 6f234db)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 17, 2019
full diff: moby/libnetwork@83d30db...09cdcc8

changes included:

- moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong
  - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong
- moby/libnetwork#2414 Allow network with --config-from to be --internal
  - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal
- moby/libnetwork#2351 Use fewer modprobes
  - relates to moby/moby#38930 Use fewer modprobes
- moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks
  - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle
  - fixes moby/libnetwork#1743 Phantom docker network

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 6f234db9fef23c591d8376f96db062e7107b658f)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: b6190c2713623ab455d29da4771b684e4eafc63f
Component: engine
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
full diff: moby/libnetwork@83d30db...09cdcc8

changes included:

- moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong
  - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong
- moby/libnetwork#2414 Allow network with --config-from to be --internal
  - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal
- moby/libnetwork#2351 Use fewer modprobes
  - relates to moby#38930 Use fewer modprobes
- moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks
  - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle
  - fixes moby/libnetwork#1743 Phantom docker network

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: zach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Network with --config-from does not honor --internal
3 participants