Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
Diff comments:
> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
> index 0ca576b..c0c415d 100644
> --- a/cloudinit/net/network_state.py
> +++ b/cloudinit/net/network_state.py
> @@ -596,6 +596,7 @@ class NetworkStateInterpreter(object):
>eno1:
> match:
>macaddress: 00:11:22:33:44:55
> + driver: hv_netsvc
ohh was just a docstring addition, wanted a more complete reference.
> wakeonlan: true
> dhcp4: true
> dhcp6: false
> diff --git a/tests/unittests/test_datasource/test_azure.py
> b/tests/unittests/test_datasource/test_azure.py
> index 2de2aea..3ed9e4e 100644
> --- a/tests/unittests/test_datasource/test_azure.py
> +++ b/tests/unittests/test_datasource/test_azure.py
> @@ -997,7 +997,7 @@ scbus-1 on xpt0 bus 0
> netconfig = dsrc.network_config
> self.assertEqual(netconfig, fallback_config)
> mock_fallback.assert_called_with(blacklist_drivers=['mlx4_core'],
> - config_driver=True)
> + config_driver=True,
> network_version=2)
oops. that should have failed/
>
> @mock.patch('cloudinit.net.get_interface_mac')
> @mock.patch('cloudinit.net.get_devicelist')
> diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
> index 1840ade..4f7e420 100644
> --- a/tests/unittests/test_net.py
> +++ b/tests/unittests/test_net.py
> @@ -2156,7 +2156,7 @@ DEFAULT_DEV_ATTRS = {
> "carrier": False,
> "dormant": False,
> "operstate": "down",
> -"address": "07-1C-C6-75-A4-BE",
> +"address": "07-1c-c6-75-a4-be",
only because out netplan we generate in other cases was lowercase. I don't have
to, just wanted it to the the same case for !fallback
> "device/driver": None,
> "device/device": None,
> "name_assign_type": "4",
> @@ -3342,13 +3375,13 @@ class TestNetplanNetRendering(CiTestCase):
>
> expected = """
> network:
> -version: 2
> ethernets:
> eth1000:
> dhcp4: true
> match:
> macaddress: 07-1c-c6-75-a4-be
> set-name: eth1000
> +version: 2
Because we are rendering directly from network v2 using yaml dump without
ordering rules instead of converting from v1 to v2 with a helper function which
intentionally ordered the output with version key first.
> """
> self.assertEqual(expected.lstrip(), contents.lstrip())
> self.assertEqual(1, mock_clean_default.call_count)
--
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
One debugging line needs removing and some questions inline.
Diff comments:
> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
> index 0ca576b..c0c415d 100644
> --- a/cloudinit/net/network_state.py
> +++ b/cloudinit/net/network_state.py
> @@ -596,6 +596,7 @@ class NetworkStateInterpreter(object):
>eno1:
> match:
>macaddress: 00:11:22:33:44:55
> + driver: hv_netsvc
Debugging code?
> wakeonlan: true
> dhcp4: true
> dhcp6: false
> diff --git a/tests/unittests/test_datasource/test_azure.py
> b/tests/unittests/test_datasource/test_azure.py
> index 2de2aea..3ed9e4e 100644
> --- a/tests/unittests/test_datasource/test_azure.py
> +++ b/tests/unittests/test_datasource/test_azure.py
> @@ -997,7 +997,7 @@ scbus-1 on xpt0 bus 0
> netconfig = dsrc.network_config
> self.assertEqual(netconfig, fallback_config)
> mock_fallback.assert_called_with(blacklist_drivers=['mlx4_core'],
> - config_driver=True)
> + config_driver=True,
> network_version=2)
We don't need this any more right?
>
> @mock.patch('cloudinit.net.get_interface_mac')
> @mock.patch('cloudinit.net.get_devicelist')
> diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
> index 1840ade..4f7e420 100644
> --- a/tests/unittests/test_net.py
> +++ b/tests/unittests/test_net.py
> @@ -2156,7 +2156,7 @@ DEFAULT_DEV_ATTRS = {
> "carrier": False,
> "dormant": False,
> "operstate": "down",
> -"address": "07-1C-C6-75-A4-BE",
> +"address": "07-1c-c6-75-a4-be",
Why change this?
> "device/driver": None,
> "device/device": None,
> "name_assign_type": "4",
> @@ -3342,13 +3375,13 @@ class TestNetplanNetRendering(CiTestCase):
>
> expected = """
> network:
> -version: 2
> ethernets:
> eth1000:
> dhcp4: true
> match:
> macaddress: 07-1c-c6-75-a4-be
> set-name: eth1000
> +version: 2
why this change?
> """
> self.assertEqual(expected.lstrip(), contents.lstrip())
> self.assertEqual(1, mock_clean_default.call_count)
--
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master has been updated. Commit message changed to: azure/net: generate_fallback_config emits network config v2 To enable Azure to send network v2, net.generate_fallback_config now generates network config v2 instead of v1. To support this shift, network_state also needed a small addition to account for setting driver params if present in the match section for a specific interface. For more details, see: https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970 -- Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
Diff comments:
> diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
> index f3cec79..fa15b96 100644
> --- a/cloudinit/net/__init__.py
> +++ b/cloudinit/net/__init__.py
> @@ -272,25 +273,30 @@ def generate_fallback_config(blacklist_drivers=None,
> config_driver=None):
> config_driver = False
>
> target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers)
> -if target_name:
> -target_mac = read_sys_net_safe(target_name, 'address')
> +if not target_name:
> +# can't read any interfaces addresses (or there are none); give up
> +return None
> +target_mac = read_sys_net_safe(target_name, 'address')
> +driver_params = {}
> +if config_driver:
> +driver = device_driver(target_name)
> +if driver:
> +driver_params = {'driver': driver,
> + 'device_id': device_devid(target_name)}
> +if network_version == 1:
> +# TODO(Drop network v1 once NetworkState parses v2)
You are correct here, I misinterpreted a test failure I ran into with
network_state not handling v2 match:driver declarations when emitting udev
rules. Got that fixed in the followup.
> nconf = {'config': [], 'version': 1}
> cfg = {'type': 'physical', 'name': target_name,
> 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]}
> -# inject the device driver name, dev_id into config if enabled and
> -# device has a valid device driver value
> -if config_driver:
> -driver = device_driver(target_name)
> -if driver:
> -cfg['params'] = {
> -'driver': driver,
> -'device_id': device_devid(target_name),
> -}
> +if driver_params:
> +cfg['params'] = driver_params
> nconf['config'].append(cfg)
> return nconf
> -else:
> -# can't read any interfaces addresses (or there are none); give up
> -return None
> +cfg = {'dhcp4': True, 'set-name': target_name,
> + 'match': {'macaddress': target_mac.lower()}}
> +cfg['match'].update(driver_params)
> +nconf = {'ethernets': {target_name: cfg}, 'version': 2}
> +return nconf
>
>
> def extract_physdevs(netcfg):
--
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master has been updated. Commit message changed to: azure/net: generate_fallback_nic emits network v2 config instead of v1 To enable Azure to send network v2, net.generate_fallback_config now generates network config v2 instead of v1. To support this shift, network_state also needed a small addition to account for setting driver params if present in the match section for a specific interface. For more details, see: https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970 -- Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master has been updated. Commit message changed to: 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. For more details, see: https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970 -- Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
One inline question.
Diff comments:
> diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
> index f3cec79..fa15b96 100644
> --- a/cloudinit/net/__init__.py
> +++ b/cloudinit/net/__init__.py
> @@ -272,25 +273,30 @@ def generate_fallback_config(blacklist_drivers=None,
> config_driver=None):
> config_driver = False
>
> target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers)
> -if target_name:
> -target_mac = read_sys_net_safe(target_name, 'address')
> +if not target_name:
> +# can't read any interfaces addresses (or there are none); give up
> +return None
> +target_mac = read_sys_net_safe(target_name, 'address')
> +driver_params = {}
> +if config_driver:
> +driver = device_driver(target_name)
> +if driver:
> +driver_params = {'driver': driver,
> + 'device_id': device_devid(target_name)}
> +if network_version == 1:
> +# TODO(Drop network v1 once NetworkState parses v2)
This is confusing to me. NetworkState definitely can parse v2?
The NetworkStateInterpreter:parse_config_v2 ?
> nconf = {'config': [], 'version': 1}
> cfg = {'type': 'physical', 'name': target_name,
> 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]}
> -# inject the device driver name, dev_id into config if enabled and
> -# device has a valid device driver value
> -if config_driver:
> -driver = device_driver(target_name)
> -if driver:
> -cfg['params'] = {
> -'driver': driver,
> -'device_id': device_devid(target_name),
> -}
> +if driver_params:
> +cfg['params'] = driver_params
> nconf['config'].append(cfg)
> return nconf
> -else:
> -# can't read any interfaces addresses (or there are none); give up
> -return None
> +cfg = {'dhcp4': True, 'set-name': target_name,
> + 'match': {'macaddress': target_mac.lower()}}
> +cfg['match'].update(driver_params)
> +nconf = {'ethernets': {target_name: cfg}, 'version': 2}
> +return nconf
>
>
> def extract_physdevs(netcfg):
--
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:feature/azure-to-network-v2 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] ~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master
Chad Smith has proposed merging
~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master.
Commit message:
azure: provide fallback network config v2 for fallback nic
To enable Azure to send network v2, net.generate_fallback_config now
accepts a network_version param to specify whether version 1 or version
2 network configuration is desired. DataSourceAzure.network_config now
requests network_version=2 when generating fallback configuration.
This branch avoids moving generate_fallback_config to v2 exclusively because we
first need NetworkState to be able to parse v2 as fallback config is passed
directly into NetworkState.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/370970
--
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:feature/azure-to-network-v2 into cloud-init:master.
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index f3cec79..fa15b96 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -264,7 +264,8 @@ def find_fallback_nic(blacklist_drivers=None):
return None
-def generate_fallback_config(blacklist_drivers=None, config_driver=None):
+def generate_fallback_config(
+blacklist_drivers=None, config_driver=None, network_version=1):
"""Determine which attached net dev is most likely to have a connection and
generate network state to run dhcp on that interface"""
@@ -272,25 +273,30 @@ def generate_fallback_config(blacklist_drivers=None, config_driver=None):
config_driver = False
target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers)
-if target_name:
-target_mac = read_sys_net_safe(target_name, 'address')
+if not target_name:
+# can't read any interfaces addresses (or there are none); give up
+return None
+target_mac = read_sys_net_safe(target_name, 'address')
+driver_params = {}
+if config_driver:
+driver = device_driver(target_name)
+if driver:
+driver_params = {'driver': driver,
+ 'device_id': device_devid(target_name)}
+if network_version == 1:
+# TODO(Drop network v1 once NetworkState parses v2)
nconf = {'config': [], 'version': 1}
cfg = {'type': 'physical', 'name': target_name,
'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]}
-# inject the device driver name, dev_id into config if enabled and
-# device has a valid device driver value
-if config_driver:
-driver = device_driver(target_name)
-if driver:
-cfg['params'] = {
-'driver': driver,
-'device_id': device_devid(target_name),
-}
+if driver_params:
+cfg['params'] = driver_params
nconf['config'].append(cfg)
return nconf
-else:
-# can't read any interfaces addresses (or there are none); give up
-return None
+cfg = {'dhcp4': True, 'set-name': target_name,
+ 'match': {'macaddress': target_mac.lower()}}
+cfg['match'].update(driver_params)
+nconf = {'ethernets': {target_name: cfg}, 'version': 2}
+return nconf
def extract_physdevs(netcfg):
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index 0ca576b..1e6ead2 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -343,7 +343,6 @@ class NetworkStateInterpreter(object):
]
}
'''
-
interfaces = self._network_state.get('interfaces', {})
iface = interfaces.get(command['name'], {})
for param, val in command.get('params', {}).items():
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index d2fad9b..ab47507 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -1268,7 +1268,8 @@ def parse_network_config(imds_metadata):
LOG.debug('Azure: generating fallback configuration')
# generate a network config, blacklist picking mlx4_core devs
netconfig = net.generate_fallback_config(
-blacklist_drivers=blacklist, config_driver=True)
+blacklist_drivers=blacklist, config_driver=True,
+network_version=2)
evt.description = "network config from fallback"
return netconfig
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 2de2aea..3ed9e4e 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -997,7 +997,7 @@ scbus-1 on xpt0 bus 0
netconfig = dsrc.network_config
self.assertEqual(netconfig, fallback_config)
