Merge "Fix Infinidat driver consistency groups feature"

cinder/tests/unit/volume/drivers/test_infinidat.py

@@ -59,15 +59,19 @@ TEST_SNAPSHOT_SOURCE_ID = 67890
 TEST_SNAPSHOT_METADATA = {'cinder_id': fake.SNAPSHOT_ID}
 
 test_volume = mock.Mock(id=fake.VOLUME_ID, name_id=fake.VOLUME_ID, size=1,
-                        volume_type_id=fake.VOLUME_TYPE_ID,
+                        volume_type_id=fake.VOLUME_TYPE_ID, group_id=None,
                         multiattach=False, volume_attachment=None)
 test_volume2 = mock.Mock(id=fake.VOLUME2_ID, name_id=fake.VOLUME2_ID, size=1,
-                         volume_type_id=fake.VOLUME_TYPE_ID,
+                         volume_type_id=None, group_id=None,
                          multiattach=False, volume_attachment=None)
-test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume,
+test_volume3 = mock.Mock(id=fake.VOLUME3_ID, name_id=fake.VOLUME3_ID, size=1,
-                          volume_id=test_volume.id)
+                         volume_type_id=fake.VOLUME_TYPE_ID,
-test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1, multiattach=False,
+                         group_id=fake.GROUP_ID, multiattach=True,
-                       volume_attachment=None)
+                         volume_attachment=None)
+test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume)
+test_clone = mock.Mock(id=fake.VOLUME4_ID, name_id=fake.VOLUME4_ID, size=1,
+                       volume_type_id=fake.VOLUME_TYPE_ID, group_id=None,
+                       multiattach=False, volume_attachment=None)
 test_group = mock.Mock(id=fake.GROUP_ID)
 test_snapgroup = mock.Mock(id=fake.GROUP_SNAPSHOT_ID, group=test_group)
 test_connector = dict(wwpns=[TEST_WWN_1],
@@ -145,6 +149,7 @@ class InfiniboxDriverTestCaseBase(test.TestCase):
         self._mock_volume.get_all_metadata.return_value = {}
         self._mock_volume.create_snapshot.return_value = self._mock_volume
         self._mock_snapshot = mock.Mock()
+        self._mock_snapshot.get_parent.return_value = self._mock_volume
         self._mock_host = mock.Mock()
         self._mock_host.get_luns.return_value = []
         self._mock_host.map_volume().get_lun.return_value = TEST_LUN
@@ -389,6 +394,22 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
             self._system.volumes.create.call_args[1]
         )
 
+    @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs')
+    @mock.patch('cinder.volume.volume_utils.group_get_by_id')
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=True)
+    def test_create_volume_within_group(self, *mocks):
+        self.driver.create_volume(test_volume3)
+        self._mock_group.add_member.assert_called_once()
+
+    @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs')
+    @mock.patch('cinder.volume.volume_utils.group_get_by_id')
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_create_volume_within_no_cg_group(self, *mocks):
+        self.driver.create_volume(test_volume3)
+        self._mock_group.add_member.assert_not_called()
+
     def test_delete_volume(self):
         self.driver.delete_volume(test_volume)
 
@@ -517,6 +538,13 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
     def test_create_group(self, *mocks):
         self.driver.create_group(None, test_group)
 
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_create_generic_group(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.create_group,
+                          None, test_group)
+
     @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
                 return_value=True)
     def test_create_group_metadata(self, *mocks):
@@ -542,6 +570,13 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
     def test_delete_group(self, *mocks):
         self.driver.delete_group(None, test_group, [test_volume])
 
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_delete_generic_group(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.delete_group,
+                          None, test_group, [test_volume])
+
     @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
                 return_value=True)
     def test_delete_group_doesnt_exist(self, *mocks):
@@ -562,6 +597,13 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
         self.driver.update_group(None, test_group,
                                  [test_volume], [test_volume])
 
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_update_generic_group_add_and_remove(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.update_group,
+                          None, test_group, [test_volume], [test_volume])
+
     @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
                 return_value=True)
     def test_update_group_api_fail(self, *mocks):
