-
Notifications
You must be signed in to change notification settings - Fork 880
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
Do not add IP to name records for aliases #2299
Conversation
Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
@fcrisciani I can see that this code have been added by your PR #1796 What are you thoughts is this correct way to fix the issue? |
@olljanat in general I would say that the answer is not "wrong" in the sense that if you try to resolve the reverse translation answer you will get the same IP as a result. Looking at the patch I have to see that the attachable container case is not going to be broken with your suggested change. Anyway I would like to dig more on the real use case and what is being broken by this behavior to better think to a proper solution |
@fcrisciani I noticed this issue when I was looking for that if it is possible to modify Redis Sentinel to store hostnames to config files instead of IPs which would make it working with static IPs and would avoid needed for: moby/moby#24170 Here is example config from my lab which is updated by Redis Sentinel
and without this change time to time there was container id instead of these redisX.redis or sentinelX.redis hostnames which makes is breaking when container is re-created by swarm and container id changes. Internally Redis uses IPs for all connections so that why I need reverse DNS for resolve hostnames for those auto discovered cluster nodes. Reason why I'm looking for this is that currently only way make Redis Sentinel working is create overlay network with manually specified subnet and join standalone containers with static IPs to it. |
@fcrisciani Hi, I have this issue on a distributed system analyser I'm working on: I'm capturing network communication between the containes in the Swarm to analyse and troubleshoot system behavior.
Having the IP constantly changing name when reverse resolving it is a pain. I overcame it by settings rules of acceptance over the name resolution result, but this is an ugly hack. |
I ran into this as HBase requires forward/reverse name lookups to match. This fixes the bug that makes HBase unusable in a docker container on an encrypted overlay network. |
@fcrisciani did you got enough explanations why this is needed? These are bit rare use cases and that why most users have been able live without this fix but at least I cannot image any use case where current approach would be needed so can we merge this one? |
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.
LGTM
I dig a bit into the logic and the attachable containers seems to still be working fine because if they have a name that will match with the container name, else if a name is not specified the container name is set to the short version of the container ID that is also the same as the only alias that is set.
Thanks @olljanat for the contribution! |
@fcrisciani good point but that is not most probably issue at least on my use cases because I'm planning to use MaxReplicas=1 setting ( moby/moby#37940 ) with these services so I can use |
- What I did
Do not add IP to name records for aliases so IP to name resolution will always reply container name.
- How I did it
Send ipMapUpdate = false to addSvcRecords function so it will not add IP to name records https://github.com/docker/libnetwork/blob/09e39d99d5c769cb7d167058504581d1cfc779e3/network.go#L1422-L1427
- How to verify it
This can be easily tested with commands:
without this one about 100-150 / 1000 tests fails.
Fixes moby/moby#34882