cloud-init-dev team mailing list archive

[Mike Gerdts <mike.gerdts@xxxxxxxxxx>]

Mike Gerdts has proposed merging ~mgerdts/cloud-init:lp1763512 into cloud-init:master.

Commit message:
DataSourceSmartOS: sdc:routes is ignored

The routes specified in sdc:routes need to be added to each subnet and
at the global scope.

Co-authored-by: Mike Gerdts <mike.gerdts@xxxxxxxxxx>

LP: #1763512

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1763512 in cloud-init: "DataSourceSmartOS ignores sdc:routes"
  https://bugs.launchpad.net/cloud-init/+bug/1763512

For more details, see:
https://code.launchpad.net/~mgerdts/cloud-init/+git/cloud-init/+merge/344168
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~mgerdts/cloud-init:lp1763512 into cloud-init:master.

diff --git a/cloudinit/sources/DataSourceSmartOS.py b/cloudinit/sources/DataSourceSmartOS.py
index 4ea00eb..db4329d 100644
--- a/cloudinit/sources/DataSourceSmartOS.py
+++ b/cloudinit/sources/DataSourceSmartOS.py
@@ -299,6 +299,7 @@ class DataSourceSmartOS(sources.DataSource):
         self.userdata_raw = ud
         self.vendordata_raw = md['vendor-data']
         self.network_data = md['network-data']
+        self.routes_data = md['routes']
 
         self._set_provisioned()
         return True
@@ -322,7 +323,8 @@ class DataSourceSmartOS(sources.DataSource):
                     convert_smartos_network_data(
                         network_data=self.network_data,
                         dns_servers=self.metadata['dns_servers'],
-                        dns_domain=self.metadata['dns_domain']))
+                        dns_domain=self.metadata['dns_domain'],
+                        routes=self.routes_data))
         return self._network_config
 
 