@@ -583,6 +625,23 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
                                           test_snapgroup, [test_snapshot],
                                           None, None)
 
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_create_genericgroup_from_src_snaps(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.create_group_from_src,
+                          None, test_group, [test_volume],
+                          test_snapgroup, [test_snapshot],
+                          None, None)
+
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=True)
+    def test_create_group_from_empty_sources(self, *mocks):
+        self.assertRaises(exception.InvalidInput,
+                          self.driver.create_group_from_src,
+                          None, test_group, [test_volume],
+                          None, None, None, None)
+
     @mock.patch("cinder.volume.volume_utils.copy_volume")
     @mock.patch("cinder.volume.volume_utils.brick_get_connector")
     @mock.patch("cinder.volume.volume_utils.brick_get_connector_properties",
@@ -599,13 +658,20 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
                 return_value=True)
     def test_create_group_snap(self, *mocks):
         mock_snapgroup = mock.Mock()
-        mock_snapgroup.get_members.return_value = [self._mock_volume]
+        mock_snapgroup.get_members.return_value = [self._mock_snapshot,
-        self._mock_volume.get_parent.return_value = self._mock_volume
+                                                   self._mock_snapshot]
-        self._mock_volume.get_name.return_value = ''
+        self._mock_volume.get_name.side_effect = [fake.VOLUME_NAME,
+                                                  fake.VOLUME2_NAME]
         self._mock_group.create_snapshot.return_value = mock_snapgroup
-        self.driver.create_group_snapshot(None,
+        self.driver.create_group_snapshot(None, test_snapgroup,
-                                          test_snapgroup,
+                                          [test_snapshot, test_snapshot])
-                                          [test_snapshot])
+
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_create_generic_group_snap(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.create_group_snapshot,
+                          None, test_snapgroup, [test_snapshot])
 
     @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
                 return_value=True)
@@ -622,6 +688,13 @@ class InfiniboxDriverTestCase(InfiniboxDriverTestCaseBase):
                                           test_snapgroup,
                                           [test_snapshot])
 
+    @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
+                return_value=False)
+    def test_delete_generic_group_snap(self, *mocks):
+        self.assertRaises(NotImplementedError,
+                          self.driver.delete_group_snapshot,
+                          None, test_snapgroup, [test_snapshot])
+
     @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type',
                 return_value=True)
     def test_delete_group_snap_does_not_exist(self, *mocks):
@@ -1258,8 +1331,7 @@ class InfiniboxDriverTestCaseQoS(InfiniboxDriverTestCaseBase):
                                                 'consumer': 'back-end',
                                                 'specs': {'maxIOPS': 1000,
                                                           'maxBWS': 10000}}}
-        test_volume = mock.Mock(id=1, size=1, volume_type_id=None)
+        self.driver.create_volume(test_volume2)
-        self.driver.create_volume(test_volume)
         self._system.qos_policies.create.assert_not_called()
         self._mock_qos_policy.assign_entity.assert_not_called()
 

cinder/volume/drivers/infinidat.py

@@ -127,10 +127,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         1.10 - added support for TLS/SSL communication
         1.11 - fixed generic volume migration
         1.12 - fixed volume multi-attach
+        1.13 - fixed consistency groups feature
 
     """
 
-    VERSION = '1.12'
+    VERSION = '1.13'
 
     # ThirdPartySystems wiki page
     CI_WIKI_NAME = "INFINIDAT_CI"
@@ -304,6 +305,18 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
             raise exception.GroupNotFound(group_id=group_name)
         return infinidat_cg
 
+    def _get_infinidat_sg(self, group_snapshot):
+        name = self._make_group_snapshot_name(group_snapshot)
+        infinidat_sg = self._system.cons_groups.safe_get(name=name)
+        if infinidat_sg is None:
+            raise exception.GroupSnapshotNotFound(
+                group_snapshot_id=group_snapshot.id)
+        if not infinidat_sg.is_snapgroup():
+            reason = (_('consistency group "%s" is not a snapshot group')
+                      % name)
+            raise exception.InvalidGroupSnapshot(reason=reason)
+        return infinidat_sg
+
     def _get_or_create_host(self, port):
         host_name = self._make_host_name(port)
         infinidat_host = self._system.hosts.safe_get(name=host_name)
@@ -600,6 +613,11 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
         infinidat_volume = self._system.volumes.create(**create_kwargs)
         self._set_qos(volume, infinidat_volume)
         self._set_cinder_object_metadata(infinidat_volume, volume)
+        if volume.group_id:
+            group = volume_utils.group_get_by_id(volume.group_id)
+            if volume_utils.is_group_a_cg_snapshot_type(group):
+                infinidat_group = self._get_infinidat_cg(group)
+                infinidat_group.add_member(infinidat_volume)
         return infinidat_volume
 
     @infinisdk_to_cinder_exceptions
@@ -797,18 +815,30 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
 
     @infinisdk_to_cinder_exceptions
     def create_group(self, context, group):
-        """Creates a group."""
+        """Creates a group.
+
+        :param context: the context of the caller.
+        :param group: the Group object of the group to be created.
+        :returns: model_update
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group):
             raise NotImplementedError()
