Merge "Fix batch member update error on empty change list"

doc/source/contributor/guides/providers.rst

@@ -1111,9 +1111,10 @@ and members in the list that do not already exist should be created.
           """
           raise NotImplementedError()
 
-      def member_batch_update(self, members):
+      def member_batch_update(self, pool_id, members):
           """Creates, updates, or deletes a set of pool members.
 
+          :param pool_id (string): The id of the pool to update.
           :param members (list): List of member objects.
           :return: Nothing if the create request was accepted.
           :raises DriverError: An unexpected error occurred in the driver.

octavia/api/drivers/amphora_driver/v1/driver.py

@@ -201,9 +201,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
                    consts.MEMBER_UPDATES: member_dict}
         self.client.cast({}, 'update_member', **payload)
 
-    def member_batch_update(self, members):
+    def member_batch_update(self, pool_id, members):
-        # Get a list of existing members
-        pool_id = members[0].pool_id
         # The DB should not have updated yet, so we can still use the pool
         db_pool = self.repositories.pool.get(db_apis.get_session(), id=pool_id)
 
@@ -236,10 +234,13 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
             if m.id not in new_member_ids:
                 deleted_members.append(m)
 
+        if deleted_members or new_members or updated_members:
             payload = {'old_member_ids': [m.id for m in deleted_members],
                        'new_member_ids': [m.member_id for m in new_members],
                        'updated_members': updated_members}
             self.client.cast({}, 'batch_update_members', **payload)
+        else:
+            LOG.info("Member batch update is a noop, returning early.")
 
     def _validate_members(self, db_pool, members):
         if db_pool.protocol == consts.PROTOCOL_UDP:

octavia/api/drivers/amphora_driver/v2/driver.py

@@ -200,9 +200,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
                    consts.MEMBER_UPDATES: member_dict}
         self.client.cast({}, 'update_member', **payload)
 
-    def member_batch_update(self, members):
+    def member_batch_update(self, pool_id, members):
-        # Get a list of existing members
-        pool_id = members[0].pool_id
         # The DB should not have updated yet, so we can still use the pool
         db_pool = self.repositories.pool.get(db_apis.get_session(), id=pool_id)
 
@@ -235,10 +233,13 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
             if m.id not in new_member_ids:
                 deleted_members.append(m)
 
+        if deleted_members or new_members or updated_members:
             payload = {'old_member_ids': [m.id for m in deleted_members],
                        'new_member_ids': [m.member_id for m in new_members],
                        'updated_members': updated_members}
             self.client.cast({}, 'batch_update_members', **payload)
+        else:
+            LOG.info("Member batch update is a noop, returning early.")
 
     def _validate_members(self, db_pool, members):
         if db_pool.protocol == consts.PROTOCOL_UDP:

octavia/api/drivers/noop_driver/driver.py

@@ -148,10 +148,11 @@ class NoopManager(object):
         self.driverconfig[new_member.member_id] = (
             new_member, 'member_update')
 
-    def member_batch_update(self, members):
+    def member_batch_update(self, pool_id, members):
         for member in members:
-            LOG.debug('Provider %s no-op, member_batch_update member %s',
+            LOG.debug('Provider %s no-op, member_batch_update pool_id %s '
-                      self.__class__.__name__, member.member_id)
+                      'member %s',
+                      self.__class__.__name__, pool_id, member.member_id)
 
             self.driverconfig[member.member_id] = (member,
                                                    'member_batch_update')
@@ -294,8 +295,8 @@ class NoopProviderDriver(driver_base.ProviderDriver):
     def member_update(self, old_member, new_member):
         self.driver.member_update(old_member, new_member)
 
-    def member_batch_update(self, members):
+    def member_batch_update(self, pool_id, members):
-        self.driver.member_batch_update(members)
+        self.driver.member_batch_update(pool_id, members)
 
     # Health Monitor
     def health_monitor_create(self, healthmonitor):

octavia/api/v2/controllers/member.py

@@ -428,4 +428,5 @@ class MembersController(MemberController):
             LOG.info("Sending Pool %s batch member update to provider %s",
                      db_pool.id, driver.name)
             driver_utils.call_provider(
-                driver.name, driver.member_batch_update, provider_members)
+                driver.name, driver.member_batch_update, db_pool.id,
+                provider_members)

octavia/tests/functional/api/v2/test_member.py

@@ -643,7 +643,7 @@ class TestMember(base.BaseAPITest):
 
         mock_provider.assert_called_once_with(u'noop_driver',
                                               mock_driver.member_batch_update,
-                                              provider_creates)
+                                              self.pool_id, provider_creates)
 
     @mock.patch('octavia.api.drivers.driver_factory.get_driver')
     @mock.patch('octavia.api.drivers.utils.call_provider')
@@ -686,7 +686,7 @@ class TestMember(base.BaseAPITest):
 
         mock_provider.assert_called_once_with(u'noop_driver',
                                               mock_driver.member_batch_update,
-                                              provider_members)
+                                              self.pool_id, provider_members)
 
     def test_create_batch_members_with_bad_subnet(self):
         subnet_id = uuidutils.generate_uuid()
