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

phantom ip/mac vlan network after a powercycle #2295

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rnataraja
Copy link

I am trying to address #1743 as part of this PR.

There are two issues as part of this

  1. When a node is power cycled unplanned or dockerd goes through a restart (unplanned), swarm scope networks are not cleaned up, Infact they are recreated when the docker daemon starts. This recreate does not work as there is already a swarmScope network has been restored and uses the same uplink.
    FIX: there was an earlier PR that tackled this problem by simply deleting the network if its created with same ID. As per comments in that PR, it was preferred to re-use instead of delete. In this PR, I am trying to essentially re-use the same network ID.

  2. Even if the swarm Scope network can be recreated there is still a problem with config-only network. As there is an additional createNetwork after the docker daemon restarts. The endpoint count on the configOnly network becomes more than what it should be. Consider the case where node has been kicked out of swarm during this unplanned reboot and added back in.
    I believe a swarm scope network to its config-only network is a one to one mapping for a particular worker node. If thats not the case, then more work is required for this PR. But if that is the case, there is no real need to track the endpoint count.
    FIX: Upon deleteNetwork of a swarm scope network, the correspnding configOnly Network endpoint count is zeroed out essentially making configOnly network as deleteable.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "issue1743" [email protected]:rnataraja/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@rnataraja
Copy link
Author

@fcrisciani what are your thoughts on these changes.

@swift1911
Copy link

is any progress about this PR?

@Greatsamps
Copy link

still present in 18.09.1

@itsgk92
Copy link

itsgk92 commented Feb 4, 2019

+1

@MalMen
Copy link

MalMen commented Feb 26, 2019

still on 18.09.2

@lukicsl
Copy link

lukicsl commented Apr 14, 2019

+1

@@ -1051,7 +1051,13 @@ func (n *network) delete(force bool, rmLBEndpoint bool) error {

if n.ConfigFrom() != "" {
if t, err := c.getConfigNetwork(n.ConfigFrom()); err == nil {
if err := t.getEpCnt().DecEndpointCnt(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

for bridge networks , multiple networks can be created from a config (--config-from), so this will break that case

Copy link
Contributor

@arkodg arkodg May 9, 2019

Choose a reason for hiding this comment

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

@@ -60,10 +60,14 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo
// empty parent and --internal are handled the same. Set here to update k/v
config.Internal = true
}
err = d.createNetwork(config)
foundExisting, err := d.createNetwork(config)
Copy link
Contributor

Choose a reason for hiding this comment

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

for the re-create path, can you please add some integration test-cases, either in libnetwork or moby (https://github.com/moby/moby/blob/master/integration/network/service_test.go) . A SIGKILL on dockerd should be enough to reproduce this case

@arkodg
Copy link
Contributor

arkodg commented Jun 21, 2019

@rnataraja still there ? :)

arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Jul 10, 2019
This commit carries forward the work done in
moby#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Signed-off-by: Arko Dasgupta <[email protected]>
@arkodg
Copy link
Contributor

arkodg commented Jul 10, 2019

thanks for driving this issue @rnataraja , I'll be taking this PR forward using #2415

arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Jul 10, 2019
This commit carries forward the work done in
moby#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Signed-off-by: Arko Dasgupta <[email protected]>
@rnataraja
Copy link
Author

thanks for driving this issue @rnataraja , I'll be taking this PR forward using #2415

Thanks @arkodg

arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Jul 15, 2019
This commit carries forward the work done in
moby#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Addresses: moby#1743

Signed-off-by: Arko Dasgupta <[email protected]>
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
arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Aug 15, 2019
This commit carries forward the work done in
moby#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Addresses: moby#1743

Signed-off-by: Arko Dasgupta <[email protected]>
arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Aug 15, 2019
This commit carries forward the work done in
moby#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Addresses: moby#1743

Signed-off-by: Arko Dasgupta <[email protected]>
@thaJeztah
Copy link
Member

@arkodg @selansen this was carried in #2415, which is now merged, so this one can be closed

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]>
cpuguy83 pushed a commit to cpuguy83/docker that referenced this pull request May 25, 2021
This commit carries forward the work done in
moby/libnetwork#2295
and fixes two things
1. Allows macvlan and ipvlan to be restored properly
after dockerd or the system is restarted
2. Makes sure the refcount for the configOnly network
is not incremented for the above case so this network
can be deleted after all the associated ConfigFrom networks
are deleted

Addresses: moby/libnetwork#1743

Signed-off-by: Arko Dasgupta <[email protected]>
@IGitYou
Copy link

IGitYou commented Jul 15, 2023

I am still seeing this Version: 24.0.4

Subscribed to watch for a fix.

@thaJeztah
Copy link
Member

/cc @akerouanton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.