Fix octavia lb deleting_race with cascading

Octavia can cascade resource deletion by passing &cascade=True as a
parameter to the HTTP request. This patch enables us to do that, fixing
the children deletion race and greatly simplifying the callstack of
service deletion

Closes-Bug: #1767074
Change-Id: I97303b54b900bc40f791309bd2589e617b61177a
Signed-off-by: Antoni Segura Puimedon <antonisp@celebdor.com>

kuryr_kubernetes/clients.py

@@ -52,6 +52,7 @@ def setup_loadbalancer_client():
     if any(ext['alias'] == 'lbaasv2' for
            ext in neutron_client.list_extensions()['extensions']):
         _clients[_LB_CLIENT] = neutron_client
+        neutron_client.cascading_capable = False
     else:
         # Since Octavia is lbaasv2 API compatible (A superset of it) we'll just
         # wire an extra neutron client instance to point to it
@@ -63,6 +64,7 @@ def setup_loadbalancer_client():
             service_type='load-balancer')
         lbaas_client.httpclient = octo_httpclient
         _clients[_LB_CLIENT] = lbaas_client
+        lbaas_client.cascading_capable = True
 
 
 def setup_kubernetes_client():

kuryr_kubernetes/controller/drivers/lbaasv2.py

@@ -40,6 +40,11 @@ _SUPPORTED_LISTENER_PROT = ('HTTP', 'HTTPS', 'TCP')
 class LBaaSv2Driver(base.LBaaSDriver):
     """LBaaSv2Driver implements LBaaSDriver for Neutron LBaaSv2 API."""
 
+    @property
+    def cascading_capable(self):
+        lbaas = clients.get_loadbalancer_client()
+        return lbaas.cascading_capable
+
     def ensure_loadbalancer(self, endpoints, project_id, subnet_id, ip,
                             security_groups_ids, service_type):
         name = "%(namespace)s/%(name)s" % endpoints['metadata']
@@ -67,16 +72,25 @@ class LBaaSv2Driver(base.LBaaSDriver):
     def release_loadbalancer(self, endpoints, loadbalancer):
         neutron = clients.get_neutron_client()
         lbaas = clients.get_loadbalancer_client()
-        self._release(loadbalancer, loadbalancer,
-                      lbaas.delete_loadbalancer, loadbalancer.id)
+        if lbaas.cascading_capable:
+            self._release(
+                loadbalancer,
+                loadbalancer,
+                lbaas.delete,
+                lbaas.lbaas_loadbalancer_path % loadbalancer.id,
+                params={'cascade': True})
 
-        sg_id = self._find_listeners_sg(loadbalancer)
-        if sg_id:
-            try:
-                neutron.delete_security_group(sg_id)
-            except n_exc.NeutronClientException:
-                LOG.exception('Error when deleting loadbalancer security '
-                              'group. Leaving it orphaned.')
+        else:
+            self._release(loadbalancer, loadbalancer,
+                          lbaas.delete_loadbalancer, loadbalancer.id)
+
+            sg_id = self._find_listeners_sg(loadbalancer)
+            if sg_id:
+                try:
+                    neutron.delete_security_group(sg_id)
+                except n_exc.NeutronClientException:
+                    LOG.exception('Error when deleting loadbalancer security '
+                                  'group. Leaving it orphaned.')
 
     def ensure_security_groups(self, endpoints, loadbalancer,
                                security_groups_ids, service_type):

kuryr_kubernetes/controller/handlers/lbaas.py

@@ -269,9 +269,17 @@ class LoadBalancerHandler(k8s_base.ResourceEventHandler):
         if not lbaas_state:
             return
         # NOTE(ivc): deleting pool deletes its members
