Merge "Removed explicit call to delete attachment"

nova/cmd/manage.py

@@ -3132,12 +3132,10 @@ class VolumeAttachmentCommands(object):
 
             # RPC call to the compute to cleanup the connections, which
             # will in turn unmap the volume from the compute host
-            # TODO(lyarwood): Add delete_attachment as a kwarg to
-            # remove_volume_connection as is available in the private
-            # method within the manager.
             if instance.host == connector['host']:
                 compute_rpcapi.remove_volume_connection(
-                    cctxt, instance, volume_id, instance.host)
+                    cctxt, instance, volume_id, instance.host,
+                    delete_attachment=True)
             else:
                 msg = (
                     f"The compute host '{connector['host']}' in the "
@@ -3145,13 +3143,6 @@ class VolumeAttachmentCommands(object):
                     f"'{instance.host}'.")
                 raise exception.HostConflict(_(msg))
 
-            # Delete the existing volume attachment if present in the bdm.
-            # This isn't present when the original attachment was made
-            # using the legacy cinderv2 APIs before the cinderv3 attachment
-            # based APIs were present.
-            if bdm.attachment_id:
-                volume_api.attachment_delete(cctxt, bdm.attachment_id)
-
             # Update the attachment with host connector, this regenerates
             # the connection_info that we can now stash in the bdm.
             new_connection_info = volume_api.attachment_update(

nova/compute/manager.py

@@ -618,7 +618,7 @@ class ComputeVirtAPI(virtapi.VirtAPI):
 class ComputeManager(manager.Manager):
     """Manages the running instances from creation to destruction."""
 
-    target = messaging.Target(version='6.2')
+    target = messaging.Target(version='6.3')
 
     def __init__(self, compute_driver=None, *args, **kwargs):
         """Load configuration options and connect to the hypervisor."""
@@ -7967,7 +7967,9 @@ class ComputeManager(manager.Manager):
             old_volume_id, new_volume_id)
 
     @wrap_exception()
-    def remove_volume_connection(self, context, volume_id, instance):
+    def remove_volume_connection(
+            self, context, volume_id, instance,
+            delete_attachment=False):
         """Remove the volume connection on this host
 
         Detach the volume from this instance on this host, and if this is
@@ -7986,7 +7988,8 @@ class ComputeManager(manager.Manager):
             # we cannot simply delete a v3 style attachment here without
             # needing to do some behavior modification of that
             # _rollback_live_migration flow which gets messy.
-            self._remove_volume_connection(context, bdm, instance)
+            self._remove_volume_connection(
+                context, bdm, instance, delete_attachment)
         except exception.NotFound:
             pass
 

nova/compute/rpcapi.py

@@ -404,6 +404,7 @@ class ComputeAPI(object):
                  flavor
         * 6.1 - Add reimage_boot_volume parameter to rebuild_instance()
         * 6.2 - Add target_state parameter to rebuild_instance()
+        * 6.3 - Add delete_attachment parameter to remove_volume_connection
     '''
 
     VERSION_ALIASES = {
@@ -1133,12 +1134,23 @@ class ComputeAPI(object):
         cctxt.cast(ctxt, 'remove_fixed_ip_from_instance',
                    instance=instance, address=address)
 
-    def remove_volume_connection(self, ctxt, instance, volume_id, host):
+    def remove_volume_connection(
-        version = self._ver(ctxt, '5.0')
+                self, ctxt, instance, volume_id, host,
-        cctxt = self.router.client(ctxt).prepare(
+                delete_attachment=False
-                server=host, version=version)
+            ):
-        return cctxt.call(ctxt, 'remove_volume_connection',
+        version = '6.3'
-                          instance=instance, volume_id=volume_id)
+        client = self.router.client(ctxt)
+        kwargs = {
+            'instance': instance,
+            'volume_id': volume_id,
+            'delete_attachment': delete_attachment
+        }
+        if not client.can_send_version(version):
+            kwargs.pop('delete_attachment')
+            version = self._ver(ctxt, '5.0')
+
+        cctxt = client.prepare(server=host, version=version)
+        return cctxt.call(ctxt, 'remove_volume_connection', **kwargs)
 
     def rescue_instance(self, ctxt, instance, rescue_password,
                         rescue_image_ref=None, clean_shutdown=True):

nova/objects/service.py

@@ -31,7 +31,7 @@ LOG = logging.getLogger(__name__)
 
 
 # NOTE(danms): This is the global service version counter
-SERVICE_VERSION = 66
+SERVICE_VERSION = 67
 
 
 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any
@@ -231,6 +231,10 @@ SERVICE_VERSION_HISTORY = (
     # Version 66: Compute RPC v6.2:
     # Add target_state parameter to rebuild_instance()
     {'compute_rpc': '6.2'},
+    # Version 67: Compute RPC v6.3:
+    # Add delete_attachment parameter to remove_volume_connection()
+    {'compute_rpc': '6.3'},
+
 )
 
 # This is the version after which we can rely on having a persistent

nova/tests/unit/cmd/test_manage.py

@@ -3732,9 +3732,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase):
             mock.ANY, uuidsentinel.volume, uuidsentinel.instance)
         fake_compute_rpcapi.remove_volume_connection.assert_called_once_with(
             mock.ANY, mock_get_instance.return_value, uuidsentinel.volume,
-            mock_get_instance.return_value.host)
+            mock_get_instance.return_value.host, delete_attachment=True)
-        fake_volume_api.attachment_delete.assert_called_once_with(
-            mock.ANY, uuidsentinel.instance)
         fake_volume_api.attachment_update.assert_called_once_with(
             mock.ANY, uuidsentinel.new_attachment, mock.ANY, device_name)
         fake_volume_api.attachment_complete.assert_called_once_with(

nova/tests/unit/compute/test_rpcapi.py

@@ -965,7 +965,7 @@ class ComputeRpcAPITestCase(test.NoDBTestCase):
     def test_remove_volume_connection(self):
         self._test_compute_api('remove_volume_connection', 'call',
                 instance=self.fake_instance_obj, volume_id='id', host='host',
-                version='6.0')
+                delete_attachment=True, version='6.3')
 
     def test_rescue_instance(self):
         self._test_compute_api('rescue_instance', 'cast',