[Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
The proposal to merge ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master has been updated. Status: Needs review => Merged For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:1c1fc3a8841a85b86b51931c9f8058832ad3b00a https://jenkins.ubuntu.com/server/job/cloud-init-ci/145/ Executed test runs: FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/145/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/145/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/145/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/145/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/145/console Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/145/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:9bcc80e67b77dbce265de68c11f5f90916988ea0 https://jenkins.ubuntu.com/server/job/cloud-init-ci/144/ Executed test runs: FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/144/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/144/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/144/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/144/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/144/console Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/144/rebuild -- 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
[Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
The proposal to merge ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master has been updated. Description changed to: cloudinit.net: add network config v2 parsing and rendering Network configuration version 2 format is implemented in a package called netplan (nplan)[1] which allows consolidated network config for multiple network controllers. - Add a new netplan renderer - Update default policy, placing eni and sysconfig first This requires explicit policy to enable netplan over eni on systems which have both (Yakkety, Zesty, UC16) - Allow any network state (parsed from any format cloud-init supports) to render to v2 if system supports netplan. - Move eni's _subnet_is_ipv6 to common code for use by other renderers - Make sysconfig renderer always emit /etc/syconfig/network configuration - Update cloud-init.service systemd unit to also wait on systemd-networkd-wait-online.service 1. https://lists.ubuntu.com/archives/ubuntu-devel/2016-July/039464.html For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
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 needed this change to fix up the unittests in
tests/unittests/test_distro/test_netconfig.py
I've not tested outside of unittest; If you remove that change and run
unittest on test_netconfig you can see.
It manifests itself as getting policy of None and then attempts to render eni
for everything (so netplan and sysconfig renders write out udev files etc).
>
> 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
ACK
> +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):
Ill revisit; I'm somewhat frustrated with the NSI vs NS objects; it maybe
unneeded changes when doing the initial plumbing of getting v2 to pass through
config to the renderers which only get (NetworkState) objects.
> 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
ACK; it should be a simple change, we basically had 'handle_bond' (v1) and
'handle_bonds' (v2); v2 being plural; That's not easy to see.
> +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
> +
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
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:
>
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:c43f407e2ac41e3e474dbfd023fc6325919819a9 https://jenkins.ubuntu.com/server/job/cloud-init-ci/139/ Executed test runs: FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/139/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/139/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/139/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/139/console FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/139/console Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/139/rebuild -- 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
[Cloud-init-dev] [Merge] ~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master
Ryan Harper has proposed merging
~raharper/cloud-init:rebased-netconfig-v2-passthrough into cloud-init:master.
Requested reviews:
cloud init development team (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/320291
cloudinit.net: add v2 parsing, and v2 rendering
Network configuration version2 format is implemented in a package
called netplan (nplan)[1] which allows consolidated network config
for multiple network controllers.
- Add a new netplan renderer
- Update default policy, placing eni and sysconfig first
This requires explicit policy to enable netplan over eni
on systems which have both (Yakkety, Zesty, UC16)
- Allow v2 configs to be passed directly to netplan
- Allow any network state (parsed from any format cloud-init supports) to
render to v2 if system supports netplan.
- Move eni's _subnet_is_ipv6 to common code for use by other renderers
- Fix to base distro class for looking up path to
system_info/network/renderers
- Make sysconfig renderer always emit /etc/syconfig/network configuration
- Update cloud-init.service systemd unit to also wait on
systemd-networkd-wait-online.service
1. https://lists.ubuntu.com/archives/ubuntu-devel/2016-July/039464.html
--
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.
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)
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
@@ -42,11 +42,16 @@ NETWORK_CONF_FN = "/etc/network/interfaces.d/50-cloud-init.cfg"
class Distro(distros.Distro):
hostname_conf_fn = "/etc/hostname"
locale_conf_fn = "/etc/default/locale"
+network_conf_fn = {
+"eni": "/etc/network/interfaces.d/50-cloud-init.cfg",
+"netplan": "/etc/netplan/50-cloud-init.yaml"
+}
renderer_configs = {
-'eni': {
-'eni_path': NETWORK_CONF_FN,
-'eni_header': ENI_HEADER,
-}
+"eni": {"eni_path": network_conf_fn["eni"],
+"eni_header": ENI_HEADER},
+"netplan": {"netplan_path": network_conf_fn["netplan"],
+"netplan_header": ENI_HEADER,
+"postcmds": True}
}
def __init__(self, name, cfg, paths):
@@ -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
+util.write_file(self.network_conf_fn["eni"], settings)
return ['all']
def _write_network_config(self, netconfig):
diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 9d39a2b..6f6cc48 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -8,6 +8,7 @@ import re
from . import ParserError
from . import renderer
+from .network_state import subnet_is_ipv6
from cloudinit import util
@@ -111,16 +112,6 @@ def _iface_start_entry(iface, index, render_hwaddress=False):
return lines
-def _subnet_is_ipv6(subnet):
-# '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 _parse_deb_config_data(ifaces, contents, src_dir, src_path):
"""Parses the file contents, placing result into ifaces.
@@ -370,7 +361,7 @@ class Renderer(renderer.Renderer):
iface['mode'] = subnet['type']
iface['control'] = subnet.get('control', 'auto')
subnet_inet = 'inet'
-if _subnet_is_ipv6(subnet):
+if subnet_is_ipv6(subnet):
subnet_inet += '6'
iface['inet'] = subnet_inet
if subnet['type'].startswith('dhcp'):
@@ -486,7 +477,7 @@ class Renderer(renderer.Renderer):
def network_state_to_eni(network_state, header=None, render_hwaddress=False):
# render the provided network state, return a string of equivalent eni
eni_path = 'etc/network/interfaces'
-renderer = Renderer({
+renderer = Renderer(config={
'eni_path': eni_path,
'eni_header': header,
'links_path_prefix':
