Merge ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master

Proposed by Chad Smith on 2019-08-05
Status: Merged
Approved by: Chad Smith on 2019-08-13
Approved revision: 5106244d7e7105079ec689ca34d93a61b3c8e045
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~chad.smith/cloud-init:feature/azure-to-network-v2
Merge into: cloud-init:master
Diff against target: 330 lines (+130/-39)
6 files modified
cloudinit/net/__init__.py (+11/-20)
cloudinit/net/network_state.py (+8/-4)
cloudinit/net/tests/test_init.py (+10/-9)
cloudinit/sources/DataSourceAzure.py (+6/-1)
tests/unittests/test_datasource/test_azure.py (+58/-1)
tests/unittests/test_net.py (+37/-4)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2019-08-06
Ryan Harper 2019-08-05 Approve on 2019-08-06
Review via email: mp+370970@code.launchpad.net

Commit message

azure/net: generate_fallback_nic emits network v2 config instead of v1

The function generate_fallback_config is used by Azure by default when
not consuming IMDS configuration data. This function is also used by any
datasource which does not implement it's own network config. This simple
fallback configuration sets up dhcp on the most likely NIC. It will now
emit network v2 instead of network v1.

This is a step toward moving all components talking in v2 and allows us
to avoid costly conversions between v1 and v2 for newer distributions
which rely on netplan.

To post a comment you must log in.

PASSED: Continuous integration, rev:850c0d90bdf26f45b5d771e77caee3d32d73ba74
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1026/
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/1026//rebuild

review: Approve (continuous-integration)
Ryan Harper (raharper) wrote :

One inline question.

2c1cc00... by Chad Smith on 2019-08-06

azure/net: generate_fallback_nic emits network v2 config instead of v1

The function generate_fallback_config is used by Azure by default when
not consuming IMDS configuration data. This function is also used by any
datasource which does not implement it's own network config. This simple
fallback configuration sets up dhcp on the most likely NIC. It will now
emit network v2 instead of network v1.

This is a step toward moving all components talking in v2 and allows us
to avoid costly conversions between v1 and v2 for newer distributions
which rely on netplan.

Ryan Harper (raharper) wrote :

One debugging line needs removing and some questions inline.

Chad Smith (chad.smith) :
Chad Smith (chad.smith) :

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

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

review: Needs Fixing (continuous-integration)
5106244... by Chad Smith on 2019-08-06

azure: add route-metric to net config on non-primary dhcp interfaces

To retain default network routes on primary dhcp interface, set the
route-metric to 100 for primary interface and 200+ for any secondary
interfaces. This ensures that any dhcp routes provided by the secondary
nics will not be used as a default route for external bound traffic.

Ryan Harper (raharper) wrote :

That looks nice! One comment on where to set maxDiff, but we can do that separately

review: Approve

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

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

review: Needs Fixing (continuous-integration)

PASSED: Continuous integration, rev:5106244d7e7105079ec689ca34d93a61b3c8e045
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1031/
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/1031//rebuild

review: Approve (continuous-integration)
Chad Smith (chad.smith) wrote :
Download full text (3.8 KiB)

Validated on disco given eth0 (with addtional secondary IP) and eth1 (dhcp secondary NIC)
# QUESTION: am I missing a metric 200 on 10.0.0.8?