-        obj = self._system.cons_groups.create(name=self._make_cg_name(group),
+        name = self._make_cg_name(group)
-                                              pool=self._get_infinidat_pool())
+        pool = self._get_infinidat_pool()
-        self._set_cinder_object_metadata(obj, group)
+        infinidat_cg = self._system.cons_groups.create(name=name, pool=pool)
+        self._set_cinder_object_metadata(infinidat_cg, group)
         return {'status': fields.GroupStatus.AVAILABLE}
 
     @infinisdk_to_cinder_exceptions
     def delete_group(self, context, group, volumes):
-        """Deletes a group."""
+        """Deletes a group.
+
+        :param context: the context of the caller.
+        :param group: the Group object of the group to be deleted.
+        :param volumes: a list of Volume objects in the group.
+        :returns: model_update, volumes_model_update
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group):
             raise NotImplementedError()
@@ -825,18 +855,25 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
     @infinisdk_to_cinder_exceptions
     def update_group(self, context, group,
                      add_volumes=None, remove_volumes=None):
-        """Updates a group."""
+        """Updates a group.
+
+        :param context: the context of the caller.
+        :param group: the Group object of the group to be updated.
+        :param add_volumes: a list of Volume objects to be added.
+        :param remove_volumes: a list of Volume objects to be removed.
+        :returns: model_update, add_volumes_update, remove_volumes_update
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group):
             raise NotImplementedError()
         add_volumes = add_volumes if add_volumes else []
         remove_volumes = remove_volumes if remove_volumes else []
         infinidat_cg = self._get_infinidat_cg(group)
-        for vol in add_volumes:
+        for volume in add_volumes:
-            infinidat_volume = self._get_infinidat_volume(vol)
+            infinidat_volume = self._get_infinidat_volume(volume)
             infinidat_cg.add_member(infinidat_volume)
-        for vol in remove_volumes:
+        for volume in remove_volumes:
-            infinidat_volume = self._get_infinidat_volume(vol)
+            infinidat_volume = self._get_infinidat_volume(volume)
             infinidat_cg.remove_member(infinidat_volume)
         return None, None, None
 
@@ -844,62 +881,78 @@ class InfiniboxVolumeDriver(san.SanISCSIDriver):
     def create_group_from_src(self, context, group, volumes,
                               group_snapshot=None, snapshots=None,
                               source_group=None, source_vols=None):
