From 15571180520de376966caf5f6bc6bbfb4611c9a6 Mon Sep 17 00:00:00 2001 From: Alberto Contreras Date: Sun, 17 Dec 2023 10:25:54 +0100 Subject: [PATCH] feat(ec2): support multi NIC/IP setups (#4799) For EC2 instances with multiple NICs, policy-based routing will be configured on secondary NICs / secondary IPs to ensure outgoing packets are routed via the correct interface. Without this extra routing config, traffic coming via secondary NICs was routed using the main routing table, which can only contain one default route and the kernel only takes the destination IP address into account when selecting a route. Packets for destination beyond local networks were always routed through the default route, the one associated with the primary NIC. If traffic based on specific source IP addresses is associated with another NIC, wihtout these routing policies, this traffic would flow over the default route and the connection couldn't be established. References: [1] https://bootstack.canonical.com/cases/00336928 [2] https://bootstack.canonical.com/cases/00377150 --- cloudinit/sources/DataSourceEc2.py | 112 ++++++++++++++++-- doc/rtd/reference/datasources/ec2.rst | 7 ++ .../integration_tests/modules/test_hotplug.py | 89 ++++++++++++++ tests/integration_tests/test_networking.py | 80 +++++++++++++ tests/integration_tests/util.py | 4 + tests/unittests/sources/test_aliyun.py | 9 ++ tests/unittests/sources/test_ec2.py | 54 +++++++-- 7 files changed, 336 insertions(+), 19 deletions(-) diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index ccad4e978f5b..8ff2e6fabb36 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -55,7 +55,6 @@ def skip_404_tag_errors(exception): class DataSourceEc2(sources.DataSource): - dsname = "Ec2" # Default metadata urls that will be used if none are provided # They will be checked for 'resolveability' and some of the @@ -402,7 +401,7 @@ def device_name_to_device(self, name): LOG.debug("block-device-mapping not a dictionary: '%s'", bdm) return None - for (entname, device) in bdm.items(): + for entname, device in bdm.items(): if entname == name: found = device break @@ -508,6 +507,7 @@ def network_config(self): # behavior on those releases. result = convert_ec2_metadata_network_config( net_md, + self.distro, fallback_nic=iface, full_network_config=util.get_cfg_option_bool( self.ds_cfg, "apply_full_imds_network_config", True @@ -872,7 +872,11 @@ def _collect_platform_data(): def convert_ec2_metadata_network_config( - network_md, macs_to_nics=None, fallback_nic=None, full_network_config=True + network_md, + distro, + macs_to_nics=None, + fallback_nic=None, + full_network_config=True, ): """Convert ec2 metadata to network config version 2 data dict. @@ -880,6 +884,7 @@ def convert_ec2_metadata_network_config( generally formed as {"interfaces": {"macs": {}} where 'macs' is a dictionary with mac address as key and contents like: {"device-number": "0", "interface-id": "...", "local-ipv4s": ...} + @param: distro: instance of Distro. @param: macs_to_nics: Optional dict of mac addresses and nic names. If not provided, get_interfaces_by_mac is called to get it from the OS. @param: fallback_nic: Optionally provide the primary nic interface name. @@ -918,10 +923,9 @@ def convert_ec2_metadata_network_config( nic_metadata = macs_metadata.get(mac) if not nic_metadata: continue # Not a physical nic represented in metadata - # device-number is zero-indexed, we want it 1-indexed for the - # multiplication on the following line - nic_idx = int(nic_metadata.get("device-number", nic_idx)) + 1 - dhcp_override = {"route-metric": nic_idx * 100} + nic_idx = int(nic_metadata.get("device-number", nic_idx)) + # nic_idx + 1 to start route_metric at 100 + dhcp_override = {"route-metric": (nic_idx + 1) * 100} dev_config = { "dhcp4": True, "dhcp4-overrides": dhcp_override, @@ -929,18 +933,110 @@ def convert_ec2_metadata_network_config( "match": {"macaddress": mac.lower()}, "set-name": nic_name, } + # Configure policy-based routing on secondary NICs / secondary IPs to + # ensure outgoing packets are routed via the correct interface. + # + # If device-number is not present (AliYun or other ec2-like platforms), + # do not configure source-routing as we cannot determine which is the + # primary NIC. + if nic_metadata.get("device-number") and nic_idx > 0: + dhcp_override["use-routes"] = True + table = 100 + nic_idx + dev_config["routes"] = [] + try: + lease = distro.dhcp_client.dhcp_discovery( + nic_name, distro=distro + ) + gateway = lease["routers"] + except Exception as e: + LOG.warning( + "Could not perform dhcp discovery on %s to find its " + "gateway. Not adding default route via the gateway. " + "Error: %s", + nic_name, + e, + ) + else: + # Add default route via the NIC's gateway + dev_config["routes"].append( + { + "to": "0.0.0.0/0", + "via": gateway, + "table": table, + }, + ) + subnet_prefix_routes = nic_metadata["subnet-ipv4-cidr-block"] + subnet_prefix_routes = ( + [subnet_prefix_routes] + if isinstance(subnet_prefix_routes, str) + else subnet_prefix_routes + ) + for prefix_route in subnet_prefix_routes: + dev_config["routes"].append( + { + "to": prefix_route, + "table": table, + }, + ) + + dev_config["routing-policy"] = [] + # Packets coming from any IPv4 associated with the current NIC + # will be routed using `table` routing table + ipv4s = nic_metadata["local-ipv4s"] + ipv4s = [ipv4s] if isinstance(ipv4s, str) else ipv4s + for ipv4 in ipv4s: + dev_config["routing-policy"].append( + { + "from": ipv4, + "table": table, + }, + ) if nic_metadata.get("ipv6s"): # Any IPv6 addresses configured dev_config["dhcp6"] = True dev_config["dhcp6-overrides"] = dhcp_override + if nic_metadata.get("device-number") and nic_idx > 0: + table = 100 + nic_idx + subnet_prefix_routes = nic_metadata["subnet-ipv6-cidr-block"] + subnet_prefix_routes = ( + [subnet_prefix_routes] + if isinstance(subnet_prefix_routes, str) + else subnet_prefix_routes + ) + for prefix_route in subnet_prefix_routes: + dev_config["routes"].append( + { + "to": prefix_route, + "table": table, + }, + ) + + dev_config["routing-policy"] = [] + ipv6s = nic_metadata["ipv6s"] + ipv6s = [ipv6s] if isinstance(ipv6s, str) else ipv6s + for ipv6 in ipv6s: + dev_config["routing-policy"].append( + { + "from": ipv6, + "table": table, + }, + ) dev_config["addresses"] = get_secondary_addresses(nic_metadata, mac) if not dev_config["addresses"]: dev_config.pop("addresses") # Since we found none configured + netcfg["ethernets"][nic_name] = dev_config - # Remove route-metric dhcp overrides if only one nic configured + + # Advance nic_idx on platforms without device-number + if not nic_metadata.get("device-number"): + nic_idx += 1 + # Remove route-metric dhcp overrides and routes / routing-policy if only + # one nic configured if len(netcfg["ethernets"]) == 1: for nic_name in netcfg["ethernets"].keys(): netcfg["ethernets"][nic_name].pop("dhcp4-overrides") netcfg["ethernets"][nic_name].pop("dhcp6-overrides", None) + netcfg["ethernets"][nic_name].pop("routes", None) + netcfg["ethernets"][nic_name].pop("routing-policy", None) return netcfg diff --git a/doc/rtd/reference/datasources/ec2.rst b/doc/rtd/reference/datasources/ec2.rst index 7e2b522a137e..6ac0a53f0e76 100644 --- a/doc/rtd/reference/datasources/ec2.rst +++ b/doc/rtd/reference/datasources/ec2.rst @@ -150,4 +150,11 @@ Notes For example: the primary NIC will have a DHCP route-metric of 100, the next NIC will have 200. + * For EC2 instances with multiple NICs, policy-based routing will be + configured on secondary NICs / secondary IPs to ensure outgoing packets + are routed via the correct interface. + At the moment of writing, this network configuration is applied at first + boot only but it can be configured to be applied on every boot and when + NICs are hotplugged, see :ref:`events`. + .. _EC2 tags user guide: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html#work-with-tags-in-IMDS diff --git a/tests/integration_tests/modules/test_hotplug.py b/tests/integration_tests/modules/test_hotplug.py index 120715e4998b..c6359f7ddd33 100644 --- a/tests/integration_tests/modules/test_hotplug.py +++ b/tests/integration_tests/modules/test_hotplug.py @@ -1,12 +1,16 @@ +import contextlib import time from collections import namedtuple import pytest import yaml +from cloudinit.subp import subp +from tests.integration_tests.clouds import IntegrationCloud from tests.integration_tests.instances import IntegrationInstance from tests.integration_tests.integration_settings import PLATFORM from tests.integration_tests.releases import CURRENT_RELEASE, FOCAL +from tests.integration_tests.util import verify_clean_log USER_DATA = """\ #cloud-config @@ -124,3 +128,88 @@ def test_no_hotplug_in_userdata(client: IntegrationInstance): assert "disabled" == client.execute( "cloud-init devel hotplug-hook -s net query" ) + + +@pytest.mark.skipif(PLATFORM != "ec2", reason="test is ec2 specific") +def test_multi_nic_hotplug(setup_image, session_cloud: IntegrationCloud): + """Tests that additional secondary NICs are routable from non-local + networks after the hotplug hook is executed when network updates + are configured on the HOTPLUG event.""" + ec2 = session_cloud.cloud_instance.client + with session_cloud.launch(launch_kwargs={}, user_data=USER_DATA) as client: + ips_before = _get_ip_addr(client) + instance_pub_ip = client.instance.ip + secondary_priv_ip = client.instance.add_network_interface() + response = ec2.describe_network_interfaces( + Filters=[ + { + "Name": "private-ip-address", + "Values": [secondary_priv_ip], + }, + ], + ) + nic_id = response["NetworkInterfaces"][0]["NetworkInterfaceId"] + + # Create Elastic IP + allocation = ec2.allocate_address(Domain="vpc") + try: + secondary_pub_ip = allocation["PublicIp"] + association = ec2.associate_address( + AllocationId=allocation["AllocationId"], + NetworkInterfaceId=nic_id, + ) + assert association["ResponseMetadata"]["HTTPStatusCode"] == 200 + + _wait_till_hotplug_complete(client) + ips_after_add = _get_ip_addr(client) + + netplan_cfg = client.read_from_file( + "/etc/netplan/50-cloud-init.yaml" + ) + config = yaml.safe_load(netplan_cfg) + new_addition = [ + ip for ip in ips_after_add if ip.ip4 == secondary_priv_ip + ][0] + assert new_addition.interface in config["network"]["ethernets"] + new_nic_cfg = config["network"]["ethernets"][ + new_addition.interface + ] + assert "routing-policy" in new_nic_cfg + assert [{"from": secondary_priv_ip, "table": 101}] == new_nic_cfg[ + "routing-policy" + ] + + assert len(ips_after_add) == len(ips_before) + 1 + + # SSH over primary NIC works + subp("nc -w 5 -zv " + instance_pub_ip + " 22", shell=True) + + # THE TEST: SSH over secondary NIC works + subp("nc -w 5 -zv " + secondary_pub_ip + " 22", shell=True) + + # Remove new NIC + client.instance.remove_network_interface(secondary_priv_ip) + _wait_till_hotplug_complete(client, expected_runs=2) + + # SSH over primary NIC works + subp("nc -w 1 -zv " + instance_pub_ip + " 22", shell=True) + + ips_after_remove = _get_ip_addr(client) + assert len(ips_after_remove) == len(ips_before) + assert secondary_priv_ip not in [ip.ip4 for ip in ips_after_remove] + + netplan_cfg = client.read_from_file( + "/etc/netplan/50-cloud-init.yaml" + ) + config = yaml.safe_load(netplan_cfg) + assert new_addition.interface not in config["network"]["ethernets"] + + log_content = client.read_from_file("/var/log/cloud-init.log") + verify_clean_log(log_content) + finally: + with contextlib.suppress(Exception): + ec2.disassociate_address( + AssociationId=association["AssociationId"] + ) + with contextlib.suppress(Exception): + ec2.release_address(AllocationId=allocation["AllocationId"]) diff --git a/tests/integration_tests/test_networking.py b/tests/integration_tests/test_networking.py index d70a29ee8f2f..0a2bc978cacd 100644 --- a/tests/integration_tests/test_networking.py +++ b/tests/integration_tests/test_networking.py @@ -1,12 +1,16 @@ """Networking-related tests.""" +import contextlib + import pytest import yaml +from cloudinit.subp import subp from tests.integration_tests import random_mac_address from tests.integration_tests.clouds import IntegrationCloud from tests.integration_tests.instances import IntegrationInstance from tests.integration_tests.integration_settings import PLATFORM from tests.integration_tests.releases import CURRENT_RELEASE, NOBLE +from tests.integration_tests.util import verify_clean_log def _add_dummy_bridge_to_netplan(client: IntegrationInstance): @@ -254,3 +258,79 @@ def test_invalid_network_v2_netplan(session_cloud: IntegrationCloud): "# E1: Invalid netplan schema. Error in network definition:" " invalid boolean value 'badval" in annotate_out ) + + +@pytest.mark.skipif(PLATFORM != "ec2", reason="test is ec2 specific") +def test_ec2_multi_nic_reboot(setup_image, session_cloud: IntegrationCloud): + """Tests that additional secondary NICs and secondary IPs on them are + routable from non-local networks after a reboot event when network updates + are configured on every boot.""" + ec2 = session_cloud.cloud_instance.client + with session_cloud.launch(launch_kwargs={}, user_data=USER_DATA) as client: + # Add secondary NIC + secondary_priv_ip_0 = client.instance.add_network_interface() + response = ec2.describe_network_interfaces( + Filters=[ + { + "Name": "private-ip-address", + "Values": [secondary_priv_ip_0], + }, + ], + ) + nic_id = response["NetworkInterfaces"][0]["NetworkInterfaceId"] + # Add secondary IP to secondary NIC + association_0 = ec2.assign_private_ip_addresses( + NetworkInterfaceId=nic_id, SecondaryPrivateIpAddressCount=1 + ) + assert association_0["ResponseMetadata"]["HTTPStatusCode"] == 200 + secondary_priv_ip_1 = association_0["AssignedPrivateIpAddresses"][0][ + "PrivateIpAddress" + ] + + # Assing elastic IPs + allocation_0 = ec2.allocate_address(Domain="vpc") + allocation_1 = ec2.allocate_address(Domain="vpc") + try: + secondary_pub_ip_0 = allocation_0["PublicIp"] + secondary_pub_ip_1 = allocation_1["PublicIp"] + + association_0 = ec2.associate_address( + AllocationId=allocation_0["AllocationId"], + NetworkInterfaceId=nic_id, + PrivateIpAddress=secondary_priv_ip_0, + ) + assert association_0["ResponseMetadata"]["HTTPStatusCode"] == 200 + association_1 = ec2.associate_address( + AllocationId=allocation_1["AllocationId"], + NetworkInterfaceId=nic_id, + PrivateIpAddress=secondary_priv_ip_1, + ) + assert association_1["ResponseMetadata"]["HTTPStatusCode"] == 200 + + # Reboot to update network config + client.execute("cloud-init clean --logs") + client.restart() + + # SSH over primary NIC works + instance_pub_ip = client.instance.ip + subp("nc -w 5 -zv " + instance_pub_ip + " 22", shell=True) + + # SSH over secondary NIC works + subp("nc -w 5 -zv " + secondary_pub_ip_0 + " 22", shell=True) + subp("nc -w 5 -zv " + secondary_pub_ip_1 + " 22", shell=True) + + log_content = client.read_from_file("/var/log/cloud-init.log") + verify_clean_log(log_content) + finally: + with contextlib.suppress(Exception): + ec2.disassociate_address( + AssociationId=association_0["AssociationId"] + ) + with contextlib.suppress(Exception): + ec2.release_address(AllocationId=allocation_0["AllocationId"]) + with contextlib.suppress(Exception): + ec2.disassociate_address( + AssociationId=association_1["AssociationId"] + ) + with contextlib.suppress(Exception): + ec2.release_address(AllocationId=allocation_1["AllocationId"]) diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py index 8e10b4872b83..3b3e140d0fca 100644 --- a/tests/integration_tests/util.py +++ b/tests/integration_tests/util.py @@ -73,6 +73,10 @@ def verify_clean_log(log: str, ignore_deprecations: bool = True): # Old Ubuntu cloud-images contain /etc/apt/sources.list "WARNING]: Removing /etc/apt/sources.list to favor deb822 source" " format", + # https://bugs.launchpad.net/ubuntu/+source/netplan.io/+bug/2041727 + "WARNING]: Running ['netplan', 'apply'] resulted in stderr output: " + "WARNING:root:Cannot call Open vSwitch: ovsdb-server.service is not " + "running.", ] traceback_texts = [] if "install canonical-livepatch" in log: diff --git a/tests/unittests/sources/test_aliyun.py b/tests/unittests/sources/test_aliyun.py index 00da2ee88e4a..01140720c4b4 100644 --- a/tests/unittests/sources/test_aliyun.py +++ b/tests/unittests/sources/test_aliyun.py @@ -291,6 +291,7 @@ def test_route_metric_calculated_without_device_number(self): } } }, + mock.Mock(), macs_to_nics={ "06:17:04:d7:26:09": "eth0", "06:17:04:d7:26:08": "eth1", @@ -303,6 +304,14 @@ def test_route_metric_calculated_without_device_number(self): # route-metric numbers should be 100 apart assert 100 == abs(met0 - met1) + # No policy routing + assert not {"routing-policy", "routes"}.intersection( + netcfg["ethernets"]["eth0"].keys() + ) + assert not {"routing-policy", "routes"}.intersection( + netcfg["ethernets"]["eth1"].keys() + ) + class TestIsAliYun(test_helpers.CiTestCase): ALIYUN_PRODUCT = "Alibaba Cloud ECS" diff --git a/tests/unittests/sources/test_ec2.py b/tests/unittests/sources/test_ec2.py index db2dbe1f692f..a545f5496a16 100644 --- a/tests/unittests/sources/test_ec2.py +++ b/tests/unittests/sources/test_ec2.py @@ -211,6 +211,7 @@ "services": {"domain": "amazonaws.com", "partition": "aws"}, } +M_PATH = "cloudinit.sources.DataSourceEc2." M_PATH_NET = "cloudinit.sources.DataSourceEc2.net." TAGS_METADATA_2021_03_23: dict = { @@ -482,6 +483,10 @@ def test_network_config_property_set_dhcp4(self): with mock.patch(patch_path) as m_get_interfaces_by_mac: with mock.patch(find_fallback_path) as m_find_fallback: with mock.patch(get_interface_mac_path) as m_get_mac: + dhcp_client = ds.distro.dhcp_client + dhcp_client.dhcp_discovery.return_value = { + "routers": "172.31.1.0" + } m_get_interfaces_by_mac.return_value = {mac1: "eth9"} m_find_fallback.return_value = "eth9" m_get_mac.return_value = mac1 @@ -908,7 +913,6 @@ def test_get_instance_tags(self): class TestGetSecondaryAddresses(test_helpers.CiTestCase): - mac = "06:17:04:d7:26:ff" with_logs = True @@ -982,10 +986,11 @@ def test_convert_ec2_metadata_network_config_skips_absent_macs(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - self.network_metadata, macs_to_nics + self.network_metadata, distro, macs_to_nics ), ) @@ -1007,10 +1012,11 @@ def test_convert_ec2_metadata_network_config_handles_only_dhcp6(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_ipv6, macs_to_nics + network_metadata_ipv6, distro, macs_to_nics ), ) @@ -1032,10 +1038,11 @@ def test_convert_ec2_metadata_network_config_local_only_dhcp4(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_ipv6, macs_to_nics + network_metadata_ipv6, distro, macs_to_nics ), ) @@ -1058,10 +1065,11 @@ def test_convert_ec2_metadata_network_config_handles_absent_dhcp4(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_ipv6, macs_to_nics, fallback_nic="eth9" + network_metadata_ipv6, distro, macs_to_nics ), ) @@ -1084,15 +1092,18 @@ def test_convert_ec2_metadata_network_config_handles_local_v4_and_v6(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_both, macs_to_nics + network_metadata_both, distro, macs_to_nics ), ) def test_convert_ec2_metadata_network_config_handles_multiple_nics(self): - """DHCP route-metric increases on secondary NICs for IPv4 and IPv6.""" + """DHCP route-metric increases on secondary NICs for IPv4 and IPv6. + Source-routing configured for secondary NICs (routing-policy and extra + routing table).""" mac2 = "06:17:04:d7:26:08" macs_to_nics = {self.mac1: "eth9", mac2: "eth10"} network_metadata_both = copy.deepcopy(self.network_metadata) @@ -1117,15 +1128,32 @@ def test_convert_ec2_metadata_network_config_handles_multiple_nics(self): "match": {"macaddress": mac2}, "set-name": "eth10", "dhcp4": True, - "dhcp4-overrides": {"route-metric": 200}, + "dhcp4-overrides": { + "route-metric": 200, + "use-routes": True, + }, "dhcp6": False, + "routes": [ + # via DHCP gateway + {"to": "0.0.0.0/0", "via": "172.31.1.0", "table": 101}, + # to NIC2_MD["subnet-ipv4-cidr-block"] + {"to": "172.31.32.0/20", "table": 101}, + ], + "routing-policy": [ + # NIC2_MD["local-ipv4s"] + {"from": "172.31.47.221", "table": 101} + ], }, }, } + distro = mock.Mock() + distro.dhcp_client.dhcp_discovery.return_value = { + "routers": "172.31.1.0" + } self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_both, macs_to_nics + network_metadata_both, distro, macs_to_nics ), ) @@ -1146,10 +1174,11 @@ def test_convert_ec2_metadata_network_config_handles_dhcp4_and_dhcp6(self): } }, } + distro = mock.Mock() self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( - network_metadata_both, macs_to_nics + network_metadata_both, distro, macs_to_nics ), ) @@ -1167,11 +1196,14 @@ def test_convert_ec2_metadata_gets_macs_from_get_interfaces_by_mac(self): }, } patch_path = M_PATH_NET + "get_interfaces_by_mac" + distro = mock.Mock() with mock.patch(patch_path) as m_get_interfaces_by_mac: m_get_interfaces_by_mac.return_value = {self.mac1: "eth9"} self.assertEqual( expected, - ec2.convert_ec2_metadata_network_config(self.network_metadata), + ec2.convert_ec2_metadata_network_config( + self.network_metadata, distro + ), )