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

Enable external DNS servers/search domains to be provided by network drivers #986

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dvavili
Copy link

@dvavili dvavili commented Mar 3, 2016

Proposal

Simplify the configuration of External DNS servers for to work in co-existence with the embedded DNS server offered in docker 1.10.
We propose offering the DNS options to the container config during container creation, as offered by DHCP Server. The proposed changes do not offer to disable embedded DNS as mentioned in Issue moby/moby#19474 ([1.10] How to disable embedded DNS?), but improve the offering of basic pluggable DNS servers by --dns option.
If the remote network plugin does not offer the DNS options there is no change in behavior and embedded DNS would continue to provide the name resolution for the containers in the network. Hence, this is backward-compatible with the existing solution.

Details

The remote network drivers can pass any external DNS server(s) and DNS search domain(s) that are configured for that network and pass this information when container address is allocated (Similar to DNS options provided by DHCP). These options are configured as external DNS servers.
Also, if the /etc/resolv.conf is modified, the modified changes take precedence over the configured DNS which is in line with the current behavior.
The precedence for answering DNS queries:

  • Embedded DNS
  • Configured external DNS servers
    • Modifications to /etc/resolv.conf takes precedence
    • DNS servers configured at container level in the order they are provided
      • provided by --dns or by remote ipam driver. The --dns provided server(s) overrides the remote ipam driver server(s).

Tested scenarios

Scenario 1:
Embedded DNS coexisting with remote ipam provided DNS Server (100.1.3.1).
Container c1,c2 attached to epg.nw1.tenant1 [Query: "ping c1" from c2]
Mar 2 22:47:13 vagrant docker[2025]: level=info msg="Querying embedded dns udp for c1.[1]"

Scenario 2:
Embedded DNS coexisting with remote ipam provided DNS Server (100.1.3.1).
Container c1,c2 attached to epg.nw1.tenant1 [Query: "dig tenant1 SRV" from c1]
Mar 2 22:51:16 vagrant docker[2025]: level=info msg="Querying ext dns udp:100.1.3.1 for tenant1.[33]"

Scenario 3:
Embedded DNS coexisting with remote ipam provided DNS Server (100.1.3.1). Container c3 is launched with --dns=10.1.3.1 option.
Container c1,c2,c3 attached to epg.nw1.tenant1 [Query: "dig tenant1 SRV" from c3]
Mar 2 22:49:10 vagrant docker[2025]: level=info msg="Querying ext dns udp:10.1.3.2 for tenant1.[33]"
Mar 2 22:49:12 vagrant docker[2025]: level=error msg="external resolution failed, read udp 100.1.3.3:53840->10.1.3.2:53: i/o timeout"
Mar 2 22:49:12 vagrant docker[2025]: level=info msg="Querying ext dns udp:100.1.3.1 for tenant1.[33]"

Note

These changes were tested with the fix in PR #895 (Source external DNS queries from container namespace)
Automating these test scenarios is a work-in-progress. Will send out a PR for it as well.

@dvavili
Copy link
Author

dvavili commented Mar 3, 2016

@@ -400,6 +400,12 @@ func (ep *endpoint) sbJoin(sbox Sandbox, options ...EndpointOption) error {
}
}()

sb.config.dnsList = append(sb.config.dnsList, ep.iface.dnsServers...)
sb.config.dnsSearchList = append(sb.config.dnsSearchList, ep.iface.dnsSearchDomains...)
if err = sb.setupResolutionFiles(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

am not quite sure if we have to call setupResolutionFiles every time from sbJoin.
Instead, I would suggest to make the change in Refresh() in sandbox.go to call it just the once after all the
endpoint Joins are processed.

@sanimej is the best person to give an authoritative answer to that.

@mavenugo
Copy link
Contributor

mavenugo commented Mar 3, 2016

@DivyaVavili thanks for the proposal. 1 question on the design though.
Is there a reason why we would want to have a different DNS and DNS Search-domain for every RequestAddress in a network / Pool ? Isn't it enough we return this in ReleasePool which will be called during Network creation events and all the endpoints in that network can share the same DNS and Search-domains ?
I understand that this is addressing one of a popular IPAM use-cases (using DHCP). Since we dont have the corresponding IPAM driver, it is hard for me to put the full picture together.

@jainvipin
Copy link

@mavenugo - modeling this PR to what DHCP would return a dhcp response. Though not likely, say we implement a DHCP ipam driver, a mismatch (different search domains for two containers in the same subnet) would create an issue. This also inline with the granularity of dns address/search domain is per container level today i.e. it can be specified differently at docker run differently for different containers.

Is there a reason why we would want to have a different DNS and DNS Search-domain for every RequestAddress in a network / Pool ? Isn't it enough we return this in ReleasePool which will be called during Network creation events and all the endpoints in that network can share the same DNS and Search-domains ?

@mavenugo
Copy link
Contributor

mavenugo commented Mar 3, 2016

@jainvipin fair enough. it would be great if we also have a DHCP driver :). but that is unrelated to this PR.

