Fix empty Batch Member Update to unlock objects

Move the decision up one layer to the API controller.
The amphora driver should now just do as it is told.

Change-Id: Idb3ad20b8539bfdb788981a8634317257d83b238
Story: 2008731
Task: 42083
(cherry picked from commit 00e3f8349d)

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

@@ -285,13 +285,10 @@ 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.")
+        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)
 
     def _validate_members(self, db_pool, members):
         if db_pool.protocol == consts.PROTOCOL_UDP:

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

@@ -309,13 +309,10 @@ 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_members': [m.to_dict() for m in deleted_members],
-                       'new_members': [m.to_dict() 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.")
+        payload = {'old_members': [m.to_dict() for m in deleted_members],
+                   'new_members': [m.to_dict() for m in new_members],
+                   'updated_members': updated_members}
+        self.client.cast({}, 'batch_update_members', **payload)
 
     def _validate_members(self, db_pool, members):
         if db_pool.protocol == consts.PROTOCOL_UDP:

octavia/api/v2/controllers/member.py

@@ -377,6 +377,12 @@ class MembersController(MemberController):
                 if (m.ip_address, m.protocol_port) not in new_member_uniques:
                     deleted_members.append(m)
 
+            if not (deleted_members or new_members or updated_members):
+                LOG.info("Member batch update is a noop, rolling back and "
+                         "returning early.")
+                lock_session.rollback()
+                return
+
             if additive_only:
                 member_count_diff = len(new_members)
             else:

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

@@ -942,9 +942,7 @@ class TestMember(base.BaseAPITest):
 
         self.assertEqual([], returned_members)
 
-        mock_provider.assert_called_once_with(u'noop_driver',
-                                              mock_driver.member_batch_update,
-                                              self.pool_id, [])
+        mock_provider.assert_not_called()
 
     def test_create_with_attached_listener(self):
         api_member = self.create_member(

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

@@ -458,13 +458,22 @@ class TestAmphoraDriver(base.TestRpc):
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_member_batch_update_clear_already_empty(
             self, mock_cast, mock_pool_get, mock_session):
+        """Expect that we will pass an empty payload if directed.
+
+        Logic for whether or not to attempt this will be done above the driver
+        layer, so our driver is responsible to forward the request even if it
+        is a perceived no-op.
+        """
         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()
+        payload = {'old_member_ids': [],
+                   'new_member_ids': [],
+                   'updated_members': []}
+        mock_cast.assert_called_with({}, 'batch_update_members', **payload)
 
     # Health Monitor
     @mock.patch('oslo_messaging.RPCClient.cast')

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

@@ -467,13 +467,22 @@ class TestAmphoraDriver(base.TestRpc):
     @mock.patch('oslo_messaging.RPCClient.cast')
     def test_member_batch_update_clear_already_empty(
             self, mock_cast, mock_pool_get, mock_session):
+        """Expect that we will pass an empty payload if directed.
+
+        Logic for whether or not to attempt this will be done above the driver
+        layer, so our driver is responsible to forward the request even if it
+        is a perceived no-op.
+        """
         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()
+        payload = {'old_members': [],
+                   'new_members': [],
+                   'updated_members': []}
+        mock_cast.assert_called_with({}, 'batch_update_members', **payload)
 
     # Health Monitor
     @mock.patch('oslo_messaging.RPCClient.cast')

releasenotes/notes/Fix-noop-batch-member-update-issue-09b76787553e7752.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixed an issue with batch member updates, that don't have any changes,
+    not properly rolling back the update.