Update Nova bdm with updated swap info

This change updates bdm for swap in finish_resize and revert_resize
functionality.

Change also adds supporting:
- unit and functional tests
- docs: releasenote

Closes-Bug: #1552777

Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
(cherry picked from commit 15dccae)
(cherry picked from commit 3d3ada4)
(cherry picked from commit 39714c2)
Signed-off-by: Elod Illes <[email protected]>

Author: auniyal61

nova/compute/manager.py

@@ -4990,6 +4990,50 @@ def _delete_volume_attachments(self, ctxt, bdms):
                               'Error: %s', bdm.attachment_id, str(e),
                               instance_uuid=bdm.instance_uuid)
 
+    def _update_bdm_for_swap_to_finish_resize(
+            self, context, instance, confirm=True):
+        """This updates bdm.swap with new swap info"""
+
+        bdms = instance.get_bdms()
+        if not (instance.old_flavor and instance.new_flavor):
+            return bdms
+
+        if instance.old_flavor.swap == instance.new_flavor.swap:
+            return bdms
+
+        old_swap = instance.old_flavor.swap
+        new_swap = instance.new_flavor.swap
+        if not confirm:
+            # revert flavor on _finish_revert_resize
+            old_swap = instance.new_flavor.swap
+            new_swap = instance.old_flavor.swap
+
+        # add swap
+        if old_swap == 0 and new_swap:
+            # (auniyal)old_swap = 0 means we did not have swap bdm
+            # for this instance.
+            # and as there is a new_swap, its a swap addition
+            new_swap_bdm = block_device.create_blank_bdm(new_swap, 'swap')
+            bdm_obj = objects.BlockDeviceMapping(
+                context, instance_uuid=instance.uuid, **new_swap_bdm)
+            bdm_obj.update_or_create()
+            return instance.get_bdms()
+
+        # update swap
+        for bdm in bdms:
+            if bdm.guest_format == 'swap' and bdm.device_type == 'disk':
+                if new_swap > 0:
+                    LOG.info('Adding swap BDM.', instance=instance)
+                    bdm.volume_size = new_swap
+                    bdm.save()
+                    break
+                elif new_swap == 0:
+                    LOG.info('Deleting swap BDM.', instance=instance)
+                    bdm.destroy()
+                    bdms.objects.remove(bdm)
+                    break
+        return bdms
+
     @wrap_exception()
     @reverts_task_state
     @wrap_instance_event(prefix='compute')
@@ -5327,8 +5371,9 @@ def _finish_revert_resize(
     ):
         """Inner version of finish_revert_resize."""
         with self._error_out_instance_on_exception(context, instance):
