From: Patrick East Date: Tue, 12 Jan 2016 18:22:18 +0000 (-0800) Subject: Remove DB calls from Pure Volume Driver CG methods X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0cd8cbdca8befd5b3c7a57faabfeda93910cf319;p=openstack-build%2Fcinder-build.git Remove DB calls from Pure Volume Driver CG methods After the updates to the CG API’s to pass in the objects we didn’t need to call the DB anymore. This finally fixes that issue and updates to return the actual database updates. Change-Id: If6c96843be1d7e1cbc83e4258b917296129df621 Closes-Bug: #1531676 --- diff --git a/cinder/tests/unit/test_pure.py b/cinder/tests/unit/test_pure.py index f4eb7956e..4bcc10f00 100644 --- a/cinder/tests/unit/test_pure.py +++ b/cinder/tests/unit/test_pure.py @@ -706,17 +706,19 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): mock_cgroup.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074" mock_cgroup['status'] = "deleted" mock_context = mock.Mock() - self.driver.db = mock.Mock() mock_volume = mock.MagicMock() - expected_volumes = [mock_volume] - self.driver.db.volume_get_all_by_group.return_value = expected_volumes - model_update, volumes = \ - self.driver.delete_consistencygroup(mock_context, mock_cgroup, []) + model_update, volumes = self.driver.delete_consistencygroup( + mock_context, mock_cgroup, [mock_volume]) expected_name = self.driver._get_pgroup_name_from_id(mock_cgroup.id) self.array.destroy_pgroup.assert_called_with(expected_name) - self.assertEqual(expected_volumes, volumes) + + expected_volume_updates = [{ + 'id': mock_volume.id, + 'status': 'deleted' + }] + self.assertEqual(expected_volume_updates, volumes) self.assertEqual(mock_cgroup['status'], model_update['status']) mock_delete_volume.assert_called_with(self.driver, mock_volume) @@ -725,7 +727,9 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): code=400, text="Protection group has been destroyed." ) - self.driver.delete_consistencygroup(mock_context, mock_cgroup, []) + self.driver.delete_consistencygroup(mock_context, + mock_cgroup, + [mock_volume]) self.array.destroy_pgroup.assert_called_with(expected_name) mock_delete_volume.assert_called_with(self.driver, mock_volume) @@ -734,7 +738,9 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): code=400, text="Protection group does not exist" ) - self.driver.delete_consistencygroup(mock_context, mock_cgroup, []) + self.driver.delete_consistencygroup(mock_context, + mock_cgroup, + [mock_volume]) self.array.destroy_pgroup.assert_called_with(expected_name) mock_delete_volume.assert_called_with(self.driver, mock_volume) @@ -747,7 +753,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.driver.delete_consistencygroup, mock_context, mock_volume, - []) + [mock_volume]) self.array.destroy_pgroup.side_effect = \ self.purestorage_module.PureHTTPError( @@ -758,12 +764,16 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.driver.delete_consistencygroup, mock_context, mock_volume, - []) + [mock_volume]) self.array.destroy_pgroup.side_effect = None self.assert_error_propagates( [self.array.destroy_pgroup], - self.driver.delete_consistencygroup, mock_context, mock_cgroup, []) + self.driver.delete_consistencygroup, + mock_context, + mock_cgroup, + [mock_volume] + ) def _create_mock_cg(self): mock_group = mock.MagicMock() @@ -836,20 +846,17 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): remvollist=[] ) - @mock.patch('cinder.objects.snapshot.SnapshotList.get_all_for_cgsnapshot') - def test_create_cgsnapshot(self, mock_snap_list): + def test_create_cgsnapshot(self): mock_cgsnap = mock.Mock() mock_cgsnap.id = "4a2f7e3a-312a-40c5-96a8-536b8a0fe074" mock_cgsnap.consistencygroup_id = \ "4a2f7e3a-312a-40c5-96a8-536b8a0fe075" mock_context = mock.Mock() mock_snap = mock.MagicMock() - expected_snaps = [mock_snap] - mock_snap_list.return_value = expected_snaps - - model_update, snapshots = \ - self.driver.create_cgsnapshot(mock_context, mock_cgsnap, []) + model_update, snapshots = self.driver.create_cgsnapshot(mock_context, + mock_cgsnap, + [mock_snap]) cg_id = mock_cgsnap.consistencygroup_id expected_pgroup_name = self.driver._get_pgroup_name_from_id(cg_id) expected_snap_suffix = self.driver._get_pgroup_snap_suffix(mock_cgsnap) @@ -857,8 +864,12 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): .assert_called_with(expected_pgroup_name, suffix=expected_snap_suffix) self.assertEqual({'status': 'available'}, model_update) - self.assertEqual(expected_snaps, snapshots) - self.assertEqual('available', mock_snap.status) + + expected_snapshot_update = [{ + 'id': mock_snap.id, + 'status': 'available' + }] + self.assertEqual(expected_snapshot_update, snapshots) self.assert_error_propagates( [self.array.create_pgroup_snapshot], @@ -866,8 +877,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): @mock.patch(BASE_DRIVER_OBJ + "._get_pgroup_snap_name", spec=pure.PureBaseVolumeDriver._get_pgroup_snap_name) - @mock.patch('cinder.objects.snapshot.SnapshotList.get_all_for_cgsnapshot') - def test_delete_cgsnapshot(self, mock_snap_list, mock_get_snap_name): + def test_delete_cgsnapshot(self, mock_get_snap_name): snap_name = "consisgroup-4a2f7e3a-312a-40c5-96a8-536b8a0f" \ "e074-cinder.4a2f7e3a-312a-40c5-96a8-536b8a0fe075" mock_get_snap_name.return_value = snap_name @@ -875,23 +885,26 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): mock_cgsnap.status = 'deleted' mock_context = mock.Mock() mock_snap = mock.Mock() - expected_snaps = [mock_snap] - mock_snap_list.return_value = expected_snaps - model_update, snapshots = \ - self.driver.delete_cgsnapshot(mock_context, mock_cgsnap, []) + model_update, snapshots = self.driver.delete_cgsnapshot(mock_context, + mock_cgsnap, + [mock_snap]) self.array.destroy_pgroup.assert_called_with(snap_name) self.assertEqual({'status': mock_cgsnap.status}, model_update) - self.assertEqual(expected_snaps, snapshots) - self.assertEqual('deleted', mock_snap.status) + + expected_snapshot_update = [{ + 'id': mock_snap.id, + 'status': 'deleted' + }] + self.assertEqual(expected_snapshot_update, snapshots) self.array.destroy_pgroup.side_effect = \ self.purestorage_module.PureHTTPError( code=400, text="Protection group snapshot has been destroyed." ) - self.driver.delete_cgsnapshot(mock_context, mock_cgsnap, []) + self.driver.delete_cgsnapshot(mock_context, mock_cgsnap, [mock_snap]) self.array.destroy_pgroup.assert_called_with(snap_name) self.array.destroy_pgroup.side_effect = \ @@ -899,7 +912,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): code=400, text="Protection group snapshot does not exist" ) - self.driver.delete_cgsnapshot(mock_context, mock_cgsnap, []) + self.driver.delete_cgsnapshot(mock_context, mock_cgsnap, [mock_snap]) self.array.destroy_pgroup.assert_called_with(snap_name) self.array.destroy_pgroup.side_effect = \ @@ -911,7 +924,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.driver.delete_cgsnapshot, mock_context, mock_cgsnap, - []) + [mock_snap]) self.array.destroy_pgroup.side_effect = \ self.purestorage_module.PureHTTPError( @@ -922,13 +935,17 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.driver.delete_cgsnapshot, mock_context, mock_cgsnap, - []) + [mock_snap]) self.array.destroy_pgroup.side_effect = None self.assert_error_propagates( [self.array.destroy_pgroup], - self.driver.delete_cgsnapshot, mock_context, mock_cgsnap, []) + self.driver.delete_cgsnapshot, + mock_context, + mock_cgsnap, + [mock_snap] + ) def test_manage_existing(self): ref_name = 'vol1' diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 5c1c9ff1d..04e023515 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -30,7 +30,6 @@ from oslo_utils import units from cinder import context from cinder import exception from cinder.i18n import _, _LE, _LI, _LW -from cinder import objects from cinder.objects import fields from cinder import utils from cinder.volume import driver @@ -483,15 +482,17 @@ class PureBaseVolumeDriver(san.SanDriver): LOG.warning(_LW("Unable to delete Protection Group: %s"), err.text) - volumes = self.db.volume_get_all_by_group(context, group.id) - + volume_updates = [] for volume in volumes: self.delete_volume(volume) - volume.status = 'deleted' + volume_updates.append({ + 'id': volume.id, + 'status': 'deleted' + }) model_update = {'status': group['status']} - return model_update, volumes + return model_update, volume_updates @log_debug_trace def update_consistencygroup(self, context, group, @@ -522,15 +523,16 @@ class PureBaseVolumeDriver(san.SanDriver): pgsnap_suffix = self._get_pgroup_snap_suffix(cgsnapshot) self._array.create_pgroup_snapshot(pgroup_name, suffix=pgsnap_suffix) - snapshots = objects.SnapshotList().get_all_for_cgsnapshot( - context, cgsnapshot.id) - + snapshot_updates = [] for snapshot in snapshots: - snapshot.status = 'available' + snapshot_updates.append({ + 'id': snapshot.id, + 'status': 'available' + }) model_update = {'status': 'available'} - return model_update, snapshots + return model_update, snapshot_updates def _delete_pgsnapshot(self, pgsnap_name): try: @@ -555,15 +557,16 @@ class PureBaseVolumeDriver(san.SanDriver): pgsnap_name = self._get_pgroup_snap_name(cgsnapshot) self._delete_pgsnapshot(pgsnap_name) - snapshots = objects.SnapshotList.get_all_for_cgsnapshot( - context, cgsnapshot.id) - + snapshot_updates = [] for snapshot in snapshots: - snapshot.status = 'deleted' + snapshot_updates.append({ + 'id': snapshot.id, + 'status': 'deleted', + }) model_update = {'status': cgsnapshot.status} - return model_update, snapshots + return model_update, snapshot_updates def _validate_manage_existing_ref(self, existing_ref, is_snap=False): """Ensure that an existing_ref is valid and return volume info