Merge "Ensure ports from pool do not reference deleted SGs/NPs"

kuryr_kubernetes/controller/drivers/base.py

@@ -741,6 +741,21 @@ class VIFPoolDriver(PodVIFDriver):
         """
         raise NotImplementedError()
 
+    @abc.abstractmethod
+    def remove_sg_from_pools(self, sg_id, net_id):
+        """Remove the SG from the ports associated to the pools.
+
+        This method ensure that ports on net_id that belongs to pools and have
+        the referenced SG are updated to clean up their SGs and put back on
+        the default pool for that network.
+
+        :param sg_id: Security Group ID that needs to be removed from pool
+                      ports
+        :param net_id: Network ID associated to the pools to clean up, and
+                       where the ports must belong to.
+        """
+        raise NotImplementedError()
+
 
 @six.add_metaclass(abc.ABCMeta)
 class ServicePubIpDriver(DriverBase):

kuryr_kubernetes/controller/drivers/vif_pool.py

@@ -122,6 +122,9 @@ class NoopVIFPool(base.VIFPoolDriver):
     def update_vif_sgs(self, pod, sgs):
         self._drv_vif.update_vif_sgs(pod, sgs)
 
+    def remove_sg_from_pools(self, sg_id, net_id):
+        pass
+
     def sync_pools(self):
         pass
 
@@ -288,6 +291,33 @@ class BaseVIFPool(base.VIFPoolDriver):
     def delete_network_pools(self, net_id):
         raise NotImplementedError()
 
+    def remove_sg_from_pools(self, sg_id, net_id):
+        neutron = clients.get_neutron_client()
+        for pool_key, pool_ports in self._available_ports_pools.items():
+            if self._get_pool_key_net(pool_key) != net_id:
+                continue
+            for sg_key, ports in pool_ports.items():
+                if sg_id not in sg_key:
+                    continue
+                # remove the pool associated to that SG
+                del self._available_ports_pools[pool_key][sg_key]
+                for port_id in ports:
+                    # remove all SGs from the port to be reused
+                    neutron.update_port(
+                        port_id,
+                        {
+                            "port": {
+                                'security_groups': []
+                            }
+                        })
+                    # add the port to the default pool
+                    self._available_ports_pools[pool_key].setdefault(
+                        tuple([]), []).append(port_id)
+                # NOTE(ltomasbo): as this ports were not created for this
+                # pool, ensuring they are used first, marking them as the
+                # most outdated
+                self._last_update[pool_key] = {tuple([]): 0}
+
     def _create_healthcheck_file(self):
         # Note(ltomasbo): Create a health check file when the pre-created
         # ports are loaded into their corresponding pools. This file is used
@@ -1025,6 +1055,12 @@ class MultiVIFPool(base.VIFPoolDriver):
         pod_vif_type = self._get_pod_vif_type(pod)
         self._vif_drvs[pod_vif_type].update_vif_sgs(pod, sgs)
 
+    def remove_sg_from_pools(self, sg_id, net_id):
+        for vif_drv in self._vif_drvs.values():
+            if str(vif_drv) == 'NoopVIFPool':
+                continue
+            vif_drv.remove_sg_from_pools(sg_id, net_id)
+
     def delete_network_pools(self, net_id):
         for vif_drv in self._vif_drvs.values():
             if str(vif_drv) == 'NoopVIFPool':

kuryr_kubernetes/controller/handlers/policy.py

@@ -20,6 +20,7 @@ from kuryr_kubernetes import clients
 from kuryr_kubernetes import constants as k_const
 from kuryr_kubernetes.controller.drivers import base as drivers
 from kuryr_kubernetes.controller.drivers import utils as driver_utils
+from kuryr_kubernetes import exceptions
 from kuryr_kubernetes.handlers import k8s_base
 from kuryr_kubernetes import utils
 
@@ -116,6 +117,10 @@ class NetworkPolicyHandler(k8s_base.ResourceEventHandler):
                             oslo_cfg.OptGroup('neutron_defaults'))
                 self._drv_vif_pool.update_vif_sgs(pod, pod_sgs)
 
+            # ensure ports at the pool don't have the NP sg associated
+            net_id = self._get_policy_net_id(policy)
+            self._drv_vif_pool.remove_sg_from_pools(crd_sg, net_id)
+
             self._drv_policy.release_network_policy(netpolicy_crd)
 
             if oslo_cfg.CONF.octavia_defaults.enforce_sg_rules:
@@ -149,3 +154,16 @@ class NetworkPolicyHandler(k8s_base.ResourceEventHandler):
         svc_pods = driver_utils.get_pods({'selector': svc_selector},
                                          svc_namespace).get('items')
         return any(pod in svc_pods for pod in affected_pods)
+
+    def _get_policy_net_id(self, policy):
+        policy_ns = policy['metadata']['namespace']
+        kuryrnet_name = 'ns-' + str(policy_ns)
+
+        kubernetes = clients.get_kubernetes_client()
+        try:
+            net_crd = kubernetes.get('{}/{}'.format(
+                k_const.K8S_API_CRD_KURYRNETS, kuryrnet_name))
+        except exceptions.K8sClientException:
+            LOG.exception("Kubernetes Client Exception.")
+            raise
+        return net_crd['spec']['netId']

kuryr_kubernetes/tests/unit/controller/handlers/test_policy.py

@@ -78,6 +78,8 @@ class TestPolicyHandler(test_base.TestCase):
         self._update_vif_sgs.return_value = None
         self._update_lbaas_sg = self._handler._drv_lbaas.update_lbaas_sg
         self._update_lbaas_sg.return_value = None
+        self._remove_sg = self._handler._drv_vif_pool.remove_sg_from_pools
+        self._remove_sg.return_value = None
 
     def _get_knp_obj(self):
         knp_obj = {
@@ -243,3 +245,4 @@ class TestPolicyHandler(test_base.TestCase):
                                                           self._project_id)
         self._update_vif_sgs.assert_called_once_with(match_pod, sg1)
         self._update_lbaas_sg.assert_not_called()
+        self._remove_sg.assert_called_once()