Merge ~raharper/cloud-init:fix/netplan-ipv6-mtu into cloud-init:master

Proposed by Ryan Harper
Status: Merged
Approved by: Ryan Harper
Approved revision: 8f46b8a1c633f8a6b112a92ccb2ba34268971dfa
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/cloud-init:fix/netplan-ipv6-mtu
Merge into: cloud-init:master
Diff against target: 190 lines (+54/-10)
3 files modified
cloudinit/cmd/devel/net_convert.py (+2/-0)
cloudinit/net/netplan.py (+27/-8)
tests/unittests/test_net.py (+25/-2)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Dan Watkins Approve
Review via email: mp+374627@code.launchpad.net

Commit message

net/netplan: use ipv6-mtu key for specifying ipv6 mtu values

netplan introduced an 'info' subcommand which emits yaml describing
implemented features that indicate new or changed fields and values
in the yaml that it accepts. Previously, cloud-init emitted the key
'mtu6' for ipv6 MTU values. This is not correct and netplan will
fail to parse these values. Netplan as of 0.98 supports both the
info subcommand and the ipv6-mtu key.

This branch modifies the netplan renderer to collect the netplan
info output into a 'features' property which is a list of available
feature flags which the renderer can use to modify its output. If
the command is not available, no feature flags are set and
cloud-init will render IPv6 MTU values just as MTU for the subnet.

To post a comment you must log in.
8f46b8a... by Ryan Harper

Drop print debug lines

Revision history for this message
Dan Watkins (oddbloke) wrote :

One inline question, but the main implementation LGTM.

review: Needs Information
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:110dd9f865b87746f633210cd8e6f15f54ee32dc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1228/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Ryan Harper (raharper) :
Revision history for this message
Dan Watkins (oddbloke) :
review: Approve
Revision history for this message
Dan Watkins (oddbloke) wrote :

