← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~akaris/cloud-init:bug1679817 into cloud-init:master

 

Andreas Karis has proposed merging ~akaris/cloud-init:bug1679817 into cloud-init:master.

Commit message:
Fix dual stack IPv4/IPv6 configuration for RHEL

Dual stack IPv4/IPv6 configuration via config drive is broken for RHEL7.
This patch fixes several scenarios for IPv4/IPv6/dual stack with multiple IP assignment
Removes unpopular IPv4 alias files and invalid IPv6 alias files

Also fixes associated unit tests

LP: #1679817
LP: #1685534
LP: #1685532

Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  cloud init development team (cloud-init-dev)

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

Fix dual stack IPv4/IPv6 configuration for RHEL

Dual stack IPv4/IPv6 configuration via config drive is broken for RHEL7.
This patch fixes several scenarios for IPv4/IPv6/dual stack with multiple IP assignment
Removes unpopular IPv4 alias files and invalid IPv6 alias files

Also fixes associated unit tests

LP: #1679817
LP: #1685534
LP: #1685532
-- 
Your team cloud init development team is requested to review the proposed merge of ~akaris/cloud-init:bug1679817 into cloud-init:master.
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 504e4d0..ee7783d 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -59,6 +59,9 @@ class ConfigMap(object):
     def __setitem__(self, key, value):
         self._conf[key] = value
 
+    def __getitem__(self, key):
+        return self._conf[key]
+
     def drop(self, key):
         self._conf.pop(key, None)
 
@@ -83,7 +86,8 @@ class ConfigMap(object):
 class Route(ConfigMap):
     """Represents a route configuration."""
 
-    route_fn_tpl = '%(base)s/network-scripts/route-%(name)s'
+    route_fn_tpl_ipv4 = '%(base)s/network-scripts/route-%(name)s'
+    route_fn_tpl_ipv6 = '%(base)s/network-scripts/route6-%(name)s'
 
     def __init__(self, route_name, base_sysconf_dir):
         super(Route, self).__init__()
@@ -102,10 +106,59 @@ class Route(ConfigMap):
         return r
 
     @property