@@ -761,7 +763,8 @@ def get_smartos_environ(uname_version=None, product_name=None):
 
 # Convert SMARTOS 'sdc:nics' data to network_config yaml
 def convert_smartos_network_data(network_data=None,
-                                 dns_servers=None, dns_domain=None):
+                                 dns_servers=None, dns_domain=None,
+                                 routes=None):
     """Return a dictionary of network_config by parsing provided
        SMARTOS sdc:nics configuration data
 
@@ -801,6 +804,10 @@ def convert_smartos_network_data(network_data=None,
             'scope',
             'type',
         ],
+        'route': [
+            'destination',
+            'gateway',
+        ],
     }
 
     if dns_servers:
@@ -815,6 +822,9 @@ def convert_smartos_network_data(network_data=None,
     else:
         dns_domain = []
 
+    if not routes:
+        routes = []
+
     def is_valid_ipv4(addr):
         return '.' in addr
 
@@ -841,6 +851,7 @@ def convert_smartos_network_data(network_data=None,
             if ip == "dhcp":
                 subnet = {'type': 'dhcp4'}
             else:
+                routeents = []
                 subnet = dict((k, v) for k, v in nic.items()
                               if k in valid_keys['subnet'])
                 subnet.update({
@@ -862,10 +873,49 @@ def convert_smartos_network_data(network_data=None,
                             pgws[proto]['gw'] = gateways[0]
                             subnet.update({'gateway': pgws[proto]['gw']})
 
+                for route in routes:
+                    rcfg = dict((k, v) for k, v in route.items()
+                                if k in valid_keys['route'])
+                    # Linux uses the value of 'gateway' to determine
+                    # automatically if the route is a forward/next-hop
+                    # (non-local IP for gateway) or an interface/resolver
+                    # (local IP for gateway).  So we can ignore the
+                    # 'interface' attribute of sdc:routes, because SDC
+                    # guarantees that the gateway is a local IP for
+                    # "interface=true".
+                    #
+                    # Eventually we should be smart and compare "gateway"
+                    # to see if it's in the prefix.  We can then smartly
+                    # add or not-add this route.  But for now,
+                    # when in doubt, use brute force! Routes for everyone!
+                    rcfg.update({
+                        'network': route['dst'],
+                        'gateway': route['gateway'],
+                    })
+                    routeents.append(rcfg)
+                    subnet.update({'routes': routeents})
+
             subnets.append(subnet)
         cfg.update({'subnets': subnets})
         config.append(cfg)
 
+    # More brute force, add routes here at the global-scope too, although
+    # this doesn't work.  Not sure if it's a documentation mismatch or a
+    # problem with certain hosts (like Ubuntu 17+ featuring netplan).
+    for route in routes:
+        cfg = dict((k, v) for k, v in route.items()
+                   if k in valid_keys['route'])
+        # Linux uses the value of 'gateway' to determine automatically if
+        # the route is a forward/next-hop (non-local IP for gateway) or an
+        # interface/resolver (local IP for gateway).  So we can ignore
+        # the 'interface' attribute of sdc:routes, because SDC guarantees
+        # that the gateway is a local IP for "interface=true".
+        cfg.update({
+            'type': 'route',
+            'destination': route['dst'],
+            'gateway': route['gateway']})
+        config.append(cfg)
+
     if dns_servers:
         config.append(
             {'type': 'nameserver', 'address': dns_servers,
@@ -905,12 +955,14 @@ if __name__ == "__main__":
             keyname = SMARTOS_ATTRIB_JSON[key]
             data[key] = client.get_json(keyname)
         elif key == "network_config":
-            for depkey in ('network-data', 'dns_servers', 'dns_domain'):
+            for depkey in ('network-data', 'dns_servers', 'dns_domain',
+                           'routes'):
                 load_key(client, depkey, data)
             data[key] = convert_smartos_network_data(
                 network_data=data['network-data'],
                 dns_servers=data['dns_servers'],
-                dns_domain=data['dns_domain'])
+                dns_domain=data['dns_domain'],
+                routes=data['routes'])
         else:
             if key in SMARTOS_ATTRIB_MAP:
                 keyname, strip = SMARTOS_ATTRIB_MAP[key]
diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py
index 706e8eb..947cef7 100644
--- a/tests/unittests/test_datasource/test_smartos.py
+++ b/tests/unittests/test_datasource/test_smartos.py
@@ -1027,6 +1027,36 @@ class TestNetworkConversion(TestCase):
         found = convert_net(SDC_NICS_SINGLE_GATEWAY)
         self.assertEqual(expected, found)
 
+    def test_routes_on_all_nics(self):
+        routes = [
+            {'linklocal': False, 'dst': '3.0.0.0/8', 'gateway': '8.12.42.3'},
+            {'linklocal': False, 'dst': '4.0.0.0/8', 'gateway': '10.210.1.4'}]
+        expected = {
+            'version': 1,
+            'config': [
+                {'mac_address': '90:b8:d0:d8:82:b4', 'mtu': 1500,
+                 'name': 'net0', 'type': 'physical',
+                 'subnets': [{'address': '8.12.42.26/24',
+                              'gateway': '8.12.42.1', 'type': 'static',
+                              'routes': [{'network': '3.0.0.0/8',
+                                          'gateway': '8.12.42.3'},
+                                         {'network': '4.0.0.0/8',
+                                         'gateway': '10.210.1.4'}]}]},
+                {'mac_address': '90:b8:d0:0a:51:31', 'mtu': 1500,
+                 'name': 'net1', 'type': 'physical',
+                 'subnets': [{'address': '10.210.1.27/24', 'type': 'static',
+                              'routes': [{'network': '3.0.0.0/8',
+                                          'gateway': '8.12.42.3'},
+                                         {'network': '4.0.0.0/8',
+                                         'gateway': '10.210.1.4'}]}]},
+                {'destination': '3.0.0.0/8', 'gateway': '8.12.42.3',
+                 'type': 'route'},
+                {'destination': '4.0.0.0/8', 'gateway': '10.210.1.4',
+                 'type': 'route'}]}
+        found = convert_net(SDC_NICS_SINGLE_GATEWAY, routes=routes)
+        self.maxDiff = None
+        self.assertEqual(expected, found)
+
 
 @unittest2.skipUnless(get_smartos_environ() == SMARTOS_ENV_KVM,
                       "Only supported on KVM and bhyve guests under SmartOS")