ubuntu@SRU-worked-azure:~$ python3 -c 'from cloudinit.sources.DataSourceAzure import get_metadata_from_imds; print(get_metadata_from_imds("eth0", 1))'
{'compute': {'location': 'eastus2', 'name': 'test-sru-disco', 'offer': 'UbuntuServer', 'osType': 'Linux', 'placementGroupId': '', 'platformFaultDomain': '0', 'platformUpdateDomain': '0', 'publisher': 'Canonical', 'resourceGroupName': 'srugroup1', 'sku': '19.04-DAILY', 'subscriptionId': '12aad61c-6de4-4e53-a6c6-5aff52a83777', 'tags': '', 'version': '19.04.201908010', 'vmId': 'b838afe4-23fd-4f34-b66c-622767857444', 'vmScaleSetName': '', 'vmSize': 'Standard_DS1_v2', 'zone': ''}, 'network': {'interface': [{'ipv4': {'ipAddress': [{'privateIpAddress': '10.0.0.5', 'publicIpAddress': '40.84.36.124'}, {'privateIpAddress': '10.0.0.23', 'publicIpAddress': ''}], 'subnet': [{'address': '10.0.0.0', 'prefix': '24'}]}, 'ipv6': {'ipAddress': []}, 'macAddress': '000D3A7B7C34'}, {'ipv4': {'ipAddress': [{'privateIpAddress': '10.0.0.8', 'publicIpAddress': ''}], 'subnet': [{'address': '10.0.0.0', 'prefix': '24'}]}, 'ipv6': {'ipAddress': []}, 'macAddress': '000D3A7B9E10'}]}}
ubuntu@SRU-worked-azure:~$ cat /etc/netplan/50-cloud-init.yaml
# This file is generated from information provided by
# the datasource. Changes to it will not persist across an instance.
# To disable cloud-init's network configuration capabilities, write a file
# /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg with the following:
# network: {config: disabled}
network:
    ethernets:
        eth0:
            addresses:
            - 10.0.0.23/24
            dhcp4: true
            dhcp4-overrides:
                route-metric: 100
            match:
                macaddress: 00:0d:3a:7b:7c:34
            set-name: eth0
        eth1:
            dhcp4: true
            dhcp4-overrides:
                route-metric: 200
            match:
                macaddress: 00:0d:3a:7b:9e:10
            set-name: eth1
    version: 2
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether 00:0d:3a:7b:7c:34 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.23/24 brd 10.0.0.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet 10.0.0.5/24 brd 10.0.0.255 scope global secondary eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::20d:3aff:fe7b:7c34/64 scope link
       valid_lft forever preferred_lft forever
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether 00:0d:3a:7b:9e:10 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.8/24 brd 10.0.0.255 scope global eth1
       valid_lft forever preferred_lft forever
    inet6 fe80::20d:3aff:fe7b:9e10/64 scope link
       valid_lft forever preferred_lft forever

ubuntu@SRU-worked-azure:~$ ip route show
default via 10.0.0.1 dev eth0 proto dhcp src 10.0.0.5 metric 100
10.0.0.0/24 dev eth0 proto kernel scope link src 10.0.0.23
10.0.0.0/24 dev eth1 proto kernel scope link src 10.0.0.8
168.63.129.16 via 10.0.0.1 dev eth0 proto dhcp src 10.0.0.5 m...

Read more...

Chad Smith (chad.smith) wrote :

