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

Fix for problem where agent is stopped and does not restart #2307

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

kylewuolle
Copy link
Contributor

This fixes a problem where the agent on a controller is stopped when a node leaves a swarm and is never restarted. By adding a check to see if the agent is nil the agent init routine will run and the other checks that guard against multiple agents will still function as intended. Here's some more information about how this problem presents itself docker/for-linux#495.

@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 "fix-agent-init-problem" git@github.com:kylewuolle/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.

@thaJeztah
Copy link
Member

ping @fcrisciani @euank ptal

@thaJeztah
Copy link
Member

whoops, wrong Euan. Meant @euanh 😊

controller.go Outdated Show resolved Hide resolved
@selansen
Copy link
Collaborator

Can you pls fix below error
^@^@🐳 check-protobuf
🐳 lint
🐳 gosimple
🐳 vet
🐳 ineffassign
🐳 fmt
controller.go
👹 please format Go code with 'gofmt -s -w'
Makefile:155: recipe for target 'fmt' failed
make: *** [fmt] Error 1
Makefile:114: recipe for target 'check' failed
make: *** [check] Error 2
Exited with code 2

@kylewuolle
Copy link
Contributor Author

ping @fcrisciani @euanh!

controller.go Outdated
@@ -268,7 +268,7 @@ func (c *controller) SetClusterProvider(provider cluster.Provider) {
}
c.Unlock()

if provider == nil || sameProvider {
if provider == nil || (sameProvider && c.getAgent() != nil) {

Choose a reason for hiding this comment

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

if I understood well, basically when the node leave the cluster never reset the cluster provider. If so, I will find much better to just reset the cluster provider to nil when the node leave the cluster.
That way sameProvider will be false and the init will continue. What do you think? is it my understanding correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might work as well. I'll test it out and see!

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if you review moby or not but I've made a PR over there that fixes it in a different way.
moby/moby#38626

@fcrisciani
Copy link

fcrisciani commented Jan 25, 2019

@kylewuolle if you can rebase I have fixed that lint failure.
also can you confirm that is fixing the issue? If so, LGTM
last thing, squash all the changes into 1 single signed commit

@kylewuolle
Copy link
Contributor Author

Will do thanks! It does for sure fix the issue!

Signed-off-by: Kyle Wuolle <kyle.wuolle@gmail.com>
@kylewuolle
Copy link
Contributor Author

@kylewuolle if you can rebase I have fixed that lint failure.
also can you confirm that is fixing the issue? If so, LGTM
last thing, squash all the changes into 1 single signed commit

All done thanks!

Copy link

@fcrisciani fcrisciani left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

@fcrisciani fcrisciani merged commit d8d4c8c into moby:master Jan 28, 2019
@fcrisciani
Copy link

fcrisciani commented Jan 28, 2019

@thaJeztah to which branch do we need this one to go?

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Apr 1, 2019
full diff: moby/libnetwork@1a06131...ebcade7

relevant changes:

- moby/libnetwork#2349 IPVS: Add support for GetConfig/SetConfig
- moby/libnetwork#2343 Revert "debian has iptables-legacy and iptables-nft now"
- moby/libnetwork#2230 Moving IPVLAN driver out of experimental
- moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- moby/libnetwork#2303 Touch-up error-message and godoc for ConfigVXLANUDPPort
- moby/libnetwork#2325 Fix possible nil pointer exception
- moby/libnetwork#2302 Use sync.RWMutex for VXLANUDPPort
- moby/libnetwork#2306 Improve error if auto-selecting IP-range failed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 3, 2019
full diff: moby/libnetwork@1a06131...ebcade7

relevant changes:

- moby/libnetwork#2349 IPVS: Add support for GetConfig/SetConfig
- moby/libnetwork#2343 Revert "debian has iptables-legacy and iptables-nft now"
- moby/libnetwork#2230 Moving IPVLAN driver out of experimental
- moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- moby/libnetwork#2303 Touch-up error-message and godoc for ConfigVXLANUDPPort
- moby/libnetwork#2325 Fix possible nil pointer exception
- moby/libnetwork#2302 Use sync.RWMutex for VXLANUDPPort
- moby/libnetwork#2306 Improve error if auto-selecting IP-range failed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 3ab093d5670e8d59f6ae0c4604b8fcabf1582854
Component: engine
adhulipa pushed a commit to adhulipa/docker that referenced this pull request Apr 11, 2019
full diff: moby/libnetwork@1a06131...ebcade7

relevant changes:

- moby/libnetwork#2349 IPVS: Add support for GetConfig/SetConfig
- moby/libnetwork#2343 Revert "debian has iptables-legacy and iptables-nft now"
- moby/libnetwork#2230 Moving IPVLAN driver out of experimental
- moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- moby/libnetwork#2303 Touch-up error-message and godoc for ConfigVXLANUDPPort
- moby/libnetwork#2325 Fix possible nil pointer exception
- moby/libnetwork#2302 Use sync.RWMutex for VXLANUDPPort
- moby/libnetwork#2306 Improve error if auto-selecting IP-range failed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Apr 23, 2019
full diff: moby/libnetwork@c902989...872f0a8

- moby/libnetwork#2354 [18.09 backport] Cleanup the cluster provider when the agent is closed
  - backport of moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
  - fixes docker/for-linux#495 Docker swarm overlay networking not working after --force-new-cluster
- moby/libnetwork#2369 [18.09 BACKPORT] Pick a random host port if the user does not specify a host port
  - backport of moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 23, 2019
full diff: moby/libnetwork@c902989...872f0a8

- moby/libnetwork#2354 [18.09 backport] Cleanup the cluster provider when the agent is closed
  - backport of moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
  - fixes docker/for-linux#495 Docker swarm overlay networking not working after --force-new-cluster
- moby/libnetwork#2369 [18.09 BACKPORT] Pick a random host port if the user does not specify a host port
  - backport of moby/libnetwork#2368 (windows) Pick a random host port if the user does not specify a host port

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 5354408039681020f9ad6afe4bf696fc90f9ce69
Component: engine
kiku-jw pushed a commit to kiku-jw/moby that referenced this pull request May 16, 2019
full diff: moby/libnetwork@1a06131...ebcade7

relevant changes:

- moby/libnetwork#2349 IPVS: Add support for GetConfig/SetConfig
- moby/libnetwork#2343 Revert "debian has iptables-legacy and iptables-nft now"
- moby/libnetwork#2230 Moving IPVLAN driver out of experimental
- moby/libnetwork#2307 Fix for problem where agent is stopped and does not restart
- moby/libnetwork#2303 Touch-up error-message and godoc for ConfigVXLANUDPPort
- moby/libnetwork#2325 Fix possible nil pointer exception
- moby/libnetwork#2302 Use sync.RWMutex for VXLANUDPPort
- moby/libnetwork#2306 Improve error if auto-selecting IP-range failed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

5 participants