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

Support dockerd and system restarts for ipvlan and macvlan networks #2415

Merged
merged 3 commits into from
Jul 26, 2019

Conversation

arkodg
Copy link
Contributor

@arkodg arkodg commented Jul 10, 2019

This commit carries forward the work done in
#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 arko.dasgupta@docker.com

@arkodg
Copy link
Contributor Author

arkodg commented Jul 10, 2019

PTAL @euanh @trapier

return false, err
}
// if driver created the networks slave link, record it for future deletion
config.CreatedSlaveLink = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am hoping CreateSlaveLink will be already set to true vlanLinkExists returns True.
If not, can you pls move this line out of if condition.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ip mac vlan code I see there is else condition with debug log "vlan already exists" . should we add similar log here for consistent purpose ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the missing logs, also config.CreatedSlaveLink is already set to true, that is why we don't set it for the restart case

}
config.CreatedSlaveLink = true
} else {
logrus.Debugf("Dummy Link %s for Mac Vlan already exists", getDummyName(stringid.TruncateID(config.ID)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we have . config.CreatedSlaveLink = true in this else loop too? ( same as ipvlan code )? just to make sure "config.CreatedSlaveLink is set to True.

drivers/ipvlan/ipvlan_network.go Show resolved Hide resolved
drivers/macvlan/macvlan_setup.go Outdated Show resolved Hide resolved
drivers/macvlan/macvlan_setup.go Outdated Show resolved Hide resolved
@selansen
Copy link
Collaborator

We need second eye to review this PR.

cc : @mavenugo

@thaJeztah
Copy link
Member

this addresses #1743 ?

@arkodg
Copy link
Contributor Author

arkodg commented Jul 12, 2019

@thaJeztah yes, thanks, I'll edit the commit with this link

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 <arko.dasgupta@docker.com>
@arkodg arkodg force-pushed the ipvlan-docker-restart-issue branch from 5479614 to 4c5094c Compare July 15, 2019 22:38
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
Copy link
Collaborator

@trapier Can you pls see if you can review it ?

@arkodg
Copy link
Contributor Author

arkodg commented Jul 24, 2019

PTAL @chiragtayal

Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
@selansen
Copy link
Collaborator

@chiragtayal , pls LGTM and approve it if you are ok.
We can merge this PR.

Copy link

@chiragtayal chiragtayal left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
@arkodg arkodg force-pushed the ipvlan-docker-restart-issue branch from c39a277 to 6d382cd Compare July 25, 2019 23:37
@selansen
Copy link
Collaborator

selansen commented Jul 26, 2019

@trapier as you already started reviewing, Pls let me know

@selansen selansen merged commit 09cdcc8 into moby:master Jul 26, 2019
@arkodg arkodg deleted the ipvlan-docker-restart-issue branch July 29, 2019 22:39
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 <github@gone.nl>
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 <github@gone.nl>
Upstream-commit: 6f234db9fef23c591d8376f96db062e7107b658f
Component: engine
@thaJeztah
Copy link
Member

thaJeztah commented Aug 22, 2019

relates to FIELD-211
relates to ENGCORE-960
relates to ENGCORE-692

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 <github@gone.nl>
(cherry picked from commit 6f234db)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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 <github@gone.nl>
(cherry picked from commit 6f234db9fef23c591d8376f96db062e7107b658f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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 <github@gone.nl>
Signed-off-by: zach <Zachary.Joyner@linux.com>
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.

None yet

4 participants