northd: Fix port group I-P for recreated groups.

When northd handles in a single iteration the following transactions:
A. port group deletion (e.g., name="pg")
B. port group addition (same name as the deleted group, i.e., "pg" but
   with a new UUID) with a logical switch port.

it should correctly reconcile the SB contents, the resulting SB port
group contents should include the "recreated" group too.

That wasn't the case because the deleted name was added to the
stale_sb_port_groups set and not removed from it when processing the new
group record.

Also, make sure we first process all deletions and then all updates (and
additions) in order to avoid pruning issues.

Fixes: a1d82e9 ("northd: Process port_group changes incrementally.")
Reported-at: https://redhat.atlassian.net/browse/FDP-3966
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Author: dceara

northd/en-port-group.c

@@ -558,15 +558,34 @@ port_group_nb_port_group_handler(struct engine_node *node, void *data_)
         HMAPX_INITIALIZER(&updated_ls_port_groups);
 
     struct sset stale_sb_port_groups = SSET_INITIALIZER(&stale_sb_port_groups);
+    /* Process all deletions first. */
     NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_pg_table) {
+        if (!nbrec_port_group_is_deleted(nb_pg)) {
+            continue;
+        }
+
         ls_port_group_process(&data->ls_port_groups,
                               &data->port_groups_lses,
                               &data->ls_port_groups_sets_changed,
                               input_data.ls_ports,
                               nb_pg, &updated_ls_port_groups,
                               &stale_sb_port_groups);
+    }
+
+    /* Then process all additions/updates. */
+    NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_pg_table) {
+        if (nbrec_port_group_is_deleted(nb_pg)) {
+            continue;
         }
 
+        ls_port_group_process(&data->ls_port_groups,
+                              &data->port_groups_lses,
+                              &data->ls_port_groups_sets_changed,
+                              input_data.ls_ports,
+                              nb_pg, &updated_ls_port_groups,
+                              &stale_sb_port_groups);
+    }
+
     /* Changes have been successfully processed incrementally now update
      * the SB too. */
     struct ovsdb_idl_index *sbrec_port_group_by_name =
@@ -598,6 +617,7 @@ port_group_nb_port_group_handler(struct engine_node *node, void *data_)
                 sorted_array_from_sset(&ls_pg_rec->ports);
             update_sb_port_group(&nb_ports, sb_pg);
             sorted_array_destroy(&nb_ports);
+            sset_find_and_delete(&stale_sb_port_groups, sb_pg_name_cstr);
         }
     }
     ds_destroy(&sb_pg_name);

tests/ovn-northd.at

@@ -11023,6 +11023,35 @@ OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Port Group recreation incremental processing])
+AT_KEYWORDS([incremental processing])
+ovn_start
+
+check ovn-nbctl ls-add ls -- lsp-add ls lsp
+check ovn-nbctl --wait=sb pg-add pg lsp
+
+ls_key=$(fetch_column Datapath_Binding tunnel_key external_ids:name=ls)
+check_column "lsp" sb:Port_Group ports name="${ls_key}_pg"
+
+dnl Simulate northd handling a batch of transactions that recreate the
+dnl port group.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+sleep_northd
+check ovn-nbctl pg-del pg
+check ovn-nbctl pg-add pg lsp
+wake_up_northd
+
+check ovn-nbctl --wait=sb sync
+check_engine_stats northd norecompute compute
+check_engine_stats port_group norecompute compute
+check_engine_stats lflow norecompute compute
+check_column "lsp" sb:Port_Group ports name="${ls_key}_pg"
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Check default drop])
 AT_KEYWORDS([drop])