Merge pull request #406 from luis5tb/bg-1898906

Bug 1898906: Ensure members are deleted from pools when there is no endpoints

kuryr_kubernetes/controller/handlers/lbaas.py

@@ -238,10 +238,12 @@ def _should_ignore(self, endpoints, lbaas_spec):
         # NOTE(ltomasbo): we must wait until service handler has annotated the
         # endpoints to process them. Thus, if annotations are not updated to
         # match the endpoints information, we should skip the event
+        lbaas_state = utils.get_lbaas_state(endpoints)
         return not(lbaas_spec and
-                   self._has_pods(endpoints) and
-                   self._svc_handler_annotations_updated(endpoints,
-                                                         lbaas_spec))
+                   ((self._has_pods(endpoints) and
+                     self._svc_handler_annotations_updated(endpoints,
+                                                           lbaas_spec)) or
+                    (not self._has_pods(endpoints) and lbaas_state)))
 
     def _svc_handler_annotations_updated(self, endpoints, lbaas_spec):
         svc_link = self._get_service_link(endpoints)
@@ -267,9 +269,7 @@ def _has_pods(self, endpoints):
     def _sync_lbaas_members(self, endpoints, lbaas_state, lbaas_spec):
         changed = False
 
-        if (self._has_pods(endpoints) and
-                self._remove_unused_members(endpoints, lbaas_state,
-                                            lbaas_spec)):
+        if (self._remove_unused_members(endpoints, lbaas_state, lbaas_spec)):
             changed = True
 
         if self._sync_lbaas_pools(endpoints, lbaas_state, lbaas_spec):
@@ -430,12 +430,14 @@ def _remove_unused_members(self, endpoints, lbaas_state, lbaas_spec):
             if port:
                 spec_ports[port.name] = pool.id
 
-        current_targets = {(a['ip'], p['port'],
-                            spec_ports.get(p.get('name')))
-                           for s in endpoints['subsets']
-                           for a in s['addresses']
-                           for p in s['ports']
-                           if p.get('name') in spec_ports}
+        current_targets = set()
+        if endpoints.get('subsets'):
+            current_targets = {(a['ip'], p['port'],
+                                spec_ports.get(p.get('name')))
+                               for s in endpoints['subsets']
+                               for a in s['addresses']
+                               for p in s['ports']
+                               if p.get('name') in spec_ports}
 
         removed_ids = set()
         for member in lbaas_state.members:

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

@@ -599,9 +599,12 @@ def test_on_cascade_deleted_lb_service(self, m_svc_spec_ctor,
         m_handler._drv_service_pub_ip.release_pub_ip.assert_called_once_with(
             lbaas_state.service_pub_ip_info)
 
-    def test_should_ignore(self):
+    @mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
+    def test_should_ignore(self, m_get_lbaas_state):
         endpoints = mock.sentinel.endpoints
         lbaas_spec = mock.sentinel.lbaas_spec
+        lbaas_state = mock.sentinel.lbaas_state
+        m_get_lbaas_state.return_value = lbaas_state
 
         # REVISIT(ivc): ddt?
         m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
@@ -612,10 +615,28 @@ def test_should_ignore(self):
             m_handler, endpoints, lbaas_spec)
         self.assertEqual(False, ret)
 
-        m_handler._has_pods.assert_called_once_with(endpoints)
+        m_handler._has_pods.assert_called()
         m_handler._svc_handler_annotations_updated.assert_called_once_with(
             endpoints, lbaas_spec)
 
+    @mock.patch('kuryr_kubernetes.utils.get_lbaas_state')
+    def test_should_ignore_member_scale_to_0(self, m_get_lbaas_state):
+        endpoints = mock.sentinel.endpoints
+        lbaas_spec = mock.sentinel.lbaas_spec
+        lbaas_state = mock.sentinel.lbaas_state
+        m_get_lbaas_state.return_value = lbaas_state
+
+        # REVISIT(ivc): ddt?
+        m_handler = mock.Mock(spec=h_lbaas.LoadBalancerHandler)
+        m_handler._has_pods.return_value = False
+        m_handler._svc_handler_annotations_updated.return_value = False
+
+        ret = h_lbaas.LoadBalancerHandler._should_ignore(
+            m_handler, endpoints, lbaas_spec)
+        self.assertEqual(False, ret)
+
+        m_handler._has_pods.assert_called()
+
     def test_has_pods(self):
         # REVISIT(ivc): ddt?
         endpoints = {'subsets': [