From f9c7a63f7aa203f6b0b480773dac9f2c47d9e0bf Mon Sep 17 00:00:00 2001
From: Thomas Bachman <tbachman@yahoo.com>
Date: Wed, 22 Mar 2023 11:15:21 +0000
Subject: [PATCH] Fix port notifications when extension is updated

The patch in [0] added support for the no-NAT CIDRs extension. This
covered the case where the agents would get extension details when a
network was created, as well as when a network was connected or
disconnected from a neutron router. However, it missed the case where
the extension on the ntwork itself was updated. This patch addresses
that gap.

The patch also adds UT coverage of the extension for AIM validation
(there is no mapping to an AIM resource, but the extension was added
to the UT for completeness).

[0]: https://review.opendev.org/c/x/group-based-policy/+/875317

Change-Id: Ibf3df8a0d48b9ba9a68c17ad70251a611aa40cab
---
 .../drivers/apic_aim/mechanism_driver.py      | 15 ++++++++++++
 .../unit/plugins/ml2plus/test_apic_aim.py     | 23 +++++++++++++++++++
 .../grouppolicy/test_aim_validation.py        |  1 +
 3 files changed, 39 insertions(+)

diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py
index d5124f8c4..28d280ae3 100644
--- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py
+++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py
@@ -1060,6 +1060,18 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
             self.aim.update(
                 aim_ctx, epg, policy_enforcement_pref=curr_masters)
 
+        # Update no-NAT CIDRs if changed.
+        curr_cidrs = current[cisco_apic.NO_NAT_CIDRS]
+        orig_cidrs = original[cisco_apic.NO_NAT_CIDRS]
+        ports_to_notify = set()
+        if curr_cidrs != orig_cidrs:
+            port_ids = self._get_non_router_ports_in_networks(
+                session, [current['id']])
+            ports_to_notify.update(port_ids)
+        if ports_to_notify:
+            self._add_postcommit_port_notifications(context._plugin_context,
+                                                    ports_to_notify)
+
         if is_ext:
             _, ext_net, ns = self._get_aim_nat_strategy(current)
             if ext_net:
@@ -1181,6 +1193,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
                     self.aim.update(aim_ctx, l3out_ext_subnet_v6, scope=scope,
                                     aggregate=aggregate)
 
+    def update_network_postcommit(self, context):
+        self._send_postcommit_notifications(context._plugin_context)
+
     def delete_network_precommit(self, context):
         current = context.current
         LOG.debug("APIC AIM MD deleting network: %s", current)
diff --git a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py
index 5bfef5404..159971fe1 100644
--- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py
+++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py
@@ -12863,6 +12863,29 @@ class TestExtensionNoNatCidrsScenarios(TestOpflexRpc, ApicAimTestCase):
         self.assertEqual(set(['10.0.1.0/24', '10.10.10.0/24']),
             set(response['vrf_subnets']))
 
+        with mock.patch.object(
+                self.driver.notifier, 'port_update',
+                autospec=True) as notifier:
+            # Test update with no nat cidrs extension
+            net = self._update(
+                'networks', net_id,
+                {'network':
+                 {'apic:no_nat_cidrs': ['20.20.20.0/24']}})['network']
+            self.assertItemsEqual(['20.20.20.0/24'],
+                net['apic:no_nat_cidrs'])
+
+            # The mechanism driver adds the bound drivers when
+            # it makes the call for the notification, so add those in.
+            port['binding:vif_details']['bound_drivers'] = {
+                '0': 'apic_aim', '1': 'apic_aim'}
+            notifier.assert_called_once_with(mock.ANY, port)
+        # Verify that the new CIDR is added, and the old one is
+        # deleted.
+        response = self.driver.request_endpoint_details(
+            n_context.get_admin_context(), request=request, host=host)
+        self.assertEqual(set(['10.0.1.0/24', '20.20.20.0/24']),
+            set(response['gbp_details']['vrf_subnets']))
+
     def test_no_nat_cidrs_external_network(self):
         # test no nat cidrs extension for external network
         ext_net = self._make_ext_network('ext-net1',
diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
index 772d2b73e..83f5eaa06 100644
--- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
+++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
@@ -474,6 +474,7 @@ class TestNeutronMapping(AimValidationTestCase):
                                                  'name': 'ec3'},
                                                 {'app_profile_name': 'ap2',
                                                  'name': 'ec4'}],
+                  'apic:no_nat_cidrs': ['10.10.10.0/24', '20.20.20.0/24'],
                   'apic:policy_enforcement_pref': 'unenforced'}
         if preexisting_bd:
             kwargs.update(