Merge ~raharper/cloud-init:sysconfig-handle-global-static-routes into cloud-init:master

[Ryan Harper]

net/sysconfig: handle global static routes

Cloud-init network-config V1 format allows configuration of "global"
static routes which are not directly associated with any interface
in particular. These routes were not rendered under sysconfig which
generally prefers routes to be part of an interface subnet configuration.
If the configuration includes global routes, sysconfig will now render
these static routes to /etc/sysconfig/static-routes file with the 'any'
parameter indicating it will be active for any network interface.

This change does not affect existing subnet routes which continue to
be rendered into /etc/sysconfig/network-scripts/route-$iface and
route6-$iface as needed.

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

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

one useful comment/question and some nits.

5afdc68... by Ryan Harper on 2018-03-27

net/sysconfig refactors

- move ip-route keyword mapping outside of loop
- move prefix 'any' to entry construction

[Ryan Harper (raharper)]

Thanks for the review, fixing up most.

We do have some discussion to be had re: sysconfig capture and redeploy w.r.t what files cloud-init will leave around, also cloud-init clean for network related configuration files.

2b459ab... by Ryan Harper on 2018-03-27

reduce vertical spacing some more

- move regex out of loop and compress to two lines
- keep list comprehension to filter non route entries from regex search
- convert match into dictionary in one line
- use ternary operator to append matched route when found
- network_config is not needed in _assert_static_routes
- no longer break out network_config to pass to static_routes
- add an else clause to assert_static_routes to make sure we don't
  have rendered static routes file when we shouldnt

c79e5fc... by Ryan Harper on 2018-03-27

Make tox happy

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

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I filed a bug
 https://bugs.launchpad.net/cloud-init/+bug/1759324
to handle "make network rendering cleanup after itself."

Unmerged commits

c79e5fc... by Ryan Harper on 2018-03-27

Make tox happy

2b459ab... by Ryan Harper on 2018-03-27

reduce vertical spacing some more

- move regex out of loop and compress to two lines
- keep list comprehension to filter non route entries from regex search
- convert match into dictionary in one line
- use ternary operator to append matched route when found
- network_config is not needed in _assert_static_routes
- no longer break out network_config to pass to static_routes
- add an else clause to assert_static_routes to make sure we don't
  have rendered static routes file when we shouldnt

5afdc68... by Ryan Harper on 2018-03-27

net/sysconfig refactors

- move ip-route keyword mapping outside of loop
- move prefix 'any' to entry construction

194f739... by Ryan Harper on 2018-03-23

net/sysconfig: handle global static routes

Cloud-init network-config V1 format allows configuration of "global"
static routes which are not directly associated with any interface
in particular. These routes were not rendered under sysconfig which
generally prefers routes to be part of an interface subnet configuration.
If the configuration includes global routes, sysconfig will now render
these static routes to /etc/sysconfig/static-routes file with the 'any'
parameter indicating it will be active for any network interface.

This change does not affect existing subnet routes which continue to
be rendered into /etc/sysconfig/network-scripts/route-$iface and
route6-$iface as needed.

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 39d89c4..f8627f3 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -550,6 +550,26 @@ class Renderer(renderer.Renderer):
             cls._render_subnet_routes(iface_cfg, route_cfg, iface_subnets)
 
     @classmethod
+    def _render_static_routes(cls, base_sysconf_dir, network_state, contents):
+        """Render global routes into /etc/sysconfig/static-routes file"""
+        sr_content = []
+        ipr_mapping = {'network': 'net', 'netmask': 'netmask',
+                       'metric': 'metric', 'gateway': 'gw'}
+        for route in network_state.iter_routes():
+            route_fmt = []
+            # use a specific order to generate a net-tools/route command
+            for kw in ['network', 'netmask', 'metric', 'gateway']:
+                if kw in route:
+                    route_fmt.extend([ipr_mapping[kw], str(route[kw])])
+            if route_fmt:
+                sr_content.append("any " + " ".join(route_fmt))
+
+        if sr_content:
+            static_routes_fn = os.path.join(base_sysconf_dir, 'static-routes')
+            sr_content = [_make_header()] + sr_content
+            contents[static_routes_fn] = "\n".join(sr_content) + "\n"
+
+    @classmethod
     def _render_sysconfig(cls, base_sysconf_dir, network_state):
         '''Given state, return /etc/sysconfig files + contents'''
         iface_contents = {}
