From 109a844a4ebd3778333e4f50acdd4dd3da0fa7e4 Mon Sep 17 00:00:00 2001 From: ld9379435 Date: Mon, 18 Mar 2024 19:56:19 +0800 Subject: [PATCH] feat: Determining route metric based on NIC name In the Alibaba Cloud scenario, we do not wish to define routing priority based on MAC addresses. In a cloud environment where the kernel parameter net.ifnames=0 has been configured, network interface card (NIC) names are determined by default according to their underlying Bus, Device, and Function (BDF) numbers, incrementing from eth0 to ethN, with eth0 acting as the default primary NIC name. In the previous logic, network-card has the highest priority, followed by device-number as the second priority. When default_nic_order is set to NicOrder.MAC, the mac address takes the third priority. On the other hand, when default_nic_order is set to NicOrder.NIC_NAME, the NIC name becomes the third priority. In AWS environments, the default setting remains as default_nic_order = NicOrder.MAC, maintaining the original behavior. However, in Alibaba Cloud scenarios, we set default_nic_order = NicOrder.NIC_NAME. In the earlier code snippet, before calling _build_nic_order, the input macs were sorted. Therefore, in the test cases, there should not be a scenario where macs are given as ["0a:f7:8d:96:f2:a1", "0a:0d:dd:44:cd:7b"]. macs = sorted(macs_to_nics.keys()) nic_order = _build_nic_order(macs_metadata, macs) For the new code implementation, I move the line macs = sorted(macs_to_nics.keys()) inside the function and pass the macs_to_nics dictionary as a parameter instead, which allows for easy access to the nic names. --- cloudinit/sources/DataSourceAliYun.py | 3 +- cloudinit/sources/DataSourceEc2.py | 26 ++- cloudinit/sources/__init__.py | 13 ++ tests/unittests/sources/test_ec2.py | 223 +++++++++++++++++++++++--- tests/unittests/test_upgrade.py | 9 +- 5 files changed, 243 insertions(+), 31 deletions(-) diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py index 8db89e467cdd..7cacd67e15f0 100644 --- a/cloudinit/sources/DataSourceAliYun.py +++ b/cloudinit/sources/DataSourceAliYun.py @@ -7,7 +7,7 @@ from cloudinit import dmi, sources from cloudinit.event import EventScope, EventType from cloudinit.sources import DataSourceEc2 as EC2 -from cloudinit.sources import DataSourceHostname +from cloudinit.sources import DataSourceHostname, NicOrder LOG = logging.getLogger(__name__) @@ -32,6 +32,7 @@ def __init__(self, sys_cfg, distro, paths): super(DataSourceAliYun, self).__init__(sys_cfg, distro, paths) self.default_update_events = copy.deepcopy(self.default_update_events) self.default_update_events[EventScope.NETWORK].add(EventType.BOOT) + self.default_nic_order = NicOrder.NIC_NAME def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): hostname = self.metadata.get("hostname") diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 06918fff0dd9..d78e97a02a51 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -21,6 +21,7 @@ from cloudinit.net import activators from cloudinit.net.dhcp import NoDHCPLeaseError from cloudinit.net.ephemeral import EphemeralIPNetwork +from cloudinit.sources import NicOrder from cloudinit.sources.helpers import ec2 LOG = logging.getLogger(__name__) @@ -117,6 +118,7 @@ def __init__(self, sys_cfg, distro, paths): super(DataSourceEc2, self).__init__(sys_cfg, distro, paths) self.metadata_address = None self.identity = None + self.default_nic_order = NicOrder.MAC def _unpickle(self, ci_pkl_version: int) -> None: super()._unpickle(ci_pkl_version) @@ -531,6 +533,7 @@ def network_config(self): full_network_config=util.get_cfg_option_bool( self.ds_cfg, "apply_full_imds_network_config", True ), + default_nic_order=self.default_nic_order, ) # Non-VPC (aka Classic) Ec2 instances need to rewrite the @@ -891,10 +894,12 @@ def _collect_platform_data(): def _build_nic_order( - macs_metadata: Dict[str, Dict], macs: List[str] + macs_metadata: Dict[str, Dict], + macs_to_nics: Dict[str, str], + default_nic_order: NicOrder = NicOrder.MAC, ) -> Dict[str, int]: """ - Builds a dictionary containing macs as keys nad nic orders as values, + Builds a dictionary containing macs as keys and nic orders as values, taking into account `network-card` and `device-number` if present. Note that the first NIC will be the primary NIC as it will be the one with @@ -902,11 +907,12 @@ def _build_nic_order( @param macs_metadata: dictionary with mac address as key and contents like: {"device-number": "0", "interface-id": "...", "local-ipv4s": ...} - @macs: list of macs to consider + @macs_to_nics: dictionary with mac address as key and nic name as value @return: Dictionary with macs as keys and nic orders as values. """ nic_order: Dict[str, int] = {} + macs = sorted(macs_to_nics.keys()) if len(macs) == 0 or len(macs_metadata) == 0: return nic_order @@ -914,7 +920,9 @@ def _build_nic_order( # filter out nics without metadata (not a physical nic) lambda mmd: mmd[1] is not None, # filter by macs - map(lambda mac: (mac, macs_metadata.get(mac)), macs), + map( + lambda mac: (mac, macs_metadata.get(mac), macs_to_nics[mac]), macs + ), ) def _get_key_as_int_or(dikt, key, alt_value): @@ -931,7 +939,7 @@ def _get_key_as_int_or(dikt, key, alt_value): # function. return { mac: i - for i, (mac, _mac_metadata) in enumerate( + for i, (mac, _mac_metadata, nic_name) in enumerate( sorted( valid_macs_metadata, key=lambda mmd: ( @@ -941,6 +949,9 @@ def _get_key_as_int_or(dikt, key, alt_value): _get_key_as_int_or( mmd[1], "device-number", float("infinity") ), + mmd[2] + if default_nic_order == NicOrder.NIC_NAME + else mmd[0], ), ) ) @@ -953,6 +964,7 @@ def convert_ec2_metadata_network_config( macs_to_nics=None, fallback_nic=None, full_network_config=True, + default_nic_order=NicOrder.MAC, ): """Convert ec2 metadata to network config version 2 data dict. @@ -995,8 +1007,10 @@ def convert_ec2_metadata_network_config( return netcfg # Apply network config for all nics and any secondary IPv4/v6 addresses is_netplan = distro.network_activator == activators.NetplanActivator + nic_order = _build_nic_order( + macs_metadata, macs_to_nics, default_nic_order + ) macs = sorted(macs_to_nics.keys()) - nic_order = _build_nic_order(macs_metadata, macs) for mac in macs: nic_name = macs_to_nics[mac] nic_metadata = macs_metadata.get(mac) diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 65222b29b372..e49219a36082 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -83,6 +83,19 @@ def __str__(self) -> str: return self.value +class NicOrder(Enum): + """ + Represents the canonical list of network config sources that cloud-init + knows about. + """ + + MAC = "mac" + NIC_NAME = "nic_name" + + def __str__(self) -> str: + return self.value + + class DatasourceUnpickleUserDataError(Exception): """Raised when userdata is unable to be unpickled due to python upgrades""" diff --git a/tests/unittests/sources/test_ec2.py b/tests/unittests/sources/test_ec2.py index d7d5581ead7c..5f8310474d15 100644 --- a/tests/unittests/sources/test_ec2.py +++ b/tests/unittests/sources/test_ec2.py @@ -13,6 +13,7 @@ from cloudinit import helpers from cloudinit.net import activators from cloudinit.sources import DataSourceEc2 as ec2 +from cloudinit.sources import NicOrder from tests.unittests import helpers as test_helpers from tests.unittests.util import MockDistro @@ -945,11 +946,15 @@ def test_invalid_ipv4_ipv6_cidr_metadata_logged_with_defaults(self): class TestBuildNicOrder: @pytest.mark.parametrize( - ["macs_metadata", "macs", "expected"], + ["macs_metadata", "macs_to_nics", "default_nic_order", "expected"], [ - pytest.param({}, [], {}, id="all_empty"), + pytest.param({}, {}, None, {}, id="all_empty"), pytest.param( - {}, ["0a:f7:8d:96:f2:a1"], {}, id="empty_macs_metadata" + {}, + {"0a:f7:8d:96:f2:a1": "eth0"}, + NicOrder.MAC, + {}, + id="empty_macs_metadata", ), pytest.param( { @@ -958,7 +963,8 @@ class TestBuildNicOrder: "mac": "0a:0d:dd:44:cd:7b", } }, - [], + {}, + NicOrder.MAC, {}, id="empty_macs", ), @@ -971,8 +977,9 @@ class TestBuildNicOrder: "mac": "0a:f7:8d:96:f2:a1", }, }, - ["0a:f7:8d:96:f2:a1", "0a:0d:dd:44:cd:7b"], - {"0a:f7:8d:96:f2:a1": 0, "0a:0d:dd:44:cd:7b": 1}, + {"0a:f7:8d:96:f2:a1": "eth0", "0a:0d:dd:44:cd:7b": "eth1"}, + NicOrder.MAC, + {"0a:0d:dd:44:cd:7b": 0, "0a:f7:8d:96:f2:a1": 1}, id="no-device-number-info", ), pytest.param( @@ -984,7 +991,8 @@ class TestBuildNicOrder: "mac": "0a:f7:8d:96:f2:a1", }, }, - ["0a:f7:8d:96:f2:a1"], + {"0a:f7:8d:96:f2:a1": "eth0"}, + NicOrder.MAC, {"0a:f7:8d:96:f2:a1": 0}, id="no-device-number-info-subset", ), @@ -999,7 +1007,8 @@ class TestBuildNicOrder: "mac": "0a:f7:8d:96:f2:a1", }, }, - ["0a:f7:8d:96:f2:a1", "0a:0d:dd:44:cd:7b"], + {"0a:0d:dd:44:cd:7b": "eth0", "0a:f7:8d:96:f2:a1": "eth1"}, + NicOrder.MAC, {"0a:0d:dd:44:cd:7b": 0, "0a:f7:8d:96:f2:a1": 1}, id="device-numbers", ), @@ -1021,11 +1030,12 @@ class TestBuildNicOrder: "mac": "0a:f7:8d:96:f2:a1", }, }, - [ - "0a:f7:8d:96:f2:a1", - "0a:0d:dd:44:cd:7b", - "0a:f7:8d:96:f2:a2", - ], + { + "0a:0d:dd:44:cd:7b": "eth0", + "0a:f7:8d:96:f2:a1": "eth1", + "0a:f7:8d:96:f2:a2": "eth2", + }, + NicOrder.MAC, { "0a:0d:dd:44:cd:7b": 0, "0a:f7:8d:96:f2:a1": 1, @@ -1047,14 +1057,15 @@ class TestBuildNicOrder: }, "0a:f7:8d:96:f2:a2": { "device-number": "1", - "mac": "0a:f7:8d:96:f2:a1", + "mac": "0a:f7:8d:96:f2:a2", }, }, - [ - "0a:f7:8d:96:f2:a1", - "0a:0d:dd:44:cd:7b", - "0a:f7:8d:96:f2:a2", - ], + { + "0a:0d:dd:44:cd:7b": "eth0", + "0a:f7:8d:96:f2:a1": "eth1", + "0a:f7:8d:96:f2:a2": "eth2", + }, + NicOrder.MAC, { "0a:0d:dd:44:cd:7b": 0, "0a:f7:8d:96:f2:a1": 1, @@ -1071,14 +1082,182 @@ class TestBuildNicOrder: "mac": "0a:f7:8d:96:f2:a1", }, }, - ["0a:f7:8d:96:f2:a9"], + {"0a:f7:8d:96:f2:a9": "eth0"}, + NicOrder.MAC, {}, id="macs-not-in-md", ), + pytest.param( + {}, + {"0a:f7:8d:96:f2:a1": "eth0"}, + NicOrder.NIC_NAME, + {}, + id="empty_macs_metadata_sort_by_nic_name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "device-number": "0", + "mac": "0a:0d:dd:44:cd:7b", + } + }, + {}, + NicOrder.NIC_NAME, + {}, + id="empty_macs_sort_by_nic_name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + {"0a:f7:8d:96:f2:a1": "eth0", "0a:0d:dd:44:cd:7b": "eth1"}, + NicOrder.NIC_NAME, + {"0a:f7:8d:96:f2:a1": 0, "0a:0d:dd:44:cd:7b": 1}, + id="no-device-number-info-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + {"0a:f7:8d:96:f2:a1": "eth0"}, + NicOrder.NIC_NAME, + {"0a:f7:8d:96:f2:a1": 0}, + id="no-device-number-info-subset-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "device-number": "0", + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "device-number": "1", + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + {"0a:0d:dd:44:cd:7b": "eth0", "0a:f7:8d:96:f2:a1": "eth1"}, + NicOrder.NIC_NAME, + {"0a:0d:dd:44:cd:7b": 0, "0a:f7:8d:96:f2:a1": 1}, + id="device-numbers-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "network-card": "1", + "device-number": "1", + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "network-card": "0", + "device-number": "0", + "mac": "0a:f7:8d:96:f2:a1", + }, + "0a:f7:8d:96:f2:a2": { + "network-card": "2", + "device-number": "1", + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + { + "0a:f7:8d:96:f2:a1": "eth0", + "0a:0d:dd:44:cd:7b": "eth1", + "0a:f7:8d:96:f2:a2": "eth2", + }, + NicOrder.MAC, + { + "0a:f7:8d:96:f2:a1": 0, + "0a:0d:dd:44:cd:7b": 1, + "0a:f7:8d:96:f2:a2": 2, + }, + id="network-cardes-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "network-card": "0", + "device-number": "0", + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "network-card": "1", + "device-number": "1", + "mac": "0a:f7:8d:96:f2:a1", + }, + "0a:f7:8d:96:f2:a2": { + "device-number": "1", + "mac": "0a:f7:8d:96:f2:a2", + }, + }, + { + "0a:0d:dd:44:cd:7b": "eth0", + "0a:f7:8d:96:f2:a1": "eth1", + "0a:f7:8d:96:f2:a2": "eth2", + }, + NicOrder.NIC_NAME, + { + "0a:0d:dd:44:cd:7b": 0, + "0a:f7:8d:96:f2:a1": 1, + "0a:f7:8d:96:f2:a2": 2, + }, + id="network-card-partially-missing-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + {"0a:f7:8d:96:f2:a9": "eth0"}, + NicOrder.NIC_NAME, + {}, + id="macs-not-in-md-sort-by-nic-name", + ), + pytest.param( + { + "0a:0d:dd:44:cd:7b": { + "mac": "0a:0d:dd:44:cd:7b", + }, + "0a:f7:8d:96:f2:a1": { + "mac": "0a:f7:8d:96:f2:a1", + }, + "0a:f7:8d:96:f2:a2": { + "mac": "0a:f7:8d:96:f2:a1", + }, + }, + { + "0a:f7:8d:96:f2:a1": "eth0", + "0a:0d:dd:44:cd:7b": "eth1", + "0a:f7:8d:96:f2:a2": "eth2", + }, + NicOrder.NIC_NAME, + { + "0a:f7:8d:96:f2:a1": 0, + "0a:0d:dd:44:cd:7b": 1, + "0a:f7:8d:96:f2:a2": 2, + }, + id="no-device-number-info-subset-sort-by-nic-name", + ), ], ) - def test_build_nic_order(self, macs_metadata, macs, expected): - assert expected == ec2._build_nic_order(macs_metadata, macs) + def test_build_nic_order( + self, macs_metadata, macs_to_nics, default_nic_order, expected + ): + assert expected == ec2._build_nic_order( + macs_metadata, macs_to_nics, default_nic_order + ) class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): diff --git a/tests/unittests/test_upgrade.py b/tests/unittests/test_upgrade.py index 0aca3683b868..a7dd8538fd4a 100644 --- a/tests/unittests/test_upgrade.py +++ b/tests/unittests/test_upgrade.py @@ -34,7 +34,12 @@ class TestUpgrade: # The presence of these attributes existed in 20.1. ds_expected_unpickle_attrs = { "AltCloud": {"seed", "supported_seed_starts"}, - "AliYun": {"identity", "metadata_address", "default_update_events"}, + "AliYun": { + "identity", + "metadata_address", + "default_update_events", + "default_nic_order", + }, "Azure": { "_ephemeral_dhcp_ctx", "_iso_dev", @@ -75,7 +80,7 @@ class TestUpgrade: "use_ip4LL", "wait_retry", }, - "Ec2": {"identity", "metadata_address"}, + "Ec2": {"identity", "metadata_address", "default_nic_order"}, "Exoscale": { "api_version", "extra_config",