From a8f69409e5cebf43767d3bb54fbad7ced1e8fc7b Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 10 Oct 2024 23:19:28 -0500 Subject: [PATCH] fix: Render bridges correctly for v2 on sysconfig with set-name (#5674) When listing interfaces in v2 format, we should expect to be able to reference other interfaces using the name in the configuration, not the name the interface will eventually take. This was broken when using `set-name`. To fix this, we now store the configuration id alongside the eventual name, and reference that instead of the name. Fixes GH-5574 --- cloudinit/net/eni.py | 6 ++ cloudinit/net/network_state.py | 29 +++------ cloudinit/net/sysconfig.py | 24 +++++--- tests/unittests/net/network_configs.py | 85 ++++++++++++++++++++++++++ tests/unittests/test_net.py | 1 + 5 files changed, 118 insertions(+), 27 deletions(-) diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 26a5ec1bcd9..146c0167435 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -5,6 +5,7 @@ import logging import os import re +from contextlib import suppress from typing import Optional from cloudinit import performance, subp, util @@ -421,6 +422,11 @@ def _render_route(self, route, indent=""): return content def _render_iface(self, iface, render_hwaddress=False): + iface = copy.deepcopy(iface) + + # Remove irrelevant keys + with suppress(KeyError): + iface.pop("config_id") sections = [] subnets = iface.get("subnets", {}) accept_ra = iface.pop("accept-ra", None) diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 10868fa8ca9..32c5a4c7151 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -411,6 +411,7 @@ def handle_physical(self, command): wakeonlan = util.is_true(wakeonlan) iface.update( { + "config_id": command.get("config_id"), "name": command.get("name"), "type": command.get("type"), "mac_address": command.get("mac_address"), @@ -424,7 +425,8 @@ def handle_physical(self, command): "wakeonlan": wakeonlan, } ) - self._network_state["interfaces"].update({command.get("name"): iface}) + iface_key = command.get("config_id", command.get("name")) + self._network_state["interfaces"].update({iface_key: iface}) self.dump_network_state() @ensure_command_keys(["name", "vlan_id", "vlan_link"]) @@ -712,6 +714,7 @@ def handle_ethernets(self, command): for eth, cfg in command.items(): phy_cmd = { + "config_id": eth, "type": "physical", } match = cfg.get("match", {}) @@ -800,26 +803,14 @@ def handle_wifis(self, command): def _v2_common(self, cfg) -> None: LOG.debug("v2_common: handling config:\n%s", cfg) for iface, dev_cfg in cfg.items(): - if "set-name" in dev_cfg: - set_name_iface = dev_cfg.get("set-name") - if set_name_iface: - iface = set_name_iface if "nameservers" in dev_cfg: - search = dev_cfg.get("nameservers").get("search", []) - dns = dev_cfg.get("nameservers").get("addresses", []) + search = dev_cfg.get("nameservers").get("search") + dns = dev_cfg.get("nameservers").get("addresses") name_cmd = {"type": "nameserver"} - if len(search) > 0: - name_cmd.update({"search": search}) - if len(dns) > 0: - name_cmd.update({"address": dns}) - - mac_address: Optional[str] = dev_cfg.get("match", {}).get( - "macaddress" - ) - if mac_address: - real_if_name = find_interface_name_from_mac(mac_address) - if real_if_name: - iface = real_if_name + if search: + name_cmd["search"] = search + if dns: + name_cmd["address"] = dns self._handle_individual_nameserver(name_cmd, iface) diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index c945f2b1440..6fe92c62f76 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -6,7 +6,7 @@ import logging import os import re -from typing import Mapping, Optional +from typing import Dict, Optional from cloudinit import subp, util from cloudinit.distros.parsers import networkmanager_conf, resolv_conf @@ -720,7 +720,7 @@ def _render_physical_interfaces( ): physical_filter = renderer.filter_by_physical for iface in network_state.iter_interfaces(physical_filter): - iface_name = iface["name"] + iface_name = iface.get("config_id") or iface["name"] iface_subnets = iface.get("subnets", []) iface_cfg = iface_contents[iface_name] route_cfg = iface_cfg.routes @@ -925,7 +925,9 @@ def _render_networkmanager_conf(network_state, templates=None): return out @classmethod - def _render_bridge_interfaces(cls, network_state, iface_contents, flavor): + def _render_bridge_interfaces( + cls, network_state: NetworkState, iface_contents, flavor + ): bridge_key_map = { old_k: new_k for old_k, new_k in cls.cfg_key_maps[flavor].items() @@ -1006,23 +1008,29 @@ def _render_ib_interfaces(cls, network_state, iface_contents, flavor): @classmethod def _render_sysconfig( - cls, base_sysconf_dir, network_state, flavor, templates=None + cls, + base_sysconf_dir, + network_state: NetworkState, + flavor, + templates=None, ): """Given state, return /etc/sysconfig files + contents""" if not templates: templates = cls.templates - iface_contents: Mapping[str, NetInterface] = {} + iface_contents: Dict[str, NetInterface] = {} for iface in network_state.iter_interfaces(): if iface["type"] == "loopback": continue - iface_name = iface["name"] - iface_cfg = NetInterface(iface_name, base_sysconf_dir, templates) + config_id: str = iface.get("config_id") or iface["name"] + iface_cfg = NetInterface( + iface["name"], base_sysconf_dir, templates + ) if flavor == "suse": iface_cfg.drop("DEVICE") # If type detection fails it is considered a bug in SUSE iface_cfg.drop("TYPE") cls._render_iface_shared(iface, iface_cfg, flavor) - iface_contents[iface_name] = iface_cfg + iface_contents[config_id] = iface_cfg cls._render_physical_interfaces(network_state, iface_contents, flavor) cls._render_bond_interfaces(network_state, iface_contents, flavor) cls._render_vlan_interfaces(network_state, iface_contents, flavor) diff --git a/tests/unittests/net/network_configs.py b/tests/unittests/net/network_configs.py index 933d206e9a8..e34092b546e 100644 --- a/tests/unittests/net/network_configs.py +++ b/tests/unittests/net/network_configs.py @@ -4912,4 +4912,89 @@ """ ), }, + "v2-bridges-set-name": { + "yaml": textwrap.dedent( + """\ + version: 2 + ethernets: + baremetalport: + match: + macaddress: 52:54:00:bd:8f:cb + set-name: baremetal0 + provisioningport: + match: + macaddress: 52:54:00:25:ae:12 + set-name: provisioning0 + bridges: + baremetal: + addresses: + - fc00:1:1::2/64 + interfaces: + - baremetalport + provisioning: + addresses: + - fc00:1:2::2/64 + interfaces: + - provisioningport + """ + ), + "expected_sysconfig_rhel": { + "ifcfg-baremetal": textwrap.dedent( + """\ + # Created by cloud-init automatically, do not edit. + # + BOOTPROTO=none + DEVICE=baremetal + IPV6ADDR=fc00:1:1::2/64 + IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no + ONBOOT=yes + TYPE=Bridge + USERCTL=no + """ + ), + "ifcfg-baremetal0": textwrap.dedent( + """\ + # Created by cloud-init automatically, do not edit. + # + BOOTPROTO=none + BRIDGE=baremetal + DEVICE=baremetal0 + HWADDR=52:54:00:bd:8f:cb + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """ + ), + "ifcfg-provisioning": textwrap.dedent( + """\ + # Created by cloud-init automatically, do not edit. + # + BOOTPROTO=none + DEVICE=provisioning + IPV6ADDR=fc00:1:2::2/64 + IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no + ONBOOT=yes + TYPE=Bridge + USERCTL=no + """ + ), + "ifcfg-provisioning0": textwrap.dedent( + """\ + # Created by cloud-init automatically, do not edit. + # + BOOTPROTO=none + BRIDGE=provisioning + DEVICE=provisioning0 + HWADDR=52:54:00:25:ae:12 + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """ + ), + }, + }, } diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 590061e039e..213ee62dc9f 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -2098,6 +2098,7 @@ def test_config_with_explicit_loopback(self): ("dhcpv6_stateful", "yaml"), ("wakeonlan_disabled", "yaml_v2"), ("wakeonlan_enabled", "yaml_v2"), + ("v2-bridges-set-name", "yaml"), pytest.param( "v1-dns", "yaml",