i have some small comments.
don't worry about fixing the 'handle_' comment, but i do worry about the
confusion.
Diff comments:
> diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
> index 803ac74..22ae998 100755
> --- a/cloudinit/distros/__init__.py
> +++ b/cloudinit/distros/__init__.py
> @@ -73,7 +73,7 @@ class Distro(object):
>
> def _supported_write_network_config(self, network_config):
> priority = util.get_cfg_by_path(
> - self._cfg, ('network', 'renderers'), None)
> + self._cfg, ('system_info', 'network', 'renderers'), None)
I'm pretty sure this is wrong.
distro gets as its config the system_info. it does not get access to the full
thing. see usage of _cfg via 'get_option' and '_get_arch_package_mirror_info'
for my reasoning. I did not test though.
>
> name, render_cls = renderers.select(priority=priority)
> LOG.debug("Selected renderer '%s' from priority list: %s",
> diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
> index 1101f02..26267c3 100644
> --- a/cloudinit/distros/debian.py
> +++ b/cloudinit/distros/debian.py
> @@ -75,7 +80,8 @@ class Distro(distros.Distro):
> self.package_command('install', pkgs=pkglist)
>
> def _write_network(self, settings):
> - util.write_file(NETWORK_CONF_FN, settings)
> + # this is always going to be legacy based
# _write_network is legacy, it will always write eni
that sound reasonable?
> + util.write_file(self.network_conf_fn["eni"], settings)
> return ['all']
>
> def _write_network_config(self, netconfig):
> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
> index 90b2835..ba84059 100644
> --- a/cloudinit/net/network_state.py
> +++ b/cloudinit/net/network_state.py
> @@ -103,14 +114,21 @@ class CommandHandlerMeta(type):
>
> class NetworkState(object):
>
> - def __init__(self, network_state, version=NETWORK_STATE_VERSION):
> + def __init__(self, network_state, version=NETWORK_STATE_VERSION,
> + config=None):
I dont understand why NetworkState is now getting a 'config', as it doesnt look
like it gets used for anything.
> self._network_state = copy.deepcopy(network_state)
> self._version = version
> + self._config = copy.deepcopy(config)
> + self.use_ipv6 = network_state.get('use_ipv6', False)
>
> @property
> def version(self):
> return self._version
>
> + @property
> + def config(self):
> + return self._config
> +
> def iter_routes(self, filter_func=None):
> for route in self._network_state.get('routes', []):
> if filter_func is not None:
> @@ -407,6 +471,239 @@ class NetworkStateInterpreter(object):
> }
> routes.append(route)
>
> + # V2 handlers
can we name these handle_v2_bonds, handle_v2_ethernets ?....
just looking at this diff, I thought "Why are we adding support now for 'bonds'
and 'vlans'?" I'm sure I'll be confused by that again.
i realize that this is a bit involved due to the CommadnHandlerMeta path.. but
this is going to get confusing.
> + def handle_bonds(self, command):
> + '''
> + v2_command = {
> + bond0: {
> + 'interfaces': ['interface0', 'interface1'],
> + 'miimon': 100,
> + 'mode': '802.3ad',
> + 'xmit_hash_policy': 'layer3+4'},
> + bond1: {
> + 'bond-slaves': ['interface2', 'interface7'],
> + 'mode': 1
> + }
> + }
> +
> + v1_command = {
> + 'type': 'bond'
> + 'name': 'bond0',
> + 'bond_interfaces': [interface0, interface1],
> + 'params': {
> + 'bond-mode': '802.3ad',
> + 'bond_miimon: 100,
> + 'bond_xmit_hash_policy': 'layer3+4',
> + }
> + }
> +
> + '''
> + self._handle_bond_bridge(command, cmd_type='bond')
> +
> + def handle_bridges(self, command):
> +
> + '''
> + v2_command = {
> + br0: {
> + 'interfaces': ['interface0', 'interface1'],
> + 'fd': 0,
> + 'stp': 'off',
> + 'maxwait': 0,
> + }
> + }
> +
> + v1_command = {
> + 'type': 'bridge'
> + 'name': 'br0',
> + 'bridge_interfaces': [interface0, interface1],
> + 'params': {
> + 'bridge_stp': 'off',
> + 'bridge_fd: 0,
> + 'bridge_maxwait': 0
> + }
> + }
> +
> + '''
> + self._handle_bond_bridge(command, cmd_type='bridge')
> +
> + def handle_ethernets(self, command):
> + '''
> + ethernets:
> + eno1:
> + match:
> + macaddress: 00:11:22:33:44:55
> + wakeonlan: true
> + dhcp4: true
> + dhcp6: false
> + addresses:
> + - 192.168.14.2/24
> + - 2001:1::1/64
> + gateway4: 192.168.14.1
> + gateway6: 2001:1::2
> + nameservers:
> + search: [foo.local, bar.local]
> + addresses: [8.8.8.8, 8.8.4.4]
> + lom:
> + match:
> + driver: ixgbe
> + set-name: lom1
> + dhcp6: true
> + switchports:
> + match:
> + name: enp2*
> + mtu: 1280
> +
> + command = {
> + 'type': 'physical',
> + 'mac_address': 'c0:d6:9f:2c:e8:80',
> + 'name': 'eth0',
> + 'subnets': [
> + {'type': 'dhcp4'}
> + ]
> + }
> + '''
> + for eth, cfg in command.items():
> + phy_cmd = {
> + 'type': 'physical',
> + 'name': cfg.get('set-name', eth),
> + }
> + mac_address = cfg.get('match', {}).get('macaddress', None)
> + if not mac_address:
> + LOG.warning('NetworkState Version2: missing macaddress')
> +
> + for key in ['mtu', 'match', 'wakeonlan']:
> + if key in cfg:
> + phy_cmd.update({key: cfg.get(key)})
> +
> + subnets = self._v2_to_v1_ipcfg(cfg)
> + if len(subnets) > 0:
> + phy_cmd.update({'subnets': subnets})
> +
> + LOG.debug('v2(ethernets) -> v1(physical):\n%s', phy_cmd)
> + self.handle_physical(phy_cmd)
> +
> + def handle_vlans(self, command):
> + '''
> + v2_vlans = {
> + 'eth0.123': {
> + 'id': 123,
> + 'link': 'eth0',
> + 'dhcp4': True,
> + }
> + }
> +
> + v1_command = {
> + 'type': 'vlan',
> + 'name': 'eth0.123',
> + 'vlan_link': 'eth0',
> + 'vlan_id': 123,
> + 'subnets': [{'type': 'dhcp4'}],
> + }
> + '''
> + for vlan, cfg in command.items():
> + vlan_cmd = {
> + 'type': 'vlan',
> + 'name': vlan,
> + 'vlan_id': cfg.get('id'),
> + 'vlan_link': cfg.get('link'),
> + }
> + subnets = self._v2_to_v1_ipcfg(cfg)
> + if len(subnets) > 0:
> + vlan_cmd.update({'subnets': subnets})
> + LOG.debug('v2(vlans) -> v1(vlan):\n%s', vlan_cmd)
> + self.handle_vlan(vlan_cmd)
> +
> + def handle_wifis(self, command):
> + raise NotImplemented('NetworkState V2: Skipping wifi configuration')
> +
> + def _v2_common(self, cfg):
> + LOG.debug('v2_common: handling config:\n%s', cfg)
> + if 'nameservers' in cfg:
> + search = cfg.get('nameservers').get('search', [])
> + dns = cfg.get('nameservers').get('addresses', [])
> + name_cmd = {'type': 'nameserver'}
> + if len(search) > 0:
> + name_cmd.update({'search': search})
> + if len(dns) > 0:
> + name_cmd.update({'addresses': dns})
> + LOG.debug('v2(nameserver) -> v1(nameserver):\n%s', name_cmd)
> + self.handle_nameserver(name_cmd)
> +
> + def _handle_bond_bridge(self, command, cmd_type=None):
> + """Common handler for bond and bridge types"""
> + for item_name, item_cfg in command.items():
> + item_params = dict((key, value) for (key, value) in
> + item_cfg.items() if key not in
> + NETWORK_V2_KEY_FILTER)
> + v1_cmd = {
> + 'type': cmd_type,
> + 'name': item_name,
> + cmd_type + '_interfaces': item_cfg.get('interfaces'),
> + 'params': item_params,
> + }
> + subnets = self._v2_to_v1_ipcfg(item_cfg)
> + if len(subnets) > 0:
> + v1_cmd.update({'subnets': subnets})
> +
> + LOG.debug('v2(%ss) -> v1(%s):\n%s', cmd_type, cmd_type, v1_cmd)
> + self.handle_bridge(v1_cmd)
> +
> + def _v2_to_v1_ipcfg(self, cfg):
> + """Common ipconfig extraction from v2 to v1 subnets array."""
> +
> + subnets = []
> + if 'dhcp4' in cfg:
> + subnets.append({'type': 'dhcp4'})
> + if 'dhcp6' in cfg:
> + self.use_ipv6 = True
> + subnets.append({'type': 'dhcp6'})
> +
> + gateway4 = None
> + gateway6 = None
> + for address in cfg.get('addresses', []):
> + subnet = {
> + 'type': 'static',
> + 'address': address,
> + }
> +
> + routes = []
> + for route in cfg.get('routes', []):
> + route_addr = route.get('to')
> + if "/" in route_addr:
> + route_addr, route_cidr = route_addr.split("/")
> + route_netmask = cidr2mask(route_cidr)
> + subnet_route = {
> + 'address': route_addr,
> + 'netmask': route_netmask,
> + 'gateway': route.get('via')
> + }
> + routes.append(subnet_route)
> + if len(routes) > 0:
> + subnet.update({'routes': routes})
> +
> + if ":" in address:
> + if 'gateway6' in cfg and gateway6 is None:
> + gateway6 = cfg.get('gateway6')
> + subnet.update({'gateway': gateway6})
> + else:
> + if 'gateway4' in cfg and gateway4 is None:
> + gateway4 = cfg.get('gateway4')
> + subnet.update({'gateway': gateway4})
> +
> + subnets.append(subnet)
> + return subnets
> +
> +
> +def subnet_is_ipv6(subnet):
> + """ Common helper for checking network_state subnets for ipv6"""
> + # 'static6' or 'dhcp6'
> + if subnet['type'].endswith('6'):
> + # This is a request for DHCPv6.
> + return True
> + elif subnet['type'] == 'static' and ":" in subnet['address']:
> + return True
> + return False
> +
>
> def cidr2mask(cidr):
> mask = [0, 0, 0, 0]
> diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
> index 117b515..23ac2e3 100644
> --- a/cloudinit/net/sysconfig.py
> +++ b/cloudinit/net/sysconfig.py
> @@ -390,19 +391,32 @@ class Renderer(renderer.Renderer):
> return contents
>
> def render_network_state(self, network_state, target=None):
> + file_mode = 0o644
> base_sysconf_dir = util.target_path(target, self.sysconf_dir)
> for path, data in self._render_sysconfig(base_sysconf_dir,
> network_state).items():
> - util.write_file(path, data)
> + util.write_file(path, data, file_mode)
> if self.dns_path:
> dns_path = util.target_path(target, self.dns_path)
> resolv_content = self._render_dns(network_state,
> existing_dns_path=dns_path)
> - util.write_file(dns_path, resolv_content)
> + util.write_file(dns_path, resolv_content, file_mode)
> if self.netrules_path:
> netrules_content = self._render_persistent_net(network_state)
> netrules_path = util.target_path(target, self.netrules_path)
> - util.write_file(netrules_path, netrules_content)
> + util.write_file(netrules_path, netrules_content, file_mode)
> +
> + # always write /etc/sysconfig/network configuration
> + sysconfig_path = util.target_path(target, "etc/sysconfig/network")
> + netcfg = [
> + ('# Created by cloud-init on instance boot automatically, '
i think this should be
netcg = _make_header() + ["NETWORKING="yes"]
> + 'do not edit.'),
> + 'NETWORKING=yes',
> + ]
> + if network_state.use_ipv6:
> + netcfg.append('NETWORKING_IPV6=yes')
> + netcfg.append('IPV6_AUTOCONF=no')
> + util.write_file(sysconfig_path, "\n".join(netcfg) + "\n", file_mode)
>
>
> def available(target=None):
--
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/320291
Your team cloud init development team is requested to review the proposed merge
of ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master.
_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
