-
Notifications
You must be signed in to change notification settings - Fork 882
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
Add anonymous container alias to service record on attachable network #1651
Conversation
n := ep.getNetwork() | ||
if !n.isClusterEligible() { | ||
return nil | ||
} | ||
|
||
c := n.getController() | ||
agent := c.getAgent() | ||
if !ep.isAnonymous() && ep.Iface().Address() != nil { |
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.
ep.isAnonymous() check has been moved up, which is needed. But why is the nil check for
ep.Iface().Address()` removed ?
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.
Thanks for catching this. I think it just got lost in the change.
I am not sure if the address can ever be nil, but better to restore the original check.
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.
yeah. Since its an explicit check I assume there might be cases where its nil.
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.
Fixed, PTAL
- Currently when a non-named container with network aliases is connected to a swarm attachable network, its aliases are not added to the service records. This is not in line with what we do when connecting to a local scope network, or to a kv-store based overlay network. Signed-off-by: Alessandro Boch <[email protected]>
LGTM |
I confirm it now works when using only one network, but not when using multiple nets and later connecting individual containers from the stack to some not previously connected networks (docker network connect --alias) Tested with Docker version 17.06.2-ce, build cec0b72 To reproduce :
Every thing looks fine so far :
But both the manually added alias and the ct_id are not resolved by the embedded DNS
|
I've been bitten by the same problem right this second. non_unique_container_name.unique_net_name My use case, is to have a service listening on 1 port (https server) to be able to redirect to 2 subnets. so instead of using aliases (which don't work currently) maybe by having a TLD called dev_dashboard_default, we could redirect to grafana.dev_dashboard_default and the same for the prod for example. |
is connected to a swarm attachable network, its aliases are
not added to the service records.
This is not in line with what we do when connecting to
a local scope network, or to a kv-store based overlay network.
Related to moby/moby/issues/31015, moby/moby/issues/31238
Signed-off-by: Alessandro Boch [email protected]