Merge ~bregeer-ctl/cloud-init:bugfix/centos7_resolv into cloud-init:master

[Bert JW Regeer]

See bugs:

https://bugs.launchpad.net/cloud-init/+bug/1620796
https://bugs.launchpad.net/cloud-init/+bug/1620807

[Scott Moser (smoser)]

This generally looks good.
cloud-init gets network configuration from a variety of sources (openstack config drive in 'interfaces(5)' format, config drive in network_data.json format, Nocloud in 'network_config' format....

Some of those have per-interface dns configuration, some do not. It'd be nice to render this per-interface information when we can.

And even in the openstack interfaces 5 format it *could* declare per-network-device dns entries (the parser supports maintaining that information).

The second thing, you need to sign the canonical contributors agreement (http://www.ubuntu.com/legal/contributors). Please do so, and mention to me that you have in irc or here. Let me know if you have any questions on that.

[Lars Kellogg-Stedman (larsks)]

This all looks fine as far as fixing the immediate problem.

I can't help but feel that there must be a better way to cooperate with networkmanager, because there are some situations in which it's really useful to have nm managing dns (vpn environments in which you want certain domains to resolve via one set of nameservers and everything else via another comes to mind), but this patch is certainly an improvement over the existing situation.

[Lars Kellogg-Stedman (larsks)]

Re: smoser's comment:

> Some of those have per-interface dns configuration, some do not. It'd be nice to render this
> per-interface information when we can.

If we tell networkmanager not to manage /etc/resolv.conf, those interface-specific nameservers are going to be effectively ignored. I think that this would only work in the case where there are no globally defined nameservers (everything is interface-specific).

[Bert JW Regeer (bregeer-ctl)]

I am not willing to sign the CLA as it currently stands. I have no issues with providing my changes under the GPLv3, I do have issues signing a document that states Canonical is allowed to re-license my changes under any license they deem necessary. I also refuse to agree to the patent clause further than what the GPLv3 requires.

Feel free to pull this change under the GPLv3 or re-implement.

For now I will have my fork available at https://gitlab.com/bertjwregeer/cloud-init, fixes are in the bugfix/centos7_resolv branch: https://gitlab.com/bertjwregeer/cloud-init/commits/bugfix/centos7_resolv

review: Disapprove

Unmerged commits

54ed5cd... by Bert JW Regeer on 2016-09-06

Add debug logging

2397d42... by Bert JW Regeer on 2016-09-06

CentOS/RHEL 7 has NetworkManager manage resolv.conf

When cloud-init first writes to /etc/resolv.conf it does so before
networking is active. As soon as networking is started, NetworkManager
sees that /etc/resolv.conf is a file (this behaviour is important...),
and will take over managing of it, replacing the entries we just added
with a blank configuration thereby removing all nameserver entries.

It expects to have the DNS entries be available on the ifcfg-eth* files,
however with the way network data is presented to cloud-init, instead we
get a listing of nameservers to use, and not what interface they belong
to.

We could blindly add the nameservers across the interface files that are
generated, but this may leave them on interfaces that are not related to
the DNS entries, and if the user has multiple interfaces and disables
one, we no longer have any nameservers.

Instead we write to /etc/resolv.conf.cloud, then we symlink
/etc/resolv.conf to /etc/resolv.conf.cloud. When NetworkManager
encounters a symlink to a file that is not within it's run state
directory it will not modify the existing file that is on disk.

This allows our cloud-init resolv.conf to stay in tact.

d8ce950... by Bert JW Regeer on 2016-09-06

Catch ValueError and log it, rather than unwinding entire cloud-init

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index c53acf7..a5d6611 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -20,8 +20,11 @@ import six
 from cloudinit.distros.parsers import resolv_conf
 from cloudinit import util
 
+from cloudinit import log as logging
+
 from . import renderer
 
+LOG = logging.getLogger(__name__)
 
 def _make_header(sep='#'):
     lines = [
@@ -206,6 +209,10 @@ class Renderer(renderer.Renderer):
         self.netrules_path = config.get(
             'netrules_path', 'etc/udev/rules.d/70-persistent-net.rules')
         self.dns_path = config.get('dns_path', 'etc/resolv.conf')
+        self.dns_symlink = config.get('dns_symlink', True)
+
+        self.dns_write_path = config.get('dns_write_path', 'etc/resolv.conf.cloud') \
+            if self.dns_symlink else self.dns_path
 
     @classmethod
     def _render_iface_shared(cls, iface, iface_cfg):
@@ -335,7 +342,10 @@ class Renderer(renderer.Renderer):
         if existing_dns_path and os.path.isfile(existing_dns_path):
             content = resolv_conf.ResolvConf(util.load_file(existing_dns_path))
         for nameserver in network_state.dns_nameservers:
-            content.add_nameserver(nameserver)
+            try:
+                content.add_nameserver(nameserver)
+            except ValueError as e:
+                LOG.debug('Unable to add nameserver %s: %s', nameserver, e)
         for searchdomain in network_state.dns_searchdomains:
             content.add_search_domain(searchdomain)
         return "\n".join([_make_header(';'), str(content)])
@@ -389,11 +399,22 @@ class Renderer(renderer.Renderer):
         for path, data in self._render_sysconfig(base_sysconf_dir,
                                                  network_state).items():
             util.write_file(path, data)
-        if self.dns_path:
+        if self.dns_write_path:
             dns_path = os.path.join(target, self.dns_path)
+            dns_write_path = os.path.join(target, self.dns_write_path)
             resolv_content = self._render_dns(network_state,
                                               existing_dns_path=dns_path)
-            util.write_file(dns_path, resolv_content)
+            util.write_file(dns_write_path, resolv_content)
+
+            if self.dns_symlink:
+                if os.path.exists(dns_path) and not os.path.islink(dns_path):
+                    LOG.debug('Removing existing resolv.conf')
+                    os.unlink(dns_path)
+
+                if not os.path.exists(dns_path):
+                    LOG.debug('Adding symlink from %s to %s', dns_path, dns_write_path)
+                    os.symlink(dns_write_path, dns_path)
+
         if self.netrules_path:
             netrules_content = self._render_persistent_net(network_state)
             netrules_path = os.path.join(target, self.netrules_path)