Merge pull request #2150 from rackerlabs/backport-ovn-ha-chassis-fix

fix(neutron-understack): ignore stale chassis when linking vxlan network HCG

Author: cardoe

python/neutron-understack/neutron_understack/routers.py

@@ -220,8 +220,11 @@ def link_vxlan_network_ha_chassis_group(_resource, _event, _trigger, payload) ->
     We do what link_network_ha_chassis_group would have done: populate the unified
     network HCG with sync_ha_chassis_group_network_unified, then anchor the internal
     router-interface LRP to that same HCG. The gateway chassis is sourced from the
-    global HA_Chassis table (all records must share one chassis_name) so the fix
-    fires even before the external gateway port is attached.
+    global HA_Chassis table (all *live* records must share one chassis_name) so the
+    fix fires even before the external gateway port is attached. Rows pointing at a
+    chassis no longer present in the Southbound DB (e.g. left behind by a
+    decommissioned/replaced host) are excluded before checking for uniqueness, so a
+    single stale row elsewhere in the fleet doesn't block every vxlan network.
 
     For VLAN/FLAT networks neutron's handler already populates the network HCG
     correctly; we detect that and return early.
@@ -239,6 +242,7 @@ def link_vxlan_network_ha_chassis_group(_resource, _event, _trigger, payload) ->
         if not client:
             return
         nb_idl = client._nb_idl
+        sb_idl = client._sb_idl
 
         # Skip if the per-network HCG is already populated — neutron handled it
         # (VLAN/FLAT gateways).  For vxlan the HCG is empty due to the neutron bug.
@@ -248,15 +252,22 @@ def link_vxlan_network_ha_chassis_group(_resource, _event, _trigger, payload) ->
         if network_hcg and network_hcg.ha_chassis:
             return
 
-        # Derive the gateway chassis from every HA_Chassis row in the NB database.
-        # If exactly one distinct chassis_name exists, that is our gateway chassis.
-        # This avoids requiring the per-router HCG to exist first.
+        # Derive the gateway chassis from every *live* HA_Chassis row in the NB
+        # database. If exactly one distinct chassis_name exists, that is our
+        # gateway chassis. This avoids requiring the per-router HCG to exist yet.
+        # Rows referencing a chassis no longer registered in the Southbound DB
+        # (e.g. a decommissioned/replaced host) are excluded so they don't block
+        # this inference for every network in the fleet.
         all_ha_chassis = nb_idl.db_list_rows("HA_Chassis").execute(check_error=True)
-        chassis_names = {row.chassis_name for row in all_ha_chassis}
+        chassis_names = {
+            row.chassis_name
+            for row in all_ha_chassis
+            if sb_idl.lookup("Chassis", row.chassis_name, default=None) is not None
+        }
         if len(chassis_names) != 1:
             LOG.debug(
                 "Cannot determine unique gateway chassis for network %(net)s "
-                "(router %(router)s): found %(n)d distinct chassis name(s)",
+                "(router %(router)s): found %(n)d distinct live chassis name(s)",
                 {"net": network_id, "router": router_id, "n": len(chassis_names)},
             )
             return

python/neutron-understack/neutron_understack/tests/test_routers.py

@@ -407,7 +407,7 @@ def _payload(mocker, router_id="router-1", port_id="port-1", network_id="net-1")
         )
 
     @staticmethod
-    def _client(mocker, ha_chassis_rows, lrp, network_hcg=None):
+    def _client(mocker, ha_chassis_rows, lrp, network_hcg=None, live_chassis=None):
         nb_idl = mocker.MagicMock()
         nb_idl.db_list_rows.return_value.execute.return_value = ha_chassis_rows
 
@@ -419,7 +419,22 @@ def lookup(table, _name, default=None):
             return default
 
         nb_idl.lookup.side_effect = lookup
-        client = mocker.Mock(_nb_idl=nb_idl)
+
+        # By default every provided HA_Chassis row is treated as live, so
+        # existing callers don't need to know about the SB liveness check.
+        if live_chassis is None:
+            live_chassis = {row.chassis_name for row in ha_chassis_rows}
+
+        sb_idl = mocker.MagicMock()
+
+        def sb_lookup(table, name, default=None):
+            if table == "Chassis" and name in live_chassis:
+                return mocker.Mock(name=name)
+            return default
+
+        sb_idl.lookup.side_effect = sb_lookup
+
+        client = mocker.Mock(_nb_idl=nb_idl, _sb_idl=sb_idl)
         return client, nb_idl
 
     def _patch_sync(self, mocker, hcg="net-hcg-uuid"):
@@ -474,6 +489,32 @@ def test_multiple_chassis_names(self, mocker):
         sync.assert_not_called()
         nb_idl.db_set.assert_not_called()
 
+    def test_stale_chassis_excluded_from_uniqueness_check(self, mocker):
+        # chassis-2 no longer exists in the Southbound DB (e.g. decommissioned
+        # host) but a stale HA_Chassis row for it still lingers in NB. It must
+        # not block inference of the one live chassis.
+        hc_live = mocker.Mock(chassis_name="chassis-1")
+        hc_dead = mocker.Mock(chassis_name="chassis-2")
+        lrp = mocker.Mock(ha_chassis_group=[])
+        client, nb_idl = self._client(
+            mocker,
+            ha_chassis_rows=[hc_live, hc_dead],
+            lrp=lrp,
+            live_chassis={"chassis-1"},
+        )
+        sync = self._patch_sync(mocker)
+        mocker.patch("neutron_understack.routers.ovn_client", return_value=client)
+
+        link_vxlan_network_ha_chassis_group(None, None, None, self._payload(mocker))
+
+        sync.assert_called_once()
+        assert sync.call_args.args[5] == {"chassis-1": 32767}  # chassis_prio
+        nb_idl.db_set.assert_called_once_with(
+            "Logical_Router_Port",
+            "lrp-port-1",
+            ("ha_chassis_group", "net-hcg-uuid"),
+        )
+
     def test_network_hcg_already_populated(self, mocker):
         # VLAN/FLAT: neutron already populated the per-network HCG — we skip.
         existing_hcg = mocker.Mock(ha_chassis=[mocker.Mock(chassis_name="chassis-1")])