@@ -788,7 +788,7 @@ class TestMember(base.BaseAPITest):
 
         mock_provider.assert_called_once_with(u'noop_driver',
                                               mock_driver.member_batch_update,
-                                              provider_creates)
+                                              self.pool_id, provider_creates)
 
     @mock.patch('octavia.api.drivers.driver_factory.get_driver')
     @mock.patch('octavia.api.drivers.utils.call_provider')
@@ -839,7 +839,7 @@ class TestMember(base.BaseAPITest):
 
         mock_provider.assert_called_once_with(u'noop_driver',
                                               mock_driver.member_batch_update,
-                                              provider_members)
+                                              self.pool_id, provider_members)
 
     @mock.patch('octavia.api.drivers.driver_factory.get_driver')
     @mock.patch('octavia.api.drivers.utils.call_provider')
@@ -881,7 +881,29 @@ class TestMember(base.BaseAPITest):
 
         mock_provider.assert_called_once_with(u'noop_driver',
                                               mock_driver.member_batch_update,
-                                              provider_members)
+                                              self.pool_id, provider_members)
+
+    @mock.patch('octavia.api.drivers.driver_factory.get_driver')
+    @mock.patch('octavia.api.drivers.utils.call_provider')
+    def test_delete_batch_members_already_empty(self, mock_provider,
+                                                mock_get_driver):
+        mock_driver = mock.MagicMock()
+        mock_driver.name = 'noop_driver'
+        mock_get_driver.return_value = mock_driver
+
+        req_dict = []
+        body = {self.root_tag_list: req_dict}
+        path = self.MEMBERS_PATH.format(pool_id=self.pool_id)
+        self.put(path, body, status=202)
+        returned_members = self.get(
+            self.MEMBERS_PATH.format(pool_id=self.pool_id)
+        ).json.get(self.root_tag_list)
+
+        self.assertEqual([], returned_members)
+
+        mock_provider.assert_called_once_with(u'noop_driver',
+                                              mock_driver.member_batch_update,
+                                              self.pool_id, [])
 
     def test_create_with_attached_listener(self):
         api_member = self.create_member(

octavia/tests/unit/api/drivers/amphora_driver/v1/test_amphora_driver.py

@@ -346,7 +346,8 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
@@ -380,13 +381,27 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
                    'updated_members': [update_mem_dict]}
         mock_cast.assert_called_with({}, 'batch_update_members', **payload)
 
+    @mock.patch('octavia.db.api.get_session')
+    @mock.patch('octavia.db.repositories.PoolRepository.get')
+    @mock.patch('oslo_messaging.RPCClient.cast')
+    def test_member_batch_update_clear_already_empty(
+            self, mock_cast, mock_pool_get, mock_session):
+        mock_pool = mock.MagicMock()
+        mock_pool_get.return_value = mock_pool
+
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, [])
+
+        mock_cast.assert_not_called()
+
     # Health Monitor
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_health_monitor_create(self, mock_cast):
@@ -441,7 +456,8 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
@@ -479,7 +495,7 @@ class TestAmphoraDriver(base.TestRpc):
 
         self.assertRaises(exceptions.UnsupportedOptionError,
                           self.amp_driver.member_batch_update,
-                          prov_members)
+                          self.sample_data.pool1_id, prov_members)
 
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_health_monitor_update(self, mock_cast):

octavia/tests/unit/api/drivers/amphora_driver/v2/test_amphora_driver.py

@@ -346,7 +346,8 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
@@ -380,13 +381,27 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
                    'updated_members': [update_mem_dict]}
         mock_cast.assert_called_with({}, 'batch_update_members', **payload)
 
+    @mock.patch('octavia.db.api.get_session')
+    @mock.patch('octavia.db.repositories.PoolRepository.get')
+    @mock.patch('oslo_messaging.RPCClient.cast')
+    def test_member_batch_update_clear_already_empty(
+            self, mock_cast, mock_pool_get, mock_session):
+        mock_pool = mock.MagicMock()
+        mock_pool_get.return_value = mock_pool
+
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, [])
+
+        mock_cast.assert_not_called()
+
     # Health Monitor
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_health_monitor_create(self, mock_cast):
@@ -441,7 +456,8 @@ class TestAmphoraDriver(base.TestRpc):
                            'protocol_port': 80,
                            'pool_id': self.sample_data.pool1_id}
 
-        self.amp_driver.member_batch_update(prov_members)
+        self.amp_driver.member_batch_update(
+            self.sample_data.pool1_id, prov_members)
 
         payload = {'old_member_ids': [self.sample_data.member1_id],
                    'new_member_ids': [self.sample_data.member3_id],
@@ -479,7 +495,7 @@ class TestAmphoraDriver(base.TestRpc):
 
         self.assertRaises(exceptions.UnsupportedOptionError,
                           self.amp_driver.member_batch_update,
-                          prov_members)
+                          self.sample_data.pool1_id, prov_members)
 
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_health_monitor_update(self, mock_cast):

octavia/tests/unit/api/drivers/test_provider_noop_driver.py

@@ -230,7 +230,7 @@ class TestNoopProviderDriver(base.TestCase):
                          self.driver.driver.driverconfig[self.member_id])
 
     def test_member_batch_update(self):
-        self.driver.member_batch_update([self.ref_member])
+        self.driver.member_batch_update(self.pool_id, [self.ref_member])
 
         self.assertEqual((self.ref_member, 'member_batch_update'),
                          self.driver.driver.driverconfig[self.member_id])