@jainvipin
Copy link

@mavenugo
yes a separate PR, coming...

it would be great if we also have a DHCP driver :). but that is unrelated to this PR.

@sanimej
Copy link

sanimej commented Mar 3, 2016

@jainvipin @DivyaVavili

With the proposed changes, how will the IPAM driver determine what DNS server/options to give to a particular container?

@@ -71,7 +71,7 @@ type Ipam interface {
// ReleasePool releases the address pool identified by the passed id
ReleasePool(poolID string) error
// Request address from the specified pool ID. Input options or required IP can be passed.
RequestAddress(string, net.IP, map[string]string) (*net.IPNet, map[string]string, error)
RequestAddress(string, net.IP, map[string]string) (*net.IPNet, []string, []string, map[string]string, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

The return parameter map[string]string was in fact added to accommodate remote drivers requirements.
In this case it was not added to carry opaque driver data only. Like we do for the one in the input parameter list, where we pass the container's MAC address if available.

Therefore, I would avoid to change the parameter list for this API.

@jainvipin
Copy link

@sanimej

As of today, the IPAM driver knows the subnet and pool-id i.e. subnet is rough indicator of the network (not docker network, but subnet itself is some indication of a local-area-network) and pool-id would approximately identify the tenancy of the container. For Contiv we plan to use those two.

It would be wonderful to allow passing more hints (aka labels equivalent) that can be configured in commercial IPAM software, so this can be automated between orchestrators and IPAM stacks. To be able to implement these functions, without breaking architectural boundaries is the trick. Any suggestions?

With the proposed changes, how will the IPAM driver determine what DNS server/options to give to a particular container?

@mrjana
Copy link
Contributor

mrjana commented Mar 3, 2016

I am 👎 on this design. We should not try to inject unrelated options (like DNS) through an unrelated extension point (like IPAM). If a particular ecosystem vendor wants its users to use a particular DNS server, then they should guide their users to pass the preferred DNS server as part of the already existing --dns option. There is nothing more needed.

@jainvipin
Copy link

We should not try to inject unrelated options (like DNS) through an unrelated extension point (like IPAM).

@mrjana - we can agree to disagree, IMHO IPAM has always included search-domain and dns-server-ip allocation. Check out about 40 commercial and non commercial products today uses this wholesome definition of IPAM: https://en.wikipedia.org/wiki/IP_address_management

If a particular ecosystem vendor wants its users to use a particular DNS server, then they should guide their users to pass the preferred DNS server as part of the already existing --dns option. There is nothing more needed.

With this change, I believe we could enable all those eco-systems with existing (e.g. above mentioned wiki link) tools, rather than forcing them to take up IPAM themselves.

@dvavili dvavili force-pushed the dnsaddr_handling branch 2 times, most recently from 4ddc9c7 to a1452ed Compare March 9, 2016 01:12
@dvavili
Copy link
Author

dvavili commented Mar 9, 2016

@mavenugo @aboch @sanimej : I have addressed the comments.

  • The return signature for RequestAddress is retained as is and the information is now passed as remote driver data.
  • The join handles setting up resolution only when remote driver provides DNS options

@dvavili
Copy link
Author

dvavili commented Mar 9, 2016

If everything looks OK, I can add these details in remote.md

@GordonTheTurtle
Copy link

@DivyaVavili It has been detected that this issue has not received any activity in over 6 months. Can you please let us know if it is still relevant:

  • For a bug: do you still experience the issue with the latest version?
  • For a feature request: was your request appropriately answered in a later version?

Thank you!
This issue will be automatically closed in 1 week unless it is commented on.
For more information please refer to #1926

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.

7 participants