From e36f17b456e9cc0bf67b400f38044c27ddf99c85 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Fri, 28 Aug 2026 16:23:32 +0200 Subject: [PATCH] fix(sonic): derive one BGP peer identity per link BGP_NEIGHBOR_AF.neighbor is a YANG leafref into BGP_NEIGHBOR for the same VRF (sonic-bgp-neighbor.yang), so the two tables have to name the same peer. The physical-interface and port-channel loops key BGP_NEIGHBOR by the resolved peer address but key BGP_NEIGHBOR_AF by the interface or port-channel name, so they name different peers: the neighbor gets no activated address family, and the address families reference a neighbor that does not exist. That is not cosmetic. frrcfgd's AF handler never creates the neighbor. It enters the address family and runs the AF commands against the key it was given (frrcfgd.py; admin_status maps to "neighbor {} activate"). Replayed against FRR 10.2.6 under SONiC's own "no bgp default ipv4-unicast": vtysh -c 'router bgp 65000' -c 'address-family ipv4 unicast' \ -c 'neighbor PortChannel1 activate' % Specify remote-as or peer-group commands first The numbered neighbor then ends up activated for no address family at all, so the session comes up and exchanges nothing. Nothing masks it: the generator emits no peer group anywhere, and the shipped base config declares none. Every attribute of the session now follows one decision. _numbered_peer_address() answers it: a numbered session needs a routable IPv4 at both ends -- the resolved peer address and a transfer-role IPv4 on this side to source it from. A peer address on its own is not enough, because with no local address the session cannot come up. Without both, the peering is unnumbered. From that one answer follow the key, v6only and the address families, in all four loops: * both tables key on the peer address when the session is numbered and on the interface or port-channel name when it is not, so they agree by construction rather than by coincidence; * v6only is kept consistent with the peering mode: "false" for a numbered key, "true" for an interface key. This one fixes no behaviour on the community SONiC path, where the field is inert -- absent from sonic-bgp-neighbor.yang, and not read by frrcfgd, which picks the "neighbor {} interface" form from the key shape alone. It is worth emitting the semantically correct value regardless: deriving it from has_transfer_ipv4 put v6only "false" on interface-keyed neighbors, a contradiction on its face, and vendor stacks outside the community tree do document and consume the field; * a numbered session runs over an IPv4 transfer link and activates ipv4_unicast alone; an unnumbered one is an IPv6 link-local session and carries both families over it. Deriving this from has_transfer_ipv4 instead is what put an ipv6_unicast row under a neighbor named by an IPv4 literal, because the peer's address and this side's transfer address are independent conditions: an interface carrying no IPv4 of its own, facing an endpoint that carries one, satisfied the outer has_transfer_ipv4-or-not-has_direct_ipv4 gate and produced a numbered BGP_NEIGHBOR this switch cannot source -- no local_addr was found for it either. The port-channel loops had none of this. They keyed on the peer address alone, emitted both address families unconditionally, hardcoded v6only "true", and left local_addr as a TODO. They now take the same path: the transfer role lookup reaches a LAG whose NetBox name is canonical, so a port channel is numbered under the same condition as a physical interface and carries the local_addr that address supplies. Looking that address up is now _local_ipv4_address(), since both neighbor loops need it and neither can assume the SONiC name and the NetBox name agree. Unnumbered peers keep working throughout: with no numbered peer to key on, both tables fall back to the interface name, which is what a link-local session wants anyway. This changes configuration pushed to switches. Where the key mismatch exists the BGP session is activated for no address family; afterwards it activates, so sessions that were previously dead can come up and traffic may move. Correct, but it wants a deliberate rollout decision rather than only a code review. The tests asserting that the two tables agree on the key keep the resolver real instead of patching it out, so they can observe which branch it takes. Every other BGP test in that module patches the resolver away and therefore emits identical output whether the two tables agree or not. Three existing tests move with the rule. TestBgpNeighborPortChannels::test_default_vrf_peer_ip_no_local_addr is dropped: it asserted that a port-channel neighbor keyed by a peer address carries no local_addr -- the numbered neighbor with no local address that this change eliminates -- and that shape is unreachable now, because a numbered port channel requires the transfer role address local_addr is read from. The input it used is asserted in full, key and every field, by a new test. TestBgpNeighborAfInterfaces::test_transfer_role_ipv4_adds_ipv4_only and TestBgpNeighborInterfaces::test_transfer_role_ipv4_v6only_false both still assert what their names say, but reaching a numbered transfer link now takes a resolvable peer, so both gain one. Their previous input, a transfer address with no peer, is an unnumbered session and is covered by a new test. UpgradeImpact Assisted-by: Claude:claude-opus-5 Assisted-by: Codex Signed-off-by: Roger Luethi --- .../tasks/conductor/sonic/config_generator.py | 176 +++++++++----- .../test_config_generator_bgp_vlan_vrf.py | 215 ++++++++++++++++-- 2 files changed, 319 insertions(+), 72 deletions(-) diff --git a/osism/tasks/conductor/sonic/config_generator.py b/osism/tasks/conductor/sonic/config_generator.py index a48d2225d..927f42be8 100644 --- a/osism/tasks/conductor/sonic/config_generator.py +++ b/osism/tasks/conductor/sonic/config_generator.py @@ -1148,6 +1148,53 @@ def _has_transfer_role_ipv4(port_name, transfer_ips, netbox_interfaces): return False +def _numbered_peer_address(has_transfer_ipv4, connected_ipv4): + """Return the peer address a *numbered* BGP session is keyed by, or None. + + A numbered session needs a routable IPv4 at both ends: the peer address the + resolver found, and a transfer-role IPv4 on this switch to source it from. + The peer's address alone is not enough -- with no local address the session + cannot come up, and keying on it would name a neighbor whose address + families this switch cannot activate. Without both, the peering is + unnumbered and is keyed by the interface or port channel name instead. + + Args: + has_transfer_ipv4: Whether this side has a transfer role IPv4 address + connected_ipv4: The connected endpoint's IPv4 address, or None + + Returns: + str or None: The peer address to key on, or None for unnumbered peering + """ + return connected_ipv4 if (connected_ipv4 and has_transfer_ipv4) else None + + +def _local_ipv4_address(port_name, interface_ips, transfer_ips, netbox_interfaces): + """Return the IPv4 address a numbered BGP session sources from, or None. + + A numbered neighbor carries local_addr so the session binds to the address + on this side of the link. Direct assignments win over transfer role ones; + both are keyed by NetBox interface name, which is not always the SONiC name. + + Args: + port_name: SONiC interface or port channel name (e.g., "Ethernet0") + interface_ips: Dict mapping NetBox interface names to IPv4 addresses + transfer_ips: Dict mapping NetBox interface names to transfer role IPv4 + netbox_interfaces: Dict mapping SONiC names to NetBox interface info + + Returns: + str or None: The local IPv4 address without its prefix length + """ + if port_name not in netbox_interfaces: + return None + + netbox_interface_name = netbox_interfaces[port_name]["netbox_name"] + if interface_ips and netbox_interface_name in interface_ips: + return interface_ips[netbox_interface_name].split("/")[0] + if transfer_ips and netbox_interface_name in transfer_ips: + return transfer_ips[netbox_interface_name].split("/")[0] + return None + + def _is_untagged_vlan_member(port_name, vlan_info, netbox_interfaces): """Check if an interface is an untagged member of any VLAN. @@ -1253,15 +1300,25 @@ def get_vrf_for_interface(interface_name): device, port_name, netbox ) - # For BGP_NEIGHBOR_AF, always use interface name like IPv6 does - neighbor_id = port_name + # Must match how the BGP_NEIGHBOR loop below keys this peer: + # BGP_NEIGHBOR_AF.neighbor is a leafref into BGP_NEIGHBOR, so an + # AF row naming anything else activates no address family. + numbered_peer = _numbered_peer_address( + has_transfer_ipv4, connected_ipv4 + ) + neighbor_id = numbered_peer if numbered_peer else port_name vrf_name = get_vrf_for_interface(port_name) ipv4_key = f"{vrf_name}|{neighbor_id}|{BGP_AF_IPV4_UNICAST}" config["BGP_NEIGHBOR_AF"][ipv4_key] = {"admin_status": "true"} - # Only add ipv6_unicast if v6only would be true (no transfer role IPv4) - if not has_transfer_ipv4: + # The address families follow the peering mode, like the key + # and v6only above. A numbered session runs over an IPv4 + # transfer link and peers over IPv4 alone; an unnumbered one is + # an IPv6 link-local session and carries both families over it. + # Gating this on anything else puts an ipv6_unicast row under a + # neighbor named by an IPv4 literal. + if not numbered_peer: ipv6_key = f"{vrf_name}|{neighbor_id}|{BGP_AF_IPV6_UNICAST}" config["BGP_NEIGHBOR_AF"][ipv6_key] = {"admin_status": "true"} logger.debug( @@ -1269,7 +1326,7 @@ def get_vrf_for_interface(interface_name): ) else: logger.debug( - f"Added BGP_NEIGHBOR_AF with ipv4_unicast only for interface {port_name} (transfer role IPv4, v6only=false)" + f"Added BGP_NEIGHBOR_AF with ipv4_unicast only for interface {port_name} (transfer role IPv4)" ) # Add l2vpn_evpn only for switch-to-switch connections (default VRF) @@ -1328,14 +1385,23 @@ def get_vrf_for_interface(interface_name): device, pc_name, netbox ) - # For BGP_NEIGHBOR_AF, always use port channel name like interfaces - neighbor_id = pc_name + # Must match how the BGP_NEIGHBOR loop below keys this peer; see the + # physical-interface loop above. + has_transfer_ipv4 = _has_transfer_role_ipv4( + pc_name, transfer_ips, netbox_interfaces + ) + numbered_peer = _numbered_peer_address(has_transfer_ipv4, connected_ipv4) + neighbor_id = numbered_peer if numbered_peer else pc_name vrf_name = get_vrf_for_interface(pc_name) ipv4_key = f"{vrf_name}|{neighbor_id}|{BGP_AF_IPV4_UNICAST}" - ipv6_key = f"{vrf_name}|{neighbor_id}|{BGP_AF_IPV6_UNICAST}" config["BGP_NEIGHBOR_AF"][ipv4_key] = {"admin_status": "true"} - config["BGP_NEIGHBOR_AF"][ipv6_key] = {"admin_status": "true"} + + # Same rule as the physical-interface loop: the address families + # follow the peering mode. + if not numbered_peer: + ipv6_key = f"{vrf_name}|{neighbor_id}|{BGP_AF_IPV6_UNICAST}" + config["BGP_NEIGHBOR_AF"][ipv6_key] = {"admin_status": "true"} # Add l2vpn_evpn only for switch-to-switch connections (default VRF) if vrf_name == "default": @@ -1395,16 +1461,20 @@ def get_vrf_for_interface(interface_name): # Get VRF for this interface vrf_name = get_vrf_for_interface(port_name) - # Use the connected interface's IPv4 address if available, otherwise use interface name - if connected_ipv4: - neighbor_key = f"{vrf_name}|{connected_ipv4}" + # Key a numbered session by the peer address, an unnumbered + # one by the interface name + numbered_peer = _numbered_peer_address( + has_transfer_ipv4, connected_ipv4 + ) + if numbered_peer: + neighbor_key = f"{vrf_name}|{numbered_peer}" logger.debug( - f"Using connected interface IPv4 address {connected_ipv4} for BGP neighbor on {port_name}" + f"Using connected interface IPv4 address {numbered_peer} for BGP neighbor on {port_name}" ) else: neighbor_key = f"{vrf_name}|{port_name}" logger.debug( - f"No connected interface IPv4 found, using interface name {port_name} for BGP neighbor" + f"No numbered peering on {port_name}, using interface name for BGP neighbor" ) # Determine peer_type based on connected device AS @@ -1417,47 +1487,34 @@ def get_vrf_for_interface(interface_name): device, connected_device, device_as_mapping ) - # Set v6only based on whether interface has transfer role IPv4 - # - Transfer role IPv4: v6only=false (dual-stack BGP) - # - No direct IPv4: v6only=true (IPv6-only BGP) - # - Non-default VRF: no v6only parameter + # v6only is a property of the key form, not of the local + # address families: frrcfgd renders an interface-keyed neighbor + # as "neighbor {} interface", the only form FRR accepts v6only + # on. So it follows the peering mode, and a non-default VRF + # carries no v6only parameter at all. bgp_neighbor_config = { "peer_type": peer_type, } if vrf_name == "default": - bgp_neighbor_config["v6only"] = ( - "false" if has_transfer_ipv4 else "true" - ) - - # If using IP address as key, also store the local address - if connected_ipv4: - # Get the local interface IPv4 address - local_ipv4 = None - if port_name in netbox_interfaces: - netbox_interface_name = netbox_interfaces[port_name][ - "netbox_name" - ] - if interface_ips and netbox_interface_name in interface_ips: - local_ipv4 = interface_ips[netbox_interface_name].split( - "/" - )[0] - elif transfer_ips and netbox_interface_name in transfer_ips: - local_ipv4 = transfer_ips[netbox_interface_name].split("/")[ - 0 - ] + bgp_neighbor_config["v6only"] = "false" if numbered_peer else "true" + # A numbered session binds to the address on this side + if numbered_peer: + local_ipv4 = _local_ipv4_address( + port_name, interface_ips, transfer_ips, netbox_interfaces + ) if local_ipv4: bgp_neighbor_config["local_addr"] = local_ipv4 config["BGP_NEIGHBOR"][neighbor_key] = bgp_neighbor_config - if has_transfer_ipv4: + if numbered_peer: logger.debug( - f"Added BGP_NEIGHBOR for interface {port_name} (transfer role IPv4, v6only=false)" + f"Added numbered BGP_NEIGHBOR for interface {port_name} (peer {numbered_peer})" ) else: logger.debug( - f"Added BGP_NEIGHBOR for interface {port_name} (no direct IPv4, v6only=true)" + f"Added unnumbered BGP_NEIGHBOR for interface {port_name}" ) # Add BGP_NEIGHBOR configuration for connected port channels @@ -1481,16 +1538,21 @@ def get_vrf_for_interface(interface_name): # Get VRF for this port channel vrf_name = get_vrf_for_interface(pc_name) - # Use the connected interface's IPv4 address if available, otherwise use port channel name - if connected_ipv4: - neighbor_key = f"{vrf_name}|{connected_ipv4}" + # Key a numbered session by the peer address, an unnumbered one by the + # port channel name + has_transfer_ipv4 = _has_transfer_role_ipv4( + pc_name, transfer_ips, netbox_interfaces + ) + numbered_peer = _numbered_peer_address(has_transfer_ipv4, connected_ipv4) + if numbered_peer: + neighbor_key = f"{vrf_name}|{numbered_peer}" logger.debug( - f"Using connected interface IPv4 address {connected_ipv4} for BGP neighbor on {pc_name}" + f"Using connected interface IPv4 address {numbered_peer} for BGP neighbor on {pc_name}" ) else: neighbor_key = f"{vrf_name}|{pc_name}" logger.debug( - f"No connected interface IPv4 found, using port channel name {pc_name} for BGP neighbor" + f"No numbered peering on {pc_name}, using port channel name for BGP neighbor" ) # Determine peer_type based on connected device AS @@ -1505,17 +1567,17 @@ def get_vrf_for_interface(interface_name): "peer_type": peer_type, } if vrf_name == "default": - bgp_neighbor_config["v6only"] = "true" - - # If using IP address as key, also store the local address - if connected_ipv4: - # For port channels, get the local IPv4 address from interface IPs - # Note: Port channels don't have direct IP assignments in NetBox, - # so we use the connected interface IP logic - local_ipv4 = None - # Port channels don't have NetBox interface entries, - # so we skip local_addr for port channels for now - # TODO: Implement port channel local address lookup if needed + bgp_neighbor_config["v6only"] = "false" if numbered_peer else "true" + + # A numbered session binds to the address on this side. A port channel + # reaches one only through a transfer role IPv4 on the aggregate, so + # that is the address the lookup finds. + if numbered_peer: + local_ipv4 = _local_ipv4_address( + pc_name, interface_ips, transfer_ips, netbox_interfaces + ) + if local_ipv4: + bgp_neighbor_config["local_addr"] = local_ipv4 config["BGP_NEIGHBOR"][neighbor_key] = bgp_neighbor_config diff --git a/tests/unit/tasks/conductor/sonic/test_config_generator_bgp_vlan_vrf.py b/tests/unit/tasks/conductor/sonic/test_config_generator_bgp_vlan_vrf.py index 3cf85a92a..c98f39218 100644 --- a/tests/unit/tasks/conductor/sonic/test_config_generator_bgp_vlan_vrf.py +++ b/tests/unit/tasks/conductor/sonic/test_config_generator_bgp_vlan_vrf.py @@ -137,10 +137,15 @@ def test_no_direct_ipv4_adds_ipv4_and_ipv6(self, bgp_config, patch_bgp): assert "default|Ethernet0|l2vpn_evpn" not in af def test_transfer_role_ipv4_adds_ipv4_only(self, bgp_config, patch_bgp): - self._base(bgp_config, transfer_ips={"eth0": "10.0.0.1/31"}) + patch_bgp.peer_ipv4.return_value = "192.0.2.5" + self._base( + bgp_config, + netbox=object(), + transfer_ips={"eth0": "10.0.0.1/31"}, + ) af = bgp_config["BGP_NEIGHBOR_AF"] - assert af["default|Ethernet0|ipv4_unicast"] == {"admin_status": "true"} - assert "default|Ethernet0|ipv6_unicast" not in af + assert af["default|192.0.2.5|ipv4_unicast"] == {"admin_status": "true"} + assert "default|192.0.2.5|ipv6_unicast" not in af def test_switch_to_switch_adds_l2vpn(self, bgp_config, patch_bgp): patch_bgp.connected_device.return_value = _switch_device() @@ -276,8 +281,13 @@ def test_peer_ip_local_addr_from_transfer_ips(self, bgp_config, patch_bgp): assert entry["v6only"] == "false" def test_transfer_role_ipv4_v6only_false(self, bgp_config, patch_bgp): - self._base(bgp_config, transfer_ips={"eth0": "10.2.2.2/31"}) - assert bgp_config["BGP_NEIGHBOR"]["default|Ethernet0"]["v6only"] == "false" + patch_bgp.peer_ipv4.return_value = "192.0.2.5" + self._base( + bgp_config, + netbox=object(), + transfer_ips={"eth0": "10.2.2.2/31"}, + ) + assert bgp_config["BGP_NEIGHBOR"]["default|192.0.2.5"]["v6only"] == "false" def test_non_default_vrf_no_v6only(self, bgp_config, patch_bgp): self._base( @@ -311,16 +321,6 @@ def test_default_vrf_no_peer_ip(self, bgp_config, patch_bgp): "v6only": "true", } - def test_default_vrf_peer_ip_no_local_addr(self, bgp_config, patch_bgp): - patch_bgp.peer_ipv4.return_value = "192.0.2.9" - _call_bgp( - bgp_config, - connected_portchannels={"PortChannel1"}, - netbox=object(), - ) - entry = bgp_config["BGP_NEIGHBOR"]["default|192.0.2.9"] - assert "local_addr" not in entry - def test_non_default_vrf_no_v6only(self, bgp_config, patch_bgp): _call_bgp( bgp_config, @@ -1079,3 +1079,188 @@ def test_svi_peer_resolved_for_untagged_port_channel(self, bgp_config, patch_bgp # without the flag the resolver stops at the uncabled bundle and this # peering disappears again. assert patch_bgp.peer_ips.call_args.kwargs["resolve_lag_members"] is True + + +# --------------------------------------------------------------------------- +# The resolver/generator seam: BGP_NEIGHBOR and BGP_NEIGHBOR_AF must agree on +# how they name a peer. These tests deliberately do NOT use ``patch_bgp`` -- +# the resolver stays real, because whether it returns an address is what +# decides the branch under test. +# --------------------------------------------------------------------------- + + +class TestNeighborAndAfAgreeOnTheKey: + def _run(self, mocker, bgp_config, peer_ipv4, transfer_ipv4=None): + """Drive the real resolver for a cabled Ethernet0 with an optional peer IP.""" + local = SimpleNamespace( + id=1, + name="eth0", + type=SimpleNamespace(value="1000base-t"), + connected_endpoints_reachable=bool(peer_ipv4), + connected_endpoints=None, + lag=None, + ) + peer = SimpleNamespace(id=99, name="Ethernet1/1") + if peer_ipv4: + local.connected_endpoints = [peer] + + mocker.patch.object( + config_generator, + "get_connected_device_for_sonic_interface", + return_value=None, + ) + mocker.patch( + "osism.tasks.conductor.sonic.connections.get_cached_device_interfaces", + return_value=[local], + ) + mocker.patch( + "osism.tasks.conductor.sonic.connections.convert_netbox_interface_to_sonic", + side_effect=lambda iface, _device: "Ethernet0", + ) + netbox = SimpleNamespace( + dcim=SimpleNamespace( + interfaces=SimpleNamespace( + get=lambda **kw: local if kw.get("name") == "Ethernet0" else None + ) + ), + ipam=SimpleNamespace( + ip_addresses=SimpleNamespace( + filter=lambda **kw: ( + [SimpleNamespace(address=f"{peer_ipv4}/31")] + if peer_ipv4 and kw.get("assigned_object_id") == peer.id + else [] + ) + ), + fhrp_group_assignments=SimpleNamespace(filter=lambda **kw: []), + ), + ) + bgp_config["PORT"] = {"Ethernet0": {}} + _call_bgp( + bgp_config, + connected_interfaces={"Ethernet0"}, + netbox_interfaces={"Ethernet0": _nbif("eth0")}, + netbox=netbox, + transfer_ips=({"eth0": f"{transfer_ipv4}/31"} if transfer_ipv4 else {}), + ) + + @staticmethod + def _neighbors_of(bgp_config): + """The vrf|neighbor identity each AF row references.""" + return {"|".join(k.split("|")[:2]) for k in bgp_config["BGP_NEIGHBOR_AF"]} + + def test_numbered_peer_keys_both_tables_by_the_peer_address( + self, mocker, bgp_config, reset_vip_cache + ): + self._run( + mocker, + bgp_config, + peer_ipv4="192.0.2.1", + transfer_ipv4="192.0.2.0", + ) + assert "default|192.0.2.1" in bgp_config["BGP_NEIGHBOR"] + # Every address family must hang off a neighbor that exists. + assert self._neighbors_of(bgp_config) == set(bgp_config["BGP_NEIGHBOR"]) + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == {"default|192.0.2.1|ipv4_unicast"} + + def test_peer_ipv4_does_not_turn_an_unnumbered_session_into_a_numbered_one( + self, mocker, bgp_config, reset_vip_cache + ): + self._run(mocker, bgp_config, peer_ipv4="192.0.2.1") + assert set(bgp_config["BGP_NEIGHBOR"]) == {"default|Ethernet0"} + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == { + "default|Ethernet0|ipv4_unicast", + "default|Ethernet0|ipv6_unicast", + } + + def test_unnumbered_peer_keys_both_tables_by_the_interface( + self, mocker, bgp_config, reset_vip_cache + ): + self._run(mocker, bgp_config, peer_ipv4=None) + assert "default|Ethernet0" in bgp_config["BGP_NEIGHBOR"] + assert self._neighbors_of(bgp_config) == set(bgp_config["BGP_NEIGHBOR"]) + + def test_local_transfer_address_without_peer_address_stays_unnumbered( + self, mocker, bgp_config, reset_vip_cache + ): + self._run( + mocker, + bgp_config, + peer_ipv4=None, + transfer_ipv4="192.0.2.0", + ) + assert bgp_config["BGP_NEIGHBOR"] == { + "default|Ethernet0": { + "peer_type": "external", + "v6only": "true", + } + } + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == { + "default|Ethernet0|ipv4_unicast", + "default|Ethernet0|ipv6_unicast", + } + + +class TestPortChannelAddressFamiliesFollowPeeringMode: + def test_ipv4_transfer_peer_does_not_enable_ipv6_af(self, bgp_config, patch_bgp): + patch_bgp.peer_ipv4.return_value = "192.0.2.1" + _call_bgp( + bgp_config, + connected_portchannels={"PortChannel1"}, + netbox_interfaces={"PortChannel1": _nbif("bond1")}, + netbox=object(), + transfer_ips={"bond1": "192.0.2.0/31"}, + ) + + assert bgp_config["BGP_NEIGHBOR"] == { + "default|192.0.2.1": { + "local_addr": "192.0.2.0", + "peer_type": "external", + "v6only": "false", + } + } + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == {"default|192.0.2.1|ipv4_unicast"} + + def test_peer_ipv4_without_local_transfer_address_stays_unnumbered( + self, bgp_config, patch_bgp + ): + patch_bgp.peer_ipv4.return_value = "192.0.2.1" + _call_bgp( + bgp_config, + connected_portchannels={"PortChannel1"}, + netbox_interfaces={"PortChannel1": _nbif("bond1")}, + netbox=object(), + transfer_ips={}, + ) + + assert bgp_config["BGP_NEIGHBOR"] == { + "default|PortChannel1": { + "peer_type": "external", + "v6only": "true", + } + } + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == { + "default|PortChannel1|ipv4_unicast", + "default|PortChannel1|ipv6_unicast", + } + + def test_local_transfer_address_without_peer_address_stays_unnumbered( + self, bgp_config, patch_bgp + ): + _call_bgp( + bgp_config, + connected_portchannels={"PortChannel1"}, + netbox_interfaces={"PortChannel1": _nbif("bond1")}, + netbox=object(), + transfer_ips={"bond1": "192.0.2.0/31"}, + ) + + assert bgp_config["BGP_NEIGHBOR"] == { + "default|PortChannel1": { + "peer_type": "external", + "v6only": "true", + } + } + assert set(bgp_config["BGP_NEIGHBOR_AF"]) == { + "default|PortChannel1|ipv4_unicast", + "default|PortChannel1|ipv6_unicast", + }