diff options
-rw-r--r-- | nova/compute/api.py | 75 | ||||
-rw-r--r-- | nova/tests/unit/api/openstack/compute/test_server_actions.py | 12 | ||||
-rw-r--r-- | nova/tests/unit/compute/test_compute_api.py | 35 |
3 files changed, 92 insertions, 30 deletions
diff --git a/nova/compute/api.py b/nova/compute/api.py index 6dfa6baad8..fe8c7af802 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2855,6 +2855,40 @@ class API(base.Base): if instance.root_device_name: properties['root_device_name'] = instance.root_device_name + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, instance.uuid) + + mapping = [] # list of BDM dicts that can go into the image properties + # Do some up-front filtering of the list of BDMs from + # which we are going to create snapshots. + volume_bdms = [] + for bdm in bdms: + if bdm.no_device: + continue + if bdm.is_volume: + # These will be handled below. + volume_bdms.append(bdm) + else: + mapping.append(bdm.get_image_mapping()) + + # Check limits in Cinder before creating snapshots to avoid going over + # quota in the middle of a list of volumes. This is a best-effort check + # but concurrently running snapshot requests from the same project + # could still fail to create volume snapshots if they go over limit. + if volume_bdms: + limits = self.volume_api.get_absolute_limits(context) + total_snapshots_used = limits['totalSnapshotsUsed'] + max_snapshots = limits['maxTotalSnapshots'] + # -1 means there is unlimited quota for snapshots + if (max_snapshots > -1 and + len(volume_bdms) + total_snapshots_used > max_snapshots): + LOG.debug('Unable to create volume snapshots for instance. ' + 'Currently has %s snapshots, requesting %s new ' + 'snapshots, with a limit of %s.', + total_snapshots_used, len(volume_bdms), + max_snapshots, instance=instance) + raise exception.OverQuota(overs='snapshots') + quiesced = False if instance.vm_state == vm_states.ACTIVE: try: @@ -2873,35 +2907,24 @@ class API(base.Base): {'reason': err}, instance=instance) - bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( - context, instance.uuid) - @wrap_instance_event(prefix='api') def snapshot_instance(self, context, instance, bdms): - mapping = [] try: - for bdm in bdms: - if bdm.no_device: - continue - - if bdm.is_volume: - # create snapshot based on volume_id - volume = self.volume_api.get(context, bdm.volume_id) - # NOTE(yamahata): Should we wait for snapshot creation? - # Linux LVM snapshot creation completes in short time, - # it doesn't matter for now. - name = _('snapshot for %s') % image_meta['name'] - LOG.debug('Creating snapshot from volume %s.', - volume['id'], instance=instance) - snapshot = self.volume_api.create_snapshot_force( - context, volume['id'], - name, volume['display_description']) - mapping_dict = block_device.snapshot_from_bdm( - snapshot['id'], bdm) - mapping_dict = mapping_dict.get_image_mapping() - else: - mapping_dict = bdm.get_image_mapping() - + for bdm in volume_bdms: + # create snapshot based on volume_id + volume = self.volume_api.get(context, bdm.volume_id) + # NOTE(yamahata): Should we wait for snapshot creation? + # Linux LVM snapshot creation completes in + # short time, it doesn't matter for now. + name = _('snapshot for %s') % image_meta['name'] + LOG.debug('Creating snapshot from volume %s.', + volume['id'], instance=instance) + snapshot = self.volume_api.create_snapshot_force( + context, volume['id'], + name, volume['display_description']) + mapping_dict = block_device.snapshot_from_bdm( + snapshot['id'], bdm) + mapping_dict = mapping_dict.get_image_mapping() mapping.append(mapping_dict) return mapping # NOTE(tasker): No error handling is done in the above for loop. diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index dd590a5023..5b846ca12e 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -982,6 +982,10 @@ class ServerActionsControllerTestV21(test.TestCase): snapshot = dict(id=_fake_id('d')) with test.nested( + mock.patch.object( + self.controller.compute_api.volume_api, 'get_absolute_limits', + return_value={'totalSnapshotsUsed': 0, + 'maxTotalSnapshots': 10}), mock.patch.object(self.controller.compute_api.compute_rpcapi, 'quiesce_instance', side_effect=exception.InstanceQuiesceNotSupported( @@ -991,7 +995,7 @@ class ServerActionsControllerTestV21(test.TestCase): mock.patch.object(self.controller.compute_api.volume_api, 'create_snapshot_force', return_value=snapshot), - ) as (mock_quiesce, mock_vol_get, mock_vol_create): + ) as (mock_get_limits, mock_quiesce, mock_vol_get, mock_vol_create): if mock_vol_create_side_effect: mock_vol_create.side_effect = mock_vol_create_side_effect @@ -1086,6 +1090,10 @@ class ServerActionsControllerTestV21(test.TestCase): snapshot = dict(id=_fake_id('d')) with test.nested( + mock.patch.object( + self.controller.compute_api.volume_api, 'get_absolute_limits', + return_value={'totalSnapshotsUsed': 0, + 'maxTotalSnapshots': 10}), mock.patch.object(self.controller.compute_api.compute_rpcapi, 'quiesce_instance', side_effect=exception.InstanceQuiesceNotSupported( @@ -1095,7 +1103,7 @@ class ServerActionsControllerTestV21(test.TestCase): mock.patch.object(self.controller.compute_api.volume_api, 'create_snapshot_force', return_value=snapshot), - ) as (mock_quiesce, mock_vol_get, mock_vol_create): + ) as (mock_get_limits, mock_quiesce, mock_vol_get, mock_vol_create): response = self.controller._action_create_image(self.req, FAKE_UUID, body=body) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 9c67da19fe..ae6ff253f9 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3044,7 +3044,7 @@ class _ComputeAPIUnitTestMixIn(object): def _test_snapshot_volume_backed(self, quiesce_required, quiesce_fails, vm_state=vm_states.ACTIVE, - snapshot_fails=False): + snapshot_fails=False, limits=None): fake_sys_meta = {'image_min_ram': '11', 'image_min_disk': '22', 'image_container_format': 'ami', @@ -3103,6 +3103,11 @@ class _ComputeAPIUnitTestMixIn(object): def fake_unquiesce_instance(context, instance, mapping=None): quiesced[1] = True + def fake_get_absolute_limits(context): + if limits is not None: + return limits + return {"totalSnapshotsUsed": 0, "maxTotalSnapshots": 10} + self.stub_out('nova.objects.BlockDeviceMappingList' '.get_by_instance_uuid', fake_bdm_list_get_by_instance_uuid) @@ -3151,6 +3156,12 @@ class _ComputeAPIUnitTestMixIn(object): 'destination_type': 'volume', 'delete_on_termination': False, 'tag': None}) + limits_patcher = mock.patch.object( + self.compute_api.volume_api, 'get_absolute_limits', + side_effect=fake_get_absolute_limits) + limits_patcher.start() + self.addCleanup(limits_patcher.stop) + with test.nested( mock.patch.object(compute_api.API, '_record_action_start'), mock.patch.object(compute_utils, 'EventReporter')) as ( @@ -3195,7 +3206,9 @@ class _ComputeAPIUnitTestMixIn(object): 'guest_format': 'swap', 'delete_on_termination': True, 'tag': None}) instance_bdms.append(bdm) - expect_meta['properties']['block_device_mapping'].append( + # The non-volume image mapping will go at the front of the list + # because the volume BDMs are processed separately. + expect_meta['properties']['block_device_mapping'].insert(0, {'guest_format': 'swap', 'boot_index': -1, 'no_device': False, 'image_id': None, 'volume_id': None, 'disk_bus': None, 'volume_size': None, 'source_type': 'blank', @@ -3241,6 +3254,24 @@ class _ComputeAPIUnitTestMixIn(object): quiesce_fails=False, snapshot_fails=True) + def test_snapshot_volume_backed_unlimited_quota(self): + """Tests that there is unlimited quota on volume snapshots so we + don't perform a quota check. + """ + limits = {'maxTotalSnapshots': -1, 'totalSnapshotsUsed': 0} + self._test_snapshot_volume_backed( + quiesce_required=False, quiesce_fails=False, limits=limits) + + def test_snapshot_volume_backed_over_quota_before_snapshot(self): + """Tests that the up-front check on quota fails before actually + attempting to snapshot any volumes. + """ + limits = {'maxTotalSnapshots': 1, 'totalSnapshotsUsed': 1} + self.assertRaises(exception.OverQuota, + self._test_snapshot_volume_backed, + quiesce_required=False, quiesce_fails=False, + limits=limits) + def test_snapshot_volume_backed_with_quiesce_skipped(self): self._test_snapshot_volume_backed(False, True) |