From 25474e26b25b396ea6d19c1a0b13f4b913cc6560 Mon Sep 17 00:00:00 2001 From: Sebastian Lohff Date: Wed, 24 Apr 2024 19:17:06 +0200 Subject: [PATCH] Support untagged infra networks We now support the already present untagged flag for infra networks. Only one network can be set as untagged for each hostgroup. As we want to make sure servers stay deployable via OpenStack, the OpenStack network will take precedence over an untagged infra network for all direct mode bindings. Due to this quirk, we also cannot properly sync the native vlan via the sync_infra_networks API call, so this will have to be done via a full switch sync. --- .../common/config/config_driver.py | 15 +++++++++++++++ .../extensions/fabricoperations.py | 4 +++- networking_ccloud/ml2/agent/common/messages.py | 18 +++++++++++------- networking_ccloud/ml2/plugin.py | 2 +- .../tests/unit/common/test_driver_config.py | 10 ++++++++++ 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/networking_ccloud/common/config/config_driver.py b/networking_ccloud/common/config/config_driver.py index 0ba3365a..ad388248 100644 --- a/networking_ccloud/common/config/config_driver.py +++ b/networking_ccloud/common/config/config_driver.py @@ -227,6 +227,8 @@ class InfraNetwork(pydantic.BaseModel): networks: List[str] = [] aggregates: List[str] = [] vni: pydantic.conint(gt=0, lt=2**24) + + # note that untagged OpenStack network will take precedence over untagged infra networks untagged: bool = False dhcp_relays: List[str] = [] @@ -327,6 +329,19 @@ def ensure_at_least_one_member(cls, v): raise ValueError("Hostgroup needs to have at least one member") return v + @pydantic.validator('infra_networks') + def ensure_only_one_untagged_infra_network(cls, v): + untagged_net = None + for infra_net in v or []: + if not infra_net.untagged: + continue + if untagged_net is None: + untagged_net = infra_net.name + else: + raise ValueError("Found two untagged InfraNetworks on same hostgroup: " + f"{untagged_net} and {infra_net.name}") + return v + @pydantic.root_validator() def ensure_hostgroups_with_role_are_not_a_metagroup(cls, values): # FIXME: constants? enum? what do we do here diff --git a/networking_ccloud/extensions/fabricoperations.py b/networking_ccloud/extensions/fabricoperations.py index 2ef40b57..4c1919e4 100644 --- a/networking_ccloud/extensions/fabricoperations.py +++ b/networking_ccloud/extensions/fabricoperations.py @@ -343,7 +343,9 @@ def sync_infra_networks(self, request, **kwargs): scul = agent_msg.SwitchConfigUpdateList(agent_msg.OperationEnum.replace, self.drv_conf) for hg in self.drv_conf.get_hostgroups_by_switches([switch.name]): if hg.infra_networks: - scul.add_infra_networks_from_hostgroup(hg, sg) + # as we don't know if a direct-binding deployment is on a port we cannot + # process native vlans here --> needs to be done in a full switch sync + scul.add_infra_networks_from_hostgroup(hg, sg, process_untagged=False) if hg.extra_vlans: scul.add_extra_vlans(hg) scul.clean_switches(switch.name) diff --git a/networking_ccloud/ml2/agent/common/messages.py b/networking_ccloud/ml2/agent/common/messages.py index 7a6f02a8..42e370f7 100644 --- a/networking_ccloud/ml2/agent/common/messages.py +++ b/networking_ccloud/ml2/agent/common/messages.py @@ -377,7 +377,8 @@ def get_or_create_switch(self, switch_name): return self.switch_config_updates[switch_name] def add_binding_host_to_config(self, hg_config, network_id, seg_vni, seg_vlan, trunk_vlan=None, - keep_mapping=False, exclude_hosts=None, is_bgw=False, gateways=None): + keep_mapping=False, exclude_hosts=None, is_bgw=False, gateways=None, + override_native=False): """Add binding host config to all required switches Given a hostgroup config this method generates and adds config to this @@ -422,10 +423,13 @@ def add_binding_host_to_config(self, hg_config, network_id, seg_vni, seg_vlan, t iface = scu.get_or_create_iface_from_switchport(sp) iface.add_trunk_vlan(seg_vlan) - if hg_config.direct_binding and not hg_config.role: - if trunk_vlan: - iface.add_vlan_translation(seg_vlan, trunk_vlan) - elif not hg_config.allow_multiple_trunk_ports: + if not hg_config.role: + if hg_config.direct_binding: + if trunk_vlan: + iface.add_vlan_translation(seg_vlan, trunk_vlan) + elif not hg_config.allow_multiple_trunk_ports: + iface.native_vlan = seg_vlan + elif not hg_config.direct_binding and override_native: iface.native_vlan = seg_vlan def add_vrf_bgp_config(self, switch_names, vrf_name, vrf_networks, vrf_aggregates): @@ -447,7 +451,7 @@ def add_vrf_bgp_config(self, switch_names, vrf_name, vrf_networks, vrf_aggregate aggregates.append(BGPVRFAggregate(network=network, az_local=az_local)) vrf.add_aggregates(aggregates) - def add_infra_networks_from_hostgroup(self, hg_config, sg): + def add_infra_networks_from_hostgroup(self, hg_config, sg, process_untagged=False): for inet in hg_config.infra_networks or []: # FIXME: exclude hosts gateways = None @@ -455,7 +459,7 @@ def add_infra_networks_from_hostgroup(self, hg_config, sg): gateways = {'vrf': inet.vrf, 'ips': inet.networks} self.add_binding_host_to_config(hg_config, inet.name, inet.vni, inet.vlan, - gateways=gateways) + gateways=gateways, override_native=inet.untagged and process_untagged) if inet.vrf: # get network address from network (clear host bits); they are az-local and non-ext-announcable diff --git a/networking_ccloud/ml2/plugin.py b/networking_ccloud/ml2/plugin.py index d1afd480..a254a37c 100644 --- a/networking_ccloud/ml2/plugin.py +++ b/networking_ccloud/ml2/plugin.py @@ -206,7 +206,7 @@ def make_switchgroup_config(self, context, sg): # add interconnects, infra networks and extra vlans for hg in self.drv_conf.get_hostgroups_by_switches([sw.name for sw in sg.members]): if hg.infra_networks: - scul.add_infra_networks_from_hostgroup(hg, sg) + scul.add_infra_networks_from_hostgroup(hg, sg, process_untagged=True) if hg.extra_vlans: scul.add_extra_vlans(hg) if hg.role: diff --git a/networking_ccloud/tests/unit/common/test_driver_config.py b/networking_ccloud/tests/unit/common/test_driver_config.py index 3d18ddfd..dc49884b 100644 --- a/networking_ccloud/tests/unit/common/test_driver_config.py +++ b/networking_ccloud/tests/unit/common/test_driver_config.py @@ -212,6 +212,16 @@ def test_infra_network_vrf_presence(self): config.DriverConfig, switchgroups=[switchgroup], hostgroups=hostgroups, global_config=global_config) + def test_hostgroup_no_two_untagged_networks(self): + sg = cfix.make_switchgroup("seagull", availability_zone="qa-de-1a"), + untagged_1 = config.InfraNetwork(name="mew-gull", vlan=23, vni=100023, untagged=True) + untagged_2 = config.InfraNetwork(name="herring-gull", vlan=42, vni=100042, untagged=True) + regular_1 = config.InfraNetwork(name="sparrow", vlan=2, vni=2) + cfix.make_hostgroups(sg, infra_networks=[untagged_1]) + cfix.make_hostgroups(sg, infra_networks=[untagged_1, regular_1]) + self.assertRaisesRegex(ValueError, "on same hostgroup: mew-gull and herring-gull", + cfix.make_hostgroups, sg, infra_networks=[untagged_1, regular_1, untagged_2]) + def test_duplicate_vrf_name(self): vrfs = cfix.make_vrfs(['ROUTE-ME', 'ROUTE-ME'])