improve how 'lo' is handled when rendering network state to interfaces

if you provide network state with a proper 'lo' entry, then
when you render network interfaces you would get 2 entries.
the additional one was because we add an 'lo' always and also because
we had to put global 'dns' entries there.

this fixes that duplicatation by handling lo specifically.

2 files changed, 243 insertions, 44 deletions

diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index f47053b2..0066561e 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -17,6 +17,7 @@
 #   along with Curtin.  If not, see <http://www.gnu.org/licenses/>.
 
 import base64
+import copy
 import errno
 import glob
 import gzip
@@ -418,7 +419,7 @@ def render_persistent_net(network_state):
 
 # TODO: switch valid_map based on mode inet/inet6
 def iface_add_subnet(iface, subnet):
-    content = ""
+    content = []
     valid_map = [
         'address',
         'netmask',
@@ -437,14 +438,14 @@ def iface_add_subnet(iface, subnet):
                 value = " ".join(value)
             if '_' in key:
                 key = key.replace('_', '-')
-            content += "    {} {}\n".format(key, value)
+            content.append("    {} {}".format(key, value))
 
     return content
 
 
 # TODO: switch to valid_map for attrs
 def iface_add_attrs(iface):
-    content = ""
+    content = []
     ignore_map = [
         'control',
         'index',
@@ -461,7 +462,7 @@ def iface_add_attrs(iface):
         if value and key not in ignore_map:
             if type(value) == list:
                 value = " ".join(value)
-            content += "    {} {}\n".format(key, value)
+            content.append("    {} {}".format(key, value))
 
     return content
 
@@ -481,10 +482,10 @@ def render_route(route, indent=""):
     1. http://askubuntu.com/questions/168033/
              how-to-set-static-routes-in-ubuntu-server
     """
-    content = ""
+    content = []
     up = indent + "post-up route add"
     down = indent + "pre-down route del"
-    eol = " || true\n"
+    eol = " || true"
     mapping = {
         'network': '-net',
         'netmask': 'netmask',
@@ -493,20 +494,20 @@ def render_route(route, indent=""):
     }
     if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0':
         default_gw = " default gw %s" % route['gateway']
-        content += up + default_gw + eol
-        content += down + default_gw + eol
+        content.append(up + default_gw + eol)
+        content.append(down + default_gw + eol)
     elif route['network'] == '::' and route['netmask'] == 0:
         # ipv6!
         default_gw = " -A inet6 default gw %s" % route['gateway']
-        content += up + default_gw + eol
-        content += down + default_gw + eol
+        content.append(up + default_gw + eol)
+        content.append(down + default_gw + eol)
     else:
         route_line = ""
         for k in ['network', 'netmask', 'gateway', 'metric']:
             if k in route:
                 route_line += " %s %s" % (mapping[k], route[k])
-        content += up + route_line + eol
-        content += down + route_line + eol
+        content.append(up + route_line + eol)
+        content.append(down + route_line + eol)
 
     return content
 
@@ -527,8 +528,38 @@ def iface_start_entry(iface, index):
     subst = iface.copy()
     subst.update({'fullname': fullname, 'cverb': cverb})
 
-    return ("{cverb} {fullname}\n"
-            "iface {fullname} {inet} {mode}\n").format(**subst)
+    if 'inet' not in subst:
+        print("bug....iface: %s" % iface)
+    return ["{cverb} {fullname}".format(**subst),
+            "iface {fullname} {inet} {mode}".format(**subst)]
+
+
+def _render_iface(iface):
+    lines = []
+    subnets = iface.get('subnets', {})
+    if subnets:
+        for index, subnet in zip(range(0, len(subnets)), subnets):
+            iface['index'] = index
+            iface['mode'] = subnet['type']
+            iface['control'] = subnet.get('control', 'auto')
+            if iface['mode'].endswith('6'):
+                iface['inet'] += '6'
+            elif iface['mode'] == 'static' and ":" in subnet['address']:
+                iface['inet'] += '6'
+            if iface['mode'].startswith('dhcp'):
+                iface['mode'] = 'dhcp'
+
+            lines.extend(iface_start_entry(iface, index))
+            lines.extend(iface_add_subnet(iface, subnet))
+            lines.extend(iface_add_attrs(iface))
+            lines.append("")
+    else:
+        # ifenslave docs say to auto the slave devices
+        if 'bond-master' in iface:
+            lines.append("auto {name}".format(**iface))
+        lines.append("iface {name} {inet} {mode}".format(**iface))
+        lines.extend(iface_add_attrs(iface))
+    return lines
 
 
 def render_interfaces(network_state):
@@ -546,44 +577,30 @@ def render_interfaces(network_state):
         'bridge': 2,
         'vlan': 3,
     }
-    content += "auto lo\niface lo inet loopback\n"
+
+    # handle 'lo' specifically as we need to insert the global dns entries
+    # there (as that is the only interface) that will be always up.
+    lo = {'name': 'lo', 'type': 'physical', 'inet': 'inet',
+          'subnets': [{'type': 'loopback', 'control': 'auto'}]}
+    for iface in interfaces.values():
+        if iface.get('name') == "lo":
+            lo = copy.deepcopy(iface)
     for dnskey, value in network_state.get('dns', {}).items():
         if len(value):
-            content += "    dns-{} {}\n".format(dnskey, " ".join(value))
+            lo['subnets'][0]["dns_" + dnskey] = value
+
+    sections = [_render_iface(lo)]
 
     for iface in sorted(interfaces.values(),
                         key=lambda k: (order[k['type']], k['name'])):
-
-        if content[-2:] != "\n\n":
-            content += "\n"
-        subnets = iface.get('subnets', {})
-        if subnets:
-            for index, subnet in zip(range(0, len(subnets)), subnets):
-                if content[-2:] != "\n\n":
-                    content += "\n"
-                iface['index'] = index
-                iface['mode'] = subnet['type']
-                iface['control'] = subnet.get('control', 'auto')
-                if iface['mode'].endswith('6'):
-                    iface['inet'] += '6'
-                elif iface['mode'] == 'static' and ":" in subnet['address']:
-                    iface['inet'] += '6'
-                if iface['mode'].startswith('dhcp'):
-                    iface['mode'] = 'dhcp'
-
-                content += iface_start_entry(iface, index)
-                content += iface_add_subnet(iface, subnet)
-                content += iface_add_attrs(iface)
-        else:
-            # ifenslave docs say to auto the slave devices
-            if 'bond-master' in iface:
-                content += "auto {name}\n".format(**iface)
-            content += "iface {name} {inet} {mode}\n".format(**iface)
-            content += iface_add_attrs(iface)
+        if iface.get('name') == "lo":
+            continue
+        sections.append(_render_iface(iface))
 
     for route in network_state.get('routes'):
-        content += render_route(route)
+        sections.append(render_route(route))
 
+    content = ''.join(['\n'.join(s) + '\n\n' for s in sections])
     # global replacements until v2 format
     content = content.replace('mac_address', 'hwaddress')
     return content
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 624a9aa8..34875f7b 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -9,6 +9,8 @@ import gzip
 import io
 import json
 import os
+import yaml
+
 
 DHCP_CONTENT_1 = """
 DEVICE='eth0'
@@ -68,6 +70,163 @@ STATIC_EXPECTED_1 = {
                  'dns_nameservers': ['10.0.1.1']}],
 }
 