-            bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
-                context, instance.uuid)
+            bdms = self._update_bdm_for_swap_to_finish_resize(
+                context, instance, confirm=False)
+
             self._notify_about_instance_usage(
                     context, instance, "resize.revert.start")
             compute_utils.notify_about_instance_action(context, instance,
@@ -6265,8 +6310,7 @@ def _finish_resize_helper(self, context, disk_info, image, instance,
         The caller must revert the instance's allocations if the migration
         process failed.
         """
-        bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
-            context, instance.uuid)
+        bdms = self._update_bdm_for_swap_to_finish_resize(context, instance)
 
         with self._error_out_instance_on_exception(context, instance):
             image_meta = objects.ImageMeta.from_dict(image)

nova/tests/functional/test_servers.py

@@ -3207,6 +3207,73 @@ def test_finish_resize_fails_allocation_cleanup(self):
         self._test_resize_to_same_host_instance_fails(
             '_finish_resize', 'compute_finish_resize')
 
+    def _verify_swap_resize_in_bdm(self, server_id, swap_size):
+        """Verify swap dev in BDM"""
+        ctxt = context.get_admin_context()
+        bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
+            ctxt, server_id)
+        if swap_size != 0:
+            self.assertIn('swap', [bdm.guest_format for bdm in bdms])
+            swaps = [
+                bdm.volume_size for bdm in bdms if bdm.guest_format == 'swap']
+            self.assertEqual(len(swaps), 1)
+            self.assertIn(swap_size, swaps)
+        else:
+            self.assertNotIn('swap', [bdm.guest_format for bdm in bdms])
+
+    def _test_swap_resize(self, swap1, swap2, confirm=True):
+        fl_1 = self._create_flavor(swap=swap1)
+        fl_2 = self._create_flavor(swap=swap2)
+        server = self._create_server(flavor_id=fl_1, networks=[])
+        # before resize
+        self.assertEqual(server['flavor']['swap'], swap1)
+        server = self._resize_server(server, fl_2)
+        self.assertEqual(server['flavor']['swap'], swap2)
+        self._verify_swap_resize_in_bdm(server['id'], swap2)
+
+        if confirm:
+            server = self._confirm_resize(server)
+            # after resize
+            self.assertEqual(server['flavor']['swap'], swap2)
+            # verify block device mapping
+            self._verify_swap_resize_in_bdm(server['id'], swap2)
+        else:
+            server = self._revert_resize(server)
+            # after revert
+            self.assertEqual(server['flavor']['swap'], swap1)
+            # verify block device mapping
+            self._verify_swap_resize_in_bdm(server['id'], swap1)
+
+    def test_swap_expand_0_to_0_confirm(self):
+        self._test_swap_resize(0, 0)
+
+    def test_swap_expand_0_to_1024_confirm(self):
+        self._test_swap_resize(0, 1024)
+
+    def test_swap_expand_0_to_1024_revert(self):
+        self._test_swap_resize(0, 1024, confirm=False)
+
+    def test_swap_expand_1024_to_2048_confirm(self):
+        self._test_swap_resize(1024, 2048)
+
+    def test_swap_expand_1024_to_2048_revert(self):
+        self._test_swap_resize(1024, 2048, confirm=False)
+
+    def test_swap_expand_2048_to_2048_confirm(self):
+        self._test_swap_resize(2048, 2048)
+
+    def test_swap_shrink_1024_to_0_confirm(self):
+        self._test_swap_resize(1024, 0)
+
+    def test_swap_shrink_1024_to_0_revert(self):
+        self._test_swap_resize(1024, 0, confirm=False)
+
+    def test_swap_shrink_2048_to_1024_confirm(self):
+        self._test_swap_resize(2048, 1024)
+
+    def test_swap_shrink_2048_to_1024_revert(self):
+        self._test_swap_resize(2048, 1024, confirm=False)
+
     def _server_created_with_host(self):
         hostname = self.compute1.host
         server_req = self._build_server(

nova/tests/unit/compute/test_compute_mgr.py

@@ -13164,3 +13164,120 @@ def test_update_compute_provider_status_unexpected_error(self, m_exc):
         self.assertIn('An error occurred while updating '
                       'COMPUTE_STATUS_DISABLED trait',
                       m_exc.call_args_list[0][0][0])
+
+
+class ComputeManagerBDMUpdateTestCase(test.TestCase):
+
+    def setUp(self):
+        super(ComputeManagerBDMUpdateTestCase, self).setUp()
+        self.compute = manager.ComputeManager()
+        self.context = context.RequestContext(fakes.FAKE_USER_ID,
+                                              fakes.FAKE_PROJECT_ID)
+        self.instance = mock.Mock()
+
+    @mock.patch('nova.block_device.create_blank_bdm')
+    @mock.patch('nova.objects.BlockDeviceMapping')
+    def test_no_flavor_change(self, mock_bdm_obj, mock_create_bdm):
+        self.instance.get_bdms.return_value = []
+        self.instance.old_flavor = None
+        self.instance.new_flavor = None
+
+        bdms = self.compute._update_bdm_for_swap_to_finish_resize(
+            self.context, self.instance)
+
+        self.assertEqual(bdms, [])
+        self.instance.get_bdms.assert_called_once()
+
+    @mock.patch('nova.block_device.create_blank_bdm')
+    @mock.patch('nova.objects.BlockDeviceMapping.create')
+    def test_no_swap_change(self, mock_bdm_obj, mock_create_bdm):
+        self.instance.old_flavor = mock.Mock(swap=1024)
+        self.instance.new_flavor = mock.Mock(swap=1024)
+
+        existing_swap_bdm = objects.BlockDeviceMapping(
+            guest_format='swap',
+            device_type='disk',
+            volume_size=1024)
+
+        bdms = objects.BlockDeviceMappingList(objects=[existing_swap_bdm])
+
+        self.instance.get_bdms.return_value = bdms
+
+        new_bdms = self.compute._update_bdm_for_swap_to_finish_resize(
+            self.context, self.instance)
+
+        self.assertEqual(new_bdms, bdms)
+        self.instance.get_bdms.assert_called_once()
+
+    @mock.patch('nova.block_device.create_blank_bdm')
+    @mock.patch('nova.objects.BlockDeviceMapping')
+    def test_add_new_swap_bdm(self, mock_bdm_obj, mock_create_bdm):
+        self.instance.old_flavor = mock.Mock(swap=0)
+        self.instance.new_flavor = mock.Mock(swap=1024)
+
+        self.instance.get_bdms.return_value = []
+
+        new_swap_bdm = {
+            'guest_format': 'swap',
+            'device_type': 'disk',
+            'volume_size': 1024}
+        mock_create_bdm.return_value = new_swap_bdm
+        mock_bdm_instance = mock_bdm_obj.return_value
+
+        self.compute._update_bdm_for_swap_to_finish_resize(
+            self.context, self.instance)
+
+        mock_bdm_obj.assert_called_once_with(
+            self.context, instance_uuid=self.instance.uuid, **new_swap_bdm
+        )
+        mock_bdm_instance.update_or_create.assert_called_once()
+        # called twice
+        self.assertEqual(self.instance.get_bdms.call_count, 2)
+
+    @mock.patch('nova.block_device.create_blank_bdm')
+    @mock.patch('nova.objects.BlockDeviceMapping.create')
+    @mock.patch('nova.objects.BlockDeviceMapping.save')
+    def test_update_swap_bdm(
+            self, mock_bdm_save, mock_bdm_create,
+            mock_create_blank_bdm):
+        self.instance.old_flavor = mock.Mock(swap=1024)  # Existing swap size
+        self.instance.new_flavor = mock.Mock(swap=2048)  # New swap size
+
+        existing_swap_bdm = objects.BlockDeviceMapping(
+            guest_format='swap',
+            device_type='disk',
+            volume_size=1024)
+
+        bdms = objects.BlockDeviceMappingList(objects=[existing_swap_bdm])
+
+        self.instance.get_bdms.return_value = bdms
+
+        self.compute._update_bdm_for_swap_to_finish_resize(
+            self.context, self.instance)
+
+        # assert bdm get saved in DB
+        existing_swap_bdm.save.assert_called_once()
+        # here we are returning same bdms object, not freh from DB
+        self.assertEqual(existing_swap_bdm.volume_size, 2048)
+        # get_bdms is called only once
+        self.assertEqual(self.instance.get_bdms.call_count, 1)
+
+    @mock.patch('nova.block_device.create_blank_bdm')
+    @mock.patch('nova.objects.BlockDeviceMapping.destroy')
+    def test_delete_swap_bdm(self, mock_bdm_destroy, mock_create_bdm):
+        self.instance.old_flavor = mock.Mock(swap=1024)
+        self.instance.new_flavor = mock.Mock(swap=0)
+
+        existing_swap_bdm = objects.BlockDeviceMapping(
+            guest_format='swap',
+            device_type='disk',
+            volume_size=1024)
+
+        self.instance.get_bdms.return_value = objects.BlockDeviceMappingList(
+            objects=[existing_swap_bdm])
+
+        self.compute._update_bdm_for_swap_to_finish_resize(
+            self.context, self.instance)
+
+        mock_bdm_destroy.assert_called_once()
+        self.instance.get_bdms.assert_called_once()

releasenotes/notes/resize-swap-size-1e15e67c436f4b95.yaml

@@ -0,0 +1,10 @@
+---
+
+fixes:
+  - |
+    With this change, operators can now resize the instance flavor swap to
+    a smaller swap size, it can be expand and shrunk down to 0 using the same
+    resize API.
+    For more details see: `bug 1552777`_
+
+    .. _`bug 1552777`: https://bugs.launchpad.net/nova/+bug/1552777