Landing as secondary NICs with unique gateways is not a primary network use-case in Azure. Netplan additional route-metric overrides on secondary NICs has been validated on OpenStack vms where unique gateways per-nic are simpler to setup.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
2index f3cec79..ea707c0 100644
3--- a/cloudinit/net/__init__.py
4+++ b/cloudinit/net/__init__.py
5@@ -265,32 +265,23 @@ def find_fallback_nic(blacklist_drivers=None):
6
7
8 def generate_fallback_config(blacklist_drivers=None, config_driver=None):
9- """Determine which attached net dev is most likely to have a connection and
10- generate network state to run dhcp on that interface"""
11-
12+ """Generate network cfg v2 for dhcp on the NIC most likely connected."""
13 if not config_driver:
14 config_driver = False
15
16 target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers)
17- if target_name:
18- target_mac = read_sys_net_safe(target_name, 'address')
19- nconf = {'config': [], 'version': 1}
20- cfg = {'type': 'physical', 'name': target_name,
21- 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]}
22- # inject the device driver name, dev_id into config if enabled and
23- # device has a valid device driver value
24- if config_driver:
25- driver = device_driver(target_name)
26- if driver:
27- cfg['params'] = {
28- 'driver': driver,
29- 'device_id': device_devid(target_name),
30- }
31- nconf['config'].append(cfg)
32- return nconf
33- else:
34+ if not target_name:
35 # can't read any interfaces addresses (or there are none); give up
36 return None
37+ target_mac = read_sys_net_safe(target_name, 'address')
38+ cfg = {'dhcp4': True, 'set-name': target_name,
39+ 'match': {'macaddress': target_mac.lower()}}
40+ if config_driver:
41+ driver = device_driver(target_name)
42+ if driver:
43+ cfg['match']['driver'] = driver
44+ nconf = {'ethernets': {target_name: cfg}, 'version': 2}
45+ return nconf
46
47
48 def extract_physdevs(netcfg):
49diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
50index 0ca576b..c0c415d 100644
51--- a/cloudinit/net/network_state.py
52+++ b/cloudinit/net/network_state.py
53@@ -596,6 +596,7 @@ class NetworkStateInterpreter(object):
54 eno1:
55 match:
56 macaddress: 00:11:22:33:44:55
57+ driver: hv_netsvc
58 wakeonlan: true
59 dhcp4: true
60 dhcp6: false
61@@ -631,15 +632,18 @@ class NetworkStateInterpreter(object):
62 'type': 'physical',
63 'name': cfg.get('set-name', eth),
64 }
65- mac_address = cfg.get('match', {}).get('macaddress', None)
66+ match = cfg.get('match', {})
67+ mac_address = match.get('macaddress', None)
68 if not mac_address:
69 LOG.debug('NetworkState Version2: missing "macaddress" info '
70 'in config entry: %s: %s', eth, str(cfg))
71- phy_cmd.update({'mac_address': mac_address})
72-
73+ phy_cmd['mac_address'] = mac_address
74+ driver = match.get('driver', None)
75+ if driver:
76+ phy_cmd['params'] = {'driver': driver}
77 for key in ['mtu', 'match', 'wakeonlan']:
78 if key in cfg:
79- phy_cmd.update({key: cfg.get(key)})
80+ phy_cmd[key] = cfg[key]
81
82 subnets = self._v2_to_v1_ipcfg(cfg)
83 if len(subnets) > 0:
84diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
85index e6e77d7..d2e38f0 100644
86--- a/cloudinit/net/tests/test_init.py
87+++ b/cloudinit/net/tests/test_init.py
88@@ -212,9 +212,9 @@ class TestGenerateFallbackConfig(CiTestCase):
89 mac = 'aa:bb:cc:aa:bb:cc'
90 write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac)
91 expected = {
92- 'config': [{'type': 'physical', 'mac_address': mac,
93- 'name': 'eth1', 'subnets': [{'type': 'dhcp'}]}],
94- 'version': 1}
95+ 'ethernets': {'eth1': {'match': {'macaddress': mac},
96+ 'dhcp4': True, 'set-name': 'eth1'}},
97+ 'version': 2}
98 self.assertEqual(expected, net.generate_fallback_config())
99
100 def test_generate_fallback_finds_dormant_eth_with_mac(self):
101@@ -223,9 +223,9 @@ class TestGenerateFallbackConfig(CiTestCase):
102 mac = 'aa:bb:cc:aa:bb:cc'
103 write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac)
104 expected = {
105- 'config': [{'type': 'physical', 'mac_address': mac,
106- 'name': 'eth0', 'subnets': [{'type': 'dhcp'}]}],
107- 'version': 1}
108+ 'ethernets': {'eth0': {'match': {'macaddress': mac}, 'dhcp4': True,
109+ 'set-name': 'eth0'}},
110+ 'version': 2}
111 self.assertEqual(expected, net.generate_fallback_config())
112
113 def test_generate_fallback_finds_eth_by_operstate(self):
114@@ -233,9 +233,10 @@ class TestGenerateFallbackConfig(CiTestCase):
115 mac = 'aa:bb:cc:aa:bb:cc'
116 write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac)
117 expected = {
118- 'config': [{'type': 'physical', 'mac_address': mac,
119- 'name': 'eth0', 'subnets': [{'type': 'dhcp'}]}],
120- 'version': 1}
121+ 'ethernets': {
122+ 'eth0': {'dhcp4': True, 'match': {'macaddress': mac},
123+ 'set-name': 'eth0'}},
124+ 'version': 2}
125 valid_operstates = ['dormant', 'down', 'lowerlayerdown', 'unknown']
126 for state in valid_operstates:
127 write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), state)
128diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
129index d2fad9b..e6ed2f3 100755
130--- a/cloudinit/sources/DataSourceAzure.py
131+++ b/cloudinit/sources/DataSourceAzure.py
132@@ -1241,7 +1241,7 @@ def parse_network_config(imds_metadata):
133 privateIpv4 = addr4['privateIpAddress']
134 if privateIpv4:
135 if dev_config.get('dhcp4', False):
136- # Append static address config for nic > 1
137+ # Append static address config for ip > 1
138 netPrefix = intf['ipv4']['subnet'][0].get(
139 'prefix', '24')
140 if not dev_config.get('addresses'):
141@@ -1251,6 +1251,11 @@ def parse_network_config(imds_metadata):
142 ip=privateIpv4, prefix=netPrefix))
143 else:
144 dev_config['dhcp4'] = True
145+ # non-primary interfaces should have a higher
146+ # route-metric (cost) so default routes prefer
147+ # primary nic due to lower route-metric value
148+ dev_config['dhcp4-overrides'] = {
149+ 'route-metric': (idx + 1) * 100}
150 for addr6 in intf['ipv6']['ipAddress']:
151 privateIpv6 = addr6['privateIpAddress']
152 if privateIpv6:
153diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
154index 2de2aea..4d57ceb 100644
155--- a/tests/unittests/test_datasource/test_azure.py
156+++ b/tests/unittests/test_datasource/test_azure.py
157@@ -12,6 +12,7 @@ from cloudinit.tests.helpers import (
158 HttprettyTestCase, CiTestCase, populate_dir, mock, wrap_and_call,
159 ExitStack, resourceLocation)
160
161+import copy
162 import crypt
163 import httpretty
164 import json
165@@ -129,6 +130,26 @@ NETWORK_METADATA = {
166 }
167 }
168
169+SECONDARY_INTERFACE = {
170+ "macAddress": "220D3A047598",
171+ "ipv6": {
172+ "ipAddress": []
173+ },
174+ "ipv4": {
175+ "subnet": [
176+ {
177+ "prefix": "24",
178+ "address": "10.0.1.0"
179+ }
180+ ],
181+ "ipAddress": [
182+ {
183+ "privateIpAddress": "10.0.1.5",
184+ }
185+ ]
186+ }
187+}
188+
189 MOCKPATH = 'cloudinit.sources.DataSourceAzure.'
190
191
192@@ -619,8 +640,43 @@ scbus-1 on xpt0 bus 0
193 'ethernets': {
194 'eth0': {'set-name': 'eth0',
195 'match': {'macaddress': '00:0d:3a:04:75:98'},
196- 'dhcp4': True}},
197+ 'dhcp4': True,
198+ 'dhcp4-overrides': {'route-metric': 100}}},
199+ 'version': 2}
200+ dsrc = self._get_ds(data)
201+ dsrc.get_data()
202+ self.assertEqual(expected_network_config, dsrc.network_config)
203+
204+ def test_network_config_set_from_imds_route_metric_for_secondary_nic(self):
205+ """Datasource.network_config adds route-metric to secondary nics."""
206+ sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}}
207+ odata = {}
208+ data = {'ovfcontent': construct_valid_ovf_env(data=odata),
209+ 'sys_cfg': sys_cfg}
210+ expected_network_config = {
211+ 'ethernets': {
212+ 'eth0': {'set-name': 'eth0',
213+ 'match': {'macaddress': '00:0d:3a:04:75:98'},
214+ 'dhcp4': True,
215+ 'dhcp4-overrides': {'route-metric': 100}},
216+ 'eth1': {'set-name': 'eth1',
217+ 'match': {'macaddress': '22:0d:3a:04:75:98'},
218+ 'dhcp4': True,
219+ 'dhcp4-overrides': {'route-metric': 200}},
220+ 'eth2': {'set-name': 'eth2',
221+ 'match': {'macaddress': '33:0d:3a:04:75:98'},
222+ 'dhcp4': True,
223+ 'dhcp4-overrides': {'route-metric': 300}}},
224 'version': 2}
225+ imds_data = copy.deepcopy(NETWORK_METADATA)
226+ imds_data['network']['interface'].append(SECONDARY_INTERFACE)
227+ third_intf = copy.deepcopy(SECONDARY_INTERFACE)
228+ third_intf['macAddress'] = third_intf['macAddress'].replace('22', '33')
229+ third_intf['ipv4']['subnet'][0]['address'] = '10.0.2.0'
230+ third_intf['ipv4']['ipAddress'][0]['privateIpAddress'] = '10.0.2.6'
231+ imds_data['network']['interface'].append(third_intf)
232+
233+ self.m_get_metadata_from_imds.return_value = imds_data
234 dsrc = self._get_ds(data)
235 dsrc.get_data()
236 self.assertEqual(expected_network_config, dsrc.network_config)
237@@ -925,6 +981,7 @@ scbus-1 on xpt0 bus 0
238 expected_cfg = {
239 'ethernets': {
240 'eth0': {'dhcp4': True,
241+ 'dhcp4-overrides': {'route-metric': 100},
242 'match': {'macaddress': '00:0d:3a:04:75:98'},
243 'set-name': 'eth0'}},
244 'version': 2}
245diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
246index 1840ade..4f7e420 100644
247--- a/tests/unittests/test_net.py
248+++ b/tests/unittests/test_net.py
249@@ -2156,7 +2156,7 @@ DEFAULT_DEV_ATTRS = {
250 "carrier": False,
251 "dormant": False,
252 "operstate": "down",
253- "address": "07-1C-C6-75-A4-BE",
254+ "address": "07-1c-c6-75-a4-be",
255 "device/driver": None,
256 "device/device": None,
257 "name_assign_type": "4",
258@@ -2207,6 +2207,39 @@ class TestGenerateFallbackConfig(CiTestCase):
259 @mock.patch("cloudinit.net.sys_dev_path")
260 @mock.patch("cloudinit.net.read_sys_net")
261 @mock.patch("cloudinit.net.get_devicelist")
262+ def test_device_driver_v2(self, mock_get_devicelist, mock_read_sys_net,
263+ mock_sys_dev_path):
264+ """Network configuration for generate_fallback_config is version 2."""
265+ devices = {
266+ 'eth0': {
267+ 'bridge': False, 'carrier': False, 'dormant': False,
268+ 'operstate': 'down', 'address': '00:11:22:33:44:55',
269+ 'device/driver': 'hv_netsvc', 'device/device': '0x3',
270+ 'name_assign_type': '4'},
271+ 'eth1': {
272+ 'bridge': False, 'carrier': False, 'dormant': False,
273+ 'operstate': 'down', 'address': '00:11:22:33:44:55',
274+ 'device/driver': 'mlx4_core', 'device/device': '0x7',
275+ 'name_assign_type': '4'},
276+
277+ }
278+
279+ tmp_dir = self.tmp_dir()
280+ _setup_test(tmp_dir, mock_get_devicelist,
281+ mock_read_sys_net, mock_sys_dev_path,
282+ dev_attrs=devices)
283+
284+ network_cfg = net.generate_fallback_config(config_driver=True)
285+ expected = {
286+ 'ethernets': {'eth0': {'dhcp4': True, 'set-name': 'eth0',
287+ 'match': {'macaddress': '00:11:22:33:44:55',
288+ 'driver': 'hv_netsvc'}}},
289+ 'version': 2}
290+ self.assertEqual(expected, network_cfg)
291+
292+ @mock.patch("cloudinit.net.sys_dev_path")
293+ @mock.patch("cloudinit.net.read_sys_net")
294+ @mock.patch("cloudinit.net.get_devicelist")
295 def test_device_driver(self, mock_get_devicelist, mock_read_sys_net,
296 mock_sys_dev_path):
297 devices = {
298@@ -2486,7 +2519,7 @@ class TestRhelSysConfigRendering(CiTestCase):
299 #
300 BOOTPROTO=dhcp
301 DEVICE=eth1000
302-HWADDR=07-1C-C6-75-A4-BE
303+HWADDR=07-1c-c6-75-a4-be
304 NM_CONTROLLED=no
305 ONBOOT=yes
306 STARTMODE=auto
307@@ -3030,7 +3063,7 @@ class TestOpenSuseSysConfigRendering(CiTestCase):
308 #
309 BOOTPROTO=dhcp
310 DEVICE=eth1000
311-HWADDR=07-1C-C6-75-A4-BE
312+HWADDR=07-1c-c6-75-a4-be
313 NM_CONTROLLED=no
314 ONBOOT=yes
315 STARTMODE=auto
316@@ -3342,13 +3375,13 @@ class TestNetplanNetRendering(CiTestCase):
317
318 expected = """
319 network:
320- version: 2
321 ethernets:
322 eth1000:
323 dhcp4: true
324 match:
325 macaddress: 07-1c-c6-75-a4-be
326 set-name: eth1000
327+ version: 2
328 """
329 self.assertEqual(expected.lstrip(), contents.lstrip())
330 self.assertEqual(1, mock_clean_default.call_count)

Subscribers

People subscribed via source and target branches