← Back to team overview

cloud-init-dev team mailing list archive

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

 

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.

Diff comments:

> diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
> index 39d89c4..87fb96e 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 = []
> +        for route in network_state.iter_routes():
> +            ipr_mapping = {'network': 'net', 'netmask': 'netmask',

OK

> +                           'metric': 'metric', 'gateway': 'gw'}
> +            route_fmt = ['any']
> +            # 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:

Yes, good catch

> +                sr_content.append(" ".join(route_fmt))
> +
> +        if sr_content:

Hrm, yes.  That makes this a bit more tricky; we don't have a mechansim in place to tag entries in a file with the instance-id for easy filtering.  Should cloud-init clean also remove these lines?

How is this file different then the ifcfg-* and route-* files?  I suspect we have a capture/new-instance bug here on sysconfig.

> +            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 = {}
> diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
> index c12a487..8861353 100644
> --- a/tests/unittests/test_net.py
> +++ b/tests/unittests/test_net.py
> @@ -1610,6 +1613,32 @@ class TestSysConfigRendering(CiTestCase):
>          if missing:
>              raise AssertionError("Missing headers in: %s" % missing)
>  
> +    def _assert_static_routes(self, network_config, 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)
> +            for line in [line for line in sroute_content.splitlines()
> +                         if line.startswith('any')]:
> +                m = re.search(r'any\snet\s(?P<network>\S+)\s'
> +                              r'netmask\s(?P<netmask>\S+)\s'
> +                              r'metric\s(?P<metric>\S+)\s'
> +                              r'gw\s(?P<gateway>\S+)', line)

OK

> +                found_route = m.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))
> +
>      @mock.patch("cloudinit.net.sys_dev_path")
>      @mock.patch("cloudinit.net.read_sys_net")
>      @mock.patch("cloudinit.net.get_devicelist")


-- 
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/342102
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:sysconfig-handle-global-static-routes into cloud-init:master.


References