cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #04497
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