@@ -576,6 +596,8 @@ class Renderer(renderer.Renderer):
                     iface_cfg.routes.to_string("ipv4")
                 contents[iface_cfg.routes.path_ipv6] = \
                     iface_cfg.routes.to_string("ipv6")
+        cls._render_static_routes(base_sysconf_dir, network_state, contents)
+
         return contents
 
     def render_network_state(self, network_state, target=None):
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index c12a487..21bb672 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -21,6 +21,7 @@ import gzip
 import io
 import json
 import os
+import re
 import textwrap
 import yaml
 
@@ -1575,6 +1576,7 @@ class TestSysConfigRendering(CiTestCase):
     def _render_and_read(self, network_config=None, state=None, dir=None):
         if dir is None:
             dir = self.tmp_dir()
+        self.render_dir = dir
 
         if network_config:
             ns = network_state.parse_net_config_data(network_config)
@@ -1582,6 +1584,7 @@ class TestSysConfigRendering(CiTestCase):
             ns = state
         else:
             raise ValueError("Expected data or state, got neither")
+        self.network_state = ns
 
         renderer = sysconfig.Renderer()
         renderer.render_network_state(ns, dir)
@@ -1610,6 +1613,32 @@ class TestSysConfigRendering(CiTestCase):
         if missing:
             raise AssertionError("Missing headers in: %s" % missing)
 
+    def _assert_static_routes(self, found):
+        static_routes = list(self.network_state.iter_routes())
+        snet_keys = ['network', 'netmask', 'metric', 'gateway']
+        if len(static_routes) > 0:
+            self.assertIn('/etc/sysconfig/static-routes', found)
+            sroute_fn = os.path.join(self.render_dir,
+                                     'etc/sysconfig/static-routes')
+            self.assertTrue(os.path.exists(sroute_fn))
+            sroute_content = util.load_file(sroute_fn)
+            sr_re = (r'^any\snet\s(?P<network>\S+)\snetmask\s(?P<netmask>\S+)'
+                     r'\smetric\s(?P<metric>\S+)\sgw\s(?P<gateway>\S+)')
+            for line in [line for line in sroute_content.splitlines()
+                         if line.startswith('any')]:
+                found_route = re.search(sr_re, line).groupdict('')
+                if 'metric' in found_route:
+                    found_route['metric'] = int(found_route['metric'])
+                found_match = []
+                for sroute in static_routes:
+                    missing = [key for key in snet_keys
+                               if found_route[key] != sroute[key]]
+                    if not missing:
+                        found_match.append(sroute)
+                self.assertEqual(1, len(found_match))
+        else:
+            self.assertNotIn('/etc/sysconfig/static-routes', found)
+
     @mock.patch("cloudinit.net.sys_dev_path")
     @mock.patch("cloudinit.net.read_sys_net")
     @mock.patch("cloudinit.net.get_devicelist")
@@ -1792,42 +1821,49 @@ USERCTL=no
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_vlan_config(self):
         entry = NETWORK_CONFIGS['vlan']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_bridge_config(self):
         entry = NETWORK_CONFIGS['bridge']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_manual_config(self):
         entry = NETWORK_CONFIGS['manual']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_all_config(self):
         entry = NETWORK_CONFIGS['all']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_small_config(self):
         entry = NETWORK_CONFIGS['small']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
     def test_v4_and_v6_static_config(self):
         entry = NETWORK_CONFIGS['v4_and_v6_static']
         found = self._render_and_read(network_config=yaml.load(entry['yaml']))
         self._compare_files_to_expected(entry['expected_sysconfig'], found)
         self._assert_headers(found)
+        self._assert_static_routes(found)
 
 
 class TestEniNetRendering(CiTestCase):