-    def path(self):
-        return self.route_fn_tpl % ({'base': self._base_sysconf_dir,
+    def path_ipv4(self):
+        return self.route_fn_tpl_ipv4 % ({'base': self._base_sysconf_dir,
+                                     'name': self._route_name})
+
+    @property
+    def path_ipv6(self):
+        return self.route_fn_tpl_ipv6 % ({'base': self._base_sysconf_dir,
                                      'name': self._route_name})
 
+    def is_ipv6_route(self, address):
+        return ':' in address
+
+    def to_string_ipv4(self):
+        buf = six.StringIO()
+        buf.write(_make_header())
+        if self._conf:
+            buf.write("\n")
+        # need to reindex IPv4 addresses (because Route can contain a mix of IPv4 and IPv6)
+        reindex = -1
+        for key in sorted(self._conf.keys()):
+            if 'ADDRESS' in key:
+              index = key.replace('ADDRESS','')
+              address_value = str(self._conf[key])
+              # if this is an IPv6 route, then ignore it
+              if self.is_ipv6_route(address_value):
+                  continue
+              netmask_value = str(self._conf['NETMASK' + index])
+              gateway_value = str(self._conf['GATEWAY' + index])
+
+              reindex = reindex + 1
+              buf.write("%s=%s\n" % ('ADDRESS' + str(reindex), _quote_value(address_value)))
+              buf.write("%s=%s\n" % ('GATEWAY' + str(reindex), _quote_value(gateway_value)))
+              buf.write("%s=%s\n" % ('NETMASK' + str(reindex), _quote_value(netmask_value)))
+        return buf.getvalue()
+
+    def to_string_ipv6(self):
+        buf = six.StringIO()
+        buf.write(_make_header())
+        if self._conf:
+            buf.write("\n")
+        for key in sorted(self._conf.keys()):
+            if 'ADDRESS' in key:
+              index = key.replace('ADDRESS','')
+              address_key = key
+              address_value = str(self._conf[address_key])
+              if not self.is_ipv6_route(address_value):
+                  continue
+              netmask_key = 'NETMASK' + index
+              netmask_value = str(self._conf[netmask_key])
+              gateway_key = 'GATEWAY' + index
+              gateway_value = str(self._conf[gateway_key])
+              buf.write("%s/%s via %s\n" % (address_value, netmask_value, gateway_value))
+        return buf.getvalue()
 
 class NetInterface(ConfigMap):
     """Represents a sysconfig/networking-script (and its config + children)."""
@@ -211,27 +264,70 @@ class Renderer(renderer.Renderer):
                 iface_cfg[new_key] = old_value
 
     @classmethod
-    def _render_subnet(cls, iface_cfg, route_cfg, subnet):
-        subnet_type = subnet.get('type')
-        if subnet_type == 'dhcp6':
-            iface_cfg['DHCPV6C'] = True
-            iface_cfg['IPV6INIT'] = True
-            iface_cfg['BOOTPROTO'] = 'dhcp'
-        elif subnet_type in ['dhcp4', 'dhcp']:
-            iface_cfg['BOOTPROTO'] = 'dhcp'
-        elif subnet_type == 'static':
-            iface_cfg['BOOTPROTO'] = 'static'
-            if subnet_is_ipv6(subnet):
-                iface_cfg['IPV6ADDR'] = subnet['address']
+    def _render_subnets(cls, iface_cfg, subnets):
+        # setting base values
+        iface_cfg['BOOTPROTO'] = 'none'
+        
+        # modifying base values according to subnets
+        for i, subnet in enumerate(subnets,start=len(iface_cfg.children)):
+            subnet_type = subnet.get('type')
+            if subnet_type == 'dhcp6':
                 iface_cfg['IPV6INIT'] = True
+                iface_cfg['DHCPV6C'] = True
+                iface_cfg['BOOTPROTO'] = 'dhcp'
+            elif subnet_type in ['dhcp4', 'dhcp']:
+                iface_cfg['BOOTPROTO'] = 'dhcp'
+            elif subnet_type == 'static':
+                # grep BOOTPROTO /usr/share/doc/initscripts-9.49.37/sysconfig.txt -A2 | head -3
+                # BOOTPROTO=none|bootp|dhcp
+                # 'bootp' or 'dhcp' cause a DHCP client to run on the device. Any other
+                # value causes any static configuration in the file to be applied.
+                # ==> the following should not be set to 'static', but should remain 'none'
+                #if iface_cfg['BOOTPROTO'] == 'none':
+                #    iface_cfg['BOOTPROTO'] = 'static'
+                if subnet_is_ipv6(subnet):
+                    iface_cfg['IPV6INIT'] = True
             else:
-                iface_cfg['IPADDR'] = subnet['address']
-        else:
-            raise ValueError("Unknown subnet type '%s' found"
-                             " for interface '%s'" % (subnet_type,
+                raise ValueError("Unknown subnet type '%s' found"
+                                 " for interface '%s'" % (subnet_type,
                                                       iface_cfg.name))
-        if 'netmask' in subnet:
-            iface_cfg['NETMASK'] = subnet['netmask']
+
+        # set IPv4 and IPv6 static addresses
+        ipv4_index = -1
+        ipv6_index = -1
+        for i, subnet in enumerate(subnets,start=len(iface_cfg.children)):
+            subnet_type = subnet.get('type')
+            if subnet_type == 'dhcp6':
+                continue
+            elif subnet_type in ['dhcp4', 'dhcp']:
+                continue
+            elif subnet_type == 'static':
+                if subnet_is_ipv6(subnet):
+                    ipv6_index = ipv6_index + 1
+                    if 'netmask' in subnet and str(subnet['netmask']) != "":
+                        ipv6_cidr = subnet['address'] + '/' + str(subnet['netmask'])
+                    else:
+                        ipv6_cidr = subnet['address']
+                    if ipv6_index == 0:
+                        iface_cfg['IPV6ADDR'] = ipv6_cidr
+                    elif ipv6_index == 1:
+                        iface_cfg['IPV6ADDR_SECONDARIES'] = ipv6_cidr
+                    else:
+                        iface_cfg['IPV6ADDR_SECONDARIES'] = iface_cfg['IPV6ADDR_SECONDARIES'] + " " + ipv6_cidr
+                else:
+                    ipv4_index = ipv4_index + 1
+                    if ipv4_index == 0:
+                        iface_cfg['IPADDR'] = subnet['address']
+                        if 'netmask' in subnet:
+                            iface_cfg['NETMASK'] = subnet['netmask']
+                    else:
+                        iface_cfg['IPADDR' + str(ipv4_index)] = subnet['address']
+                        if 'netmask' in subnet:
+                            iface_cfg['NETMASK' + str(ipv4_index)] = subnet['netmask']
+
+    @classmethod 
+    def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets):
+      for i, subnet in enumerate(subnets,start=len(iface_cfg.children)):
         for route in subnet.get('routes', []):
             if subnet.get('ipv6'):
                 gw_cfg = 'IPV6_DEFAULTGW'
@@ -295,15 +391,9 @@ class Renderer(renderer.Renderer):
             iface_subnets = iface.get("subnets", [])
             iface_cfg = iface_contents[iface_name]
             route_cfg = iface_cfg.routes
-            if len(iface_subnets) == 1:
-                cls._render_subnet(iface_cfg, route_cfg, iface_subnets[0])
-            elif len(iface_subnets) > 1:
-                for i, isubnet in enumerate(iface_subnets,
-                                            start=len(iface_cfg.children)):
-                    iface_sub_cfg = iface_cfg.copy()
-                    iface_sub_cfg.name = "%s:%s" % (iface_name, i)
-                    iface_cfg.children.append(iface_sub_cfg)
-                    cls._render_subnet(iface_sub_cfg, route_cfg, isubnet)
+
+            cls._render_subnets(iface_cfg, iface_subnets)
+            cls._render_subnet_routes(iface_cfg, route_cfg, iface_subnets)
 
     @classmethod
     def _render_bond_interfaces(cls, network_state, iface_contents):
@@ -387,7 +477,8 @@ class Renderer(renderer.Renderer):
                     if iface_cfg:
                         contents[iface_cfg.path] = iface_cfg.to_string()
             if iface_cfg.routes:
-                contents[iface_cfg.routes.path] = iface_cfg.routes.to_string()
+                contents[iface_cfg.routes.path_ipv4] = iface_cfg.routes.to_string_ipv4()
+                contents[iface_cfg.routes.path_ipv6] = iface_cfg.routes.to_string_ipv6()
         return contents
 
     def render_network_state(self, network_state, target=None):
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index 8837066..6d6b985 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -431,7 +431,7 @@ NETWORKING=yes
             expected_buf = '''
 # Created by cloud-init on instance boot automatically, do not edit.
 #
-BOOTPROTO=static
+BOOTPROTO=none
 DEVICE=eth0
 IPADDR=192.168.1.5
 NETMASK=255.255.255.0
@@ -488,7 +488,6 @@ NETWORKING=yes
                 mock.patch.object(util, 'load_file', return_value=''))
             mocks.enter_context(
                 mock.patch.object(os.path, 'isfile', return_value=False))
-
             rh_distro.apply_network(BASE_NET_CFG_IPV6, False)
 
             self.assertEqual(len(write_bufs), 4)
@@ -581,11 +580,10 @@ IPV6_AUTOCONF=no
             expected_buf = '''
 # Created by cloud-init on instance boot automatically, do not edit.
 #
-BOOTPROTO=static
+BOOTPROTO=none
 DEVICE=eth0
-IPV6ADDR=2607:f0d0:1002:0011::2
+IPV6ADDR=2607:f0d0:1002:0011::2/64
 IPV6INIT=yes
-NETMASK=64
 NM_CONTROLLED=no
 ONBOOT=yes
 TYPE=Ethernet
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 89e7536..05fa8f9 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -136,7 +136,7 @@ OS_SAMPLES = [
              """
 # Created by cloud-init on instance boot automatically, do not edit.
 #
-BOOTPROTO=static
+BOOTPROTO=none
 DEFROUTE=yes
 DEVICE=eth0
 GATEWAY=172.19.3.254
@@ -204,38 +204,14 @@ nameserver 172.19.0.12
 # Created by cloud-init on instance boot automatically, do not edit.
 #
 BOOTPROTO=none
-DEVICE=eth0
-HWADDR=fa:16:3e:ed:9a:59
-NM_CONTROLLED=no
-ONBOOT=yes
-TYPE=Ethernet
-USERCTL=no
-""".lstrip()),
-            ('etc/sysconfig/network-scripts/ifcfg-eth0:0',
-             """
-# Created by cloud-init on instance boot automatically, do not edit.
-#
-BOOTPROTO=static
 DEFROUTE=yes
-DEVICE=eth0:0
+DEVICE=eth0
 GATEWAY=172.19.3.254
 HWADDR=fa:16:3e:ed:9a:59
 IPADDR=172.19.1.34
+IPADDR1=10.0.0.10
 NETMASK=255.255.252.0
-NM_CONTROLLED=no
-ONBOOT=yes
-TYPE=Ethernet
-USERCTL=no
-""".lstrip()),
-            ('etc/sysconfig/network-scripts/ifcfg-eth0:1',
-             """
-# Created by cloud-init on instance boot automatically, do not edit.
-#
-BOOTPROTO=static
-DEVICE=eth0:1
-HWADDR=fa:16:3e:ed:9a:59
-IPADDR=10.0.0.10
-NETMASK=255.255.255.0
+NETMASK1=255.255.255.0
 NM_CONTROLLED=no
 ONBOOT=yes
 TYPE=Ethernet
@@ -264,8 +240,9 @@ nameserver 172.19.0.12
                     "gateway": "172.19.3.254",
                 }],
                 "ip_address": "172.19.1.34", "id": "network0"
-            }, {
-                "network_id": "public-ipv6",
+            }, 
+            {
+                "network_id": "public-ipv6-a",
                 "type": "ipv6", "netmask": "",
                 "link": "tap1a81968a-79",
                 "routes": [
@@ -276,7 +253,24 @@ nameserver 172.19.0.12
                     }
                 ],
                 "ip_address": "2001:DB8::10", "id": "network1"
-            }],
+            },
+            {
+                "network_id": "public-ipv6-b",
+                "type": "ipv6", "netmask": "64",
+                "link": "tap1a81968a-79",
+                "routes": [
+                ],
+                "ip_address": "2001:DB9::10", "id": "network2"
+            },
+            {
+                "network_id": "public-ipv6-c",
+                "type": "ipv6", "netmask": "64",
+                "link": "tap1a81968a-79",
+                "routes": [
+                ],
+                "ip_address": "2001:DB10::10", "id": "network3"
+            }
+            ],
             "links": [
                 {
                     "ethernet_mac_address": "fa:16:3e:ed:9a:59",
@@ -295,41 +289,16 @@ nameserver 172.19.0.12
 # Created by cloud-init on instance boot automatically, do not edit.
 #
 BOOTPROTO=none
-DEVICE=eth0
-HWADDR=fa:16:3e:ed:9a:59
-NM_CONTROLLED=no
-ONBOOT=yes
-TYPE=Ethernet
-USERCTL=no
-""".lstrip()),
-            ('etc/sysconfig/network-scripts/ifcfg-eth0:0',
-             """
-# Created by cloud-init on instance boot automatically, do not edit.
-#
-BOOTPROTO=static
 DEFROUTE=yes
-DEVICE=eth0:0
+DEVICE=eth0
 GATEWAY=172.19.3.254
 HWADDR=fa:16:3e:ed:9a:59
 IPADDR=172.19.1.34
-NETMASK=255.255.252.0
-NM_CONTROLLED=no
-ONBOOT=yes
-TYPE=Ethernet
-USERCTL=no
-""".lstrip()),
-            ('etc/sysconfig/network-scripts/ifcfg-eth0:1',
-             """
-# Created by cloud-init on instance boot automatically, do not edit.
-#
-BOOTPROTO=static
-DEFROUTE=yes
-DEVICE=eth0:1
-HWADDR=fa:16:3e:ed:9a:59
 IPV6ADDR=2001:DB8::10
+IPV6ADDR_SECONDARIES="2001:DB9::10/64 2001:DB10::10/64"
 IPV6INIT=yes
 IPV6_DEFAULTGW=2001:DB8::1
-NETMASK=
+NETMASK=255.255.252.0
 NM_CONTROLLED=no
 ONBOOT=yes
 TYPE=Ethernet

Follow ups