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

Avoid returning early on agent join failures #1473

Merged
merged 1 commit into from
Sep 27, 2016
Merged

Conversation

mrjana
Copy link
Contributor

@mrjana mrjana commented Sep 27, 2016

When a gossip join failure happens do not return early in the call chain because a join failure is most likely transient and the retry logic built in the networkdb is going to retry and succeed. Returning early makes the initialization of ingress network/sandbox to not happen which causes a problem even after the gossip join on retry is successful.

Signed-off-by: Jana Radhakrishnan mrjana@docker.com

@@ -192,7 +192,6 @@ func (c *controller) agentSetup() error {
if remoteAddr != "" {
if err := c.agentJoin(remoteAddr); err != nil {
logrus.Errorf("Error in agentJoin : %v", err)
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not returning the error on the first place (returning nil). Am not sure how this will fix the issue mentioned in the description. The only possible effect this has is the fact that agentInitDone is not reinitialized. Is that the issue that you are intending to fix ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we return early agentInitDone is not closed. If it is not closed the go routing waiting on it to proceed to plumbing the ingress sandbox will not happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Just noticed that even though the networkDB retry logic takes effect, call to nDB.sendNodeEvent is not involved in that retry. am not sure if that is required. Just giving my observation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not strictly needed as the other node will be able to work with just getting a join notification from memberlist. But I think it is still correct to make that fix. Will update the PR.

@mrjana mrjana changed the title Avoid propogating agent join failures Avoid returning early on agent join failures Sep 27, 2016
@sanimej
Copy link

sanimej commented Sep 27, 2016

In what cases do we see gossip join failure ? Since we already got the keys the gRPC session should be working.

@mrjana
Copy link
Contributor Author

mrjana commented Sep 27, 2016

@sanimej When we are restarting all the nodes in the cluster at the same time the daemon which joined the cluster using swarm join might try to join the node which inited the cluster using swarm init before it has reached the part where it has initialized gossip. In that case this can happen.

When a gossip join failure happens do not return early in the call chain
because a join failure is most likely transient and the retry logic
built in the networkdb is going to retry and succeed. Returning early
makes the initialization of ingress network/sandbox to not happen which
causes a problem even after the gossip join on retry is successful.

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
@sanimej
Copy link

sanimej commented Sep 27, 2016

LGTM. As a side note we don't need NodeEventTypeJoin since the memberlist join is sufficient.

@mavenugo
Copy link
Contributor

LGTM.

@mavenugo mavenugo merged commit f1aa086 into moby:master Sep 27, 2016
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.

4 participants