Merge ~raharper/cloud-init:fix/sysconfig-skip-resolvconf-no-dns-config into cloud-init:master

[Ryan Harper]

sysconfig: only write resolv.conf if network_state has DNS values

If an OS image provided an /etc/resolv.conf file that was not empty
cloud-init would read and re-write it with a cloud-init header even
if no DNS network configuration was provided (e.g. DHCP only).

This can cause problems for some network services which don't
ignore cloud-init's header.

LP: #1843634

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:cc70b43e9becd6a469ed17430c5907b85b5f2b2d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1139/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1139//rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

It can be argued that cloud-init *was* doing the right thing.
It was told that there were no dns servers and no dns 'search' (by lack of that in network config).

dhcp muddies that argument for sure.

That said, based on the content in the description of the bug (the
previously-existing /etc/resolv.conf), cloud-init should *never* be
writing that file when 'netconfig' is involved. It seems rather that
resolv.conf is generated based on contents of
/etc/sysconfig/network/config .

Would writing /etc/sysconfig/network/config with valid content for NETCONFIG_DNS_STATIC_SEARCHLIST and friends fix this?

[Ryan Harper (raharper)]

Maybe, but I don't think we really wanted to always append a header to the file, and we certainly wanted to avoid writing the file if we didn't have a config. Lastly the side-effect of modifying resolvconf without dns settings disabled system auto-generation.

We do need to be careful w.r.t detecting whether netconfig is present or not. I think that we could replace updating resolvconf directly if netconfig is present. This renderer is shared by a range of SuSE and RHEL releases which may or maynot have netconfig (or something equivalent).

I'd like Robert to comment on whether we want to move to netconfig or not. That can be done incrementally (land this fix of not touching resolv.conf at all if we don't have settings) and then separately replacing writing resolv.conf directly with using netconfig if present.

71a4b90... by Ryan Harper on 2019-09-13

Move network_state check up to top of _render_dns

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:485384fe14caf0a3da01dfd7d5c79027e2c00161
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1141/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1141//rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:ac38c650c1b6cb9df4021786d772450470f17aff
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1176/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1176//rebuild

review: Approve (continuous-integration)

[Dan Watkins (oddbloke)]

One nit inline, so leaving this to Ryan to merge (whether or not the nit is addressed). Thanks!

review: Approve

6ac5b74... by Ryan Harper on 2019-09-26

Clarify content variable names

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:6ac5b74e1a91cfad0edb999bfbbad9715ccd68a2
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1182/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1182//rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index be5dede..1708990 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -578,6 +578,10 @@ class Renderer(renderer.Renderer):
 
     @staticmethod
     def _render_dns(network_state, existing_dns_path=None):
+        # skip writing resolv.conf if network_state doesn't include any input.
+        if not any([len(network_state.dns_nameservers),
+                    len(network_state.dns_searchdomains)]):
+            return None
         content = resolv_conf.ResolvConf("")
         if existing_dns_path and os.path.isfile(existing_dns_path):
             content = resolv_conf.ResolvConf(util.load_file(existing_dns_path))
@@ -585,8 +589,6 @@ class Renderer(renderer.Renderer):
             content.add_nameserver(nameserver)
         for searchdomain in network_state.dns_searchdomains:
             content.add_search_domain(searchdomain)
-        if not str(content):
-            return None
         header = _make_header(';')
         content_str = str(content)
         if not content_str.startswith(header):
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index e578992..92c7c3c 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -2701,6 +2701,10 @@ USERCTL=no
         ns = network_state.parse_net_config_data(CONFIG_V1_EXPLICIT_LOOPBACK)
         render_dir = self.tmp_path("render")
         os.makedirs(render_dir)
+        # write an etc/resolv.conf and expect it to not be modified
+        resolvconf = os.path.join(render_dir, 'etc/resolv.conf')
+        resolvconf_content = "# Original Content"
+        util.write_file(resolvconf, resolvconf_content)
         renderer = self._get_renderer()
         renderer.render_network_state(ns, target=render_dir)
         found = dir2dict(render_dir)
@@ -2718,6 +2722,8 @@ TYPE=Ethernet
 USERCTL=no
 """
         self.assertEqual(expected, found[nspath + 'ifcfg-eth0'])
+        # a dhcp only config should not modify resolv.conf
+        self.assertEqual(resolvconf_content, found['/etc/resolv.conf'])
 
     def test_bond_config(self):
         expected_name = 'expected_sysconfig_rhel'
@@ -3202,6 +3208,10 @@ USERCTL=no
         ns = network_state.parse_net_config_data(CONFIG_V1_EXPLICIT_LOOPBACK)
         render_dir = self.tmp_path("render")
         os.makedirs(render_dir)
+        # write an etc/resolv.conf and expect it to not be modified
+        resolvconf = os.path.join(render_dir, 'etc/resolv.conf')
+        resolvconf_content = "# Original Content"
+        util.write_file(resolvconf, resolvconf_content)
         renderer = self._get_renderer()
         renderer.render_network_state(ns, target=render_dir)
         found = dir2dict(render_dir)
@@ -3219,6 +3229,8 @@ TYPE=Ethernet
 USERCTL=no
 """
         self.assertEqual(expected, found[nspath + 'ifcfg-eth0'])
+        # a dhcp only config should not modify resolv.conf
+        self.assertEqual(resolvconf_content, found['/etc/resolv.conf'])
 
     def test_bond_config(self):
         expected_name = 'expected_sysconfig_opensuse'