This looks like it needs rebasing to fix CI.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:8f46b8a1c633f8a6b112a92ccb2ba34268971dfa
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1230/
Executed test runs:
    FAILED: Checkout

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py
index 1ad7e0b..6f513d9 100755
--- a/cloudinit/cmd/devel/net_convert.py
+++ b/cloudinit/cmd/devel/net_convert.py
@@ -116,6 +116,8 @@ def handle_args(name, args):
116 config['postcmds'] = False116 config['postcmds'] = False
117 # trim leading slash117 # trim leading slash
118 config['netplan_path'] = config['netplan_path'][1:]118 config['netplan_path'] = config['netplan_path'][1:]
119 # enable some netplan features
120 config['features'] = ['dhcp-use-domains', 'ipv6-mtu']
119 else:121 else:
120 r_cls = sysconfig.Renderer122 r_cls = sysconfig.Renderer
121 config = distro.renderer_configs.get('sysconfig')123 config = distro.renderer_configs.get('sysconfig')
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index e54a34e..1b039ab 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -34,7 +34,7 @@ def _get_params_dict_by_match(config, match):
34 if key.startswith(match))34 if key.startswith(match))
3535
3636
37def _extract_addresses(config, entry, ifname):37def _extract_addresses(config, entry, ifname, features=None):
38 """This method parse a cloudinit.net.network_state dictionary (config) and38 """This method parse a cloudinit.net.network_state dictionary (config) and
39 maps netstate keys/values into a dictionary (entry) to represent39 maps netstate keys/values into a dictionary (entry) to represent
40 netplan yaml.40 netplan yaml.
@@ -66,7 +66,7 @@ def _extract_addresses(config, entry, ifname):
66 'match': {'macaddress': '52:54:00:12:34:00'},66 'match': {'macaddress': '52:54:00:12:34:00'},
67 'mtu': 1501,67 'mtu': 1501,
68 'address': ['192.168.1.2/24', '2001:4800:78ff:1b:be76:4eff:fe06:1000"],68 'address': ['192.168.1.2/24', '2001:4800:78ff:1b:be76:4eff:fe06:1000"],
69 'mtu6': 1480}69 'ipv6-mtu': 1480}
7070
71 """71 """
7272
@@ -79,6 +79,8 @@ def _extract_addresses(config, entry, ifname):
79 else:79 else:
80 return [obj, ]80 return [obj, ]
8181
82 if features is None:
83 features = []
82 addresses = []84 addresses = []
83 routes = []85 routes = []
84 nameservers = []86 nameservers = []
@@ -108,8 +110,8 @@ def _extract_addresses(config, entry, ifname):
108 searchdomains += _listify(subnet.get('dns_search', []))110 searchdomains += _listify(subnet.get('dns_search', []))
109 if 'mtu' in subnet:111 if 'mtu' in subnet:
110 mtukey = 'mtu'112 mtukey = 'mtu'
111 if subnet_is_ipv6(subnet):113 if subnet_is_ipv6(subnet) and 'ipv6-mtu' in features:
112 mtukey += '6'114 mtukey = 'ipv6-mtu'
113 entry.update({mtukey: subnet.get('mtu')})115 entry.update({mtukey: subnet.get('mtu')})
114 for route in subnet.get('routes', []):116 for route in subnet.get('routes', []):
115 to_net = "%s/%s" % (route.get('network'),117 to_net = "%s/%s" % (route.get('network'),
@@ -179,6 +181,7 @@ class Renderer(renderer.Renderer):
179 """Renders network information in a /etc/netplan/network.yaml format."""181 """Renders network information in a /etc/netplan/network.yaml format."""
180182
181 NETPLAN_GENERATE = ['netplan', 'generate']183 NETPLAN_GENERATE = ['netplan', 'generate']
184 NETPLAN_INFO = ['netplan', 'info']
182185
183 def __init__(self, config=None):186 def __init__(self, config=None):
184 if not config:187 if not config:
@@ -188,6 +191,22 @@ class Renderer(renderer.Renderer):
188 self.netplan_header = config.get('netplan_header', None)191 self.netplan_header = config.get('netplan_header', None)
189 self._postcmds = config.get('postcmds', False)192 self._postcmds = config.get('postcmds', False)
190 self.clean_default = config.get('clean_default', True)193 self.clean_default = config.get('clean_default', True)
194 self._features = config.get('features', None)
195
196 @property
197 def features(self):
198 if self._features is None:
199 try:
200 info_blob, _err = util.subp(self.NETPLAN_INFO, capture=True)
201 info = util.load_yaml(info_blob)
202 self._features = info['netplan.io']['features']
203 except util.ProcessExecutionError:
204 # if the info subcommand is not present then we don't have any
205 # new features
206 pass
207 except (TypeError, KeyError) as e:
208 LOG.debug('Failed to list features from netplan info: %s', e)
209 return self._features
191210
192 def render_network_state(self, network_state, templates=None, target=None):211 def render_network_state(self, network_state, templates=None, target=None):
193 # check network state for version212 # check network state for version
@@ -271,7 +290,7 @@ class Renderer(renderer.Renderer):
271 else:290 else:
272 del eth['match']291 del eth['match']
273 del eth['set-name']292 del eth['set-name']
274 _extract_addresses(ifcfg, eth, ifname)293 _extract_addresses(ifcfg, eth, ifname, self.features)
275 ethernets.update({ifname: eth})294 ethernets.update({ifname: eth})
276295
277 elif if_type == 'bond':296 elif if_type == 'bond':
@@ -296,7 +315,7 @@ class Renderer(renderer.Renderer):
296 slave_interfaces = ifcfg.get('bond-slaves')315 slave_interfaces = ifcfg.get('bond-slaves')
297 if slave_interfaces == 'none':316 if slave_interfaces == 'none':
298 _extract_bond_slaves_by_name(interfaces, bond, ifname)317 _extract_bond_slaves_by_name(interfaces, bond, ifname)
299 _extract_addresses(ifcfg, bond, ifname)318 _extract_addresses(ifcfg, bond, ifname, self.features)
300 bonds.update({ifname: bond})319 bonds.update({ifname: bond})
301320
302 elif if_type == 'bridge':321 elif if_type == 'bridge':
@@ -331,7 +350,7 @@ class Renderer(renderer.Renderer):
331 bridge.update({'parameters': br_config})350 bridge.update({'parameters': br_config})
332 if ifcfg.get('mac_address'):351 if ifcfg.get('mac_address'):
333 bridge['macaddress'] = ifcfg.get('mac_address').lower()352 bridge['macaddress'] = ifcfg.get('mac_address').lower()
334 _extract_addresses(ifcfg, bridge, ifname)353 _extract_addresses(ifcfg, bridge, ifname, self.features)
335 bridges.update({ifname: bridge})354 bridges.update({ifname: bridge})
336355
337 elif if_type == 'vlan':356 elif if_type == 'vlan':
@@ -343,7 +362,7 @@ class Renderer(renderer.Renderer):
343 macaddr = ifcfg.get('mac_address', None)362 macaddr = ifcfg.get('mac_address', None)
344 if macaddr is not None:363 if macaddr is not None:
345 vlan['macaddress'] = macaddr.lower()364 vlan['macaddress'] = macaddr.lower()
346 _extract_addresses(ifcfg, vlan, ifname)365 _extract_addresses(ifcfg, vlan, ifname, self.features)
347 vlans.update({ifname: vlan})366 vlans.update({ifname: vlan})
348367
349 # inject global nameserver values under each all interface which368 # inject global nameserver values under each all interface which
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index d220199..21604b1 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -996,8 +996,8 @@ NETWORK_CONFIGS = {
996 addresses:996 addresses:
997 - 192.168.14.2/24997 - 192.168.14.2/24
998 - 2001:1::1/64998 - 2001:1::1/64
999 ipv6-mtu: 1500
999 mtu: 90001000 mtu: 9000
1000 mtu6: 1500
1001 """).rstrip(' '),1001 """).rstrip(' '),
1002 'yaml': textwrap.dedent("""\1002 'yaml': textwrap.dedent("""\
1003 version: 11003 version: 1
@@ -3585,7 +3585,9 @@ class TestNetplanPostcommands(CiTestCase):
35853585
3586 @mock.patch.object(netplan.Renderer, '_netplan_generate')3586 @mock.patch.object(netplan.Renderer, '_netplan_generate')
3587 @mock.patch.object(netplan.Renderer, '_net_setup_link')3587 @mock.patch.object(netplan.Renderer, '_net_setup_link')
3588 def test_netplan_render_calls_postcmds(self, mock_netplan_generate,3588 @mock.patch('cloudinit.util.subp')
3589 def test_netplan_render_calls_postcmds(self, mock_subp,
3590 mock_netplan_generate,
3589 mock_net_setup_link):3591 mock_net_setup_link):
3590 tmp_dir = self.tmp_dir()3592 tmp_dir = self.tmp_dir()
3591 ns = network_state.parse_net_config_data(self.mycfg,3593 ns = network_state.parse_net_config_data(self.mycfg,
@@ -3597,6 +3599,7 @@ class TestNetplanPostcommands(CiTestCase):
3597 render_target = 'netplan.yaml'3599 render_target = 'netplan.yaml'
3598 renderer = netplan.Renderer(3600 renderer = netplan.Renderer(
3599 {'netplan_path': render_target, 'postcmds': True})3601 {'netplan_path': render_target, 'postcmds': True})
3602 mock_subp.side_effect = iter([util.ProcessExecutionError])
3600 renderer.render_network_state(ns, target=render_dir)3603 renderer.render_network_state(ns, target=render_dir)
36013604
3602 mock_netplan_generate.assert_called_with(run=True)3605 mock_netplan_generate.assert_called_with(run=True)
@@ -3619,7 +3622,13 @@ class TestNetplanPostcommands(CiTestCase):
3619 render_target = 'netplan.yaml'3622 render_target = 'netplan.yaml'
3620 renderer = netplan.Renderer(3623 renderer = netplan.Renderer(
3621 {'netplan_path': render_target, 'postcmds': True})3624 {'netplan_path': render_target, 'postcmds': True})
3625 mock_subp.side_effect = iter([
3626 util.ProcessExecutionError,
3627 ('', ''),
3628 ('', ''),
3629 ])
3622 expected = [3630 expected = [
3631 mock.call(['netplan', 'info'], capture=True),
3623 mock.call(['netplan', 'generate'], capture=True),3632 mock.call(['netplan', 'generate'], capture=True),
3624 mock.call(['udevadm', 'test-builtin', 'net_setup_link',3633 mock.call(['udevadm', 'test-builtin', 'net_setup_link',
3625 '/sys/class/net/lo'], capture=True),3634 '/sys/class/net/lo'], capture=True),
@@ -3875,6 +3884,20 @@ class TestReadInitramfsConfig(CiTestCase):
38753884
38763885
3877class TestNetplanRoundTrip(CiTestCase):3886class TestNetplanRoundTrip(CiTestCase):
3887
3888 NETPLAN_INFO_OUT = textwrap.dedent("""
3889 netplan.io:
3890 features:
3891 - dhcp-use-domains
3892 - ipv6-mtu
3893 website: https://netplan.io/
3894 """)
3895
3896 def setUp(self):
3897 super(TestNetplanRoundTrip, self).setUp()
3898 self.add_patch('cloudinit.net.netplan.util.subp', 'm_subp')
3899 self.m_subp.return_value = (self.NETPLAN_INFO_OUT, '')
3900
3878 def _render_and_read(self, network_config=None, state=None,3901 def _render_and_read(self, network_config=None, state=None,
3879 netplan_path=None, target=None):3902 netplan_path=None, target=None):
3880 if target is None:3903 if target is None:

Subscribers

People subscribed via source and target branches