-
Notifications
You must be signed in to change notification settings - Fork 615
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
Convert code to use connectionbroker package #1851
Conversation
ping @cyli @dongluochen @LK4D4 |
} | ||
}(ready) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the bit that all of this change was made to refactor out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes connections to the local manager use a unix socket or similar local connection (via connectionbroker
) instead of connecting over TCP. As a consequence, this code, which finds the manager's IP address and adds it to the Remotes
list to bootstrap Remotes
, isn't necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see.
This all looks good to me. |
LGTM |
}), | ||
) | ||
if err != nil { | ||
logger.WithError(err).Errorf("failed to connect to CA after locking the cluster") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this say "failed to connect to local manager socket after locking the cluster"? The manager may not be a CA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the message.
LGTM |
By using connectionbroker instead of using remotes directly, subsystems like the agent can connect directly to the local manager when running on a node that acts as a manager. This avoids the need for the manager to expose a TCP port at all times. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
96b5974
to
d15f8ca
Compare
By using
connectionbroker
instead of usingremotes
directly, subsystemslike the agent can connect directly to the local manager when running on
a node that acts as a manager. This avoids the need for the manager to
expose a TCP port at all times.