cloud-init-dev team mailing list archive

[Ryan McCabe <rmccabe@xxxxxxxxxx>]

Ryan McCabe has proposed merging ~rmccabe/cloud-init:bug1693251 into cloud-init:master.

Commit message:
net: Allow for NetworkManager configuration

In cases where the config json specifies nameserver entries, if there are
interfaces configured to use dhcp, NetworkManager, if enabled, will
clobber the /etc/resolv.conf that cloud-init has produced, which can break
dns.

This patch adds a mechanism for dropping additional configuration into
/etc/NetworkManager/conf.d/ and disables management of /etc/resolv.conf by
NetworkManager when nameserver information is provided in the config.

LP: #1693251

Signed-off-by: Ryan McCabe <rmccabe@xxxxxxxxxx>

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1693251 in cloud-init: "cloud-init should configure networkmanager to not manage /etc/resolv.conf"
  https://bugs.launchpad.net/cloud-init/+bug/1693251

For more details, see:
https://code.launchpad.net/~rmccabe/cloud-init/+git/cloud-init/+merge/325239

In cases where the config json specifies nameserver entries, NetworkManager, if enabled, will clobber the /etc/resolv.conf that cloud-init has produced, which can break dns. If at least one interface is configured for dhcp, you might end up with a resolv.conf that doesn't function as intended, and if you don't have any interfaces that get dns from dhcp, NetworkManager could clobber resolv.conf with an empty file.

This patch adds a mechanism for dropping additional configuration into /etc/NetworkManager/conf.d/. I figured at some point, somebody will need to set some other keypair to stop NetworkManager from misbehaving, so I added something for that instead of simply writing the particular 2 lines of config to fix the resolv.conf issue.

This patch only writes out that configuration file if necessary (as opposed to creating an empty file when there is no config needed). I wasn't sure which was preferred, so if that's not ideal, I could write out the empty file for consistency.

This patch will also write that config "(dns=none") out, too, for the specific problem noted in #1693251

LP: #1693251


-- 
Your team cloud-init commiters is requested to review the proposed merge of ~rmccabe/cloud-init:bug1693251 into cloud-init:master.

diff --git a/cloudinit/distros/parsers/networkmanager_conf.py b/cloudinit/distros/parsers/networkmanager_conf.py
new file mode 100644
index 0000000..aa57582
--- /dev/null
+++ b/cloudinit/distros/parsers/networkmanager_conf.py
@@ -0,0 +1,25 @@
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# Author: Ryan McCabe <rmccabe@xxxxxxxxxx>
+#
+# This file is part of cloud-init. See LICENSE file for license information.
+
+import six
+from six import StringIO
+
+import configobj
+
+# This module is used to set additional NetworkManager configuration
+# in /etc/NetworkManager/conf.d
+#
+
+class NetworkManagerConf(configobj.ConfigObj):
+    def __init__(self, contents):
+        configobj.ConfigObj.__init__(self, contents,
+                                     interpolation=False,
+                                     write_empty_values=False)
+
+    def set_section_keypair(self, section_name, key, value):
+        if not section_name in self.sections:
+            self.main[section_name] = {}
+        self.main[section_name] = {key: value}
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 58c5713..6659d3b 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -6,6 +6,7 @@ import re
 import six
 
 from cloudinit.distros.parsers import resolv_conf
+from cloudinit.distros.parsers import networkmanager_conf
 from cloudinit import util
 
 from . import renderer
@@ -252,6 +253,9 @@ 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')
+        nm_conf_path = 'etc/NetworkManager/conf.d/99-cloud-init.conf'
+        self.networkmanager_conf_path = config.get('networkmanager_conf_path',
+                                                   nm_conf_path)
 
     @classmethod
     def _render_iface_shared(cls, iface, iface_cfg):
@@ -445,6 +449,20 @@ class Renderer(renderer.Renderer):
             content.add_search_domain(searchdomain)
         return "\n".join([_make_header(';'), str(content)])
 
+    @staticmethod
+    def _render_networkmanager_conf(network_state):
+        content = networkmanager_conf.NetworkManagerConf("")
+
+        # If DNS server information is provided, configure
+        # NetworkManager to not manage dns, so that /etc/resolv.conf
+        # does not get clobbered.
+        if network_state.dns_nameservers:
+            content.set_section_keypair('main', 'dns', 'none')
+
+        if len(content) == 0:
+            return None
+        return "".join([_make_header(), "\n", "\n".join(content.write()), "\n"])
+
     @classmethod
     def _render_bridge_interfaces(cls, network_state, iface_contents):
         bridge_filter = renderer.filter_by_type('bridge')
@@ -505,6 +523,12 @@ class Renderer(renderer.Renderer):
             resolv_content = self._render_dns(network_state,
                                               existing_dns_path=dns_path)
             util.write_file(dns_path, resolv_content, file_mode)
+        if self.networkmanager_conf_path:
+            nm_conf_path = util.target_path(target,
+                                            self.networkmanager_conf_path)
+            nm_conf_content = self._render_networkmanager_conf(network_state)
+            if nm_conf_content:
+                util.write_file(nm_conf_path, nm_conf_content, file_mode)
         if self.netrules_path:
             netrules_content = self._render_persistent_net(network_state)
             netrules_path = util.target_path(target, self.netrules_path)
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 0000b07..ac1a16a 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -164,6 +164,13 @@ NETMASK0=0.0.0.0
 ;
 nameserver 172.19.0.12
 """.lstrip()),
+            ('etc/NetworkManager/conf.d/99-cloud-init.conf',
+             """
+# Created by cloud-init on instance boot automatically, do not edit.
+#
+[main]
+dns = none
+""".lstrip()),
             ('etc/udev/rules.d/70-persistent-net.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))]
@@ -225,6 +232,13 @@ USERCTL=no
 ;
 nameserver 172.19.0.12
 """.lstrip()),
+            ('etc/NetworkManager/conf.d/99-cloud-init.conf',
+             """
+# Created by cloud-init on instance boot automatically, do not edit.
+#
+[main]
+dns = none
+""".lstrip()),
             ('etc/udev/rules.d/70-persistent-net.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))]
@@ -308,6 +322,13 @@ USERCTL=no
 ;
 nameserver 172.19.0.12
 """.lstrip()),
+            ('etc/NetworkManager/conf.d/99-cloud-init.conf',
+             """
+# Created by cloud-init on instance boot automatically, do not edit.
+#
+[main]
+dns = none
+""".lstrip()),
             ('etc/udev/rules.d/70-persistent-net.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))]