-        lbaas_state.members = []
-        self._sync_lbaas_members(endpoints, lbaas_state,
-                                 obj_lbaas.LBaaSServiceSpec())
+        if self._drv_lbaas.cascading_capable:
+            self._drv_lbaas.release_loadbalancer(
+                endpoints=endpoints,
+                loadbalancer=lbaas_state.loadbalancer)
+            if lbaas_state.service_pub_ip_info:
+                self._drv_service_pub_ip.release_pub_ip(
+                    lbaas_state.service_pub_ip_info)
+        else:
+            lbaas_state.members = []
+            self._sync_lbaas_members(endpoints, lbaas_state,
+                                     obj_lbaas.LBaaSServiceSpec())
 
     def _should_ignore(self, endpoints, lbaas_spec):
         return not(lbaas_spec and

kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py

@@ -75,6 +75,7 @@ class TestLBaaSv2Driver(test_base.TestCase):
     def test_release_loadbalancer(self):
         self.useFixture(k_fix.MockNeutronClient()).client
         lbaas = self.useFixture(k_fix.MockLBaaSClient()).client
+        lbaas.cascading_capable = False
         cls = d_lbaasv2.LBaaSv2Driver
         m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
         endpoints = mock.sentinel.endpoints
@@ -86,6 +87,23 @@ class TestLBaaSv2Driver(test_base.TestCase):
                                                   lbaas.delete_loadbalancer,
                                                   loadbalancer.id)
 
+    def test_cascade_release_loadbalancer(self):
+        self.useFixture(k_fix.MockNeutronClient()).client
+        lbaas = self.useFixture(k_fix.MockLBaaSClient()).client
+        lbaas.cascading_capable = True
+        lbaas.lbaas_loadbalancer_path = "boo %s"
+        cls = d_lbaasv2.LBaaSv2Driver
+        m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)
+        endpoints = mock.sentinel.endpoints
+        loadbalancer = mock.Mock()
+
+        cls.release_loadbalancer(m_driver, endpoints, loadbalancer)
+
+        m_driver._release.assert_called_once_with(
+            loadbalancer, loadbalancer, lbaas.delete,
+            lbaas.lbaas_loadbalancer_path % loadbalancer.id,
+            params={'cascade': True})
+
     def test_ensure_listener(self):
         cls = d_lbaasv2.LBaaSv2Driver
         m_driver = mock.Mock(spec=d_lbaasv2.LBaaSv2Driver)

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

@@ -516,6 +516,8 @@ class TestLoadBalancerHandler(test_base.TestCase):
 
         m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
         m_handler._get_lbaas_state.return_value = lbaas_state
+        m_handler._drv_lbaas = mock.Mock()
+        m_handler._drv_lbaas.cascading_capable = False
 
         h_lbaas.LoadBalancerHandler.on_deleted(m_handler, endpoints)
 
@@ -523,6 +525,29 @@ class TestLoadBalancerHandler(test_base.TestCase):
         m_handler._sync_lbaas_members.assert_called_once_with(
             endpoints, lbaas_state, empty_spec)
 
+    @mock.patch('kuryr_kubernetes.objects.lbaas'
+                '.LBaaSServiceSpec')
+    def test_on_cascade_deleted_lb_service(self, m_svc_spec_ctor):
+        endpoints = mock.sentinel.endpoints
+        empty_spec = mock.sentinel.empty_spec
+        lbaas_state = mock.Mock()
+        lbaas_state.loadbalancer = mock.sentinel.loadbalancer
+        lbaas_state.service_pub_ip_info = mock.sentinel.pub_ip
+        m_svc_spec_ctor.return_value = empty_spec
+
+        m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
+        m_handler._get_lbaas_state.return_value = lbaas_state
+        m_handler._drv_lbaas = mock.Mock()
+        m_handler._drv_service_pub_ip = mock.Mock()
+        m_handler._drv_lbaas.cascading_capable = True
+
+        h_lbaas.LoadBalancerHandler.on_deleted(m_handler, endpoints)
+
+        m_handler._drv_lbaas.release_loadbalancer.assert_called_once_with(
+            endpoints=endpoints, loadbalancer=lbaas_state.loadbalancer)
+        m_handler._drv_service_pub_ip.release_pub_ip.assert_called_once_with(
+            lbaas_state.service_pub_ip_info)
+
     def test_should_ignore(self):
         endpoints = mock.sentinel.endpoints
         lbaas_spec = mock.sentinel.lbaas_spec