-        """Creates a group from source."""
+        """Creates a group from source.
-        # The source is either group_snapshot+snapshots or
-        # source_group+source_vols. The target is group+volumes
-        # we assume the source (source_vols / snapshots) are in the same
-        # order as the target (volumes)
 
+        :param context: the context of the caller.
+        :param group: the Group object to be created.
+        :param volumes: a list of Volume objects in the group.
+        :param group_snapshot: the GroupSnapshot object as source.
+        :param snapshots: a list of Snapshot objects in group_snapshot.
+        :param source_group: the Group object as source.
+        :param source_vols: a list of Volume objects in the source_group.
+        :returns: model_update, volumes_model_update
+
+        The source can be group_snapshot or a source_group.
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group):
             raise NotImplementedError()
         self.create_group(context, group)
-        new_infinidat_group = self._get_infinidat_cg(group)
+        if group_snapshot and snapshots:
-        if group_snapshot is not None and snapshots is not None:
             for volume, snapshot in zip(volumes, snapshots):
                 self.create_volume_from_snapshot(volume, snapshot)
-                new_infinidat_volume = self._get_infinidat_volume(volume)
+        elif source_group and source_vols:
-                new_infinidat_group.add_member(new_infinidat_volume)
+            for volume, source_vol in zip(volumes, source_vols):
-        elif source_group is not None and source_vols is not None:
+                self.create_cloned_volume(volume, source_vol)
-            for volume, src_vol in zip(volumes, source_vols):
+        else:
-                self.create_cloned_volume(volume, src_vol)
+            message = _('creating a group from source is possible '
-                new_infinidat_volume = self._get_infinidat_volume(volume)
+                        'from an existing group or a group snapshot.')
-                new_infinidat_group.add_member(new_infinidat_volume)
+            raise exception.InvalidInput(message=message)
         return None, None
 
     @infinisdk_to_cinder_exceptions
     def create_group_snapshot(self, context, group_snapshot, snapshots):
-        """Creates a group_snapshot."""
+        """Creates a group_snapshot.
+
+        :param context: the context of the caller.
+        :param group_snapshot: the GroupSnapshot object to be created.
+        :param snapshots: a list of Snapshot objects in the group_snapshot.
+        :returns: model_update, snapshots_model_update
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group_snapshot):
             raise NotImplementedError()
         infinidat_cg = self._get_infinidat_cg(group_snapshot.group)
-        group_snap_name = self._make_group_snapshot_name(group_snapshot)
+        group_snapshot_name = self._make_group_snapshot_name(group_snapshot)
-        new_group = infinidat_cg.create_snapshot(name=group_snap_name)
+        infinidat_sg = infinidat_cg.create_snapshot(name=group_snapshot_name)
         # update the names of the individual snapshots in the new snapgroup
         # to match the names we use for cinder snapshots
-        for infinidat_snapshot in new_group.get_members():
+        for infinidat_snapshot in infinidat_sg.get_members():
             parent_name = infinidat_snapshot.get_parent().get_name()
-            for cinder_snapshot in snapshots:
+            for snapshot in snapshots:
-                if cinder_snapshot.volume_id in parent_name:
+                if snapshot.volume.name_id in parent_name:
-                    snapshot_name = self._make_snapshot_name(cinder_snapshot)
+                    snapshot_name = self._make_snapshot_name(snapshot)
                     infinidat_snapshot.update_name(snapshot_name)
         return None, None
 
     @infinisdk_to_cinder_exceptions
     def delete_group_snapshot(self, context, group_snapshot, snapshots):
-        """Deletes a group_snapshot."""
+        """Deletes a group_snapshot.
+
+        :param context: the context of the caller.
+        :param group_snapshot: the GroupSnapshot object to be deleted.
+        :param snapshots: a list of Snapshot objects in the group_snapshot.
+        :returns: model_update, snapshots_model_update
+        """
         # let generic volume group support handle non-cgsnapshots
         if not volume_utils.is_group_a_cg_snapshot_type(group_snapshot):
             raise NotImplementedError()
-        cgsnap_name = self._make_group_snapshot_name(group_snapshot)
+        try:
-        infinidat_cgsnap = self._system.cons_groups.safe_get(name=cgsnap_name)
+            infinidat_sg = self._get_infinidat_sg(group_snapshot)
-        if infinidat_cgsnap is not None:
+        except exception.GroupSnapshotNotFound:
-            if not infinidat_cgsnap.is_snapgroup():
+            pass
-                msg = _('Group "%s" is not a snapshot group') % cgsnap_name
+        else:
-                LOG.error(msg)
+            infinidat_sg.safe_delete()
-                raise exception.InvalidGroupSnapshot(message=msg)
-            infinidat_cgsnap.safe_delete()
         for snapshot in snapshots:
             self.delete_snapshot(snapshot)
         return None, None

releasenotes/notes/bug-1984000-infinidat-fix-consistency-groups-cf5b9c85dbf972ee.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Infinidat Driver `bug #1984000
+    <https://bugs.launchpad.net/cinder/+bug/1984000>`_:
+    Fixed Infinidat driver to take into account the group
+    identifier property when creating a new volume and
+    add the volume to the consistency group.