Fix A/A 'resource_backend' when scheduling volumes

Fix an issue with the 'resource_backend' included in the scheduler spec
for creating a volume associated with another volume, snapshot, or
group/cg. When running A/A, the 'resource_backend' must reference the
cluster, not the host.

Enhance the unit tests that cover this area. This includes fixing the
'expected_spec' so it copies a dictionary rather than referencing it,
so that external changes to the dictionary don't inadvertently update
the unit test's expected results.

Closes-Bug: #1808343
Change-Id: I7d414844d094945b55a094a8426687595f22de28

cinder/objects/base.py

@@ -528,6 +528,9 @@ class ClusteredObject(object):
                     'storage backends.')
             raise exception.InvalidInput(reason=msg)
 
+    # The object's resource backend depends on whether it's clustered.
+    resource_backend = service_topic_queue
+
 
 class CinderObjectSerializer(base.VersionedObjectSerializer):
     OBJ_BASE_CLASS = CinderObject

cinder/tests/unit/volume/flows/fake_volume_api.py

@@ -13,7 +13,7 @@
 
 class FakeVolumeAPI(object):
     def __init__(self, expected_spec, test_inst):
-        self.expected_spec = expected_spec
+        self.expected_spec = expected_spec.copy()
         self.test_inst = test_inst
 
     def create_volume(self, ctxt, volume, host,
@@ -31,7 +31,7 @@ class FakeVolumeAPI(object):
 
 class FakeSchedulerRpcAPI(object):
     def __init__(self, expected_spec, test_inst):
-        self.expected_spec = expected_spec
+        self.expected_spec = expected_spec.copy()
         self.test_inst = test_inst
 
     def create_volume(self, ctxt, volume, snapshot_id=None, image_id=None,

cinder/tests/unit/volume/flows/test_create_volume_flow.py

@@ -67,12 +67,18 @@ class CreateVolumeFlowTestCase(test.TestCase):
                                               mock_time, mock_extract_host,
                                               volume_get_by_id):
         mock_time.side_effect = self.time_inc
-        volume = fake_volume.fake_volume_obj(self.ctxt,
-                                             host='host@backend#pool')
+        volume = fake_volume.fake_volume_obj(
+            self.ctxt,
+            host='host@backend#pool',
+            cluster_name='cluster@backend#pool')
         volume_get_by_id.return_value = volume
 
+        # This is the spec for a volume created from another resource. It
+        # includes the 'resource_backend'. When the volume is associated
+        # with a cluster the 'resource_backend' should use the cluster name.
         spec = {'volume_id': volume.id,
                 'volume': volume,
+                'resource_backend': 'cluster@backend#pool',
                 'source_volid': volume.id,
                 'snapshot_id': None,
                 'image_id': 4,
@@ -87,7 +93,13 @@ class CreateVolumeFlowTestCase(test.TestCase):
             fake_volume_api.FakeVolumeAPI(spec, self),
             fake_volume_api.FakeDb())
 
+        # Remove 'resource_backend' prior to calling task._cast_create_volume
+        # (the point of the test is to confirm that it adds it to the spec
+        # sent to the scheduler).
+        spec.pop('resource_backend')
+
         task._cast_create_volume(self.ctxt, spec, {})
+        mock_snapshot_get.assert_not_called()
         mock_extract_host.assert_not_called()
 
         snapshot = fake_snapshot.fake_snapshot_obj(self.ctxt,
@@ -96,6 +108,7 @@ class CreateVolumeFlowTestCase(test.TestCase):
 
         spec = {'volume_id': volume.id,
                 'volume': volume,
+                'resource_backend': 'cluster@backend#pool',
                 'source_volid': None,
                 'snapshot_id': snapshot.id,
                 'image_id': 4,
@@ -110,6 +123,7 @@ class CreateVolumeFlowTestCase(test.TestCase):
             fake_volume_api.FakeVolumeAPI(spec, self),
             fake_volume_api.FakeDb())
 
+        spec.pop('resource_backend')
         task._cast_create_volume(self.ctxt, spec, {})
         mock_snapshot_get.assert_called_once_with(self.ctxt, snapshot.id)
         mock_extract_host.assert_not_called()
@@ -124,10 +138,14 @@ class CreateVolumeFlowTestCase(test.TestCase):
         volume = fake_volume.fake_volume_obj(self.ctxt)
         volume_get_by_id.return_value = volume
         props = {}
-        cg_obj = (fake_consistencygroup.
-                  fake_consistencyobject_obj(self.ctxt, consistencygroup_id=1,
-                                             host='host@backend#pool'))
+        cg_obj = fake_consistencygroup.fake_consistencyobject_obj(
+            self.ctxt,
+            consistencygroup_id=1,
+            host='host@backend#pool',
+            cluster_name='cluster@backend#pool')
         consistencygroup_get_by_id.return_value = cg_obj
+        mock_extract_host.return_value = 'cluster@backend'
+
         spec = {'volume_id': None,
                 'volume': None,
                 'source_volid': None,
@@ -145,9 +163,14 @@ class CreateVolumeFlowTestCase(test.TestCase):
             fake_volume_api.FakeDb())
 
         task._cast_create_volume(self.ctxt, spec, props)
+        consistencygroup_get_by_id.assert_not_called()
+        mock_extract_host.assert_not_called()
 
+        # This is the spec for a volume created from a consistency group. It
+        # includes the 'resource_backend'.
         spec = {'volume_id': volume.id,
                 'volume': volume,
+                'resource_backend': 'cluster@backend',
                 'source_volid': 2,
                 'snapshot_id': 3,
                 'image_id': 4,
@@ -162,9 +185,14 @@ class CreateVolumeFlowTestCase(test.TestCase):
             fake_volume_api.FakeVolumeAPI(spec, self),
             fake_volume_api.FakeDb())
 
+        # Remove 'resource_backend' prior to calling task._cast_create_volume
+        # (the point of the test is to confirm that it adds it to the spec
+        # sent to the scheduler).
+        spec.pop('resource_backend')
+
         task._cast_create_volume(self.ctxt, spec, props)
         consistencygroup_get_by_id.assert_called_once_with(self.ctxt, 5)
-        mock_extract_host.assert_called_once_with('host@backend#pool')
+        mock_extract_host.assert_called_once_with('cluster@backend#pool')
 
     @mock.patch('cinder.db.volume_create')
     @mock.patch('cinder.objects.Volume.get_by_id')

cinder/volume/flows/api/create_volume.py

@@ -741,13 +741,13 @@ class VolumeCastTask(flow_utils.CinderTask):
             # to choose a proper pool whose backend is same as CG's backend.
             cgroup = objects.ConsistencyGroup.get_by_id(context, cgroup_id)
             request_spec['resource_backend'] = vol_utils.extract_host(
-                cgroup.host)
+                cgroup.resource_backend)
         elif group_id:
             # If group_id exists, we should cast volume to the scheduler
             # to choose a proper pool whose backend is same as group's backend.
             group = objects.Group.get_by_id(context, group_id)
             request_spec['resource_backend'] = vol_utils.extract_host(
-                group.host)
+                group.resource_backend)
         elif snapshot_id and CONF.snapshot_same_host:
             # NOTE(Rongze Zhu): A simple solution for bug 1008866.
             #
@@ -759,10 +759,11 @@ class VolumeCastTask(flow_utils.CinderTask):
             # before creating volume, we schedule this request to scheduler
             # service with the desired backend information.
             snapshot = objects.Snapshot.get_by_id(context, snapshot_id)
-            request_spec['resource_backend'] = snapshot.volume.host
+            request_spec['resource_backend'] = snapshot.volume.resource_backend
         elif source_volid:
             source_volume_ref = objects.Volume.get_by_id(context, source_volid)
-            request_spec['resource_backend'] = source_volume_ref.host
+            request_spec['resource_backend'] = (
+                source_volume_ref.resource_backend)
 
         self.scheduler_rpcapi.create_volume(
             context,