+EXAMPLE_ENI = """
+auto lo
+iface lo inet loopback
+   dns-nameservers 10.0.0.1
+   dns-search foo.com
+
+auto eth0
+iface eth0 inet static
+        address 1.2.3.12
+        netmask 255.255.255.248
+        broadcast 1.2.3.15
+        gateway 1.2.3.9
+        dns-nameservers 69.9.160.191 69.9.191.4
+auto eth1
+iface eth1 inet static
+        address 10.248.2.4
+        netmask 255.255.255.248
+        broadcast 10.248.2.7
+"""
+
+NETWORK_YAML_SMALL = """
+version: 1
+config:
+    # Physical interfaces.
+    - type: physical
+      name: eth0
+      mac_address: "c0:d6:9f:2c:e8:80"
+      subnets:
+          - type: dhcp4
+          - type: static
+            address: 192.168.21.3/24
+            dns_nameservers:
+              - 8.8.8.8
+              - 8.8.4.4
+            dns_search: barley.maas sach.maas
+    - type: physical
+      name: eth1
+      mac_address: "cf:d6:af:48:e8:80"
+    - type: nameserver
+      address:
+        - 1.2.3.4
+        - 5.6.7.8
+      search:
+        - wark.maas
+"""
+NETWORK_YAML_ALL = """
+version: 1
+config:
+    # Physical interfaces.
+    - type: physical
+      name: eth0
+      mac_address: "c0:d6:9f:2c:e8:80"
+    - type: physical
+      name: eth1
+      mac_address: "aa:d6:9f:2c:e8:80"
+    - type: physical
+      name: eth2
+      mac_address: "c0:bb:9f:2c:e8:80"
+    - type: physical
+      name: eth3
+      mac_address: "66:bb:9f:2c:e8:80"
+    - type: physical
+      name: eth4
+      mac_address: "98:bb:9f:2c:e8:80"
+    # specify how ifupdown should treat iface
+    # control is one of ['auto', 'hotplug', 'manual']
+    # with manual meaning ifup/ifdown should not affect the iface
+    # useful for things like iscsi root + dhcp
+    - type: physical
+      name: eth5
+      mac_address: "98:bb:9f:2c:e8:8a"
+      subnets:
+        - type: dhcp
+          control: manual
+    # VLAN interface.
+    - type: vlan
+      name: eth0.101
+      vlan_link: eth0
+      vlan_id: 101
+      mtu: 1500
+      subnets:
+        - type: static
+          address: 192.168.0.2/24
+          gateway: 192.168.0.1
+          dns_nameservers:
+            - 192.168.0.10
+            - 10.23.23.134
+          dns_search:
+            - barley.maas
+            - sacchromyces.maas
+            - brettanomyces.maas
+        - type: static
+          address: 192.168.2.10/24
+    # Bond.
+    - type: bond
+      name: bond0
+      # if 'mac_address' is omitted, the MAC is taken from
+      # the first slave.
+      mac_address: "aa:bb:cc:dd:ee:ff"
+      bond_interfaces:
+        - eth1
+        - eth2
+      params:
+        bond-mode: active-backup
+      subnets:
+        - type: dhcp6
+    # A Bond VLAN.
+    - type: vlan
+      name: bond0.200
+      vlan_link: bond0
+      vlan_id: 200
+      subnets:
+          - type: dhcp4
+    # A bridge.
+    - type: bridge
+      name: br0
+      bridge_interfaces:
+          - eth3
+          - eth4
+      ipv4_conf:
+          rp_filter: 1
+          proxy_arp: 0
+          forwarding: 1
+      ipv6_conf:
+          autoconf: 1
+          disable_ipv6: 1
+          use_tempaddr: 1
+          forwarding: 1
+          # basically anything in /proc/sys/net/ipv6/conf/.../
+      params:
+          bridge_stp: 'off'
+          bridge_fd: 0
+          bridge_maxwait: 0
+      subnets:
+          - type: static
+            address: 192.168.14.2/24
+          - type: static
+            address: 2001:1::1/64 # default to /64
+    # A global nameserver.
+    - type: nameserver
+      address: 8.8.8.8
+      search: barley.maas
+    # global nameservers and search in list form
+    - type: nameserver
+      address:
+        - 4.4.4.4
+        - 8.8.4.4
+      search:
+        - wark.maas
+        - foobar.maas
+    # A global route.
+    - type: route
+      destination: 10.0.0.0/8
+      gateway: 11.0.0.1
+      metric: 3
+"""
+
 
 class TestNetConfigParsing(TestCase):
     simple_cfg = {
@@ -122,6 +281,29 @@ class TestNetConfigParsing(TestCase):
         self.assertEqual(found, self.simple_cfg)
 
 
+class TestEniRoundTrip(TestCase):
+    def testsimple_convert_and_render(self):
+        network_config = net.convert_eni_data(EXAMPLE_ENI)
+        ns = net.parse_net_config_data(network_config)
+        eni = net.render_interfaces(ns)
+        print("Eni looks like:\n%s" % eni)
+        raise Exception("FOO")
+
+    def testsimple_render_all(self):
+        network_config = yaml.load(NETWORK_YAML_ALL)
+        ns = net.parse_net_config_data(network_config)
+        eni = net.render_interfaces(ns)
+        print("Eni looks like:\n%s" % eni)
+        raise Exception("FOO")
+
+    def testsimple_render_small(self):
+        network_config = yaml.load(NETWORK_YAML_SMALL)
+        ns = net.parse_net_config_data(network_config)
+        eni = net.render_interfaces(ns)
+        print("Eni looks like:\n%s" % eni)
+        raise Exception("FOO")
+
+
 def _gzip_data(data):
     with io.BytesIO() as iobuf:
         gzfp = gzip.GzipFile(mode="wb", fileobj=iobuf)