diff --git a/nova/compute/api.py b/nova/compute/api.py index ae8a6eca357b..9f35dea42f84 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -315,111 +315,6 @@ class API(base.Base): else: raise exception.OnsetFileContentLimitExceeded() - def _get_headroom(self, quotas, usages, deltas): - headroom = {res: quotas[res] - - (usages[res]['in_use'] + usages[res]['reserved']) - for res in quotas.keys()} - # If quota_cores is unlimited [-1]: - # - set cores headroom based on instances headroom: - if quotas.get('cores') == -1: - if deltas.get('cores'): - hc = headroom.get('instances', 1) * deltas['cores'] - headroom['cores'] = hc / deltas.get('instances', 1) - else: - headroom['cores'] = headroom.get('instances', 1) - - # If quota_ram is unlimited [-1]: - # - set ram headroom based on instances headroom: - if quotas.get('ram') == -1: - if deltas.get('ram'): - hr = headroom.get('instances', 1) * deltas['ram'] - headroom['ram'] = hr / deltas.get('instances', 1) - else: - headroom['ram'] = headroom.get('instances', 1) - - return headroom - - def _check_num_instances_quota(self, context, instance_type, min_count, - max_count, project_id=None, user_id=None): - """Enforce quota limits on number of instances created.""" - - # Determine requested cores and ram - req_cores = max_count * instance_type['vcpus'] - req_ram = max_count * instance_type['memory_mb'] - - # Check the quota - try: - quotas = objects.Quotas(context=context) - quotas.reserve(instances=max_count, - cores=req_cores, ram=req_ram, - project_id=project_id, user_id=user_id) - except exception.OverQuota as exc: - # OK, we exceeded quota; let's figure out why... - quotas = exc.kwargs['quotas'] - overs = exc.kwargs['overs'] - usages = exc.kwargs['usages'] - deltas = {'instances': max_count, - 'cores': req_cores, 'ram': req_ram} - headroom = self._get_headroom(quotas, usages, deltas) - - allowed = headroom.get('instances', 1) - # Reduce 'allowed' instances in line with the cores & ram headroom - if instance_type['vcpus']: - allowed = min(allowed, - headroom['cores'] // instance_type['vcpus']) - if instance_type['memory_mb']: - allowed = min(allowed, - headroom['ram'] // instance_type['memory_mb']) - - # Convert to the appropriate exception message - if allowed <= 0: - msg = _("Cannot run any more instances of this type.") - elif min_count <= allowed <= max_count: - # We're actually OK, but still need reservations - return self._check_num_instances_quota(context, instance_type, - min_count, allowed) - else: - msg = (_("Can only run %s more instances of this type.") % - allowed) - - num_instances = (str(min_count) if min_count == max_count else - "%s-%s" % (min_count, max_count)) - requested = dict(instances=num_instances, cores=req_cores, - ram=req_ram) - (overs, reqs, total_alloweds, useds) = self._get_over_quota_detail( - headroom, overs, quotas, requested) - params = {'overs': overs, 'pid': context.project_id, - 'min_count': min_count, 'max_count': max_count, - 'msg': msg} - - if min_count == max_count: - LOG.debug(("%(overs)s quota exceeded for %(pid)s," - " tried to run %(min_count)d instances. " - "%(msg)s"), params) - else: - LOG.debug(("%(overs)s quota exceeded for %(pid)s," - " tried to run between %(min_count)d and" - " %(max_count)d instances. %(msg)s"), - params) - raise exception.TooManyInstances(overs=overs, - req=reqs, - used=useds, - allowed=total_alloweds) - - return max_count, quotas - - def _get_over_quota_detail(self, headroom, overs, quotas, requested): - reqs = [] - useds = [] - total_alloweds = [] - for resource in overs: - reqs.append(str(requested[resource])) - useds.append(str(quotas[resource] - headroom[resource])) - total_alloweds.append(str(quotas[resource])) - (overs, reqs, useds, total_alloweds) = map(', '.join, ( - overs, reqs, useds, total_alloweds)) - return overs, reqs, total_alloweds, useds - def _check_metadata_properties_quota(self, context, metadata=None): """Enforce quota limits on metadata properties.""" if not metadata: @@ -987,8 +882,8 @@ class API(base.Base): block_device_mapping, shutdown_terminate, instance_group, check_server_group_quota, filter_properties, key_pair, tags): - # Reserve quotas - num_instances, quotas = self._check_num_instances_quota( + # Check quotas + num_instances = compute_utils.check_num_instances_quota( context, instance_type, min_count, max_count) security_groups = self.security_group_api.populate_security_groups( security_groups) @@ -1086,29 +981,10 @@ class API(base.Base): # instance instance_group.members.extend(members) - # In the case of any exceptions, attempt DB cleanup and rollback the - # quota reservations. + # In the case of any exceptions, attempt DB cleanup except Exception: with excutils.save_and_reraise_exception(): - try: - for rs, br, im in instances_to_build: - try: - rs.destroy() - except exception.RequestSpecNotFound: - pass - try: - im.destroy() - except exception.InstanceMappingNotFound: - pass - try: - br.destroy() - except exception.BuildRequestNotFound: - pass - finally: - quotas.rollback() - - # Commit the reservations - quotas.commit() + self._cleanup_build_artifacts(None, instances_to_build) return instances_to_build @@ -1263,6 +1139,24 @@ class API(base.Base): # we stop supporting v1. for instance in instances: instance.create() + # NOTE(melwitt): We recheck the quota after creating the objects + # to prevent users from allocating more resources than their + # allowed quota in the event of a race. This is configurable + # because it can be expensive if strict quota limits are not + # required in a deployment. + if CONF.quota.recheck_quota: + try: + compute_utils.check_num_instances_quota( + context, instance_type, 0, 0, + orig_num_req=len(instances)) + except exception.TooManyInstances: + with excutils.save_and_reraise_exception(): + # Need to clean up all the instances we created + # along with the build requests, request specs, + # and instance mappings. + self._cleanup_build_artifacts(instances, + instances_to_build) + self.compute_task_api.build_instances(context, instances=instances, image=boot_meta, filter_properties=filter_properties, @@ -1286,6 +1180,31 @@ class API(base.Base): return (instances, reservation_id) + @staticmethod + def _cleanup_build_artifacts(instances, instances_to_build): + # instances_to_build is a list of tuples: + # (RequestSpec, BuildRequest, InstanceMapping) + + # Be paranoid about artifacts being deleted underneath us. + for instance in instances or []: + try: + instance.destroy() + except exception.InstanceNotFound: + pass + for rs, build_request, im in instances_to_build or []: + try: + rs.destroy() + except exception.RequestSpecNotFound: + pass + try: + build_request.destroy() + except exception.BuildRequestNotFound: + pass + try: + im.destroy() + except exception.InstanceMappingNotFound: + pass + @staticmethod def _volume_size(instance_type, bdm): size = bdm.get('volume_size') @@ -1815,26 +1734,17 @@ class API(base.Base): # buildrequest indicates that the build process should be halted by # the conductor. - # Since conductor has halted the build process no cleanup of the - # instance is necessary, but quotas must still be decremented. - project_id, user_id = quotas_obj.ids_from_instance( - context, instance) - # This is confusing but actually decrements quota. - quotas = self._create_reservations(context, - instance, - instance.task_state, - project_id, user_id) + # NOTE(alaski): Though the conductor halts the build process it + # does not currently delete the instance record. This is + # because in the near future the instance record will not be + # created if the buildrequest has been deleted here. For now we + # ensure the instance has been set to deleted at this point. + # Yes this directly contradicts the comment earlier in this + # method, but this is a temporary measure. + # Look up the instance because the current instance object was + # stashed on the buildrequest and therefore not complete enough + # to run .destroy(). try: - # NOTE(alaski): Though the conductor halts the build process it - # does not currently delete the instance record. This is - # because in the near future the instance record will not be - # created if the buildrequest has been deleted here. For now we - # ensure the instance has been set to deleted at this point. - # Yes this directly contradicts the comment earlier in this - # method, but this is a temporary measure. - # Look up the instance because the current instance object was - # stashed on the buildrequest and therefore not complete enough - # to run .destroy(). instance_uuid = instance.uuid cell, instance = self._lookup_instance(context, instance_uuid) if instance is not None: @@ -1848,13 +1758,8 @@ class API(base.Base): instance.destroy() else: instance.destroy() - quotas.commit() - else: - # The instance is already deleted so rollback the quota - # usage decrement reservation in the not found block below. - raise exception.InstanceNotFound(instance_id=instance_uuid) except exception.InstanceNotFound: - quotas.rollback() + pass return True return False @@ -1898,41 +1803,13 @@ class API(base.Base): # acceptable to skip the rest of the delete processing. cell, instance = self._lookup_instance(context, instance.uuid) if cell and instance: - # Conductor may have buried the instance in cell0 but - # quotas must still be decremented in the main cell DB. - project_id, user_id = quotas_obj.ids_from_instance( - context, instance) - - # TODO(mriedem): This is a hack until we have quotas in the - # API database. When we looked up the instance in - # _get_instance if the instance has a mapping then the - # context is modified to set the target cell permanently. - # However, if the instance is in cell0 then the context - # is targeting cell0 and the quotas would be decremented - # from cell0 and we actually need them decremented from - # the cell database. So we temporarily untarget the - # context while we do the quota stuff and re-target after - # we're done. - - # We have to get the flavor from the instance while the - # context is still targeted to where the instance lives. - quota_flavor = self._get_flavor_for_reservation(instance) - - with nova_context.target_cell(context, None) as cctxt: - # This is confusing but actually decrements quota usage - quotas = self._create_reservations( - cctxt, instance, instance.task_state, - project_id, user_id, flavor=quota_flavor) - try: # Now destroy the instance from the cell it lives in. with compute_utils.notify_about_instance_delete( self.notifier, context, instance): instance.destroy() - # Now commit the quota reservation to decrement usage. - quotas.commit() except exception.InstanceNotFound: - quotas.rollback() + pass # The instance was deleted or is already gone. return if not instance: @@ -1954,8 +1831,6 @@ class API(base.Base): bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) - project_id, user_id = quotas_obj.ids_from_instance(context, instance) - # At these states an instance has a snapshot associate. if instance.vm_state in (vm_states.SHELVED, vm_states.SHELVED_OFFLOADED): @@ -1976,21 +1851,12 @@ class API(base.Base): instance=instance) original_task_state = instance.task_state - quotas = None try: # NOTE(maoy): no expected_task_state needs to be set instance.update(instance_attrs) instance.progress = 0 instance.save() - # NOTE(comstud): If we delete the instance locally, we'll - # commit the reservations here. Otherwise, the manager side - # will commit or rollback the reservations based on success. - quotas = self._create_reservations(context, - instance, - original_task_state, - project_id, user_id) - # NOTE(dtp): cells.enable = False means "use cells v2". # Run everywhere except v1 compute cells. if not CONF.cells.enable or self.cell_type == 'api': @@ -2000,11 +1866,8 @@ class API(base.Base): if self.cell_type == 'api': # NOTE(comstud): If we're in the API cell, we need to # skip all remaining logic and just call the callback, - # which will cause a cast to the child cell. Also, - # commit reservations here early until we have a better - # way to deal with quotas with cells. - cb(context, instance, bdms, reservations=None) - quotas.commit() + # which will cause a cast to the child cell. + cb(context, instance, bdms) return shelved_offloaded = (instance.vm_state == vm_states.SHELVED_OFFLOADED) @@ -2018,7 +1881,6 @@ class API(base.Base): self.notifier, context, instance, "%s.end" % delete_type, system_metadata=instance.system_metadata) - quotas.commit() LOG.info('Instance deleted and does not have host ' 'field, its vm_state is %(state)s.', {'state': instance.vm_state}, @@ -2043,21 +1905,11 @@ class API(base.Base): LOG.info('Instance is already in deleting state, ' 'ignoring this request', instance=instance) - quotas.rollback() return self._record_action_start(context, instance, instance_actions.DELETE) - # NOTE(snikitin): If instance's vm_state is 'soft-delete', - # we should not count reservations here, because instance - # in soft-delete vm_state have already had quotas - # decremented. More details: - # https://bugs.launchpad.net/nova/+bug/1333145 - if instance.vm_state == vm_states.SOFT_DELETED: - quotas.rollback() - - cb(context, instance, bdms, - reservations=quotas.reservations) + cb(context, instance, bdms) except exception.ComputeHostNotFound: pass @@ -2081,16 +1933,10 @@ class API(base.Base): instance=instance) with nova_context.target_cell(context, cell) as cctxt: self._local_delete(cctxt, instance, bdms, delete_type, cb) - quotas.commit() except exception.InstanceNotFound: # NOTE(comstud): Race condition. Instance already gone. - if quotas: - quotas.rollback() - except Exception: - with excutils.save_and_reraise_exception(): - if quotas: - quotas.rollback() + pass def _confirm_resize_on_deleting(self, context, instance): # If in the middle of a resize, use confirm_resize to @@ -2115,65 +1961,12 @@ class API(base.Base): return src_host = migration.source_compute - # Call since this can race with the terminate_instance. - # The resize is done but awaiting confirmation/reversion, - # so there are two cases: - # 1. up-resize: here -instance['vcpus'/'memory_mb'] match - # the quota usages accounted for this instance, - # so no further quota adjustment is needed - # 2. down-resize: here -instance['vcpus'/'memory_mb'] are - # shy by delta(old, new) from the quota usages accounted - # for this instance, so we must adjust - try: - deltas = compute_utils.downsize_quota_delta(context, instance) - except KeyError: - LOG.info('Migration %s may have been confirmed during delete', - migration.id, instance=instance) - return - quotas = compute_utils.reserve_quota_delta(context, deltas, instance) self._record_action_start(context, instance, instance_actions.CONFIRM_RESIZE) self.compute_rpcapi.confirm_resize(context, - instance, migration, - src_host, quotas.reservations, - cast=False) - - def _get_flavor_for_reservation(self, instance): - """Returns the flavor needed for _create_reservations. - - This is used when the context is targeted to a cell that is - different from the one that the instance lives in. - """ - if instance.task_state in (task_states.RESIZE_MIGRATED, - task_states.RESIZE_FINISH): - return instance.old_flavor - return instance.flavor - - def _create_reservations(self, context, instance, original_task_state, - project_id, user_id, flavor=None): - # NOTE(wangpan): if the instance is resizing, and the resources - # are updated to new instance type, we should use - # the old instance type to create reservation. - # see https://bugs.launchpad.net/nova/+bug/1099729 for more details - if original_task_state in (task_states.RESIZE_MIGRATED, - task_states.RESIZE_FINISH): - old_flavor = flavor or instance.old_flavor - instance_vcpus = old_flavor.vcpus - instance_memory_mb = old_flavor.memory_mb - else: - flavor = flavor or instance.flavor - instance_vcpus = flavor.vcpus - instance_memory_mb = flavor.memory_mb - - quotas = objects.Quotas(context=context) - quotas.reserve(project_id=project_id, - user_id=user_id, - instances=-1, - cores=-instance_vcpus, - ram=-instance_memory_mb) - return quotas + instance, migration, src_host, cast=False) def _get_stashed_volume_connector(self, bdm, instance): """Lookup a connector dict from the bdm.connection_info if set @@ -2277,8 +2070,7 @@ class API(base.Base): self.notifier, context, instance, "%s.end" % delete_type, system_metadata=sys_meta) - def _do_delete(self, context, instance, bdms, reservations=None, - local=False): + def _do_delete(self, context, instance, bdms, local=False): if local: instance.vm_state = vm_states.DELETED instance.task_state = None @@ -2286,11 +2078,9 @@ class API(base.Base): instance.save() else: self.compute_rpcapi.terminate_instance(context, instance, bdms, - reservations=reservations, delete_type='delete') - def _do_force_delete(self, context, instance, bdms, reservations=None, - local=False): + def _do_force_delete(self, context, instance, bdms, local=False): if local: instance.vm_state = vm_states.DELETED instance.task_state = None @@ -2298,19 +2088,16 @@ class API(base.Base): instance.save() else: self.compute_rpcapi.terminate_instance(context, instance, bdms, - reservations=reservations, delete_type='force_delete') - def _do_soft_delete(self, context, instance, bdms, reservations=None, - local=False): + def _do_soft_delete(self, context, instance, bdms, local=False): if local: instance.vm_state = vm_states.SOFT_DELETED instance.task_state = None instance.terminated_at = timeutils.utcnow() instance.save() else: - self.compute_rpcapi.soft_delete_instance(context, instance, - reservations=reservations) + self.compute_rpcapi.soft_delete_instance(context, instance) # NOTE(maoy): we allow delete to be called no matter what vm_state says. @check_instance_lock @@ -2343,31 +2130,28 @@ class API(base.Base): @check_instance_state(vm_state=[vm_states.SOFT_DELETED]) def restore(self, context, instance): """Restore a previously deleted (but not reclaimed) instance.""" - # Reserve quotas + # Check quotas flavor = instance.get_flavor() project_id, user_id = quotas_obj.ids_from_instance(context, instance) - num_instances, quotas = self._check_num_instances_quota( - context, flavor, 1, 1, + compute_utils.check_num_instances_quota(context, flavor, 1, 1, project_id=project_id, user_id=user_id) self._record_action_start(context, instance, instance_actions.RESTORE) - try: - if instance.host: - instance.task_state = task_states.RESTORING - instance.deleted_at = None - instance.save(expected_task_state=[None]) - self.compute_rpcapi.restore_instance(context, instance) - else: - instance.vm_state = vm_states.ACTIVE - instance.task_state = None - instance.deleted_at = None - instance.save(expected_task_state=[None]) - - quotas.commit() - except Exception: - with excutils.save_and_reraise_exception(): - quotas.rollback() + if instance.host: + instance.task_state = task_states.RESTORING + instance.deleted_at = None + instance.save(expected_task_state=[None]) + # TODO(melwitt): We're not rechecking for strict quota here to + # guard against going over quota during a race at this time because + # the resource consumption for this operation is written to the + # database by compute. + self.compute_rpcapi.restore_instance(context, instance) + else: + instance.vm_state = vm_states.ACTIVE + instance.task_state = None + instance.deleted_at = None + instance.save(expected_task_state=[None]) @check_instance_lock @check_instance_state(must_have_launched=False) @@ -3210,6 +2994,40 @@ class API(base.Base): request_spec=request_spec, kwargs=kwargs) + @staticmethod + def _check_quota_for_upsize(context, instance, current_flavor, new_flavor): + project_id, user_id = quotas_obj.ids_from_instance(context, + instance) + # Deltas will be empty if the resize is not an upsize. + deltas = compute_utils.upsize_quota_delta(context, new_flavor, + current_flavor) + if deltas: + try: + res_deltas = {'cores': deltas.get('cores', 0), + 'ram': deltas.get('ram', 0)} + objects.Quotas.check_deltas(context, res_deltas, + project_id, user_id=user_id, + check_project_id=project_id, + check_user_id=user_id) + except exception.OverQuota as exc: + quotas = exc.kwargs['quotas'] + overs = exc.kwargs['overs'] + usages = exc.kwargs['usages'] + headroom = compute_utils.get_headroom(quotas, usages, + deltas) + (overs, reqs, total_alloweds, + useds) = compute_utils.get_over_quota_detail(headroom, + overs, + quotas, + deltas) + LOG.warning("%(overs)s quota exceeded for %(pid)s," + " tried to resize instance.", + {'overs': overs, 'pid': context.project_id}) + raise exception.TooManyInstances(overs=overs, + req=reqs, + used=useds, + allowed=total_alloweds) + @check_instance_lock @check_instance_cell @check_instance_state(vm_state=[vm_states.RESIZED]) @@ -3219,31 +3037,26 @@ class API(base.Base): migration = objects.Migration.get_by_instance_and_status( elevated, instance.uuid, 'finished') - # reverse quota reservation for increased resource usage - deltas = compute_utils.reverse_upsize_quota_delta(context, instance) - quotas = compute_utils.reserve_quota_delta(context, deltas, instance) + # If this is a resize down, a revert might go over quota. + self._check_quota_for_upsize(context, instance, instance.flavor, + instance.old_flavor) instance.task_state = task_states.RESIZE_REVERTING - try: - instance.save(expected_task_state=[None]) - except Exception: - with excutils.save_and_reraise_exception(): - quotas.rollback() + instance.save(expected_task_state=[None]) migration.status = 'reverting' migration.save() - # With cells, the best we can do right now is commit the reservations - # immediately... - if CONF.cells.enable: - quotas.commit() self._record_action_start(context, instance, instance_actions.REVERT_RESIZE) + # TODO(melwitt): We're not rechecking for strict quota here to guard + # against going over quota during a race at this time because the + # resource consumption for this operation is written to the database + # by compute. self.compute_rpcapi.revert_resize(context, instance, migration, - migration.dest_compute, - quotas.reservations or []) + migration.dest_compute) @check_instance_lock @check_instance_cell @@ -3251,20 +3064,17 @@ class API(base.Base): def confirm_resize(self, context, instance, migration=None): """Confirms a migration/resize and deletes the 'old' instance.""" elevated = context.elevated() + # NOTE(melwitt): We're not checking quota here because there isn't a + # change in resource usage when confirming a resize. Resource + # consumption for resizes are written to the database by compute, so + # a confirm resize is just a clean up of the migration objects and a + # state change in compute. if migration is None: migration = objects.Migration.get_by_instance_and_status( elevated, instance.uuid, 'finished') - # reserve quota only for any decrease in resource usage - deltas = compute_utils.downsize_quota_delta(context, instance) - quotas = compute_utils.reserve_quota_delta(context, deltas, instance) - migration.status = 'confirming' migration.save() - # With cells, the best we can do right now is commit the reservations - # immediately... - if CONF.cells.enable: - quotas.commit() self._record_action_start(context, instance, instance_actions.CONFIRM_RESIZE) @@ -3272,19 +3082,15 @@ class API(base.Base): self.compute_rpcapi.confirm_resize(context, instance, migration, - migration.source_compute, - quotas.reservations or []) + migration.source_compute) @staticmethod - def _resize_cells_support(context, quotas, instance, + def _resize_cells_support(context, instance, current_instance_type, new_instance_type): """Special API cell logic for resize.""" - # With cells, the best we can do right now is commit the - # reservations immediately... - quotas.commit() # NOTE(johannes/comstud): The API cell needs a local migration - # record for later resize_confirm and resize_reverts to deal - # with quotas. We don't need source and/or destination + # record for later resize_confirm and resize_reverts. + # We don't need source and/or destination # information, just the old and new flavors. Status is set to # 'finished' since nothing else will update the status along # the way. @@ -3352,29 +3158,9 @@ class API(base.Base): # ensure there is sufficient headroom for upsizes if flavor_id: - deltas = compute_utils.upsize_quota_delta(context, - new_instance_type, - current_instance_type) - try: - quotas = compute_utils.reserve_quota_delta(context, deltas, - instance) - except exception.OverQuota as exc: - quotas = exc.kwargs['quotas'] - overs = exc.kwargs['overs'] - usages = exc.kwargs['usages'] - headroom = self._get_headroom(quotas, usages, deltas) - (overs, reqs, total_alloweds, - useds) = self._get_over_quota_detail(headroom, overs, quotas, - deltas) - LOG.warning("%(overs)s quota exceeded for %(pid)s," - " tried to resize instance.", - {'overs': overs, 'pid': context.project_id}) - raise exception.TooManyInstances(overs=overs, - req=reqs, - used=useds, - allowed=total_alloweds) - else: - quotas = objects.Quotas(context=context) + self._check_quota_for_upsize(context, instance, + current_instance_type, + new_instance_type) instance.task_state = task_states.RESIZE_PREP instance.progress = 0 @@ -3387,8 +3173,8 @@ class API(base.Base): filter_properties['ignore_hosts'].append(instance.host) if self.cell_type == 'api': - # Commit reservations early and create migration record. - self._resize_cells_support(context, quotas, instance, + # Create migration record. + self._resize_cells_support(context, instance, current_instance_type, new_instance_type) @@ -3412,11 +3198,14 @@ class API(base.Base): # to them, we need to support the old way request_spec = None + # TODO(melwitt): We're not rechecking for strict quota here to guard + # against going over quota during a race at this time because the + # resource consumption for this operation is written to the database + # by compute. scheduler_hint = {'filter_properties': filter_properties} self.compute_task_api.resize_instance(context, instance, extra_instance_updates, scheduler_hint=scheduler_hint, flavor=new_instance_type, - reservations=quotas.reservations or [], clean_shutdown=clean_shutdown, request_spec=request_spec) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 07ddf321fee2..09d0923192c2 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -697,23 +697,13 @@ class ComputeManager(manager.Manager): instance.destroy() bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) - quotas = objects.Quotas(context=context) - project_id, user_id = objects.quotas.ids_from_instance(context, - instance) - quotas.reserve(project_id=project_id, user_id=user_id, instances=-1, - cores=-instance.flavor.vcpus, - ram=-instance.flavor.memory_mb) self._complete_deletion(context, instance, bdms, - quotas, system_meta) def _complete_deletion(self, context, instance, bdms, - quotas, system_meta): - if quotas: - quotas.commit() - + system_meta): # ensure block device mappings are not leaked for bdm in bdms: bdm.destroy() @@ -726,18 +716,6 @@ class ComputeManager(manager.Manager): phase=fields.NotificationPhase.END) self._delete_scheduler_instance_info(context, instance.uuid) - def _create_reservations(self, context, instance, project_id, user_id): - vcpus = instance.flavor.vcpus - mem_mb = instance.flavor.memory_mb - - quotas = objects.Quotas(context=context) - quotas.reserve(project_id=project_id, - user_id=user_id, - instances=-1, - cores=-vcpus, - ram=-mem_mb) - return quotas - def _init_instance(self, context, instance): '''Initialize this instance during service init.''' @@ -838,12 +816,7 @@ class ComputeManager(manager.Manager): instance.obj_load_attr('system_metadata') bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) - project_id, user_id = objects.quotas.ids_from_instance( - context, instance) - quotas = self._create_reservations(context, instance, - project_id, user_id) - - self._delete_instance(context, instance, bdms, quotas) + self._delete_instance(context, instance, bdms) except Exception: # we don't want that an exception blocks the init_host LOG.exception('Failed to complete a deletion', @@ -2351,78 +2324,62 @@ class ComputeManager(manager.Manager): six.reraise(exc_info[0], exc_info[1], exc_info[2]) @hooks.add_hook("delete_instance") - def _delete_instance(self, context, instance, bdms, quotas): - """Delete an instance on this host. Commit or rollback quotas - as necessary. + def _delete_instance(self, context, instance, bdms): + """Delete an instance on this host. :param context: nova request context :param instance: nova.objects.instance.Instance object :param bdms: nova.objects.block_device.BlockDeviceMappingList object - :param quotas: nova.objects.quotas.Quotas object """ - was_soft_deleted = instance.vm_state == vm_states.SOFT_DELETED - if was_soft_deleted: - # Instances in SOFT_DELETED vm_state have already had quotas - # decremented. - try: - quotas.rollback() - except Exception: - pass + events = self.instance_events.clear_events_for_instance(instance) + if events: + LOG.debug('Events pending at deletion: %(events)s', + {'events': ','.join(events.keys())}, + instance=instance) + self._notify_about_instance_usage(context, instance, + "delete.start") + compute_utils.notify_about_instance_action(context, instance, + self.host, action=fields.NotificationAction.DELETE, + phase=fields.NotificationPhase.START) - try: - events = self.instance_events.clear_events_for_instance(instance) - if events: - LOG.debug('Events pending at deletion: %(events)s', - {'events': ','.join(events.keys())}, - instance=instance) - self._notify_about_instance_usage(context, instance, - "delete.start") - compute_utils.notify_about_instance_action(context, instance, - self.host, action=fields.NotificationAction.DELETE, - phase=fields.NotificationPhase.START) + self._shutdown_instance(context, instance, bdms) + # NOTE(dims): instance.info_cache.delete() should be called after + # _shutdown_instance in the compute manager as shutdown calls + # deallocate_for_instance so the info_cache is still needed + # at this point. + if instance.info_cache is not None: + instance.info_cache.delete() + else: + # NOTE(yoshimatsu): Avoid AttributeError if instance.info_cache + # is None. When the root cause that instance.info_cache becomes + # None is fixed, the log level should be reconsidered. + LOG.warning("Info cache for instance could not be found. " + "Ignore.", instance=instance) - self._shutdown_instance(context, instance, bdms) - # NOTE(dims): instance.info_cache.delete() should be called after - # _shutdown_instance in the compute manager as shutdown calls - # deallocate_for_instance so the info_cache is still needed - # at this point. - if instance.info_cache is not None: - instance.info_cache.delete() - else: - # NOTE(yoshimatsu): Avoid AttributeError if instance.info_cache - # is None. When the root cause that instance.info_cache becomes - # None is fixed, the log level should be reconsidered. - LOG.warning("Info cache for instance could not be found. " - "Ignore.", instance=instance) - - # NOTE(vish): We have already deleted the instance, so we have - # to ignore problems cleaning up the volumes. It - # would be nice to let the user know somehow that - # the volume deletion failed, but it is not - # acceptable to have an instance that can not be - # deleted. Perhaps this could be reworked in the - # future to set an instance fault the first time - # and to only ignore the failure if the instance - # is already in ERROR. - self._cleanup_volumes(context, instance.uuid, bdms, - raise_exc=False) - # if a delete task succeeded, always update vm state and task - # state without expecting task state to be DELETING - instance.vm_state = vm_states.DELETED - instance.task_state = None - instance.power_state = power_state.NOSTATE - instance.terminated_at = timeutils.utcnow() - instance.save() - system_meta = instance.system_metadata - instance.destroy() - except Exception: - with excutils.save_and_reraise_exception(): - quotas.rollback() + # NOTE(vish): We have already deleted the instance, so we have + # to ignore problems cleaning up the volumes. It + # would be nice to let the user know somehow that + # the volume deletion failed, but it is not + # acceptable to have an instance that can not be + # deleted. Perhaps this could be reworked in the + # future to set an instance fault the first time + # and to only ignore the failure if the instance + # is already in ERROR. + self._cleanup_volumes(context, instance.uuid, bdms, + raise_exc=False) + # if a delete task succeeded, always update vm state and task + # state without expecting task state to be DELETING + instance.vm_state = vm_states.DELETED + instance.task_state = None + instance.power_state = power_state.NOSTATE + instance.terminated_at = timeutils.utcnow() + instance.save() + system_meta = instance.system_metadata + instance.destroy() self._complete_deletion(context, instance, bdms, - quotas, system_meta) @wrap_exception() @@ -2431,10 +2388,6 @@ class ComputeManager(manager.Manager): @wrap_instance_fault def terminate_instance(self, context, instance, bdms, reservations): """Terminate an instance on this host.""" - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - @utils.synchronized(instance.uuid) def do_terminate_instance(instance, bdms): # NOTE(mriedem): If we are deleting the instance while it was @@ -2454,7 +2407,7 @@ class ComputeManager(manager.Manager): context, instance.uuid) break try: - self._delete_instance(context, instance, bdms, quotas) + self._delete_instance(context, instance, bdms) except exception.InstanceNotFound: LOG.info("Instance disappeared during terminate", instance=instance) @@ -2607,30 +2560,21 @@ class ComputeManager(manager.Manager): @wrap_instance_fault def soft_delete_instance(self, context, instance, reservations): """Soft delete an instance on this host.""" - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) + self._notify_about_instance_usage(context, instance, + "soft_delete.start") + compute_utils.notify_about_instance_action(context, instance, + self.host, action=fields.NotificationAction.SOFT_DELETE, + phase=fields.NotificationPhase.START) try: - self._notify_about_instance_usage(context, instance, - "soft_delete.start") - compute_utils.notify_about_instance_action(context, instance, - self.host, action=fields.NotificationAction.SOFT_DELETE, - phase=fields.NotificationPhase.START) - try: - self.driver.soft_delete(instance) - except NotImplementedError: - # Fallback to just powering off the instance if the - # hypervisor doesn't implement the soft_delete method - self.driver.power_off(instance) - instance.power_state = self._get_power_state(context, instance) - instance.vm_state = vm_states.SOFT_DELETED - instance.task_state = None - instance.save(expected_task_state=[task_states.SOFT_DELETING]) - except Exception: - with excutils.save_and_reraise_exception(): - quotas.rollback() - quotas.commit() + self.driver.soft_delete(instance) + except NotImplementedError: + # Fallback to just powering off the instance if the + # hypervisor doesn't implement the soft_delete method + self.driver.power_off(instance) + instance.power_state = self._get_power_state(context, instance) + instance.vm_state = vm_states.SOFT_DELETED + instance.task_state = None + instance.save(expected_task_state=[task_states.SOFT_DELETING]) self._notify_about_instance_usage(context, instance, "soft_delete.end") compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.SOFT_DELETE, @@ -3504,11 +3448,6 @@ class ComputeManager(manager.Manager): @wrap_instance_event(prefix='compute') @wrap_instance_fault def confirm_resize(self, context, instance, reservations, migration): - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - @utils.synchronized(instance.uuid) def do_confirm_resize(context, instance, migration_id): # NOTE(wangpan): Get the migration status from db, if it has been @@ -3523,20 +3462,17 @@ class ComputeManager(manager.Manager): except exception.MigrationNotFound: LOG.error("Migration %s is not found during confirmation", migration_id, instance=instance) - quotas.rollback() return if migration.status == 'confirmed': LOG.info("Migration %s is already confirmed", migration_id, instance=instance) - quotas.rollback() return elif migration.status not in ('finished', 'confirming'): LOG.warning("Unexpected confirmation status '%(status)s' " "of migration %(id)s, exit confirmation process", {"status": migration.status, "id": migration_id}, instance=instance) - quotas.rollback() return # NOTE(wangpan): Get the instance from db, if it has been @@ -3549,22 +3485,18 @@ class ComputeManager(manager.Manager): except exception.InstanceNotFound: LOG.info("Instance is not found during confirmation", instance=instance) - quotas.rollback() return - self._confirm_resize(context, instance, quotas, - migration=migration) + self._confirm_resize(context, instance, migration=migration) do_confirm_resize(context, instance, migration.id) - def _confirm_resize(self, context, instance, quotas, - migration=None): + def _confirm_resize(self, context, instance, migration=None): """Destroys the source instance.""" self._notify_about_instance_usage(context, instance, "resize.confirm.start") - with self._error_out_instance_on_exception(context, instance, - quotas=quotas): + with self._error_out_instance_on_exception(context, instance): # NOTE(danms): delete stashed migration information old_instance_type = instance.old_flavor instance.old_flavor = None @@ -3614,8 +3546,6 @@ class ComputeManager(manager.Manager): context, instance, "resize.confirm.end", network_info=network_info) - quotas.commit() - @wrap_exception() @reverts_task_state @wrap_instance_event(prefix='compute') @@ -3628,18 +3558,12 @@ class ComputeManager(manager.Manager): source machine. """ - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - # NOTE(comstud): A revert_resize is essentially a resize back to # the old size, so we need to send a usage event here. compute_utils.notify_usage_exists(self.notifier, context, instance, current_period=True) - with self._error_out_instance_on_exception(context, instance, - quotas=quotas): + with self._error_out_instance_on_exception(context, instance): # NOTE(tr3buchet): tear down networks on destination host self.network_api.setup_networks_on_host(context, instance, teardown=True) @@ -3681,8 +3605,7 @@ class ComputeManager(manager.Manager): rt.drop_move_claim(context, instance, instance.node) self.compute_rpcapi.finish_revert_resize(context, instance, - migration, migration.source_compute, - quotas.reservations) + migration, migration.source_compute) @wrap_exception() @reverts_task_state @@ -3696,13 +3619,7 @@ class ComputeManager(manager.Manager): revert the resized attributes in the database. """ - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - - with self._error_out_instance_on_exception(context, instance, - quotas=quotas): + with self._error_out_instance_on_exception(context, instance): self._notify_about_instance_usage( context, instance, "resize.revert.start") @@ -3762,11 +3679,9 @@ class ComputeManager(manager.Manager): self._notify_about_instance_usage( context, instance, "resize.revert.end") - quotas.commit() def _prep_resize(self, context, image, instance, instance_type, - quotas, request_spec, filter_properties, node, - clean_shutdown=True): + request_spec, filter_properties, node, clean_shutdown=True): if not filter_properties: filter_properties = {} @@ -3801,8 +3716,7 @@ class ComputeManager(manager.Manager): LOG.info('Migrating', instance=instance) self.compute_rpcapi.resize_instance( context, instance, claim.migration, image, - instance_type, quotas.reservations, - clean_shutdown) + instance_type, clean_shutdown) @wrap_exception() @reverts_task_state @@ -3827,21 +3741,15 @@ class ComputeManager(manager.Manager): if not isinstance(instance_type, objects.Flavor): instance_type = objects.Flavor.get_by_id(context, instance_type['id']) - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - with self._error_out_instance_on_exception(context, instance, - quotas=quotas): + with self._error_out_instance_on_exception(context, instance): compute_utils.notify_usage_exists(self.notifier, context, instance, current_period=True) self._notify_about_instance_usage( context, instance, "resize.prep.start") try: self._prep_resize(context, image, instance, - instance_type, quotas, - request_spec, filter_properties, - node, clean_shutdown) + instance_type, request_spec, + filter_properties, node, clean_shutdown) # NOTE(dgenin): This is thrown in LibvirtDriver when the # instance to be migrated is backed by LVM. # Remove when LVM migration is implemented. @@ -3851,7 +3759,7 @@ class ComputeManager(manager.Manager): # try to re-schedule the resize elsewhere: exc_info = sys.exc_info() self._reschedule_resize_or_reraise(context, image, instance, - exc_info, instance_type, quotas, request_spec, + exc_info, instance_type, request_spec, filter_properties) finally: extra_usage_info = dict( @@ -3863,7 +3771,7 @@ class ComputeManager(manager.Manager): extra_usage_info=extra_usage_info) def _reschedule_resize_or_reraise(self, context, image, instance, exc_info, - instance_type, quotas, request_spec, filter_properties): + instance_type, request_spec, filter_properties): """Try to re-schedule the resize or re-raise the original error to error out the instance. """ @@ -3878,8 +3786,7 @@ class ComputeManager(manager.Manager): try: reschedule_method = self.compute_task_api.resize_instance scheduler_hint = dict(filter_properties=filter_properties) - method_args = (instance, None, scheduler_hint, instance_type, - quotas.reservations) + method_args = (instance, None, scheduler_hint, instance_type) task_state = task_states.RESIZE_PREP rescheduled = self._reschedule(context, request_spec, @@ -3914,12 +3821,7 @@ class ComputeManager(manager.Manager): reservations, migration, instance_type, clean_shutdown): """Starts the migration of a running instance to another host.""" - - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) - with self._error_out_instance_on_exception(context, instance, - quotas=quotas): + with self._error_out_instance_on_exception(context, instance): # TODO(chaochin) Remove this until v5 RPC API # Code downstream may expect extra_specs to be populated since it # is receiving an object, so lookup the flavor to ensure this. @@ -3975,8 +3877,7 @@ class ComputeManager(manager.Manager): instance.save(expected_task_state=task_states.RESIZE_MIGRATING) self.compute_rpcapi.finish_resize(context, instance, - migration, image, disk_info, - migration.dest_compute, reservations=quotas.reservations) + migration, image, disk_info, migration.dest_compute) self._notify_about_instance_usage(context, instance, "resize.end", network_info=network_info) @@ -4003,8 +3904,6 @@ class ComputeManager(manager.Manager): @staticmethod def _set_instance_info(instance, instance_type): instance.instance_type_id = instance_type.id - # NOTE(danms): These are purely for any legacy code that still - # looks at them. instance.memory_mb = instance_type.memory_mb instance.vcpus = instance_type.vcpus instance.root_gb = instance_type.root_gb @@ -4103,24 +4002,14 @@ class ComputeManager(manager.Manager): new host machine. """ - quotas = objects.Quotas.from_reservations(context, - reservations, - instance=instance) try: image_meta = objects.ImageMeta.from_dict(image) self._finish_resize(context, instance, migration, disk_info, image_meta) - quotas.commit() except Exception: LOG.exception('Setting instance vm_state to ERROR', instance=instance) with excutils.save_and_reraise_exception(): - try: - quotas.rollback() - except Exception: - LOG.exception("Failed to rollback quota for failed " - "finish_resize", - instance=instance) self._set_instance_obj_error_state(context, instance) @wrap_exception() @@ -6651,14 +6540,6 @@ class ComputeManager(manager.Manager): LOG.debug("CONF.reclaim_instance_interval <= 0, skipping...") return - # TODO(comstud, jichenjc): Dummy quota object for now See bug 1296414. - # The only case that the quota might be inconsistent is - # the compute node died between set instance state to SOFT_DELETED - # and quota commit to DB. When compute node starts again - # it will have no idea the reservation is committed or not or even - # expired, since it's a rare case, so marked as todo. - quotas = objects.Quotas.from_reservations(context, None) - filters = {'vm_state': vm_states.SOFT_DELETED, 'task_state': None, 'host': self.host} @@ -6672,7 +6553,7 @@ class ComputeManager(manager.Manager): context, instance.uuid) LOG.info('Reclaiming deleted instance', instance=instance) try: - self._delete_instance(context, instance, bdms, quotas) + self._delete_instance(context, instance, bdms) except Exception as e: LOG.warning("Periodic reclaim failed to delete " "instance: %s", @@ -6848,15 +6729,12 @@ class ComputeManager(manager.Manager): @contextlib.contextmanager def _error_out_instance_on_exception(self, context, instance, - quotas=None, instance_state=vm_states.ACTIVE): instance_uuid = instance.uuid try: yield except NotImplementedError as error: with excutils.save_and_reraise_exception(): - if quotas: - quotas.rollback() LOG.info("Setting instance back to %(state)s after: " "%(error)s", {'state': instance_state, 'error': error}, @@ -6865,8 +6743,6 @@ class ComputeManager(manager.Manager): vm_state=instance_state, task_state=None) except exception.InstanceFaultRollback as error: - if quotas: - quotas.rollback() LOG.info("Setting instance back to ACTIVE after: %s", error, instance_uuid=instance_uuid) self._instance_update(context, instance, @@ -6877,8 +6753,6 @@ class ComputeManager(manager.Manager): LOG.exception('Setting instance vm_state to ERROR', instance_uuid=instance_uuid) with excutils.save_and_reraise_exception(): - if quotas: - quotas.rollback() self._set_instance_obj_error_state(context, instance) @wrap_exception() diff --git a/nova/compute/utils.py b/nova/compute/utils.py index f2e3eb6ba0ae..f33e83220a4c 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -687,6 +687,132 @@ def reserve_quota_delta(context, deltas, instance): return quotas +def get_headroom(quotas, usages, deltas): + headroom = {res: quotas[res] - usages[res] + for res in quotas.keys()} + # If quota_cores is unlimited [-1]: + # - set cores headroom based on instances headroom: + if quotas.get('cores') == -1: + if deltas.get('cores'): + hc = headroom.get('instances', 1) * deltas['cores'] + headroom['cores'] = hc / deltas.get('instances', 1) + else: + headroom['cores'] = headroom.get('instances', 1) + + # If quota_ram is unlimited [-1]: + # - set ram headroom based on instances headroom: + if quotas.get('ram') == -1: + if deltas.get('ram'): + hr = headroom.get('instances', 1) * deltas['ram'] + headroom['ram'] = hr / deltas.get('instances', 1) + else: + headroom['ram'] = headroom.get('instances', 1) + + return headroom + + +def check_num_instances_quota(context, instance_type, min_count, + max_count, project_id=None, user_id=None, + orig_num_req=None): + """Enforce quota limits on number of instances created.""" + # project_id is used for the TooManyInstances error message + if project_id is None: + project_id = context.project_id + # Determine requested cores and ram + req_cores = max_count * instance_type.vcpus + req_ram = max_count * instance_type.memory_mb + deltas = {'instances': max_count, 'cores': req_cores, 'ram': req_ram} + + try: + objects.Quotas.check_deltas(context, deltas, + project_id, user_id=user_id, + check_project_id=project_id, + check_user_id=user_id) + except exception.OverQuota as exc: + quotas = exc.kwargs['quotas'] + overs = exc.kwargs['overs'] + usages = exc.kwargs['usages'] + # This is for the recheck quota case where we used a delta of zero. + if min_count == max_count == 0: + # orig_num_req is the original number of instances requested in the + # case of a recheck quota, for use in the over quota exception. + req_cores = orig_num_req * instance_type.vcpus + req_ram = orig_num_req * instance_type.memory_mb + requested = {'instances': orig_num_req, 'cores': req_cores, + 'ram': req_ram} + (overs, reqs, total_alloweds, useds) = get_over_quota_detail( + deltas, overs, quotas, requested) + msg = "Cannot run any more instances of this type." + params = {'overs': overs, 'pid': project_id, 'msg': msg} + LOG.debug("%(overs)s quota exceeded for %(pid)s. %(msg)s", + params) + raise exception.TooManyInstances(overs=overs, + req=reqs, + used=useds, + allowed=total_alloweds) + # OK, we exceeded quota; let's figure out why... + headroom = get_headroom(quotas, usages, deltas) + + allowed = headroom.get('instances', 1) + # Reduce 'allowed' instances in line with the cores & ram headroom + if instance_type.vcpus: + allowed = min(allowed, + headroom['cores'] // instance_type.vcpus) + if instance_type.memory_mb: + allowed = min(allowed, + headroom['ram'] // instance_type.memory_mb) + + # Convert to the appropriate exception message + if allowed <= 0: + msg = "Cannot run any more instances of this type." + elif min_count <= allowed <= max_count: + # We're actually OK, but still need to check against allowed + return check_num_instances_quota(context, instance_type, min_count, + allowed, project_id=project_id, + user_id=user_id) + else: + msg = "Can only run %s more instances of this type." % allowed + + num_instances = (str(min_count) if min_count == max_count else + "%s-%s" % (min_count, max_count)) + requested = dict(instances=num_instances, cores=req_cores, + ram=req_ram) + (overs, reqs, total_alloweds, useds) = get_over_quota_detail( + headroom, overs, quotas, requested) + params = {'overs': overs, 'pid': project_id, + 'min_count': min_count, 'max_count': max_count, + 'msg': msg} + + if min_count == max_count: + LOG.debug("%(overs)s quota exceeded for %(pid)s," + " tried to run %(min_count)d instances. " + "%(msg)s", params) + else: + LOG.debug("%(overs)s quota exceeded for %(pid)s," + " tried to run between %(min_count)d and" + " %(max_count)d instances. %(msg)s", + params) + raise exception.TooManyInstances(overs=overs, + req=reqs, + used=useds, + allowed=total_alloweds) + + return max_count + + +def get_over_quota_detail(headroom, overs, quotas, requested): + reqs = [] + useds = [] + total_alloweds = [] + for resource in overs: + reqs.append(str(requested[resource])) + useds.append(str(quotas[resource] - headroom[resource])) + total_alloweds.append(str(quotas[resource])) + (overs, reqs, useds, total_alloweds) = map(', '.join, ( + overs, reqs, useds, total_alloweds)) + return overs, reqs, total_alloweds, useds + + def remove_shelved_keys_from_system_metadata(instance): # Delete system_metadata for a shelved instance for key in ['shelved_at', 'shelved_image_id', 'shelved_host']: diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 3f92a731ae1e..55ff733a870a 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -925,15 +925,11 @@ class ComputeTaskManager(base.Base): return host_mapping_cache = {} + instances = [] for (build_request, request_spec, host) in six.moves.zip( build_requests, request_specs, hosts): - filter_props = request_spec.to_legacy_filter_properties_dict() instance = build_request.get_new_instance(context) - scheduler_utils.populate_retry(filter_props, instance.uuid) - scheduler_utils.populate_filter_properties(filter_props, - host) - # Convert host from the scheduler into a cell record if host['host'] not in host_mapping_cache: try: @@ -947,6 +943,8 @@ class ComputeTaskManager(base.Base): self._bury_in_cell0(context, request_spec, exc, build_requests=[build_request], instances=[instance]) + # This is a placeholder in case the quota recheck fails. + instances.append(None) continue else: host_mapping = host_mapping_cache[host['host']] @@ -963,6 +961,8 @@ class ComputeTaskManager(base.Base): # the build request is gone so we're done for this instance LOG.debug('While scheduling instance, the build request ' 'was already deleted.', instance=instance) + # This is a placeholder in case the quota recheck fails. + instances.append(None) continue else: instance.availability_zone = ( @@ -970,7 +970,34 @@ class ComputeTaskManager(base.Base): context, host['host'])) with obj_target_cell(instance, cell): instance.create() + instances.append(instance) + # NOTE(melwitt): We recheck the quota after creating the + # objects to prevent users from allocating more resources + # than their allowed quota in the event of a race. This is + # configurable because it can be expensive if strict quota + # limits are not required in a deployment. + if CONF.quota.recheck_quota: + try: + compute_utils.check_num_instances_quota( + context, instance.flavor, 0, 0, + orig_num_req=len(build_requests)) + except exception.TooManyInstances as exc: + with excutils.save_and_reraise_exception(): + self._cleanup_build_artifacts(context, exc, instances, + build_requests, + request_specs) + + for (build_request, request_spec, host, instance) in six.moves.zip( + build_requests, request_specs, hosts, instances): + if instance is None: + # Skip placeholders that were buried in cell0 or had their + # build requests deleted by the user before instance create. + continue + filter_props = request_spec.to_legacy_filter_properties_dict() + scheduler_utils.populate_retry(filter_props, instance.uuid) + scheduler_utils.populate_filter_properties(filter_props, + host) # send a state update notification for the initial create to # show it going from non-existent to BUILDING notifications.send_update_with_states(context, instance, None, @@ -1019,6 +1046,29 @@ class ComputeTaskManager(base.Base): host=host['host'], node=host['nodename'], limits=host['limits']) + def _cleanup_build_artifacts(self, context, exc, instances, build_requests, + request_specs): + for (instance, build_request, request_spec) in six.moves.zip( + instances, build_requests, request_specs): + # Skip placeholders that were buried in cell0 or had their + # build requests deleted by the user before instance create. + if instance is None: + continue + updates = {'vm_state': vm_states.ERROR, 'task_state': None} + legacy_spec = request_spec.to_legacy_request_spec_dict() + self._set_vm_state_and_notify(context, instance.uuid, + 'build_instances', updates, exc, + legacy_spec) + # Be paranoid about artifacts being deleted underneath us. + try: + build_request.destroy() + except exception.BuildRequestNotFound: + pass + try: + request_spec.destroy() + except exception.RequestSpecNotFound: + pass + def _delete_build_request(self, context, build_request, instance, cell, instance_bdms, instance_tags): """Delete a build request after creating the instance in the cell. diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 7c23e4ed93a7..f64a08a2a254 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -30,15 +30,11 @@ class MigrationTask(base.TaskBase): self.request_spec = request_spec self.reservations = reservations self.flavor = flavor - self.quotas = None self.compute_rpcapi = compute_rpcapi self.scheduler_client = scheduler_client def _execute(self): - self.quotas = objects.Quotas.from_reservations(self.context, - self.reservations, - instance=self.instance) # TODO(sbauza): Remove that once prep_resize() accepts a RequestSpec # object in the signature and all the scheduler.utils methods too legacy_spec = self.request_spec.to_legacy_request_spec_dict() @@ -96,5 +92,4 @@ class MigrationTask(base.TaskBase): node=node, clean_shutdown=self.clean_shutdown) def rollback(self): - if self.quotas: - self.quotas.rollback() + pass diff --git a/nova/objects/instance.py b/nova/objects/instance.py index a73e85c202ff..145ee8aae99d 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -20,7 +20,10 @@ from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_utils import timeutils from oslo_utils import versionutils +from sqlalchemy import or_ from sqlalchemy.orm import joinedload +from sqlalchemy.sql import func +from sqlalchemy.sql import null from nova.cells import opts as cells_opts from nova.cells import rpcapi as cells_rpcapi @@ -1206,7 +1209,8 @@ class InstanceList(base.ObjectListBase, base.NovaObject): # Version 2.1: Add get_uuids_by_host() # Version 2.2: Pagination for get_active_by_window_joined() # Version 2.3: Add get_count_by_vm_state() - VERSION = '2.3' + # Version 2.4: Add get_counts() + VERSION = '2.4' fields = { 'objects': fields.ListOfObjectsField('Instance'), @@ -1407,6 +1411,55 @@ class InstanceList(base.ObjectListBase, base.NovaObject): return cls._get_count_by_vm_state_in_db(context, project_id, user_id, vm_state) + @staticmethod + @db_api.pick_context_manager_reader + def _get_counts_in_db(context, project_id, user_id=None): + # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py: + # It would be better to have vm_state not be nullable + # but until then we test it explicitly as a workaround. + not_soft_deleted = or_( + models.Instance.vm_state != vm_states.SOFT_DELETED, + models.Instance.vm_state == null() + ) + project_query = context.session.query( + func.count(models.Instance.id), + func.sum(models.Instance.vcpus), + func.sum(models.Instance.memory_mb)).\ + filter_by(deleted=0).\ + filter(not_soft_deleted).\ + filter_by(project_id=project_id) + + project_result = project_query.first() + fields = ('instances', 'cores', 'ram') + project_counts = {field: int(project_result[idx] or 0) + for idx, field in enumerate(fields)} + counts = {'project': project_counts} + if user_id: + user_result = project_query.filter_by(user_id=user_id).first() + user_counts = {field: int(user_result[idx] or 0) + for idx, field in enumerate(fields)} + counts['user'] = user_counts + return counts + + @base.remotable_classmethod + def get_counts(cls, context, project_id, user_id=None): + """Get the counts of Instance objects in the database. + + :param context: The request context for database access + :param project_id: The project_id to count across + :param user_id: The user_id to count across + :returns: A dict containing the project-scoped counts and user-scoped + counts if user_id is specified. For example: + + {'project': {'instances': , + 'cores': , + 'ram': , + 'cores': , + 'ram': }} + """ + return cls._get_counts_in_db(context, project_id, user_id=user_id) + @db_api.pick_context_manager_writer def _migrate_instance_keypairs(ctxt, count): diff --git a/nova/quota.py b/nova/quota.py index 4fd72bd30bca..70b89f5378aa 100644 --- a/nova/quota.py +++ b/nova/quota.py @@ -136,7 +136,7 @@ class DbQuotaDriver(object): usage = usages.get(resource.name, {}) modified_quotas[resource.name].update( in_use=usage.get('in_use', 0), - reserved=usage.get('reserved', 0), + reserved=0, ) # Initialize remains quotas with the default limits. @@ -238,8 +238,7 @@ class DbQuotaDriver(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. :param project_quotas: Quotas dictionary for the specified project. :param user_quotas: Quotas dictionary for the specified project and user. @@ -260,17 +259,6 @@ class DbQuotaDriver(object): if usages: user_usages = self._get_usages(context, resources, project_id, user_id=user_id) - # TODO(melwitt): This is for compat with ReservableResources and - # should be removed when all of the ReservableResources have been - # removed. ReservableResource usage comes from the quota_usages - # table in the database, so we need to query usages from there as - # long as we still have ReservableResources. - from_usages_table = db.quota_usage_get_all_by_project_and_user( - context, project_id, user_id) - for k, v in from_usages_table.items(): - if k in user_usages: - continue - user_usages[k] = v return self._process_quotas(context, resources, project_id, user_quotas, quota_class, defaults=defaults, usages=user_usages) @@ -293,8 +281,7 @@ class DbQuotaDriver(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. :param remains: If True, the current remains of the project will will be returned. :param project_quotas: Quotas dictionary for the specified project. @@ -304,17 +291,6 @@ class DbQuotaDriver(object): project_usages = {} if usages: project_usages = self._get_usages(context, resources, project_id) - # TODO(melwitt): This is for compat with ReservableResources and - # should be removed when all of the ReservableResources have been - # removed. ReservableResource usage comes from the quota_usages - # table in the database, so we need to query usages from there as - # long as we still have ReservableResources. - from_usages_table = db.quota_usage_get_all_by_project(context, - project_id) - for k, v in from_usages_table.items(): - if k in project_usages: - continue - project_usages[k] = v return self._process_quotas(context, resources, project_id, project_quotas, quota_class, defaults=defaults, usages=project_usages, @@ -385,14 +361,14 @@ class DbQuotaDriver(object): # attempts to update a quota, the value chosen must be at least # as much as the current usage and less than or equal to the # project limit less the sum of existing per user limits. - minimum = value['in_use'] + value['reserved'] + minimum = value['in_use'] settable_quotas[key] = {'minimum': minimum, 'maximum': maximum} else: for key, value in project_quotas.items(): minimum = \ max(int(self._sub_quota_values(value['limit'], value['remains'])), - int(value['in_use'] + value['reserved'])) + int(value['in_use'])) settable_quotas[key] = {'minimum': minimum, 'maximum': -1} return settable_quotas @@ -423,7 +399,7 @@ class DbQuotaDriver(object): syncable_resources.append(key) return syncable_resources - def _get_quotas(self, context, resources, keys, has_sync, project_id=None, + def _get_quotas(self, context, resources, keys, project_id=None, user_id=None, project_quotas=None): """A helper method which retrieves the quotas for the specific resources identified by keys, and which apply to the current @@ -432,10 +408,6 @@ class DbQuotaDriver(object): :param context: The request context, for access checks. :param resources: A dictionary of the registered resources. :param keys: A list of the desired quotas to retrieve. - :param has_sync: If True, indicates that the resource must - have a sync function; if False, indicates - that the resource must NOT have a sync - function. :param project_id: Specify the project_id if current context is admin and admin wants to impact on common user's tenant. @@ -446,13 +418,8 @@ class DbQuotaDriver(object): """ # Filter resources - if has_sync: - sync_filt = lambda x: hasattr(x, 'sync') - else: - sync_filt = lambda x: not hasattr(x, 'sync') desired = set(keys) - sub_resources = {k: v for k, v in resources.items() - if k in desired and sync_filt(v)} + sub_resources = {k: v for k, v in resources.items() if k in desired} # Make sure we accounted for all of them... if len(keys) != len(sub_resources): @@ -526,10 +493,10 @@ class DbQuotaDriver(object): # Get the applicable quotas project_quotas = db.quota_get_all_by_project(context, project_id) quotas = self._get_quotas(context, resources, values.keys(), - has_sync=False, project_id=project_id, + project_id=project_id, project_quotas=project_quotas) user_quotas = self._get_quotas(context, resources, values.keys(), - has_sync=False, project_id=project_id, + project_id=project_id, user_id=user_id, project_quotas=project_quotas) @@ -635,12 +602,12 @@ class DbQuotaDriver(object): # per user quotas, project quota limits (for quotas that have # user-scoping, limits for the project) quotas = self._get_quotas(context, resources, all_keys, - has_sync=False, project_id=project_id, + project_id=project_id, project_quotas=project_quotas) # per user quotas, user quota limits (for quotas that have # user-scoping, the limits for the user) user_quotas = self._get_quotas(context, resources, all_keys, - has_sync=False, project_id=project_id, + project_id=project_id, user_id=user_id, project_quotas=project_quotas) @@ -770,12 +737,12 @@ class DbQuotaDriver(object): 'project_quotas': project_quotas}) quotas = self._get_quotas(context, resources, deltas.keys(), - has_sync=True, project_id=project_id, + project_id=project_id, project_quotas=project_quotas) LOG.debug('Quotas for project %(project_id)s after resource sync: ' '%(quotas)s', {'project_id': project_id, 'quotas': quotas}) user_quotas = self._get_quotas(context, resources, deltas.keys(), - has_sync=True, project_id=project_id, + project_id=project_id, user_id=user_id, project_quotas=project_quotas) LOG.debug('Quotas for project %(project_id)s and user %(user_id)s ' @@ -1031,8 +998,7 @@ class NoopQuotaDriver(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. """ return self._get_noop_quotas(resources, usages=usages) @@ -1054,8 +1020,7 @@ class NoopQuotaDriver(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. :param remains: If True, the current remains of the project will will be returned. """ @@ -1536,8 +1501,7 @@ class QuotaEngine(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. """ return self._driver.get_user_quotas(context, self._resources, @@ -1559,8 +1523,7 @@ class QuotaEngine(object): default value, if there is no value from the quota class) will be reported if there is no specific value for the resource. - :param usages: If True, the current in_use and reserved counts - will also be returned. + :param usages: If True, the current counts will also be returned. :param remains: If True, the current remains of the project will will be returned. """ @@ -1906,6 +1869,42 @@ def _floating_ip_count(context, project_id): return {'project': {'floating_ips': count}} +def _instances_cores_ram_count(context, project_id, user_id=None): + """Get the counts of instances, cores, and ram in the database. + + :param context: The request context for database access + :param project_id: The project_id to count across + :param user_id: The user_id to count across + :returns: A dict containing the project-scoped counts and user-scoped + counts if user_id is specified. For example: + + {'project': {'instances': , + 'cores': , + 'ram': }, + 'user': {'instances': , + 'cores': , + 'ram': }} + """ + # TODO(melwitt): Counting across cells for instances means we will miss + # counting resources if a cell is down. In the future, we should query + # placement for cores/ram and InstanceMappings for instances (once we are + # deleting InstanceMappings when we delete instances). + results = nova_context.scatter_gather_all_cells( + context, objects.InstanceList.get_counts, project_id, user_id=user_id) + total_counts = {'project': {'instances': 0, 'cores': 0, 'ram': 0}} + if user_id: + total_counts['user'] = {'instances': 0, 'cores': 0, 'ram': 0} + for cell_uuid, result in results.items(): + if result not in (nova_context.did_not_respond_sentinel, + nova_context.raised_exception_sentinel): + for resource, count in result['project'].items(): + total_counts['project'][resource] += count + if user_id: + for resource, count in result['user'].items(): + total_counts['user'][resource] += count + return total_counts + + def _server_group_count(context, project_id, user_id=None): """Get the counts of server groups in the database. @@ -1935,9 +1934,9 @@ QUOTAS = QuotaEngine() resources = [ - ReservableResource('instances', '_sync_instances', 'instances'), - ReservableResource('cores', '_sync_instances', 'cores'), - ReservableResource('ram', '_sync_instances', 'ram'), + CountableResource('instances', _instances_cores_ram_count, 'instances'), + CountableResource('cores', _instances_cores_ram_count, 'cores'), + CountableResource('ram', _instances_cores_ram_count, 'ram'), CountableResource('security_groups', _security_group_count, 'security_groups'), CountableResource('fixed_ips', _fixed_ip_count, 'fixed_ips'), diff --git a/nova/tests/functional/db/test_quota.py b/nova/tests/functional/db/test_quota.py index 9101cab8a84f..2f812fc919ec 100644 --- a/nova/tests/functional/db/test_quota.py +++ b/nova/tests/functional/db/test_quota.py @@ -75,3 +75,51 @@ class QuotaTestCase(test.NoDBTestCase): 'fake-user') self.assertEqual(2, count['user']['server_group_members']) + + def test_instances_cores_ram_count(self): + ctxt = context.RequestContext('fake-user', 'fake-project') + mapping1 = objects.CellMapping(context=ctxt, + uuid=uuidutils.generate_uuid(), + database_connection='cell1', + transport_url='none:///') + mapping2 = objects.CellMapping(context=ctxt, + uuid=uuidutils.generate_uuid(), + database_connection='cell2', + transport_url='none:///') + mapping1.create() + mapping2.create() + + # Create an instance in cell1 + with context.target_cell(ctxt, mapping1) as cctxt: + instance = objects.Instance(context=cctxt, + project_id='fake-project', + user_id='fake-user', + vcpus=2, memory_mb=512) + instance.create() + + # Create an instance in cell2 + with context.target_cell(ctxt, mapping2) as cctxt: + instance = objects.Instance(context=cctxt, + project_id='fake-project', + user_id='fake-user', + vcpus=4, memory_mb=1024) + instance.create() + + # Create an instance in cell2 for a different user + with context.target_cell(ctxt, mapping2) as cctxt: + instance = objects.Instance(context=cctxt, + project_id='fake-project', + user_id='other-fake-user', + vcpus=4, memory_mb=1024) + instance.create() + + # Count instances, cores, and ram across cells + count = quota._instances_cores_ram_count(ctxt, 'fake-project', + user_id='fake-user') + + self.assertEqual(3, count['project']['instances']) + self.assertEqual(10, count['project']['cores']) + self.assertEqual(2560, count['project']['ram']) + self.assertEqual(2, count['user']['instances']) + self.assertEqual(6, count['user']['cores']) + self.assertEqual(1536, count['user']['ram']) diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 0388c28df929..aba16035b4d5 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -274,6 +274,53 @@ class ServersTest(ServersTestBase): found_server = self._wait_for_state_change(found_server, 'DELETED') self.assertEqual('ACTIVE', found_server['status']) + def test_deferred_delete_restore_overquota(self): + # Test that a restore that would put the user over quota fails + self.flags(instances=1, group='quota') + # Creates, deletes and restores a server. + self.flags(reclaim_instance_interval=3600) + + # Create server + server = self._build_minimal_create_server_request() + + created_server1 = self.api.post_server({'server': server}) + LOG.debug("created_server: %s", created_server1) + self.assertTrue(created_server1['id']) + created_server_id1 = created_server1['id'] + + # Wait for it to finish being created + found_server1 = self._wait_for_state_change(created_server1, 'BUILD') + + # It should be available... + self.assertEqual('ACTIVE', found_server1['status']) + + # Delete the server + self.api.delete_server(created_server_id1) + + # Wait for queued deletion + found_server1 = self._wait_for_state_change(found_server1, 'ACTIVE') + self.assertEqual('SOFT_DELETED', found_server1['status']) + + # Create a second server + server = self._build_minimal_create_server_request() + + created_server2 = self.api.post_server({'server': server}) + LOG.debug("created_server: %s", created_server2) + self.assertTrue(created_server2['id']) + + # Wait for it to finish being created + found_server2 = self._wait_for_state_change(created_server2, 'BUILD') + + # It should be available... + self.assertEqual('ACTIVE', found_server2['status']) + + # Try to restore the first server, it should fail + ex = self.assertRaises(client.OpenStackApiException, + self.api.post_server_action, + created_server_id1, {'restore': {}}) + self.assertEqual(403, ex.response.status_code) + self.assertEqual('SOFT_DELETED', found_server1['status']) + def test_deferred_delete_force(self): # Creates, deletes and force deletes a server. self.flags(reclaim_instance_interval=3600) @@ -668,6 +715,24 @@ class ServersTest(ServersTestBase): # Cleanup self._delete_server(created_server_id) + def test_resize_server_overquota(self): + self.flags(cores=1, group='quota') + self.flags(ram=512, group='quota') + # Create server with default flavor, 1 core, 512 ram + server = self._build_minimal_create_server_request() + created_server = self.api.post_server({"server": server}) + created_server_id = created_server['id'] + + found_server = self._wait_for_state_change(created_server, 'BUILD') + self.assertEqual('ACTIVE', found_server['status']) + + # Try to resize to flavorid 2, 1 core, 2048 ram + post = {'resize': {'flavorRef': '2'}} + ex = self.assertRaises(client.OpenStackApiException, + self.api.post_server_action, + created_server_id, post) + self.assertEqual(403, ex.response.status_code) + class ServersTestV21(ServersTest): api_major_version = 'v2.1' diff --git a/nova/tests/unit/api/openstack/compute/test_multiple_create.py b/nova/tests/unit/api/openstack/compute/test_multiple_create.py index fc699b946f36..19790692f9e9 100644 --- a/nova/tests/unit/api/openstack/compute/test_multiple_create.py +++ b/nova/tests/unit/api/openstack/compute/test_multiple_create.py @@ -431,3 +431,36 @@ class MultiCreateExtensionTestV21(test.TestCase): self.assertRaises(self.validation_error, self.controller.create, self.req, body=body) + + def test_create_multiple_instance_max_count_overquota_min_count_ok(self): + self.flags(instances=3, group='quota') + image_href = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' + flavor_ref = 'http://localhost/123/flavors/3' + body = { + 'server': { + multiple_create_v21.MIN_ATTRIBUTE_NAME: 2, + multiple_create_v21.MAX_ATTRIBUTE_NAME: 5, + 'name': 'server_test', + 'imageRef': image_href, + 'flavorRef': flavor_ref, + } + } + res = self.controller.create(self.req, body=body).obj + instance_uuids = self.instance_cache_by_uuid.keys() + self.assertIn(res["server"]["id"], instance_uuids) + + def test_create_multiple_instance_max_count_overquota_min_count_over(self): + self.flags(instances=3, group='quota') + image_href = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' + flavor_ref = 'http://localhost/123/flavors/3' + body = { + 'server': { + multiple_create_v21.MIN_ATTRIBUTE_NAME: 4, + multiple_create_v21.MAX_ATTRIBUTE_NAME: 5, + 'name': 'server_test', + 'imageRef': image_href, + 'flavorRef': flavor_ref, + } + } + self.assertRaises(webob.exc.HTTPForbidden, self.controller.create, + self.req, body=body) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index b6c6bfc6da0d..1a900a0de138 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -51,6 +51,7 @@ from nova.compute import vm_states import nova.conf from nova import context from nova import db +from nova.db.sqlalchemy import api as db_api from nova.db.sqlalchemy import models from nova import exception from nova.image import glance @@ -3231,8 +3232,27 @@ class ServersControllerCreateTest(test.TestCase): self.assertEqual(encodeutils.safe_decode(robj['Location']), selfhref) - def _do_test_create_instance_above_quota(self, resource, allowed, quota, - expected_msg): + @mock.patch('nova.db.quota_get_all_by_project') + @mock.patch('nova.db.quota_get_all_by_project_and_user') + @mock.patch('nova.objects.Quotas.count_as_dict') + def _do_test_create_instance_above_quota(self, resource, allowed, + quota, expected_msg, mock_count, mock_get_all_pu, + mock_get_all_p): + count = {'project': {}, 'user': {}} + for res in ('instances', 'ram', 'cores'): + if res == resource: + value = quota - allowed + count['project'][res] = count['user'][res] = value + else: + count['project'][res] = count['user'][res] = 0 + mock_count.return_value = count + mock_get_all_p.return_value = {'project_id': 'fake'} + mock_get_all_pu.return_value = {'project_id': 'fake', + 'user_id': 'fake_user'} + if resource in db_api.PER_PROJECT_QUOTAS: + mock_get_all_p.return_value[resource] = quota + else: + mock_get_all_pu.return_value[resource] = quota fakes.stub_out_instance_quota(self, allowed, quota, resource) self.body['server']['flavorRef'] = 3 self.req.body = jsonutils.dump_as_bytes(self.body) @@ -3264,12 +3284,16 @@ class ServersControllerCreateTest(test.TestCase): fake_group.user_id = ctxt.user_id fake_group.create() + real_count = fakes.QUOTAS.count_as_dict + def fake_count(context, name, group, user_id): - self.assertEqual(name, "server_group_members") - self.assertEqual(group.uuid, fake_group.uuid) - self.assertEqual(user_id, - self.req.environ['nova.context'].user_id) - return {'user': {'server_group_members': 10}} + if name == 'server_group_members': + self.assertEqual(group.uuid, fake_group.uuid) + self.assertEqual(user_id, + self.req.environ['nova.context'].user_id) + return {'user': {'server_group_members': 10}} + else: + return real_count(context, name, group, user_id) def fake_limit_check(context, **kwargs): if 'server_group_members' in kwargs: diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 90f956dcdd33..fb170ad1f381 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -65,7 +65,6 @@ from nova.objects import block_device as block_device_obj from nova.objects import fields as obj_fields from nova.objects import instance as instance_obj from nova.objects import migrate_data as migrate_data_obj -from nova import quota from nova.scheduler import client as scheduler_client from nova import test from nova.tests import fixtures @@ -94,7 +93,6 @@ from nova.virt import fake from nova.virt import hardware from nova.volume import cinder -QUOTAS = quota.QUOTAS LOG = logging.getLogger(__name__) CONF = nova.conf.CONF @@ -216,8 +214,6 @@ class BaseTestCase(test.TestCase): self.project_id = 'fake' self.context = context.RequestContext(self.user_id, self.project_id) - self.none_quotas = objects.Quotas.from_reservations( - self.context, None) def fake_show(meh, context, id, **kwargs): if id: @@ -4322,8 +4318,7 @@ class ComputeTestCase(BaseTestCase, self.compute._delete_instance, self.context, instance, - [], - self.none_quotas) + []) mock_destroy.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, mock.ANY) @@ -4340,8 +4335,7 @@ class ComputeTestCase(BaseTestCase, self.compute._delete_instance, self.context, instance, - [], - self.none_quotas) + []) mock_destroy.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, mock.ANY) @@ -4350,8 +4344,7 @@ class ComputeTestCase(BaseTestCase, def test_delete_instance_changes_power_state(self): """Test that the power state is NOSTATE after deleting an instance.""" instance = self._create_fake_instance_obj() - self.compute._delete_instance(self.context, instance, [], - self.none_quotas) + self.compute._delete_instance(self.context, instance, []) self.assertEqual(power_state.NOSTATE, instance.power_state) def test_instance_termination_exception_sets_error(self): @@ -4360,8 +4353,7 @@ class ComputeTestCase(BaseTestCase, """ instance = self._create_fake_instance_obj() - def fake_delete_instance(self, context, instance, bdms, - reservations=None): + def fake_delete_instance(self, context, instance, bdms): raise exception.InstanceTerminationFailure(reason='') self.stub_out('nova.compute.manager.ComputeManager._delete_instance', @@ -4495,23 +4487,12 @@ class ComputeTestCase(BaseTestCase, fake_migration_save) self._test_state_revert(instance, *operation) - def _ensure_quota_reservations(self, instance, - reservations, mock_quota): - """Mock up commit/rollback of quota reservations.""" - mock_quota.assert_called_once_with(mock.ANY, reservations, - project_id=instance['project_id'], - user_id=instance['user_id']) - - @mock.patch.object(nova.quota.QUOTAS, 'commit') - def test_quotas_successful_delete(self, mock_commit): + def test_quotas_successful_delete(self): instance = self._create_fake_instance_obj() - resvs = list('fake_res') self.compute.terminate_instance(self.context, instance, - bdms=[], reservations=resvs) - self._ensure_quota_reservations(instance, resvs, mock_commit) + bdms=[], reservations=[]) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_quotas_failed_delete(self, mock_rollback): + def test_quotas_failed_delete(self): instance = self._create_fake_instance_obj() def fake_shutdown_instance(*args, **kwargs): @@ -4520,25 +4501,18 @@ class ComputeTestCase(BaseTestCase, self.stub_out('nova.compute.manager.ComputeManager._shutdown_instance', fake_shutdown_instance) - resvs = list('fake_res') self.assertRaises(test.TestingException, self.compute.terminate_instance, self.context, instance, - bdms=[], reservations=resvs) - self._ensure_quota_reservations(instance, - resvs, mock_rollback) + bdms=[], reservations=[]) - @mock.patch.object(nova.quota.QUOTAS, 'commit') - def test_quotas_successful_soft_delete(self, mock_commit): + def test_quotas_successful_soft_delete(self): instance = self._create_fake_instance_obj( params=dict(task_state=task_states.SOFT_DELETING)) - resvs = list('fake_res') self.compute.soft_delete_instance(self.context, instance, - reservations=resvs) - self._ensure_quota_reservations(instance, resvs, mock_commit) + reservations=[]) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_quotas_failed_soft_delete(self, mock_rollback): + def test_quotas_failed_soft_delete(self): instance = self._create_fake_instance_obj( params=dict(task_state=task_states.SOFT_DELETING)) @@ -4548,29 +4522,17 @@ class ComputeTestCase(BaseTestCase, self.stub_out('nova.virt.fake.FakeDriver.soft_delete', fake_soft_delete) - resvs = list('fake_res') - self.assertRaises(test.TestingException, self.compute.soft_delete_instance, self.context, instance, - reservations=resvs) + reservations=[]) - self._ensure_quota_reservations(instance, - resvs, mock_rollback) - - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_quotas_destroy_of_soft_deleted_instance(self, mock_rollback): + def test_quotas_destroy_of_soft_deleted_instance(self): instance = self._create_fake_instance_obj( params=dict(vm_state=vm_states.SOFT_DELETED)) - # Termination should be successful, but quota reservations - # rolled back because the instance was in SOFT_DELETED state. - resvs = list('fake_res') self.compute.terminate_instance(self.context, instance, - bdms=[], reservations=resvs) - - self._ensure_quota_reservations(instance, - resvs, mock_rollback) + bdms=[], reservations=[]) def _stub_out_resize_network_methods(self): def fake(cls, ctxt, instance, *args, **kwargs): @@ -4637,10 +4599,9 @@ class ComputeTestCase(BaseTestCase, mock.patch.object(self.compute, '_get_instance_block_device_info'), mock.patch.object(migration, 'save'), mock.patch.object(instance, 'save'), - mock.patch.object(nova.quota.QUOTAS, 'commit') ) as (mock_setup, mock_net_mig, mock_get_nw, mock_notify, mock_notify_action, mock_virt_mig, mock_get_blk, mock_mig_save, - mock_inst_save, mock_commit): + mock_inst_save): def _mig_save(): self.assertEqual(migration.status, 'finished') self.assertEqual(vm_state, instance.vm_state) @@ -4715,8 +4676,6 @@ class ComputeTestCase(BaseTestCase, refresh_conn_info=True) mock_inst_save.assert_has_calls(inst_call_list) mock_mig_save.assert_called_once_with() - self._ensure_quota_reservations(instance, reservations, - mock_commit) def test_finish_resize_from_active(self): self._test_finish_resize(power_on=True) @@ -4727,8 +4686,7 @@ class ComputeTestCase(BaseTestCase, def test_finish_resize_without_resize_instance(self): self._test_finish_resize(power_on=True, resize_instance=False) - @mock.patch.object(nova.quota.QUOTAS, 'commit') - def test_finish_resize_with_volumes(self, mock_commit): + def test_finish_resize_with_volumes(self): """Contrived test to ensure finish_resize doesn't raise anything.""" # create instance @@ -4888,14 +4846,11 @@ class ComputeTestCase(BaseTestCase, volume['device_path'] = None volume['instance_uuid'] = None self.stub_out('nova.volume.cinder.API.detach', fake_detach) - self._ensure_quota_reservations(instance, - reservations, mock_commit) # clean up self.compute.terminate_instance(self.context, instance, [], []) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_finish_resize_handles_error(self, mock_rollback): + def test_finish_resize_handles_error(self): # Make sure we don't leave the instance in RESIZE on error. def throw_up(*args, **kwargs): @@ -4907,13 +4862,12 @@ class ComputeTestCase(BaseTestCase, old_flavor_name = 'm1.tiny' instance = self._create_fake_instance_obj(type_name=old_flavor_name) - reservations = list('fake_res') instance_type = flavors.get_flavor_by_name('m1.small') self.compute.prep_resize(self.context, instance=instance, instance_type=instance_type, - image={}, reservations=reservations, + image={}, reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) @@ -4928,7 +4882,7 @@ class ComputeTestCase(BaseTestCase, self.context, migration=migration, disk_info={}, image={}, instance=instance, - reservations=reservations) + reservations=[]) instance.refresh() self.assertEqual(vm_states.ERROR, instance.vm_state) @@ -4939,8 +4893,6 @@ class ComputeTestCase(BaseTestCase, self.assertEqual(old_flavor['ephemeral_gb'], instance.ephemeral_gb) self.assertEqual(old_flavor['id'], instance.instance_type_id) self.assertNotEqual(instance_type['id'], instance.instance_type_id) - self._ensure_quota_reservations(instance, reservations, - mock_rollback) def test_set_instance_info(self): old_flavor_name = 'm1.tiny' @@ -5135,16 +5087,12 @@ class ComputeTestCase(BaseTestCase, self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_prep_resize_instance_migration_error_on_none_host(self, - mock_rollback): + def test_prep_resize_instance_migration_error_on_none_host(self): """Ensure prep_resize raises a migration error if destination host is not defined """ instance = self._create_fake_instance_obj() - reservations = list('fake_res') - self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, block_device_mapping=[]) instance.host = None @@ -5154,15 +5102,12 @@ class ComputeTestCase(BaseTestCase, self.assertRaises(exception.MigrationError, self.compute.prep_resize, self.context, instance=instance, instance_type=instance_type, image={}, - reservations=reservations, request_spec={}, + reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) self.compute.terminate_instance(self.context, instance, [], []) - self._ensure_quota_reservations(instance, reservations, - mock_rollback) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_resize_instance_driver_error(self, mock_rollback): + def test_resize_instance_driver_error(self): # Ensure instance status set to Error on resize error. def throw_up(*args, **kwargs): @@ -5174,15 +5119,13 @@ class ComputeTestCase(BaseTestCase, instance = self._create_fake_instance_obj() instance_type = flavors.get_default_flavor() - reservations = list('fake_res') - self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, block_device_mapping=[]) instance.host = 'foo' instance.save() self.compute.prep_resize(self.context, instance=instance, instance_type=instance_type, image={}, - reservations=reservations, request_spec={}, + reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) instance.task_state = task_states.RESIZE_PREP @@ -5195,7 +5138,7 @@ class ComputeTestCase(BaseTestCase, self.assertRaises(test.TestingException, self.compute.resize_instance, self.context, instance=instance, migration=migration, image={}, - reservations=reservations, + reservations=[], instance_type=jsonutils.to_primitive(instance_type), clean_shutdown=True) # NOTE(comstud): error path doesn't use objects, so our object @@ -5203,11 +5146,8 @@ class ComputeTestCase(BaseTestCase, instance.refresh() self.assertEqual(instance.vm_state, vm_states.ERROR) self.compute.terminate_instance(self.context, instance, [], []) - self._ensure_quota_reservations(instance, reservations, - mock_rollback) - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_resize_instance_driver_rollback(self, mock_rollback): + def test_resize_instance_driver_rollback(self): # Ensure instance status set to Running after rollback. def throw_up(*args, **kwargs): @@ -5218,14 +5158,13 @@ class ComputeTestCase(BaseTestCase, instance = self._create_fake_instance_obj() instance_type = flavors.get_default_flavor() - reservations = list('fake_res') self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, block_device_mapping=[]) instance.host = 'foo' instance.save() self.compute.prep_resize(self.context, instance=instance, instance_type=instance_type, image={}, - reservations=reservations, request_spec={}, + reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) instance.task_state = task_states.RESIZE_PREP @@ -5238,7 +5177,7 @@ class ComputeTestCase(BaseTestCase, self.assertRaises(test.TestingException, self.compute.resize_instance, self.context, instance=instance, migration=migration, image={}, - reservations=reservations, + reservations=[], instance_type=jsonutils.to_primitive(instance_type), clean_shutdown=True) # NOTE(comstud): error path doesn't use objects, so our object @@ -5247,8 +5186,6 @@ class ComputeTestCase(BaseTestCase, self.assertEqual(instance.vm_state, vm_states.ACTIVE) self.assertIsNone(instance.task_state) self.compute.terminate_instance(self.context, instance, [], []) - self._ensure_quota_reservations(instance, reservations, - mock_rollback) def _test_resize_instance(self, clean_shutdown=True): # Ensure instance can be migrated/resized. @@ -5316,8 +5253,7 @@ class ComputeTestCase(BaseTestCase, def test_resize_instance_forced_shutdown(self): self._test_resize_instance(clean_shutdown=False) - @mock.patch.object(nova.quota.QUOTAS, 'commit') - def _test_confirm_resize(self, mock_commit, power_on, numa_topology=None): + def _test_confirm_resize(self, power_on, numa_topology=None): # Common test case method for confirm_resize def fake(*args, **kwargs): pass @@ -5344,8 +5280,6 @@ class ComputeTestCase(BaseTestCase, self._stub_out_resize_network_methods() - reservations = list('fake_res') - # Get initial memory usage memory_mb_used = self.rt.compute_nodes[NODENAME].memory_mb_used @@ -5371,7 +5305,7 @@ class ComputeTestCase(BaseTestCase, self.compute.prep_resize(self.context, instance=instance, instance_type=new_instance_type_ref, - image={}, reservations=reservations, request_spec={}, + image={}, reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) # Memory usage should increase after the resize as well @@ -5420,7 +5354,7 @@ class ComputeTestCase(BaseTestCase, instance.task_state = None instance.save() self.compute.confirm_resize(self.context, instance=instance, - reservations=reservations, + reservations=[], migration=migration) # Resources from the migration (based on initial flavor) should @@ -5439,8 +5373,6 @@ class ComputeTestCase(BaseTestCase, self.assertIsNone(instance.migration_context) self.assertEqual(p_state, instance.power_state) self.compute.terminate_instance(self.context, instance, [], []) - self._ensure_quota_reservations(instance, reservations, - mock_commit) def test_confirm_resize_from_active(self): self._test_confirm_resize(power_on=True) @@ -5600,8 +5532,7 @@ class ComputeTestCase(BaseTestCase, self._test_resize_with_pci( self.compute.revert_resize, '0000:0b:00.1') - @mock.patch.object(nova.quota.QUOTAS, 'commit') - def _test_finish_revert_resize(self, mock_commit, power_on, + def _test_finish_revert_resize(self, power_on, remove_old_vm_state=False, numa_topology=None): """Convenience method that does most of the work for the @@ -5634,8 +5565,6 @@ class ComputeTestCase(BaseTestCase, self._stub_out_resize_network_methods() - reservations = list('fake_res') - # Get initial memory usage memory_mb_used = self.rt.compute_nodes[NODENAME].memory_mb_used @@ -5661,7 +5590,7 @@ class ComputeTestCase(BaseTestCase, self.compute.prep_resize(self.context, instance=instance, instance_type=new_instance_type_ref, - image={}, reservations=reservations, request_spec={}, + image={}, reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) @@ -5710,7 +5639,7 @@ class ComputeTestCase(BaseTestCase, self.compute.revert_resize(self.context, migration=migration, instance=instance, - reservations=reservations) + reservations=[]) # Resources from the migration (based on initial flavor) should # be freed now @@ -5729,7 +5658,7 @@ class ComputeTestCase(BaseTestCase, self.compute.finish_revert_resize(self.context, migration=migration, - instance=instance, reservations=reservations) + instance=instance, reservations=[]) self.assertIsNone(instance.task_state) @@ -5744,9 +5673,6 @@ class ComputeTestCase(BaseTestCase, else: self.assertEqual(old_vm_state, instance.vm_state) - self._ensure_quota_reservations(instance, reservations, - mock_commit) - def test_finish_revert_resize_from_active(self): self._test_finish_revert_resize(power_on=True) @@ -5844,8 +5770,7 @@ class ComputeTestCase(BaseTestCase, flavor_type = flavors.get_flavor_by_flavor_id(1) self.assertEqual(flavor_type['name'], 'm1.tiny') - @mock.patch.object(nova.quota.QUOTAS, 'rollback') - def test_resize_instance_handles_migration_error(self, mock_rollback): + def test_resize_instance_handles_migration_error(self): # Ensure vm_state is ERROR when error occurs. def raise_migration_failure(*args): raise test.TestingException() @@ -5853,7 +5778,6 @@ class ComputeTestCase(BaseTestCase, raise_migration_failure) instance = self._create_fake_instance_obj() - reservations = list('fake_res') instance_type = flavors.get_default_flavor() @@ -5863,7 +5787,7 @@ class ComputeTestCase(BaseTestCase, instance.save() self.compute.prep_resize(self.context, instance=instance, instance_type=instance_type, - image={}, reservations=reservations, + image={}, reservations=[], request_spec={}, filter_properties={}, node=None, clean_shutdown=True) migration = objects.Migration.get_by_instance_and_status( @@ -5874,7 +5798,7 @@ class ComputeTestCase(BaseTestCase, self.assertRaises(test.TestingException, self.compute.resize_instance, self.context, instance=instance, migration=migration, image={}, - reservations=reservations, + reservations=[], instance_type=jsonutils.to_primitive(instance_type), clean_shutdown=True) # NOTE(comstud): error path doesn't use objects, so our object @@ -5882,8 +5806,6 @@ class ComputeTestCase(BaseTestCase, instance.refresh() self.assertEqual(instance.vm_state, vm_states.ERROR) self.compute.terminate_instance(self.context, instance, [], []) - self._ensure_quota_reservations(instance, reservations, - mock_rollback) def test_pre_live_migration_instance_has_no_fixed_ip(self): # Confirm that no exception is raised if there is no fixed ip on @@ -7485,17 +7407,15 @@ class ComputeTestCase(BaseTestCase, self.compute._reclaim_queued_deletes(ctxt) mock_delete.assert_called_once_with( - ctxt, test.MatchType(objects.Instance), [], - test.MatchType(objects.Quotas)) + ctxt, test.MatchType(objects.Instance), []) mock_bdms.assert_called_once_with(ctxt, mock.ANY) - @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(objects.InstanceList, 'get_by_filters') @mock.patch.object(compute_manager.ComputeManager, '_deleted_old_enough') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(compute_manager.ComputeManager, '_delete_instance') def test_reclaim_queued_deletes_continue_on_error(self, mock_delete_inst, - mock_get_uuid, mock_delete_old, mock_get_filter, mock_quota): + mock_get_uuid, mock_delete_old, mock_get_filter): # Verify that reclaim continues on error. self.flags(reclaim_instance_interval=3600) ctxt = context.get_admin_context() @@ -7515,7 +7435,6 @@ class ComputeTestCase(BaseTestCase, mock_delete_old.side_effect = (True, True) mock_get_uuid.side_effect = ([], []) mock_delete_inst.side_effect = (test.TestingException, None) - mock_quota.return_value = self.none_quotas self.compute._reclaim_queued_deletes(ctxt) @@ -7527,9 +7446,8 @@ class ComputeTestCase(BaseTestCase, mock_get_uuid.assert_has_calls([mock.call(ctxt, instance1.uuid), mock.call(ctxt, instance2.uuid)]) mock_delete_inst.assert_has_calls([ - mock.call(ctxt, instance1, [], self.none_quotas), - mock.call(ctxt, instance2, [], self.none_quotas)]) - mock_quota.assert_called_once_with(ctxt, None) + mock.call(ctxt, instance1, []), + mock.call(ctxt, instance2, [])]) @mock.patch.object(fake.FakeDriver, 'get_info') @mock.patch.object(compute_manager.ComputeManager, @@ -7600,9 +7518,8 @@ class ComputeTestCase(BaseTestCase, self.compute.handle_events(event_instance) @mock.patch.object(objects.Migration, 'get_by_id') - @mock.patch.object(objects.Quotas, 'rollback') def test_confirm_resize_roll_back_quota_migration_not_found(self, - mock_rollback, mock_get_by_id): + mock_get_by_id): instance = self._create_fake_instance_obj() migration = objects.Migration() @@ -7614,12 +7531,10 @@ class ComputeTestCase(BaseTestCase, migration_id=0) self.compute.confirm_resize(self.context, instance=instance, migration=migration, reservations=[]) - self.assertTrue(mock_rollback.called) @mock.patch.object(instance_obj.Instance, 'get_by_uuid') - @mock.patch.object(objects.Quotas, 'rollback') def test_confirm_resize_roll_back_quota_instance_not_found(self, - mock_rollback, mock_get_by_id): + mock_get_by_id): instance = self._create_fake_instance_obj() migration = objects.Migration() @@ -7631,12 +7546,10 @@ class ComputeTestCase(BaseTestCase, instance_id=instance.uuid) self.compute.confirm_resize(self.context, instance=instance, migration=migration, reservations=[]) - self.assertTrue(mock_rollback.called) @mock.patch.object(objects.Migration, 'get_by_id') - @mock.patch.object(objects.Quotas, 'rollback') def test_confirm_resize_roll_back_quota_status_confirmed(self, - mock_rollback, mock_get_by_id): + mock_get_by_id): instance = self._create_fake_instance_obj() migration = objects.Migration() @@ -7647,12 +7560,10 @@ class ComputeTestCase(BaseTestCase, mock_get_by_id.return_value = migration self.compute.confirm_resize(self.context, instance=instance, migration=migration, reservations=[]) - self.assertTrue(mock_rollback.called) @mock.patch.object(objects.Migration, 'get_by_id') - @mock.patch.object(objects.Quotas, 'rollback') def test_confirm_resize_roll_back_quota_status_dummy(self, - mock_rollback, mock_get_by_id): + mock_get_by_id): instance = self._create_fake_instance_obj() migration = objects.Migration() @@ -7663,7 +7574,6 @@ class ComputeTestCase(BaseTestCase, mock_get_by_id.return_value = migration self.compute.confirm_resize(self.context, instance=instance, migration=migration, reservations=[]) - self.assertTrue(mock_rollback.called) def test_allow_confirm_resize_on_instance_in_deleting_task_state(self): instance = self._create_fake_instance_obj() @@ -8450,8 +8360,10 @@ class ComputeAPITestCase(BaseTestCase): self.fake_show) # Simulate a race where the first check passes and the recheck fails. + # The first call is for checking instances/cores/ram, second and third + # calls are for checking server group members. check_deltas_mock.side_effect = [ - None, exception.OverQuota(overs='server_group_members')] + None, None, exception.OverQuota(overs='server_group_members')] group = objects.InstanceGroup(self.context) group.uuid = uuids.fake @@ -8466,7 +8378,8 @@ class ComputeAPITestCase(BaseTestCase): scheduler_hints={'group': group.uuid}, check_server_group_quota=True) - self.assertEqual(2, check_deltas_mock.call_count) + # The first call was for the instances/cores/ram check. + self.assertEqual(3, check_deltas_mock.call_count) call1 = mock.call(self.context, {'server_group_members': 1}, group, self.context.user_id) call2 = mock.call(self.context, {'server_group_members': 0}, group, @@ -8502,10 +8415,19 @@ class ComputeAPITestCase(BaseTestCase): check_server_group_quota=True) self.assertEqual(len(refs), len(group.members)) - # check_deltas should have been called only once. - check_deltas_mock.assert_called_once_with( - self.context, {'server_group_members': 1}, group, - self.context.user_id) + # check_deltas should have been called only once for server group + # members. + self.assertEqual(2, check_deltas_mock.call_count) + call1 = mock.call(self.context, {'instances': 1, + 'cores': inst_type.vcpus, + 'ram': inst_type.memory_mb}, + self.context.project_id, + user_id=None, + check_project_id=self.context.project_id, + check_user_id=None) + call2 = mock.call(self.context, {'server_group_members': 1}, group, + self.context.user_id) + check_deltas_mock.assert_has_calls([call1, call2]) group = objects.InstanceGroup.get_by_uuid(self.context, group.uuid) self.assertIn(refs[0]['uuid'], group.members) @@ -11913,7 +11835,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): mock_mig.assert_called_once_with(mock.ANY, mock.ANY) mock_res.assert_called_once_with(mock.ANY, None, inst_obj, mock.ANY, - self.instance_type, mock.ANY, {}, {}) + self.instance_type, {}, {}) @mock.patch.object(compute_manager.ComputeManager, "_reschedule") def test_reschedule_fails_with_exception(self, mock_res): @@ -11922,8 +11844,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): """ instance = self._create_fake_instance_obj() scheduler_hint = dict(filter_properties={}) - method_args = (instance, None, scheduler_hint, self.instance_type, - None) + method_args = (instance, None, scheduler_hint, self.instance_type) mock_res.side_effect = InnerTestingException("Inner") try: @@ -11932,8 +11853,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): exc_info = sys.exc_info() self.assertRaises(test.TestingException, self.compute._reschedule_resize_or_reraise, self.context, - None, instance, exc_info, self.instance_type, - self.none_quotas, {}, {}) + None, instance, exc_info, self.instance_type, {}, {}) mock_res.assert_called_once_with( self.context, {}, {}, instance, @@ -11947,8 +11867,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): """ instance = self._create_fake_instance_obj() scheduler_hint = dict(filter_properties={}) - method_args = (instance, None, scheduler_hint, self.instance_type, - None) + method_args = (instance, None, scheduler_hint, self.instance_type) mock_res.return_value = False try: @@ -11957,8 +11876,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): exc_info = sys.exc_info() self.assertRaises(test.TestingException, self.compute._reschedule_resize_or_reraise, self.context, - None, instance, exc_info, self.instance_type, - self.none_quotas, {}, {}) + None, instance, exc_info, self.instance_type, {}, {}) mock_res.assert_called_once_with( self.context, {}, {}, instance, @@ -11971,8 +11889,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): # If rescheduled, the original resize exception should be logged. instance = self._create_fake_instance_obj() scheduler_hint = dict(filter_properties={}) - method_args = (instance, None, scheduler_hint, self.instance_type, - None) + method_args = (instance, None, scheduler_hint, self.instance_type) try: raise test.TestingException("Original") @@ -11982,7 +11899,7 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): self.compute._reschedule_resize_or_reraise( self.context, None, instance, exc_info, - self.instance_type, self.none_quotas, {}, {}) + self.instance_type, {}, {}) mock_res.assert_called_once_with(self.context, {}, {}, instance, self.compute.compute_task_api.resize_instance, diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index ca4152ea21bf..72bc899865ae 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -45,7 +45,6 @@ from nova.objects import block_device as block_device_obj from nova.objects import fields as fields_obj from nova.objects import quotas as quotas_obj from nova.objects import security_group as secgroup_obj -from nova import quota from nova import test from nova.tests import fixtures from nova.tests.unit import fake_block_device @@ -168,17 +167,19 @@ class _ComputeAPIUnitTestMixIn(object): list_obj.obj_reset_changes() return list_obj + @mock.patch('nova.objects.Quotas.check_deltas') @mock.patch('nova.conductor.conductor_api.ComputeTaskAPI.build_instances') @mock.patch('nova.compute.api.API._record_action_start') @mock.patch('nova.compute.api.API._check_requested_networks') - @mock.patch('nova.quota.QUOTAS.limit_check') + @mock.patch('nova.objects.Quotas.limit_check') @mock.patch('nova.compute.api.API._get_image') @mock.patch('nova.compute.api.API._provision_instances') def test_create_with_networks_max_count_none(self, provision_instances, get_image, check_limit, check_requested_networks, record_action_start, - build_instances): + build_instances, + check_deltas): # Make sure max_count is checked for None, as Python3 doesn't allow # comparison between NoneType and Integer, something that's allowed in # Python 2. @@ -200,21 +201,25 @@ class _ComputeAPIUnitTestMixIn(object): requested_networks=requested_networks, max_count=None) - @mock.patch('nova.quota.QUOTAS.reserve') - @mock.patch('nova.quota.QUOTAS.limit_check') - def test_create_quota_exceeded_messages(self, mock_limit_check, - mock_reserve): + @mock.patch('nova.objects.Quotas.count_as_dict') + @mock.patch('nova.objects.Quotas.limit_check') + @mock.patch('nova.objects.Quotas.limit_check_project_and_user') + def test_create_quota_exceeded_messages(self, mock_limit_check_pu, + mock_limit_check, mock_count): image_href = "image_href" image_id = 0 instance_type = self._create_flavor() quotas = {'instances': 1, 'cores': 1, 'ram': 1} - usages = {r: {'in_use': 1, 'reserved': 1} for r in - ['instances', 'cores', 'ram']} quota_exception = exception.OverQuota(quotas=quotas, - usages=usages, overs=['instances']) + usages={'instances': 1, 'cores': 1, 'ram': 1}, + overs=['instances']) - mock_reserve.side_effect = quota_exception + proj_count = {'instances': 1, 'cores': 1, 'ram': 1} + user_count = proj_count.copy() + mock_count.return_value = {'project': proj_count, 'user': user_count} + # instances/cores/ram quota + mock_limit_check_pu.side_effect = quota_exception # We don't care about network validation in this test. self.compute_api.network_api.validate_networks = ( @@ -232,8 +237,7 @@ class _ComputeAPIUnitTestMixIn(object): self.fail("Exception not raised") mock_get_image.assert_called_with(self.context, image_href) self.assertEqual(2, mock_get_image.call_count) - self.assertEqual(2, mock_reserve.call_count) - self.assertEqual(2, mock_limit_check.call_count) + self.assertEqual(2, mock_limit_check_pu.call_count) def _test_create_max_net_count(self, max_net_count, min_count, max_count): with test.nested( @@ -825,17 +829,11 @@ class _ComputeAPIUnitTestMixIn(object): self.context.elevated().AndReturn(self.context) objects.Migration.get_by_instance_and_status( self.context, inst.uuid, 'finished').AndReturn(migration) - compute_utils.downsize_quota_delta(self.context, - inst).AndReturn('deltas') - fake_quotas = objects.Quotas.from_reservations(self.context, - ['rsvs']) - compute_utils.reserve_quota_delta(self.context, 'deltas', - inst).AndReturn(fake_quotas) self.compute_api._record_action_start( self.context, inst, instance_actions.CONFIRM_RESIZE) self.compute_api.compute_rpcapi.confirm_resize( self.context, inst, migration, - migration['source_compute'], fake_quotas.reservations, cast=False) + migration['source_compute'], cast=False) def _test_delete_shelved_part(self, inst): image_api = self.compute_api.image_api @@ -886,7 +884,6 @@ class _ComputeAPIUnitTestMixIn(object): self.context, inst.uuid).AndReturn(im) def _test_delete(self, delete_type, **attrs): - reservations = ['fake-resv'] inst = self._create_instance_obj() inst.update(attrs) inst._context = self.context @@ -904,13 +901,10 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(inst, 'save') self.mox.StubOutWithMock(objects.BlockDeviceMappingList, 'get_by_instance_uuid') - self.mox.StubOutWithMock(quota.QUOTAS, 'reserve') self.mox.StubOutWithMock(self.context, 'elevated') self.mox.StubOutWithMock(objects.Service, 'get_by_compute_host') self.mox.StubOutWithMock(self.compute_api.servicegroup_api, 'service_is_up') - self.mox.StubOutWithMock(compute_utils, 'downsize_quota_delta') - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(db, 'instance_update_and_get_original') self.mox.StubOutWithMock(self.compute_api.network_api, @@ -919,8 +913,6 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(db, 'instance_destroy') self.mox.StubOutWithMock(compute_utils, 'notify_about_instance_usage') - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') - self.mox.StubOutWithMock(quota.QUOTAS, 'rollback') rpcapi = self.compute_api.compute_rpcapi self.mox.StubOutWithMock(rpcapi, 'confirm_resize') self.mox.StubOutWithMock(self.compute_api.consoleauth_rpcapi, @@ -942,34 +934,24 @@ class _ComputeAPIUnitTestMixIn(object): inst.save() if inst.task_state == task_states.RESIZE_FINISH: self._test_delete_resizing_part(inst, deltas) - quota.QUOTAS.reserve(self.context, project_id=inst.project_id, - user_id=inst.user_id, - expire=mox.IgnoreArg(), - **deltas).AndReturn(reservations) # NOTE(comstud): This is getting messy. But what we are wanting # to test is: # If cells is enabled and we're the API cell: - # * Cast to cells_rpcapi. with reservations=None - # * Commit reservations + # * Cast to cells_rpcapi. # Otherwise: # * Check for downed host # * If downed host: # * Clean up instance, destroying it, sending notifications. # (Tested in _test_downed_host_part()) - # * Commit reservations # * If not downed host: # * Record the action start. - # * Cast to compute_rpcapi. with the reservations + # * Cast to compute_rpcapi. cast = True - commit_quotas = True - soft_delete = False if self.cell_type != 'api': if inst.vm_state == vm_states.RESIZED: self._test_delete_resized_part(inst) - if inst.vm_state == vm_states.SOFT_DELETED: - soft_delete = True if inst.vm_state != vm_states.SHELVED_OFFLOADED: self.context.elevated().AndReturn(self.context) objects.Service.get_by_compute_host(self.context, @@ -984,37 +966,17 @@ class _ComputeAPIUnitTestMixIn(object): self._test_downed_host_part(inst, updates, delete_time, delete_type) cast = False - else: - # Happens on the manager side - commit_quotas = False if cast: if self.cell_type != 'api': self.compute_api._record_action_start(self.context, inst, instance_actions.DELETE) - if commit_quotas or soft_delete: - cast_reservations = None - else: - cast_reservations = reservations if delete_type == 'soft_delete': - rpcapi.soft_delete_instance(self.context, inst, - reservations=cast_reservations) + rpcapi.soft_delete_instance(self.context, inst) elif delete_type in ['delete', 'force_delete']: rpcapi.terminate_instance(self.context, inst, [], - reservations=cast_reservations, delete_type=delete_type) - if soft_delete: - quota.QUOTAS.rollback(self.context, reservations, - project_id=inst.project_id, - user_id=inst.user_id) - - if commit_quotas: - # Local delete or when we're testing API cell. - quota.QUOTAS.commit(self.context, reservations, - project_id=inst.project_id, - user_id=inst.user_id) - if self.cell_type is None or self.cell_type == 'api': self.compute_api.consoleauth_rpcapi.delete_tokens_for_instance( self.context, inst.uuid) @@ -1094,7 +1056,6 @@ class _ComputeAPIUnitTestMixIn(object): self.useFixture(fixtures.AllServicesCurrent()) inst = self._create_instance_obj() inst.host = '' - quotas = quotas_obj.Quotas(self.context) updates = {'progress': 0, 'task_state': task_states.DELETING} self.mox.StubOutWithMock(objects.BuildRequest, @@ -1105,7 +1066,6 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(db, 'constraint') self.mox.StubOutWithMock(db, 'instance_destroy') - self.mox.StubOutWithMock(self.compute_api, '_create_reservations') self.mox.StubOutWithMock(self.compute_api, '_lookup_instance') self.mox.StubOutWithMock(compute_utils, 'notify_about_instance_usage') @@ -1124,16 +1084,12 @@ class _ComputeAPIUnitTestMixIn(object): self.context, inst.uuid).AndRaise( exception.BuildRequestNotFound(uuid=inst.uuid)) inst.save() - self.compute_api._create_reservations(self.context, - inst, inst.task_state, - inst.project_id, inst.user_id - ).AndReturn(quotas) if self.cell_type == 'api': rpcapi.terminate_instance( self.context, inst, mox.IsA(objects.BlockDeviceMappingList), - reservations=None, delete_type='delete') + delete_type='delete') else: compute_utils.notify_about_instance_usage( self.compute_api.notifier, self.context, @@ -1418,81 +1374,51 @@ class _ComputeAPIUnitTestMixIn(object): def test_delete_while_booting_buildreq_deleted_instance_none(self): self.useFixture(fixtures.AllServicesCurrent()) inst = self._create_instance_obj() - quota_mock = mock.MagicMock() @mock.patch.object(self.compute_api, '_attempt_delete_of_buildrequest', return_value=True) @mock.patch.object(self.compute_api, '_lookup_instance', return_value=(None, None)) - @mock.patch.object(self.compute_api, '_create_reservations', - return_value=quota_mock) - def test(mock_create_res, mock_lookup, mock_attempt): + def test(mock_lookup, mock_attempt): self.assertTrue( self.compute_api._delete_while_booting(self.context, inst)) - self.assertFalse(quota_mock.commit.called) - quota_mock.rollback.assert_called_once_with() test() def test_delete_while_booting_buildreq_deleted_instance_not_found(self): self.useFixture(fixtures.AllServicesCurrent()) inst = self._create_instance_obj() - quota_mock = mock.MagicMock() @mock.patch.object(self.compute_api, '_attempt_delete_of_buildrequest', return_value=True) @mock.patch.object(self.compute_api, '_lookup_instance', side_effect=exception.InstanceNotFound( instance_id='fake')) - @mock.patch.object(self.compute_api, '_create_reservations', - return_value=quota_mock) - def test(mock_create_res, mock_lookup, mock_attempt): + def test(mock_lookup, mock_attempt): self.assertTrue( self.compute_api._delete_while_booting(self.context, inst)) - self.assertFalse(quota_mock.commit.called) - self.assertTrue(quota_mock.rollback.called) test() - @ddt.data((task_states.RESIZE_MIGRATED, True), - (task_states.RESIZE_FINISH, True), - (None, False)) - @ddt.unpack - def test_get_flavor_for_reservation(self, task_state, is_old): - instance = self._create_instance_obj({'task_state': task_state}) - flavor = self.compute_api._get_flavor_for_reservation(instance) - expected_flavor = instance.old_flavor if is_old else instance.flavor - self.assertEqual(expected_flavor, flavor) - - @mock.patch('nova.context.target_cell') @mock.patch('nova.compute.utils.notify_about_instance_delete') @mock.patch('nova.objects.Instance.destroy') - def test_delete_instance_from_cell0(self, destroy_mock, notify_mock, - target_cell_mock): + def test_delete_instance_from_cell0(self, destroy_mock, notify_mock): """Tests the case that the instance does not have a host and was not deleted while building, so conductor put it into cell0 so the API has - to delete the instance from cell0 and decrement the quota from the - main cell. + to delete the instance from cell0. """ instance = self._create_instance_obj({'host': None}) cell0 = objects.CellMapping(uuid=objects.CellMapping.CELL0_UUID) - quota_mock = mock.MagicMock() - target_cell_mock().__enter__.return_value = mock.sentinel.cctxt with test.nested( mock.patch.object(self.compute_api, '_delete_while_booting', return_value=False), mock.patch.object(self.compute_api, '_lookup_instance', return_value=(cell0, instance)), - mock.patch.object(self.compute_api, '_get_flavor_for_reservation', - return_value=instance.flavor), - mock.patch.object(self.compute_api, '_create_reservations', - return_value=quota_mock) ) as ( _delete_while_booting, _lookup_instance, - _get_flavor_for_reservation, _create_reservations ): self.compute_api._delete( self.context, instance, 'delete', mock.NonCallableMock()) @@ -1500,81 +1426,10 @@ class _ComputeAPIUnitTestMixIn(object): self.context, instance) _lookup_instance.assert_called_once_with( self.context, instance.uuid) - _get_flavor_for_reservation.assert_called_once_with(instance) - _create_reservations.assert_called_once_with( - mock.sentinel.cctxt, instance, instance.task_state, - self.context.project_id, instance.user_id, - flavor=instance.flavor) - quota_mock.commit.assert_called_once_with() - expected_target_cell_calls = [ - # Create the quota reservation. - mock.call(self.context, None), - mock.call().__enter__(), - mock.call().__exit__(None, None, None), - ] - target_cell_mock.assert_has_calls(expected_target_cell_calls) notify_mock.assert_called_once_with( self.compute_api.notifier, self.context, instance) destroy_mock.assert_called_once_with() - @mock.patch('nova.context.target_cell') - @mock.patch('nova.compute.utils.notify_about_instance_delete') - @mock.patch('nova.objects.Instance.destroy') - @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid') - def test_delete_instance_from_cell0_rollback_quota( - self, bdms_get_by_instance_uuid, destroy_mock, notify_mock, - target_cell_mock): - """Tests the case that the instance does not have a host and was not - deleted while building, so conductor put it into cell0 so the API has - to delete the instance from cell0 and decrement the quota from the - main cell. When we go to delete the instance, it's already gone so we - rollback the quota change. - """ - instance = self._create_instance_obj({'host': None}) - cell0 = objects.CellMapping(uuid=objects.CellMapping.CELL0_UUID) - quota_mock = mock.MagicMock() - destroy_mock.side_effect = exception.InstanceNotFound( - instance_id=instance.uuid) - target_cell_mock().__enter__.return_value = mock.sentinel.cctxt - - with test.nested( - mock.patch.object(self.compute_api, '_delete_while_booting', - return_value=False), - mock.patch.object(self.compute_api, '_lookup_instance', - return_value=(cell0, instance)), - mock.patch.object(self.compute_api, '_get_flavor_for_reservation', - return_value=instance.flavor), - mock.patch.object(self.compute_api, '_create_reservations', - return_value=quota_mock) - ) as ( - _delete_while_booting, _lookup_instance, - _get_flavor_for_reservation, _create_reservations - ): - self.compute_api._delete( - self.context, instance, 'delete', mock.NonCallableMock()) - _delete_while_booting.assert_called_once_with( - self.context, instance) - _lookup_instance.assert_called_once_with( - self.context, instance.uuid) - _get_flavor_for_reservation.assert_called_once_with(instance) - _create_reservations.assert_called_once_with( - mock.sentinel.cctxt, instance, instance.task_state, - self.context.project_id, instance.user_id, - flavor=instance.flavor) - notify_mock.assert_called_once_with( - self.compute_api.notifier, self.context, instance) - destroy_mock.assert_called_once_with() - expected_target_cell_calls = [ - # Create the quota reservation. - mock.call(self.context, None), - mock.call().__enter__(), - mock.call().__exit__(None, None, None), - ] - target_cell_mock.assert_has_calls(expected_target_cell_calls) - quota_mock.rollback.assert_called_once_with() - # Make sure we short-circuited and returned. - bdms_get_by_instance_uuid.assert_not_called() - @mock.patch.object(context, 'target_cell') @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid', side_effect=exception.InstanceMappingNotFound( @@ -1645,10 +1500,7 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(self.context, 'elevated') self.mox.StubOutWithMock(objects.Migration, 'get_by_instance_and_status') - self.mox.StubOutWithMock(compute_utils, 'downsize_quota_delta') - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') self.mox.StubOutWithMock(fake_mig, 'save') - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_rpcapi, 'confirm_resize') @@ -1658,30 +1510,17 @@ class _ComputeAPIUnitTestMixIn(object): objects.Migration.get_by_instance_and_status( self.context, fake_inst['uuid'], 'finished').AndReturn( fake_mig) - compute_utils.downsize_quota_delta(self.context, - fake_inst).AndReturn('deltas') - - resvs = ['resvs'] - fake_quotas = objects.Quotas.from_reservations(self.context, resvs) - - compute_utils.reserve_quota_delta(self.context, 'deltas', - fake_inst).AndReturn(fake_quotas) def _check_mig(expected_task_state=None): self.assertEqual('confirming', fake_mig.status) fake_mig.save().WithSideEffects(_check_mig) - if self.cell_type: - quota.QUOTAS.commit(self.context, resvs, project_id=None, - user_id=None) - self.compute_api._record_action_start(self.context, fake_inst, 'confirmResize') self.compute_api.compute_rpcapi.confirm_resize( - self.context, fake_inst, fake_mig, 'compute-source', - [] if self.cell_type else fake_quotas.reservations) + self.context, fake_inst, fake_mig, 'compute-source') self.mox.ReplayAll() @@ -1697,28 +1536,20 @@ class _ComputeAPIUnitTestMixIn(object): def test_confirm_resize_with_migration_ref(self): self._test_confirm_resize(mig_ref_passed=True) - @mock.patch('nova.quota.QUOTAS.commit') - @mock.patch.object(compute_utils, 'reserve_quota_delta') - @mock.patch.object(compute_utils, 'reverse_upsize_quota_delta') + @mock.patch('nova.objects.Quotas.check_deltas') @mock.patch('nova.objects.Migration.get_by_instance_and_status') @mock.patch('nova.context.RequestContext.elevated') def _test_revert_resize(self, mock_elevated, mock_get_migration, - mock_reverse_upsize_quota_delta, - mock_reserve_quota_delta, - mock_quota_commit): + mock_check): params = dict(vm_state=vm_states.RESIZED) fake_inst = self._create_instance_obj(params=params) + fake_inst.old_flavor = fake_inst.flavor fake_mig = objects.Migration._from_db_object( self.context, objects.Migration(), test_migration.fake_db_migration()) - resvs = ['resvs'] - fake_quotas = objects.Quotas.from_reservations(self.context, resvs) - mock_elevated.return_value = self.context mock_get_migration.return_value = fake_mig - mock_reverse_upsize_quota_delta.return_value = 'deltas' - mock_reserve_quota_delta.return_value = fake_quotas def _check_state(expected_task_state=None): self.assertEqual(task_states.RESIZE_REVERTING, @@ -1739,47 +1570,30 @@ class _ComputeAPIUnitTestMixIn(object): mock_elevated.assert_called_once_with() mock_get_migration.assert_called_once_with( self.context, fake_inst['uuid'], 'finished') - mock_reverse_upsize_quota_delta.assert_called_once_with( - self.context, fake_inst) - mock_reserve_quota_delta.assert_called_once_with( - self.context, 'deltas', fake_inst) mock_inst_save.assert_called_once_with(expected_task_state=[None]) mock_mig_save.assert_called_once_with() - if self.cell_type: - mock_quota_commit.assert_called_once_with( - self.context, resvs, project_id=None, user_id=None) mock_record_action.assert_called_once_with(self.context, fake_inst, 'revertResize') mock_revert_resize.assert_called_once_with( - self.context, fake_inst, fake_mig, 'compute-dest', - [] if self.cell_type else fake_quotas.reservations) + self.context, fake_inst, fake_mig, 'compute-dest') def test_revert_resize(self): self._test_revert_resize() - @mock.patch('nova.quota.QUOTAS.rollback') - @mock.patch.object(compute_utils, 'reserve_quota_delta') - @mock.patch.object(compute_utils, 'reverse_upsize_quota_delta') + @mock.patch('nova.objects.Quotas.check_deltas') @mock.patch('nova.objects.Migration.get_by_instance_and_status') @mock.patch('nova.context.RequestContext.elevated') def test_revert_resize_concurrent_fail(self, mock_elevated, - mock_get_migration, - mock_reverse_upsize_quota_delta, - mock_reserve_quota_delta, - mock_quota_rollback): + mock_get_migration, mock_check): params = dict(vm_state=vm_states.RESIZED) fake_inst = self._create_instance_obj(params=params) + fake_inst.old_flavor = fake_inst.flavor fake_mig = objects.Migration._from_db_object( self.context, objects.Migration(), test_migration.fake_db_migration()) mock_elevated.return_value = self.context mock_get_migration.return_value = fake_mig - delta = ['delta'] - mock_reverse_upsize_quota_delta.return_value = delta - resvs = ['resvs'] - fake_quotas = objects.Quotas.from_reservations(self.context, resvs) - mock_reserve_quota_delta.return_value = fake_quotas exc = exception.UnexpectedTaskStateError( instance_uuid=fake_inst['uuid'], @@ -1796,13 +1610,7 @@ class _ComputeAPIUnitTestMixIn(object): mock_elevated.assert_called_once_with() mock_get_migration.assert_called_once_with( self.context, fake_inst['uuid'], 'finished') - mock_reverse_upsize_quota_delta.assert_called_once_with( - self.context, fake_inst) - mock_reserve_quota_delta.assert_called_once_with( - self.context, delta, fake_inst) mock_inst_save.assert_called_once_with(expected_task_state=[None]) - mock_quota_rollback.assert_called_once_with( - self.context, resvs, project_id=None, user_id=None) def _test_resize(self, flavor_id_passed=True, same_host=False, allow_same_host=False, @@ -1823,9 +1631,10 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') self.mox.StubOutWithMock(compute_utils, 'upsize_quota_delta') - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') self.mox.StubOutWithMock(fake_inst, 'save') - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') + self.mox.StubOutWithMock(quotas_obj.Quotas, 'count_as_dict') + self.mox.StubOutWithMock(quotas_obj.Quotas, + 'limit_check_project_and_user') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(objects.RequestSpec, 'get_by_instance_uuid') self.mox.StubOutWithMock(self.compute_api.compute_task_api, @@ -1845,17 +1654,30 @@ class _ComputeAPIUnitTestMixIn(object): if (self.cell_type == 'compute' or not (flavor_id_passed and same_flavor)): - resvs = ['resvs'] project_id, user_id = quotas_obj.ids_from_instance(self.context, fake_inst) - fake_quotas = objects.Quotas.from_reservations(self.context, - resvs) if flavor_id_passed: compute_utils.upsize_quota_delta( self.context, mox.IsA(objects.Flavor), - mox.IsA(objects.Flavor)).AndReturn('deltas') - compute_utils.reserve_quota_delta( - self.context, 'deltas', fake_inst).AndReturn(fake_quotas) + mox.IsA(objects.Flavor)).AndReturn({'cores': 0, 'ram': 0}) + + proj_count = {'instances': 1, 'cores': current_flavor.vcpus, + 'ram': current_flavor.memory_mb} + user_count = proj_count.copy() + # mox.IgnoreArg() might be 'instances', 'cores', or 'ram' + # depending on how the deltas dict is iterated in check_deltas + quotas_obj.Quotas.count_as_dict(self.context, mox.IgnoreArg(), + project_id, + user_id=user_id).AndReturn( + {'project': proj_count, + 'user': user_count}) + # The current and new flavor have the same cores/ram + req_cores = current_flavor.vcpus + req_ram = current_flavor.memory_mb + values = {'cores': req_cores, 'ram': req_ram} + quotas_obj.Quotas.limit_check_project_and_user( + self.context, user_values=values, project_values=values, + project_id=project_id, user_id=user_id) def _check_state(expected_task_state=None): self.assertEqual(task_states.RESIZE_PREP, @@ -1872,15 +1694,7 @@ class _ComputeAPIUnitTestMixIn(object): else: filter_properties = {'ignore_hosts': [fake_inst['host']]} - if flavor_id_passed: - expected_reservations = fake_quotas.reservations - else: - expected_reservations = [] if self.cell_type == 'api': - if flavor_id_passed: - quota.QUOTAS.commit(self.context, resvs, project_id=None, - user_id=None) - expected_reservations = [] mig = objects.Migration() def _get_migration(context=None): @@ -1922,7 +1736,6 @@ class _ComputeAPIUnitTestMixIn(object): self.context, fake_inst, extra_kwargs, scheduler_hint=scheduler_hint, flavor=mox.IsA(objects.Flavor), - reservations=expected_reservations, clean_shutdown=clean_shutdown, request_spec=fake_spec) @@ -1964,6 +1777,37 @@ class _ComputeAPIUnitTestMixIn(object): def test_resize_forced_shutdown(self): self._test_resize(clean_shutdown=False) + @mock.patch('nova.compute.flavors.get_flavor_by_flavor_id') + @mock.patch('nova.objects.Quotas.count_as_dict') + @mock.patch('nova.objects.Quotas.limit_check_project_and_user') + def test_resize_quota_check(self, mock_check, mock_count, mock_get): + self.flags(cores=1, group='quota') + self.flags(ram=2048, group='quota') + proj_count = {'instances': 1, 'cores': 1, 'ram': 1024} + user_count = proj_count.copy() + mock_count.return_value = {'project': proj_count, + 'user': user_count} + + cur_flavor = objects.Flavor(id=1, name='foo', vcpus=1, memory_mb=512, + root_gb=10, disabled=False) + fake_inst = self._create_instance_obj() + fake_inst.flavor = cur_flavor + new_flavor = objects.Flavor(id=2, name='bar', vcpus=1, memory_mb=2048, + root_gb=10, disabled=False) + mock_get.return_value = new_flavor + mock_check.side_effect = exception.OverQuota( + overs=['ram'], quotas={'cores': 1, 'ram': 2048}, + usages={'instances': 1, 'cores': 1, 'ram': 2048}, + headroom={'ram': 2048}) + + self.assertRaises(exception.TooManyInstances, self.compute_api.resize, + self.context, fake_inst, flavor_id='new') + mock_check.assert_called_once_with( + self.context, + user_values={'cores': 1, 'ram': 2560}, + project_values={'cores': 1, 'ram': 2560}, + project_id=fake_inst.project_id, user_id=fake_inst.user_id) + def test_migrate(self): self._test_migrate() @@ -1982,8 +1826,9 @@ class _ComputeAPIUnitTestMixIn(object): def test_resize_invalid_flavor_fails(self): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') # Should never reach these. - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') + self.mox.StubOutWithMock(quotas_obj.Quotas, 'count_as_dict') + self.mox.StubOutWithMock(quotas_obj.Quotas, + 'limit_check_project_and_user') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, 'resize_instance') @@ -2005,8 +1850,9 @@ class _ComputeAPIUnitTestMixIn(object): def test_resize_disabled_flavor_fails(self): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') # Should never reach these. - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') + self.mox.StubOutWithMock(quotas_obj.Quotas, 'count_as_dict') + self.mox.StubOutWithMock(quotas_obj.Quotas, + 'limit_check_project_and_user') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, 'resize_instance') @@ -2072,9 +1918,10 @@ class _ComputeAPIUnitTestMixIn(object): def test_resize_quota_exceeds_fails(self): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') self.mox.StubOutWithMock(compute_utils, 'upsize_quota_delta') - self.mox.StubOutWithMock(compute_utils, 'reserve_quota_delta') + self.mox.StubOutWithMock(quotas_obj.Quotas, 'count_as_dict') + self.mox.StubOutWithMock(quotas_obj.Quotas, + 'limit_check_project_and_user') # Should never reach these. - self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') self.mox.StubOutWithMock(self.compute_api.compute_task_api, 'resize_instance') @@ -2084,21 +1931,34 @@ class _ComputeAPIUnitTestMixIn(object): name='foo', disabled=False) flavors.get_flavor_by_flavor_id( 'flavor-id', read_deleted='no').AndReturn(fake_flavor) - deltas = dict(resource=0) + deltas = dict(cores=0) compute_utils.upsize_quota_delta( self.context, mox.IsA(objects.Flavor), mox.IsA(objects.Flavor)).AndReturn(deltas) - usage = dict(in_use=0, reserved=0) - quotas = {'resource': 0} - usages = {'resource': usage} - overs = ['resource'] + quotas = {'cores': 0} + overs = ['cores'] over_quota_args = dict(quotas=quotas, - usages=usages, + usages={'instances': 1, 'cores': 1, 'ram': 512}, overs=overs) - compute_utils.reserve_quota_delta(self.context, deltas, - fake_inst).AndRaise( - exception.OverQuota(**over_quota_args)) + proj_count = {'instances': 1, 'cores': fake_inst.flavor.vcpus, + 'ram': fake_inst.flavor.memory_mb} + user_count = proj_count.copy() + # mox.IgnoreArg() might be 'instances', 'cores', or 'ram' + # depending on how the deltas dict is iterated in check_deltas + quotas_obj.Quotas.count_as_dict(self.context, mox.IgnoreArg(), + fake_inst.project_id, + user_id=fake_inst.user_id).AndReturn( + {'project': proj_count, + 'user': user_count}) + req_cores = fake_inst.flavor.vcpus + req_ram = fake_inst.flavor.memory_mb + values = {'cores': req_cores, 'ram': req_ram} + quotas_obj.Quotas.limit_check_project_and_user( + self.context, user_values=values, project_values=values, + project_id=fake_inst.project_id, + user_id=fake_inst.user_id).AndRaise( + exception.OverQuota(**over_quota_args)) self.mox.ReplayAll() @@ -2110,8 +1970,9 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(flavors, 'get_flavor_by_flavor_id') @mock.patch.object(compute_utils, 'upsize_quota_delta') - @mock.patch.object(compute_utils, 'reserve_quota_delta') - def test_resize_quota_exceeds_fails_instance(self, mock_reserve, + @mock.patch.object(quotas_obj.Quotas, 'count_as_dict') + @mock.patch.object(quotas_obj.Quotas, 'limit_check_project_and_user') + def test_resize_quota_exceeds_fails_instance(self, mock_check, mock_count, mock_upsize, mock_flavor): fake_inst = self._create_instance_obj() fake_flavor = self._create_flavor(id=200, flavorid='flavor-id', @@ -2119,14 +1980,12 @@ class _ComputeAPIUnitTestMixIn(object): mock_flavor.return_value = fake_flavor deltas = dict(cores=1, ram=1) mock_upsize.return_value = deltas - usage = dict(in_use=0, reserved=0) quotas = {'instances': 1, 'cores': -1, 'ram': -1} - usages = {'instances': usage, 'cores': usage, 'ram': usage} overs = ['ram'] over_quota_args = dict(quotas=quotas, - usages=usages, + usages={'instances': 1, 'cores': 1, 'ram': 512}, overs=overs) - mock_reserve.side_effect = exception.OverQuota(**over_quota_args) + mock_check.side_effect = exception.OverQuota(**over_quota_args) with mock.patch.object(fake_inst, 'save') as mock_save: self.assertRaises(exception.TooManyInstances, @@ -2134,45 +1993,21 @@ class _ComputeAPIUnitTestMixIn(object): fake_inst, flavor_id='flavor-id') self.assertFalse(mock_save.called) - def test_check_instance_quota_exceeds_with_multiple_resources(self): - quotas = {'cores': 1, 'instances': 1, 'ram': 512} - usages = {'cores': dict(in_use=1, reserved=0), - 'instances': dict(in_use=1, reserved=0), - 'ram': dict(in_use=512, reserved=0)} - overs = ['cores', 'instances', 'ram'] - over_quota_args = dict(quotas=quotas, - usages=usages, - overs=overs) - e = exception.OverQuota(**over_quota_args) - fake_flavor = self._create_flavor() - instance_num = 1 - with mock.patch.object(objects.Quotas, 'reserve', side_effect=e): - try: - self.compute_api._check_num_instances_quota(self.context, - fake_flavor, - instance_num, - instance_num) - except exception.TooManyInstances as e: - self.assertEqual('cores, instances, ram', e.kwargs['overs']) - self.assertEqual('1, 1, 512', e.kwargs['req']) - self.assertEqual('1, 1, 512', e.kwargs['used']) - self.assertEqual('1, 1, 512', e.kwargs['allowed']) - else: - self.fail("Exception not raised") - @mock.patch.object(flavors, 'get_flavor_by_flavor_id') - @mock.patch.object(objects.Quotas, 'reserve') + @mock.patch.object(objects.Quotas, 'count_as_dict') + @mock.patch.object(objects.Quotas, 'limit_check_project_and_user') def test_resize_instance_quota_exceeds_with_multiple_resources( - self, mock_reserve, mock_get_flavor): + self, mock_check, mock_count, mock_get_flavor): quotas = {'cores': 1, 'ram': 512} - usages = {'cores': dict(in_use=1, reserved=0), - 'ram': dict(in_use=512, reserved=0)} overs = ['cores', 'ram'] over_quota_args = dict(quotas=quotas, - usages=usages, + usages={'instances': 1, 'cores': 1, 'ram': 512}, overs=overs) - mock_reserve.side_effect = exception.OverQuota(**over_quota_args) + proj_count = {'instances': 1, 'cores': 1, 'ram': 512} + user_count = proj_count.copy() + mock_count.return_value = {'project': proj_count, 'user': user_count} + mock_check.side_effect = exception.OverQuota(**over_quota_args) mock_get_flavor.return_value = self._create_flavor(id=333, vcpus=3, memory_mb=1536) @@ -3365,8 +3200,8 @@ class _ComputeAPIUnitTestMixIn(object): "contents": "foo" } ] - with mock.patch.object(quota.QUOTAS, 'limit_check', - side_effect=side_effect): + with mock.patch('nova.objects.Quotas.limit_check', + side_effect=side_effect): self.compute_api._check_injected_file_quota( self.context, injected_files) @@ -3393,15 +3228,18 @@ class _ComputeAPIUnitTestMixIn(object): self._test_check_injected_file_quota_onset_file_limit_exceeded, side_effect) - @mock.patch('nova.objects.Quotas.commit') - @mock.patch('nova.objects.Quotas.reserve') + @mock.patch('nova.objects.Quotas.count_as_dict') + @mock.patch('nova.objects.Quotas.limit_check_project_and_user') @mock.patch('nova.objects.Instance.save') @mock.patch('nova.objects.InstanceAction.action_start') def test_restore_by_admin(self, action_start, instance_save, - quota_reserve, quota_commit): + quota_check, quota_count): admin_context = context.RequestContext('admin_user', 'admin_project', True) + proj_count = {'instances': 1, 'cores': 1, 'ram': 512} + user_count = proj_count.copy() + quota_count.return_value = {'project': proj_count, 'user': user_count} instance = self._create_instance_obj() instance.vm_state = vm_states.SOFT_DELETED instance.task_state = None @@ -3411,17 +3249,30 @@ class _ComputeAPIUnitTestMixIn(object): rpc.restore_instance.assert_called_once_with(admin_context, instance) self.assertEqual(instance.task_state, task_states.RESTORING) - self.assertEqual(1, quota_commit.call_count) - quota_reserve.assert_called_once_with(instances=1, - cores=instance.flavor.vcpus, ram=instance.flavor.memory_mb, + # mock.ANY might be 'instances', 'cores', or 'ram' depending on how the + # deltas dict is iterated in check_deltas + quota_count.assert_called_once_with(admin_context, mock.ANY, + instance.project_id, + user_id=instance.user_id) + quota_check.assert_called_once_with( + admin_context, + user_values={'instances': 2, + 'cores': 1 + instance.flavor.vcpus, + 'ram': 512 + instance.flavor.memory_mb}, + project_values={'instances': 2, + 'cores': 1 + instance.flavor.vcpus, + 'ram': 512 + instance.flavor.memory_mb}, project_id=instance.project_id, user_id=instance.user_id) - @mock.patch('nova.objects.Quotas.commit') - @mock.patch('nova.objects.Quotas.reserve') + @mock.patch('nova.objects.Quotas.count_as_dict') + @mock.patch('nova.objects.Quotas.limit_check_project_and_user') @mock.patch('nova.objects.Instance.save') @mock.patch('nova.objects.InstanceAction.action_start') def test_restore_by_instance_owner(self, action_start, instance_save, - quota_reserve, quota_commit): + quota_check, quota_count): + proj_count = {'instances': 1, 'cores': 1, 'ram': 512} + user_count = proj_count.copy() + quota_count.return_value = {'project': proj_count, 'user': user_count} instance = self._create_instance_obj() instance.vm_state = vm_states.SOFT_DELETED instance.task_state = None @@ -3432,9 +3283,19 @@ class _ComputeAPIUnitTestMixIn(object): instance) self.assertEqual(instance.project_id, self.context.project_id) self.assertEqual(instance.task_state, task_states.RESTORING) - self.assertEqual(1, quota_commit.call_count) - quota_reserve.assert_called_once_with(instances=1, - cores=instance.flavor.vcpus, ram=instance.flavor.memory_mb, + # mock.ANY might be 'instances', 'cores', or 'ram' depending on how the + # deltas dict is iterated in check_deltas + quota_count.assert_called_once_with(self.context, mock.ANY, + instance.project_id, + user_id=instance.user_id) + quota_check.assert_called_once_with( + self.context, + user_values={'instances': 2, + 'cores': 1 + instance.flavor.vcpus, + 'ram': 512 + instance.flavor.memory_mb}, + project_values={'instances': 2, + 'cores': 1 + instance.flavor.vcpus, + 'ram': 512 + instance.flavor.memory_mb}, project_id=instance.project_id, user_id=instance.user_id) @mock.patch.object(objects.InstanceAction, 'action_start') @@ -3649,7 +3510,7 @@ class _ComputeAPIUnitTestMixIn(object): def _test_provision_instances_with_cinder_error(self, expected_exception): - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects.Instance, 'create') @mock.patch.object(self.compute_api.security_group_api, 'ensure_default') @@ -3658,10 +3519,9 @@ class _ComputeAPIUnitTestMixIn(object): def do_test( mock_req_spec_from_components, _mock_create_bdm, _mock_ensure_default, _mock_create, mock_check_num_inst_quota): - quota_mock = mock.MagicMock() req_spec_mock = mock.MagicMock() - mock_check_num_inst_quota.return_value = (1, quota_mock) + mock_check_num_inst_quota.return_value = 1 mock_req_spec_from_components.return_value = req_spec_mock ctxt = context.RequestContext('fake-user', 'fake-project') @@ -3746,7 +3606,7 @@ class _ComputeAPIUnitTestMixIn(object): mock_br, mock_rs): fake_keypair = objects.KeyPair(name='test') - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(self.compute_api, 'security_group_api') @mock.patch.object(self.compute_api, 'create_db_entry_for_new_instance') @@ -3754,7 +3614,7 @@ class _ComputeAPIUnitTestMixIn(object): '_bdm_validate_set_size_and_instance') @mock.patch.object(self.compute_api, '_create_block_device_mapping') def do_test(mock_cbdm, mock_bdm_v, mock_cdb, mock_sg, mock_cniq): - mock_cniq.return_value = 1, mock.MagicMock() + mock_cniq.return_value = 1 self.compute_api._provision_instances(self.context, mock.sentinel.flavor, 1, 1, mock.MagicMock(), @@ -3780,7 +3640,7 @@ class _ComputeAPIUnitTestMixIn(object): def test_provision_instances_creates_build_request(self): @mock.patch.object(objects.Instance, 'create') @mock.patch.object(self.compute_api, 'volume_api') - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(self.compute_api.security_group_api, 'ensure_default') @mock.patch.object(objects.RequestSpec, 'from_components') @@ -3795,8 +3655,7 @@ class _ComputeAPIUnitTestMixIn(object): min_count = 1 max_count = 2 - quota_mock = mock.MagicMock() - mock_check_num_inst_quota.return_value = (2, quota_mock) + mock_check_num_inst_quota.return_value = 2 ctxt = context.RequestContext('fake-user', 'fake-project') flavor = self._create_flavor() @@ -3859,7 +3718,7 @@ class _ComputeAPIUnitTestMixIn(object): do_test() def test_provision_instances_creates_instance_mapping(self): - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects.Instance, 'create', new=mock.MagicMock()) @mock.patch.object(self.compute_api.security_group_api, 'ensure_default', new=mock.MagicMock()) @@ -3873,10 +3732,9 @@ class _ComputeAPIUnitTestMixIn(object): new=mock.MagicMock()) @mock.patch('nova.objects.InstanceMapping') def do_test(mock_inst_mapping, mock_check_num_inst_quota): - quota_mock = mock.MagicMock() inst_mapping_mock = mock.MagicMock() - mock_check_num_inst_quota.return_value = (1, quota_mock) + mock_check_num_inst_quota.return_value = 1 mock_inst_mapping.return_value = inst_mapping_mock ctxt = context.RequestContext('fake-user', 'fake-project') @@ -3947,7 +3805,7 @@ class _ComputeAPIUnitTestMixIn(object): _mock_cinder_reserve_volume, _mock_cinder_check_availability_zone, _mock_cinder_get, _mock_get_min_ver): - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects, 'Instance') @mock.patch.object(self.compute_api.security_group_api, 'ensure_default') @@ -3958,11 +3816,10 @@ class _ComputeAPIUnitTestMixIn(object): def do_test(mock_inst_mapping, mock_build_req, mock_req_spec_from_components, _mock_create_bdm, _mock_ensure_default, mock_inst, mock_check_num_inst_quota): - quota_mock = mock.MagicMock() min_count = 1 max_count = 2 - mock_check_num_inst_quota.return_value = (2, quota_mock) + mock_check_num_inst_quota.return_value = 2 req_spec_mock = mock.MagicMock() mock_req_spec_from_components.return_value = req_spec_mock inst_mocks = [mock.MagicMock() for i in range(max_count)] @@ -4036,7 +3893,7 @@ class _ComputeAPIUnitTestMixIn(object): do_test() def test_provision_instances_creates_reqspec_with_secgroups(self): - @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(self.compute_api, 'security_group_api') @mock.patch.object(compute_api, 'objects') @mock.patch.object(self.compute_api, '_create_block_device_mapping', @@ -4049,7 +3906,7 @@ class _ComputeAPIUnitTestMixIn(object): new=mock.MagicMock()) def test(mock_objects, mock_secgroup, mock_cniq): ctxt = context.RequestContext('fake-user', 'fake-project') - mock_cniq.return_value = (1, mock.MagicMock()) + mock_cniq.return_value = 1 self.compute_api._provision_instances(ctxt, None, None, None, mock.MagicMock(), None, None, [], None, None, None, None, diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index c4343b3b0b9e..97a31eabaed4 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -33,6 +33,8 @@ from nova.compute import vm_states import nova.conf from nova import context from nova import db +from nova.db.sqlalchemy import api as db_api +from nova.db.sqlalchemy import api_models from nova import exception from nova import objects from nova.objects import fields as obj_fields @@ -160,6 +162,81 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase): def test_create_instance_associates_security_groups(self): self.skipTest("Test is incompatible with cells.") + @mock.patch('nova.objects.quotas.Quotas.check_deltas') + def test_create_instance_over_quota_during_recheck( + self, check_deltas_mock): + self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', + self.fake_show) + + # Simulate a race where the first check passes and the recheck fails. + fake_quotas = {'instances': 5, 'cores': 10, 'ram': 4096} + fake_headroom = {'instances': 5, 'cores': 10, 'ram': 4096} + fake_usages = {'instances': 5, 'cores': 10, 'ram': 4096} + exc = exception.OverQuota(overs=['instances'], quotas=fake_quotas, + headroom=fake_headroom, usages=fake_usages) + check_deltas_mock.side_effect = [None, exc] + + inst_type = flavors.get_default_flavor() + # Try to create 3 instances. + self.assertRaises(exception.QuotaError, self.compute_api.create, + self.context, inst_type, self.fake_image['id'], min_count=3) + + project_id = self.context.project_id + + self.assertEqual(2, check_deltas_mock.call_count) + call1 = mock.call(self.context, + {'instances': 3, 'cores': inst_type.vcpus * 3, + 'ram': inst_type.memory_mb * 3}, + project_id, user_id=None, + check_project_id=project_id, check_user_id=None) + call2 = mock.call(self.context, {'instances': 0, 'cores': 0, 'ram': 0}, + project_id, user_id=None, + check_project_id=project_id, check_user_id=None) + check_deltas_mock.assert_has_calls([call1, call2]) + + # Verify we removed the artifacts that were added after the first + # quota check passed. + instances = objects.InstanceList.get_all(self.context) + self.assertEqual(0, len(instances)) + build_requests = objects.BuildRequestList.get_all(self.context) + self.assertEqual(0, len(build_requests)) + + @db_api.api_context_manager.reader + def request_spec_get_all(context): + return context.session.query(api_models.RequestSpec).all() + + request_specs = request_spec_get_all(self.context) + self.assertEqual(0, len(request_specs)) + + instance_mappings = objects.InstanceMappingList.get_by_project_id( + self.context, project_id) + self.assertEqual(0, len(instance_mappings)) + + @mock.patch('nova.objects.quotas.Quotas.check_deltas') + def test_create_instance_no_quota_recheck( + self, check_deltas_mock): + self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', + self.fake_show) + # Disable recheck_quota. + self.flags(recheck_quota=False, group='quota') + + inst_type = flavors.get_default_flavor() + (refs, resv_id) = self.compute_api.create(self.context, + inst_type, + self.fake_image['id']) + self.assertEqual(1, len(refs)) + + project_id = self.context.project_id + + # check_deltas should have been called only once. + check_deltas_mock.assert_called_once_with(self.context, + {'instances': 1, + 'cores': inst_type.vcpus, + 'ram': inst_type.memory_mb}, + project_id, user_id=None, + check_project_id=project_id, + check_user_id=None) + @mock.patch.object(compute_api.API, '_local_delete') @mock.patch.object(compute_api.API, '_lookup_instance', return_value=(None, None)) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index f19df48fcccc..d7222338095b 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -154,7 +154,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): specd_compute._delete_instance(specd_compute, self.context, mock_inst, - mock.Mock(), mock.Mock()) methods_called = [n for n, a, k in call_tracker.mock_calls] @@ -277,7 +276,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): vm_state=vm_states.ERROR, host=self.compute.host, expected_attrs=['system_metadata']) - quotas = mock.create_autospec(objects.Quotas, spec_set=True) with test.nested( mock.patch.object(self.compute, '_notify_about_instance_usage'), @@ -290,7 +288,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): instance_obj_load_attr, instance_save, instance_destroy ): instance.info_cache = None - self.compute._delete_instance(self.context, instance, [], quotas) + self.compute._delete_instance(self.context, instance, []) mock_notify.assert_has_calls([ mock.call(self.context, instance, 'fake-mini', @@ -779,11 +777,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'destroy') @mock.patch.object(objects.Instance, 'obj_load_attr') - @mock.patch.object(objects.quotas.Quotas, 'commit') - @mock.patch.object(objects.quotas.Quotas, 'reserve') @mock.patch.object(objects.quotas, 'ids_from_instance') def test_init_instance_complete_partial_deletion( - self, mock_ids_from_instance, mock_reserve, mock_commit, + self, mock_ids_from_instance, mock_inst_destroy, mock_obj_load_attr, mock_get_by_instance_uuid, mock_bdm_destroy): """Test to complete deletion for instances in DELETED status but not @@ -810,10 +806,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertEqual(vm_states.DELETED, instance.vm_state) self.assertFalse(instance.deleted) - deltas = {'instances': -1, - 'cores': -instance.flavor.vcpus, - 'ram': -instance.flavor.memory_mb} - def fake_inst_destroy(): instance.deleted = True instance.deleted_at = timeutils.utcnow() @@ -826,12 +818,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): # Make sure that instance.destroy method was called and # instance was deleted from db. - self.assertTrue(mock_reserve.called) - self.assertTrue(mock_commit.called) self.assertNotEqual(0, instance.deleted) - mock_reserve.assert_called_once_with(project_id=instance.project_id, - user_id=instance.user_id, - **deltas) @mock.patch('nova.compute.manager.LOG') def test_init_instance_complete_partial_deletion_raises_exception( @@ -872,24 +859,18 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): task_state=task_states.DELETING) bdms = [] - quotas = objects.quotas.Quotas(self.context) with test.nested( mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid', return_value=bdms), mock.patch.object(self.compute, '_delete_instance'), - mock.patch.object(instance, 'obj_load_attr'), - mock.patch.object(self.compute, '_create_reservations', - return_value=quotas) - ) as (mock_get, mock_delete, mock_load, mock_create): + mock.patch.object(instance, 'obj_load_attr') + ) as (mock_get, mock_delete, mock_load): self.compute._init_instance(self.context, instance) mock_get.assert_called_once_with(self.context, instance.uuid) - mock_create.assert_called_once_with(self.context, instance, - instance.project_id, - instance.user_id) mock_delete.assert_called_once_with(self.context, instance, - bdms, mock.ANY) + bdms) @mock.patch.object(objects.Instance, 'get_by_uuid') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @@ -910,7 +891,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): expected_attrs=['metadata', 'system_metadata']) bdms = [] - reservations = ['fake-resv'] def _create_patch(name, attr): patcher = mock.patch.object(name, attr) @@ -921,10 +901,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): mock_delete_instance = _create_patch(self.compute, '_delete_instance') mock_set_instance_error_state = _create_patch( self.compute, '_set_instance_obj_error_state') - mock_create_reservations = _create_patch(self.compute, - '_create_reservations') - - mock_create_reservations.return_value = reservations mock_get_by_instance_uuid.return_value = bdms mock_get_by_uuid.return_value = instance mock_delete_instance.side_effect = test.TestingException('test') @@ -1173,28 +1149,18 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): host=self.compute.host, task_state=task_states.DELETING) bdms = [] - quotas = objects.quotas.Quotas(self.context) with test.nested( mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid', return_value=bdms), mock.patch.object(self.compute, '_delete_instance'), - mock.patch.object(instance, 'obj_load_attr'), - mock.patch.object(self.compute, '_create_reservations', - return_value=quotas), - mock.patch.object(objects.quotas, 'ids_from_instance', - return_value=(instance.project_id, - instance.user_id)) - ) as (mock_get, mock_delete, mock_load, mock_create, mock_ids): + mock.patch.object(instance, 'obj_load_attr') + ) as (mock_get, mock_delete, mock_load): self.compute._init_instance(self.context, instance) mock_get.assert_called_once_with(self.context, instance.uuid) - mock_create.assert_called_once_with(self.context, instance, - instance.project_id, - instance.user_id) mock_delete.assert_called_once_with(self.context, instance, - bdms, mock.ANY) - mock_ids.assert_called_once_with(self.context, instance) + bdms) def test_init_instance_resize_prep(self): instance = fake_instance.fake_instance_obj( @@ -3425,16 +3391,13 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): def test_error_out_instance_on_exception_unknown_with_quotas(self, set_error): instance = fake_instance.fake_instance_obj(self.context) - quotas = mock.create_autospec(objects.Quotas, spec_set=True) def do_test(): with self.compute._error_out_instance_on_exception( - self.context, instance, quotas): + self.context, instance): raise test.TestingException('test') self.assertRaises(test.TestingException, do_test) - self.assertEqual(1, len(quotas.method_calls)) - self.assertEqual(mock.call.rollback(), quotas.method_calls[0]) set_error.assert_called_once_with(self.context, instance) def test_cleanup_volumes(self): @@ -3871,7 +3834,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): mock_bdm_get_by_inst.assert_called_once_with( self.context, instance.uuid) mock_delete_instance.assert_called_once_with( - self.context, instance, bdms, mock.ANY) + self.context, instance, bdms) @mock.patch.object(nova.compute.manager.ComputeManager, '_notify_about_instance_usage') @@ -5501,7 +5464,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): mock.ANY, mock.ANY, not is_shared) mock_finish_revert.assert_called_once_with( self.context, self.instance, self.migration, - self.migration.source_compute, mock.ANY) + self.migration.source_compute) do_test() diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index b71c89e2ac54..ffaee73e061b 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -928,9 +928,9 @@ class ComputeUtilsTestCase(test.NoDBTestCase): mock_notify_usage.assert_has_calls(expected_notify_calls) -class ComputeUtilsQuotaDeltaTestCase(test.TestCase): +class ComputeUtilsQuotaTestCase(test.TestCase): def setUp(self): - super(ComputeUtilsQuotaDeltaTestCase, self).setUp() + super(ComputeUtilsQuotaTestCase, self).setUp() self.context = context.RequestContext('fake', 'fake') def test_upsize_quota_delta(self): @@ -995,6 +995,35 @@ class ComputeUtilsQuotaDeltaTestCase(test.TestCase): mock_reserve.assert_called_once_with(project_id=inst.project_id, user_id=inst.user_id, **deltas) + @mock.patch('nova.objects.Quotas.count_as_dict') + def test_check_instance_quota_exceeds_with_multiple_resources(self, + mock_count): + quotas = {'cores': 1, 'instances': 1, 'ram': 512} + overs = ['cores', 'instances', 'ram'] + over_quota_args = dict(quotas=quotas, + usages={'instances': 1, 'cores': 1, 'ram': 512}, + overs=overs) + e = exception.OverQuota(**over_quota_args) + fake_flavor = objects.Flavor(vcpus=1, memory_mb=512) + instance_num = 1 + proj_count = {'instances': 1, 'cores': 1, 'ram': 512} + user_count = proj_count.copy() + mock_count.return_value = {'project': proj_count, 'user': user_count} + with mock.patch.object(objects.Quotas, 'limit_check_project_and_user', + side_effect=e): + try: + compute_utils.check_num_instances_quota(self.context, + fake_flavor, + instance_num, + instance_num) + except exception.TooManyInstances as e: + self.assertEqual('cores, instances, ram', e.kwargs['overs']) + self.assertEqual('1, 1, 512', e.kwargs['req']) + self.assertEqual('1, 1, 512', e.kwargs['used']) + self.assertEqual('1, 1, 512', e.kwargs['allowed']) + else: + self.fail("Exception not raised") + class IsVolumeBackedInstanceTestCase(test.TestCase): def setUp(self): diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index cbba18bb86b6..6086eb2c5c38 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -58,17 +58,13 @@ class MigrationTaskTestCase(test.NoDBTestCase): @mock.patch.object(scheduler_utils, 'setup_instance_group') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize') - @mock.patch.object(objects.Quotas, 'from_reservations') - def test_execute(self, quotas_mock, prep_resize_mock, - sel_dest_mock, sig_mock, az_mock): + def test_execute(self, prep_resize_mock, sel_dest_mock, sig_mock, az_mock): sel_dest_mock.return_value = self.hosts az_mock.return_value = 'myaz' task = self._generate_task() legacy_request_spec = self.request_spec.to_legacy_request_spec_dict() task.execute() - quotas_mock.assert_called_once_with(self.context, self.reservations, - instance=self.instance) sig_mock.assert_called_once_with(self.context, self.request_spec) task.scheduler_client.select_destinations.assert_called_once_with( self.context, self.request_spec, [self.instance.uuid]) @@ -78,11 +74,4 @@ class MigrationTaskTestCase(test.NoDBTestCase): request_spec=legacy_request_spec, filter_properties=self.filter_properties, node=self.hosts[0]['nodename'], clean_shutdown=self.clean_shutdown) - self.assertFalse(quotas_mock.return_value.rollback.called) az_mock.assert_called_once_with(self.context, 'host1') - - def test_rollback(self): - task = self._generate_task() - task.quotas = mock.MagicMock() - task.rollback() - task.quotas.rollback.assert_called_once_with() diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index d1bb0d03590f..4124374443bd 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -38,6 +38,8 @@ from nova.conductor.tasks import migrate from nova import conf from nova import context from nova import db +from nova.db.sqlalchemy import api as db_api +from nova.db.sqlalchemy import api_models from nova import exception as exc from nova.image import api as image_api from nova import objects @@ -1337,9 +1339,14 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.ctxt, instance_uuid=build_request.instance.uuid, cell_mapping=None, project_id=self.ctxt.project_id) im.create() - params['request_specs'] = [objects.RequestSpec( - instance_uuid=build_request.instance_uuid, - instance_group=None)] + rs = fake_request_spec.fake_spec_obj(remove_id=True) + rs._context = self.ctxt + rs.instance_uuid = build_request.instance_uuid + rs.instance_group = None + rs.retry = None + rs.limits = None + rs.create() + params['request_specs'] = [rs] params['image'] = {'fake_data': 'should_pass_silently'} params['admin_password'] = 'admin_password', params['injected_files'] = 'injected_files' @@ -1677,6 +1684,73 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertTrue(bury.called) self.assertFalse(build_and_run.called) + @mock.patch('nova.objects.quotas.Quotas.check_deltas') + @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') + def test_schedule_and_build_over_quota_during_recheck(self, mock_select, + mock_check): + mock_select.return_value = [{'host': 'fake-host', + 'nodename': 'fake-nodename', + 'limits': None}] + # Simulate a race where the first check passes and the recheck fails. + # First check occurs in compute/api. + fake_quotas = {'instances': 5, 'cores': 10, 'ram': 4096} + fake_headroom = {'instances': 5, 'cores': 10, 'ram': 4096} + fake_usages = {'instances': 5, 'cores': 10, 'ram': 4096} + e = exc.OverQuota(overs=['instances'], quotas=fake_quotas, + headroom=fake_headroom, usages=fake_usages) + mock_check.side_effect = e + + # This is needed to register the compute node in a cell. + self.start_service('compute', host='fake-host') + self.assertRaises( + exc.TooManyInstances, + self.conductor.schedule_and_build_instances, **self.params) + + project_id = self.params['context'].project_id + mock_check.assert_called_once_with( + self.params['context'], {'instances': 0, 'cores': 0, 'ram': 0}, + project_id, user_id=None, check_project_id=project_id, + check_user_id=None) + + # Verify we set the instance to ERROR state and set the fault message. + instances = objects.InstanceList.get_all(self.ctxt) + self.assertEqual(1, len(instances)) + instance = instances[0] + self.assertEqual(vm_states.ERROR, instance.vm_state) + self.assertIsNone(instance.task_state) + self.assertIn('Quota exceeded', instance.fault.message) + # Verify we removed the build objects. + build_requests = objects.BuildRequestList.get_all(self.ctxt) + + self.assertEqual(0, len(build_requests)) + + @db_api.api_context_manager.reader + def request_spec_get_all(context): + return context.session.query(api_models.RequestSpec).all() + + request_specs = request_spec_get_all(self.ctxt) + self.assertEqual(0, len(request_specs)) + + @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') + @mock.patch('nova.objects.quotas.Quotas.check_deltas') + @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') + def test_schedule_and_build_no_quota_recheck(self, mock_select, + mock_check, mock_build): + mock_select.return_value = [{'host': 'fake-host', + 'nodename': 'fake-nodename', + 'limits': None}] + # Disable recheck_quota. + self.flags(recheck_quota=False, group='quota') + # This is needed to register the compute node in a cell. + self.start_service('compute', host='fake-host') + self.conductor.schedule_and_build_instances(**self.params) + + # check_deltas should not have been called a second time. The first + # check occurs in compute/api. + mock_check.assert_not_called() + + self.assertTrue(mock_build.called) + @mock.patch('nova.objects.CellMapping.get_by_uuid') def test_bury_in_cell0_no_cell0(self, mock_cm_get): mock_cm_get.side_effect = exc.CellMappingNotFound(uuid='0') @@ -1893,13 +1967,12 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(scheduler_utils, 'setup_instance_group') @mock.patch.object(utils, 'get_image_from_system_metadata') - @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(conductor_manager.ComputeTaskManager, '_set_vm_state_and_notify') @mock.patch.object(migrate.MigrationTask, 'rollback') def test_cold_migrate_no_valid_host_back_in_active_state( - self, rollback_mock, notify_mock, select_dest_mock, quotas_mock, + self, rollback_mock, notify_mock, select_dest_mock, metadata_mock, sig_mock, spec_fc_mock, im_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( @@ -1934,8 +2007,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): flavor, {}, [resvs], True, None) metadata_mock.assert_called_with({}) - quotas_mock.assert_called_once_with(self.context, [resvs], - instance=inst_obj) sig_mock.assert_called_once_with(self.context, fake_spec) notify_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, @@ -1946,13 +2017,12 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(scheduler_utils, 'setup_instance_group') @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(utils, 'get_image_from_system_metadata') - @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(conductor_manager.ComputeTaskManager, '_set_vm_state_and_notify') @mock.patch.object(migrate.MigrationTask, 'rollback') def test_cold_migrate_no_valid_host_back_in_stopped_state( - self, rollback_mock, notify_mock, select_dest_mock, quotas_mock, + self, rollback_mock, notify_mock, select_dest_mock, metadata_mock, spec_fc_mock, sig_mock, im_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( @@ -1988,8 +2058,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): flavor, {}, [resvs], True, None) metadata_mock.assert_called_with({}) - quotas_mock.assert_called_once_with(self.context, [resvs], - instance=inst_obj) sig_mock.assert_called_once_with(self.context, fake_spec) notify_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, @@ -2072,7 +2140,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(scheduler_utils, 'setup_instance_group') @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(utils, 'get_image_from_system_metadata') - @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(conductor_manager.ComputeTaskManager, '_set_vm_state_and_notify') @@ -2080,7 +2147,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize') def test_cold_migrate_exception_host_in_error_state_and_raise( self, prep_resize_mock, rollback_mock, notify_mock, - select_dest_mock, quotas_mock, metadata_mock, spec_fc_mock, + select_dest_mock, metadata_mock, spec_fc_mock, sig_mock, im_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( @@ -2123,8 +2190,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 'limits': {}} metadata_mock.assert_called_with({}) - quotas_mock.assert_called_once_with(self.context, [resvs], - instance=inst_obj) sig_mock.assert_called_once_with(self.context, fake_spec) select_dest_mock.assert_called_once_with( self.context, fake_spec, [inst_obj.uuid]) diff --git a/nova/tests/unit/fake_instance.py b/nova/tests/unit/fake_instance.py index cfce7297dde9..fd0abad7e291 100644 --- a/nova/tests/unit/fake_instance.py +++ b/nova/tests/unit/fake_instance.py @@ -124,6 +124,12 @@ def fake_instance_obj(context, obj_instance_class=None, **updates): inst.keypairs = objects.KeyPairList(objects=[]) if flavor: inst.flavor = flavor + # This is needed for instance quota counting until we have the + # ability to count allocations in placement. + if 'vcpus' in flavor and 'vcpus' not in updates: + inst.vcpus = flavor.vcpus + if 'memory_mb' in flavor and 'memory_mb' not in updates: + inst.memory_mb = flavor.memory_mb inst.old_flavor = None inst.new_flavor = None inst.obj_reset_changes() diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 07c360d589ce..6b6bcca425d9 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1112,7 +1112,7 @@ object_data = { 'InstanceGroup': '1.10-1a0c8c7447dc7ecb9da53849430c4a5f', 'InstanceGroupList': '1.8-90f8f1a445552bb3bbc9fa1ae7da27d4', 'InstanceInfoCache': '1.5-cd8b96fefe0fc8d4d337243ba0bf0e1e', - 'InstanceList': '2.3-7a3c541e6e7b5a75afe7afe125f9712d', + 'InstanceList': '2.4-d2c5723da8c1d08e07cb00160edfd292', 'InstanceMapping': '1.0-65de80c491f54d19374703c0753c4d47', 'InstanceMappingList': '1.2-ee638619aa3d8a82a59c0c83bfa64d78', 'InstanceNUMACell': '1.4-7c1eb9a198dee076b4de0840e45f4f55', diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index fffba9d91c49..9a249484939b 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -363,12 +363,6 @@ class ProjectCommandsTestCase(test.TestCase): self.assertIsNone(self.commands.quota_usage_refresh( 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa')) - def test_quota_usage_refresh_with_keys(self): - self.assertIsNone(self.commands.quota_usage_refresh( - 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', - 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaab', - 'ram')) - def test_quota_usage_refresh_invalid_user_key(self): self.assertEqual(2, self.commands.quota_usage_refresh( 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', diff --git a/nova/tests/unit/test_quota.py b/nova/tests/unit/test_quota.py index 2a41ca15f9dd..680c0f4cbc05 100644 --- a/nova/tests/unit/test_quota.py +++ b/nova/tests/unit/test_quota.py @@ -14,11 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. -import datetime - import mock from oslo_db.sqlalchemy import enginefacade -from oslo_utils import timeutils from six.moves import range from nova import compute @@ -26,9 +23,9 @@ from nova.compute import flavors import nova.conf from nova import context from nova import db -from nova.db.sqlalchemy import api as sqa_api from nova.db.sqlalchemy import models as sqa_models from nova import exception +from nova import objects from nova import quota from nova import test import nova.tests.unit.image.fake @@ -43,7 +40,10 @@ def _get_fake_get_usages(updates=None): usages = {'security_group_rules': {'in_use': 1}, 'key_pairs': {'in_use': 2}, 'server_group_members': {'in_use': 3}, - 'floating_ips': {'in_use': 2}} + 'floating_ips': {'in_use': 2}, + 'instances': {'in_use': 2}, + 'cores': {'in_use': 4}, + 'ram': {'in_use': 10 * 1024}} if updates: usages.update(updates) @@ -85,22 +85,24 @@ class QuotaIntegrationTestCase(test.TestCase): super(QuotaIntegrationTestCase, self).tearDown() nova.tests.unit.image.fake.FakeImageService_reset() - def _create_instance(self, cores=2): + def _create_instance(self, flavor_name='m1.large'): """Create a test instance.""" - inst = {} - inst['image_id'] = 'cedef40a-ed67-4d10-800e-17455edce175' - inst['reservation_id'] = 'r-fakeres' - inst['user_id'] = self.user_id - inst['project_id'] = self.project_id - inst['instance_type_id'] = '3' # m1.large - inst['vcpus'] = cores - return db.instance_create(self.context, inst) + inst = objects.Instance(context=self.context) + inst.image_id = 'cedef40a-ed67-4d10-800e-17455edce175' + inst.reservation_id = 'r-fakeres' + inst.user_id = self.user_id + inst.project_id = self.project_id + inst.flavor = flavors.get_flavor_by_name(flavor_name) + # This is needed for instance quota counting until we have the + # ability to count allocations in placement. + inst.vcpus = inst.flavor.vcpus + inst.memory_mb = inst.flavor.memory_mb + inst.create() + return inst def test_too_many_instances(self): - instance_uuids = [] for i in range(CONF.quota.instances): - instance = self._create_instance() - instance_uuids.append(instance['uuid']) + self._create_instance() inst_type = flavors.get_flavor_by_name('m1.small') image_uuid = 'cedef40a-ed67-4d10-800e-17455edce175' try: @@ -110,17 +112,15 @@ class QuotaIntegrationTestCase(test.TestCase): except exception.QuotaError as e: expected_kwargs = {'code': 413, 'req': '1, 1', - 'used': '4, 2', + 'used': '8, 2', 'allowed': '4, 2', 'overs': 'cores, instances'} self.assertEqual(expected_kwargs, e.kwargs) else: self.fail('Expected QuotaError exception') - for instance_uuid in instance_uuids: - db.instance_destroy(self.context, instance_uuid) def test_too_many_cores(self): - instance = self._create_instance(cores=4) + self._create_instance() inst_type = flavors.get_flavor_by_name('m1.small') image_uuid = 'cedef40a-ed67-4d10-800e-17455edce175' try: @@ -136,13 +136,14 @@ class QuotaIntegrationTestCase(test.TestCase): self.assertEqual(expected_kwargs, e.kwargs) else: self.fail('Expected QuotaError exception') - db.instance_destroy(self.context, instance['uuid']) def test_many_cores_with_unlimited_quota(self): # Setting cores quota to unlimited: self.flags(cores=-1, group='quota') - instance = self._create_instance(cores=4) - db.instance_destroy(self.context, instance['uuid']) + # Default is 20 cores, so create 3x m1.xlarge with + # 8 cores each. + for i in range(3): + self._create_instance(flavor_name='m1.xlarge') def test_too_many_addresses(self): # This test is specifically relying on nova-network. @@ -253,26 +254,6 @@ class QuotaIntegrationTestCase(test.TestCase): self.assertRaises(exception.QuotaError, self._create_with_injected_files, files) - def test_reservation_expire(self): - self.useFixture(test.TimeOverride()) - - def assertInstancesReserved(reserved): - result = quota.QUOTAS.get_project_quotas(self.context, - self.context.project_id) - self.assertEqual(result['instances']['reserved'], reserved) - - quota.QUOTAS.reserve(self.context, - expire=60, - instances=2) - - assertInstancesReserved(2) - - timeutils.advance_time_seconds(80) - - quota.QUOTAS.expire(self.context) - - assertInstancesReserved(0) - @enginefacade.transaction_context_provider class FakeContext(context.RequestContext): @@ -512,17 +493,23 @@ class BaseResourceTestCase(test.TestCase): def test_valid_method_call_check_wrong_method_reserve(self): resources = {'key_pairs': 1} + engine_resources = {'key_pairs': quota.CountableResource('key_pairs', + None, + 'key_pairs')} self.assertRaises(exception.InvalidQuotaMethodUsage, quota._valid_method_call_check_resources, - resources, 'reserve', quota.QUOTAS._resources) + resources, 'reserve', engine_resources) def test_valid_method_call_check_wrong_method_check(self): resources = {'instances': 1} + engine_resources = {'instances': quota.ReservableResource('instances', + None, + 'instances')} self.assertRaises(exception.InvalidQuotaMethodUsage, quota._valid_method_call_check_resources, - resources, 'check', quota.QUOTAS._resources) + resources, 'check', engine_resources) class QuotaEngineTestCase(test.TestCase): @@ -1000,25 +987,9 @@ class DbQuotaDriverTestCase(test.TestCase): 'injected_file_path_bytes': 127, } - def fake_qugabpau(context, project_id, user_id): - self.calls.append('quota_usage_get_all_by_project_and_user') - self.assertEqual(project_id, 'test_project') - self.assertEqual(user_id, 'fake_user') - return dict( - instances=dict(in_use=2, reserved=2), - cores=dict(in_use=4, reserved=4), - ram=dict(in_use=10 * 1024, reserved=0), - metadata_items=dict(in_use=0, reserved=0), - injected_files=dict(in_use=0, reserved=0), - injected_file_content_bytes=dict(in_use=0, reserved=0), - injected_file_path_bytes=dict(in_use=0, reserved=0), - ) - self.stub_out('nova.db.quota_get_all_by_project_and_user', fake_qgabpau) self.stub_out('nova.db.quota_get_all_by_project', fake_qgabp) - self.stub_out('nova.db.quota_usage_get_all_by_project_and_user', - fake_qugabpau) self._stub_quota_class_get_all_by_name() @@ -1104,13 +1075,9 @@ class DbQuotaDriverTestCase(test.TestCase): 'security_group_rules': {'in_use': 0}} self.assertEqual(expected, actual) - @mock.patch('nova.quota.DbQuotaDriver._get_usages') + @mock.patch('nova.quota.DbQuotaDriver._get_usages', + side_effect=_get_fake_get_usages()) def test_get_user_quotas(self, mock_get_usages): - # This will test that the counted usage will not be overwritten by - # the quota_usages records (in_use=2, reserved=2) from the database. - usages = {'instances': {'in_use': 5}} - mock_get_usages.side_effect = _get_fake_get_usages(updates=usages) - self.maxDiff = None self._stub_get_by_project_and_user() ctxt = FakeContext('test_project', 'test_class') @@ -1120,7 +1087,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project_and_user', 'quota_get_all_by_project', - 'quota_usage_get_all_by_project_and_user', 'quota_class_get_all_by_name', ]) mock_get_usages.assert_called_once_with(ctxt, quota.QUOTAS._resources, @@ -1129,13 +1095,13 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(result, dict( instances=dict( limit=5, - in_use=5, + in_use=2, reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=25 * 1024, @@ -1231,19 +1197,6 @@ class DbQuotaDriverTestCase(test.TestCase): injected_file_path_bytes=127, ) - def fake_qugabp(context, project_id): - self.calls.append('quota_usage_get_all_by_project') - self.assertEqual(project_id, 'test_project') - return dict( - instances=dict(in_use=2, reserved=2), - cores=dict(in_use=4, reserved=4), - ram=dict(in_use=10 * 1024, reserved=0), - metadata_items=dict(in_use=0, reserved=0), - injected_files=dict(in_use=0, reserved=0), - injected_file_content_bytes=dict(in_use=0, reserved=0), - injected_file_path_bytes=dict(in_use=0, reserved=0), - ) - def fake_quota_get_all(context, project_id): self.calls.append('quota_get_all') self.assertEqual(project_id, 'test_project') @@ -1253,19 +1206,14 @@ class DbQuotaDriverTestCase(test.TestCase): hard_limit=2)] self.stub_out('nova.db.quota_get_all_by_project', fake_qgabp) - self.stub_out('nova.db.quota_usage_get_all_by_project', fake_qugabp) self.stub_out('nova.db.quota_get_all', fake_quota_get_all) self._stub_quota_class_get_all_by_name() self._stub_quota_class_get_default() - @mock.patch('nova.quota.DbQuotaDriver._get_usages') + @mock.patch('nova.quota.DbQuotaDriver._get_usages', + side_effect=_get_fake_get_usages()) def test_get_project_quotas(self, mock_get_usages): - # This will test that the counted usage will not be overwritten by - # the quota_usages records (in_use=2, reserved=2) from the database. - usages = {'instances': {'in_use': 5}} - mock_get_usages.side_effect = _get_fake_get_usages(updates=usages) - self.maxDiff = None self._stub_get_by_project() ctxt = FakeContext('test_project', 'test_class') @@ -1274,7 +1222,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project', - 'quota_usage_get_all_by_project', 'quota_class_get_all_by_name', 'quota_class_get_default', ]) @@ -1283,13 +1230,13 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(result, dict( instances=dict( limit=5, - in_use=5, + in_use=2, reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=25 * 1024, @@ -1364,7 +1311,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project', - 'quota_usage_get_all_by_project', 'quota_class_get_all_by_name', 'quota_class_get_default', 'quota_get_all', @@ -1375,13 +1321,13 @@ class DbQuotaDriverTestCase(test.TestCase): instances=dict( limit=5, in_use=2, - reserved=2, + reserved=0, remains=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, remains=8, ), ram=dict( @@ -1470,7 +1416,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project_and_user', 'quota_get_all_by_project', - 'quota_usage_get_all_by_project_and_user', ]) mock_get_usages.assert_called_once_with(ctxt, quota.QUOTAS._resources, 'test_project', @@ -1479,12 +1424,12 @@ class DbQuotaDriverTestCase(test.TestCase): instances=dict( limit=10, in_use=2, - reserved=2, + reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=50 * 1024, @@ -1559,7 +1504,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project', - 'quota_usage_get_all_by_project', 'quota_class_get_default', ]) mock_get_usages.assert_called_once_with(ctxt, quota.QUOTAS._resources, @@ -1568,12 +1512,12 @@ class DbQuotaDriverTestCase(test.TestCase): instances=dict( limit=5, in_use=2, - reserved=2, + reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=25 * 1024, @@ -1650,7 +1594,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project_and_user', 'quota_get_all_by_project', - 'quota_usage_get_all_by_project_and_user', 'quota_class_get_all_by_name', ]) mock_get_usages.assert_called_once_with(ctxt, quota.QUOTAS._resources, @@ -1660,12 +1603,12 @@ class DbQuotaDriverTestCase(test.TestCase): instances=dict( limit=5, in_use=2, - reserved=2, + reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=25 * 1024, @@ -1741,7 +1684,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project', - 'quota_usage_get_all_by_project', 'quota_class_get_all_by_name', 'quota_class_get_default', ]) @@ -1751,12 +1693,12 @@ class DbQuotaDriverTestCase(test.TestCase): instances=dict( limit=5, in_use=2, - reserved=2, + reserved=0, ), cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), ram=dict( limit=25 * 1024, @@ -1832,7 +1774,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project_and_user', 'quota_get_all_by_project', - 'quota_usage_get_all_by_project_and_user', 'quota_class_get_all_by_name', ]) mock_get_usages.assert_called_once_with(ctxt, quota.QUOTAS._resources, @@ -1842,7 +1783,7 @@ class DbQuotaDriverTestCase(test.TestCase): cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), injected_files=dict( limit=2, @@ -1866,7 +1807,6 @@ class DbQuotaDriverTestCase(test.TestCase): self.assertEqual(self.calls, [ 'quota_get_all_by_project', - 'quota_usage_get_all_by_project', 'quota_class_get_all_by_name', 'quota_class_get_default', ]) @@ -1876,7 +1816,7 @@ class DbQuotaDriverTestCase(test.TestCase): cores=dict( limit=10, in_use=4, - reserved=4, + reserved=0, ), injected_files=dict( limit=2, @@ -2016,7 +1956,6 @@ class DbQuotaDriverTestCase(test.TestCase): result = {} for k, v in resources.items(): limit = v.default - reserved = 0 if k == 'instances': remains = v.default - 5 in_use = 1 @@ -2032,7 +1971,7 @@ class DbQuotaDriverTestCase(test.TestCase): remains = v.default in_use = 0 result[k] = {'limit': limit, 'in_use': in_use, - 'reserved': reserved, 'remains': remains} + 'remains': remains} return result def fake_process_quotas_in_get_user_quotas(dbdrv, context, resources, @@ -2043,16 +1982,14 @@ class DbQuotaDriverTestCase(test.TestCase): self.calls.append('_process_quotas') result = {} for k, v in resources.items(): - reserved = 0 if k == 'instances': in_use = 1 elif k == 'cores': - in_use = 5 - reserved = 10 + in_use = 15 else: in_use = 0 result[k] = {'limit': v.default, - 'in_use': in_use, 'reserved': reserved} + 'in_use': in_use} return result def fake_qgabpau(context, project_id, user_id): @@ -2290,82 +2227,14 @@ class DbQuotaDriverTestCase(test.TestCase): self.stub_out('nova.quota.DbQuotaDriver.get_project_quotas', fake_get_project_quotas) - def test_get_quotas_has_sync_unknown(self): + def test_get_quotas_unknown(self): self._stub_get_project_quotas() self.assertRaises(exception.QuotaResourceUnknown, self.driver._get_quotas, None, quota.QUOTAS._resources, - ['unknown'], True) + ['unknown']) self.assertEqual(self.calls, []) - def test_get_quotas_no_sync_unknown(self): - self._stub_get_project_quotas() - self.assertRaises(exception.QuotaResourceUnknown, - self.driver._get_quotas, - None, quota.QUOTAS._resources, - ['unknown'], False) - self.assertEqual(self.calls, []) - - def test_get_quotas_has_sync_no_sync_resource(self): - self._stub_get_project_quotas() - self.assertRaises(exception.QuotaResourceUnknown, - self.driver._get_quotas, - None, quota.QUOTAS._resources, - ['metadata_items'], True) - self.assertEqual(self.calls, []) - - def test_get_quotas_no_sync_has_sync_resource(self): - self._stub_get_project_quotas() - self.assertRaises(exception.QuotaResourceUnknown, - self.driver._get_quotas, - None, quota.QUOTAS._resources, - ['instances'], False) - self.assertEqual(self.calls, []) - - def test_get_quotas_has_sync(self): - self._stub_get_project_quotas() - result = self.driver._get_quotas(FakeContext('test_project', - 'test_class'), - quota.QUOTAS._resources, - ['instances', 'cores', 'ram'], - True, - project_id='test_project') - - self.assertEqual(self.calls, ['get_project_quotas']) - self.assertEqual(result, dict( - instances=10, - cores=20, - ram=50 * 1024, - )) - - def test_get_quotas_no_sync(self): - self._stub_get_project_quotas() - result = self.driver._get_quotas(FakeContext('test_project', - 'test_class'), - quota.QUOTAS._resources, - ['metadata_items', 'injected_files', - 'injected_file_content_bytes', - 'injected_file_path_bytes', - 'security_group_rules', - 'server_group_members', - 'server_groups', 'security_groups', - 'floating_ips'], - False, - project_id='test_project') - - self.assertEqual(self.calls, ['get_project_quotas']) - self.assertEqual(result, dict( - metadata_items=128, - injected_files=5, - injected_file_content_bytes=10 * 1024, - injected_file_path_bytes=255, - security_group_rules=20, - server_group_members=10, - server_groups=10, - security_groups=10, - floating_ips=10, - )) - def test_limit_check_under(self): self._stub_get_project_quotas() self.assertRaises(exception.InvalidQuotaValue, @@ -2508,713 +2377,6 @@ class DbQuotaDriverTestCase(test.TestCase): for kwarg in kwargs: self.driver.limit_check_project_and_user(ctxt, resources, **kwarg) - def _stub_quota_reserve(self): - def fake_quota_reserve(context, resources, quotas, user_quotas, deltas, - expire, until_refresh, max_age, project_id=None, - user_id=None): - self.calls.append(('quota_reserve', expire, until_refresh, - max_age)) - return ['resv-1', 'resv-2', 'resv-3'] - self.stub_out('nova.db.quota_reserve', fake_quota_reserve) - - def test_reserve_bad_expire(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - self.assertRaises(exception.InvalidReservationExpiration, - self.driver.reserve, - FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire='invalid') - self.assertEqual(self.calls, []) - - def test_reserve_default_expire(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2)) - - expire = timeutils.utcnow() + datetime.timedelta(seconds=86400) - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 0, 0), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_reserve_int_expire(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire=3600) - - expire = timeutils.utcnow() + datetime.timedelta(seconds=3600) - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 0, 0), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_reserve_timedelta_expire(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - expire_delta = datetime.timedelta(seconds=60) - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire=expire_delta) - - expire = timeutils.utcnow() + expire_delta - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 0, 0), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_reserve_datetime_expire(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - expire = timeutils.utcnow() + datetime.timedelta(seconds=120) - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire=expire) - - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 0, 0), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_reserve_until_refresh(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - self.flags(until_refresh=500, group='quota') - expire = timeutils.utcnow() + datetime.timedelta(seconds=120) - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire=expire) - - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 500, 0), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_reserve_max_age(self): - self._stub_get_project_quotas() - self._stub_quota_reserve() - self.flags(max_age=86400, group='quota') - expire = timeutils.utcnow() + datetime.timedelta(seconds=120) - result = self.driver.reserve(FakeContext('test_project', 'test_class'), - quota.QUOTAS._resources, - dict(instances=2), expire=expire) - - self.assertEqual(self.calls, [ - 'get_project_quotas', - ('quota_reserve', expire, 0, 86400), - ]) - self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - - def test_usage_reset(self): - calls = [] - - def fake_quota_usage_update(context, project_id, user_id, resource, - **kwargs): - calls.append(('quota_usage_update', context, project_id, user_id, - resource, kwargs)) - if resource == 'nonexist': - raise exception.QuotaUsageNotFound(project_id=project_id) - self.stub_out('nova.db.quota_usage_update', fake_quota_usage_update) - - ctx = FakeContext('test_project', 'test_class') - resources = ['res1', 'res2', 'nonexist', 'res4'] - self.driver.usage_reset(ctx, resources) - - # Make sure we had some calls - self.assertEqual(len(calls), len(resources)) - - # Extract the elevated context that was used and do some - # sanity checks - elevated = calls[0][1] - self.assertEqual(elevated.project_id, ctx.project_id) - self.assertEqual(elevated.quota_class, ctx.quota_class) - self.assertTrue(elevated.is_admin) - - # Now check that all the expected calls were made - exemplar = [('quota_usage_update', elevated, 'test_project', - 'fake_user', res, dict(in_use=-1)) for res in resources] - self.assertEqual(calls, exemplar) - - -class FakeSession(object): - def begin(self): - return self - - def add(self, instance): - pass - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc_value, exc_traceback): - return False - - -class FakeUsage(sqa_models.QuotaUsage): - def save(self, *args, **kwargs): - pass - - -class QuotaSqlAlchemyBase(test.TestCase): - def setUp(self): - super(QuotaSqlAlchemyBase, self).setUp() - self.sync_called = set() - self.quotas = dict( - instances=5, - cores=10, - ram=10 * 1024, - ) - self.deltas = dict( - instances=2, - cores=4, - ram=2 * 1024, - ) - - def make_sync(res_name): - def sync(context, project_id, user_id): - self.sync_called.add(res_name) - if res_name in self.usages: - if self.usages[res_name].in_use < 0: - return {res_name: 2} - else: - return {res_name: self.usages[res_name].in_use - 1} - return {res_name: 0} - return sync - self.resources = {} - - _existing_quota_sync_func_dict = dict(sqa_api.QUOTA_SYNC_FUNCTIONS) - - def restore_sync_functions(): - sqa_api.QUOTA_SYNC_FUNCTIONS.clear() - sqa_api.QUOTA_SYNC_FUNCTIONS.update(_existing_quota_sync_func_dict) - - self.addCleanup(restore_sync_functions) - - for res_name in ('instances', 'cores', 'ram'): - method_name = '_sync_%s' % res_name - sqa_api.QUOTA_SYNC_FUNCTIONS[method_name] = make_sync(res_name) - res = quota.ReservableResource(res_name, '_sync_%s' % res_name) - self.resources[res_name] = res - - self.expire = timeutils.utcnow() + datetime.timedelta(seconds=3600) - self.usages = {} - self.usages_created = {} - self.reservations_created = {} - self.usages_list = [ - dict(resource='instances', - project_id='test_project', - user_id='fake_user', - in_use=2, - reserved=2, - until_refresh=None), - dict(resource='cores', - project_id='test_project', - user_id='fake_user', - in_use=2, - reserved=4, - until_refresh=None), - dict(resource='ram', - project_id='test_project', - user_id='fake_user', - in_use=2, - reserved=2 * 1024, - until_refresh=None), - ] - - def fake_get_project_user_quota_usages(context, project_id, user_id): - return self.usages.copy(), self.usages.copy() - - def fake_quota_usage_create(project_id, user_id, resource, - in_use, reserved, until_refresh, - session): - quota_usage_ref = self._make_quota_usage( - project_id, user_id, resource, in_use, reserved, until_refresh, - timeutils.utcnow(), timeutils.utcnow()) - - self.usages_created[resource] = quota_usage_ref - - return quota_usage_ref - - def fake_reservation_create(uuid, usage_id, project_id, - user_id, resource, delta, expire, - session): - reservation_ref = self._make_reservation( - uuid, usage_id, project_id, user_id, resource, delta, expire, - timeutils.utcnow(), timeutils.utcnow()) - - self.reservations_created[resource] = reservation_ref - - return reservation_ref - - self.stub_out('nova.db.sqlalchemy.api._get_project_user_quota_usages', - fake_get_project_user_quota_usages) - self.stub_out('nova.db.sqlalchemy.api._quota_usage_create', - fake_quota_usage_create) - self.stub_out('nova.db.sqlalchemy.api._reservation_create', - fake_reservation_create) - - self.useFixture(test.TimeOverride()) - - def _make_quota_usage(self, project_id, user_id, resource, in_use, - reserved, until_refresh, created_at, updated_at): - quota_usage_ref = FakeUsage() - quota_usage_ref.id = len(self.usages) + len(self.usages_created) - quota_usage_ref.project_id = project_id - quota_usage_ref.user_id = user_id - quota_usage_ref.resource = resource - quota_usage_ref.in_use = in_use - quota_usage_ref.reserved = reserved - quota_usage_ref.until_refresh = until_refresh - quota_usage_ref.created_at = created_at - quota_usage_ref.updated_at = updated_at - quota_usage_ref.deleted_at = None - quota_usage_ref.deleted = False - - return quota_usage_ref - - def init_usage(self, project_id, user_id, resource, in_use, reserved=0, - until_refresh=None, created_at=None, updated_at=None): - if created_at is None: - created_at = timeutils.utcnow() - if updated_at is None: - updated_at = timeutils.utcnow() - if resource == 'fixed_ips': - user_id = None - - quota_usage_ref = self._make_quota_usage(project_id, user_id, resource, - in_use, reserved, - until_refresh, - created_at, updated_at) - - self.usages[resource] = quota_usage_ref - - def compare_usage(self, usage_dict, expected): - for usage in expected: - resource = usage['resource'] - for key, value in usage.items(): - actual = getattr(usage_dict[resource], key) - self.assertEqual(actual, value, - "%s != %s on usage for resource %s, key %s" % - (actual, value, resource, key)) - - def _make_reservation(self, uuid, usage_id, project_id, user_id, resource, - delta, expire, created_at, updated_at): - reservation_ref = sqa_models.Reservation() - reservation_ref.id = len(self.reservations_created) - reservation_ref.uuid = uuid - reservation_ref.usage_id = usage_id - reservation_ref.project_id = project_id - reservation_ref.user_id = user_id - reservation_ref.resource = resource - reservation_ref.delta = delta - reservation_ref.expire = expire - reservation_ref.created_at = created_at - reservation_ref.updated_at = updated_at - reservation_ref.deleted_at = None - reservation_ref.deleted = False - - return reservation_ref - - def compare_reservation(self, reservations, expected): - reservations = set(reservations) - for resv in expected: - resource = resv['resource'] - resv_obj = self.reservations_created[resource] - - self.assertIn(resv_obj.uuid, reservations) - reservations.discard(resv_obj.uuid) - - for key, value in resv.items(): - actual = getattr(resv_obj, key) - self.assertEqual(actual, value, - "%s != %s on reservation for resource %s" % - (actual, value, resource)) - - self.assertEqual(len(reservations), 0) - - def _update_reservations_list(self, usage_id_change=False, - delta_change=False): - reservations_list = [ - dict(resource='instances', - project_id='test_project', - delta=2), - dict(resource='cores', - project_id='test_project', - delta=4), - dict(resource='ram', - delta=2 * 1024), - ] - if usage_id_change: - reservations_list[0]["usage_id"] = self.usages_created['instances'] - reservations_list[1]["usage_id"] = self.usages_created['cores'] - reservations_list[2]["usage_id"] = self.usages_created['ram'] - else: - reservations_list[0]["usage_id"] = self.usages['instances'] - reservations_list[1]["usage_id"] = self.usages['cores'] - reservations_list[2]["usage_id"] = self.usages['ram'] - if delta_change: - reservations_list[0]["delta"] = -2 - reservations_list[1]["delta"] = -4 - reservations_list[2]["delta"] = -2 * 1024 - return reservations_list - - def _init_usages(self, *in_use, **kwargs): - for i, option in enumerate(('instances', 'cores', 'ram')): - self.init_usage('test_project', 'fake_user', - option, in_use[i], **kwargs) - return FakeContext('test_project', 'test_class') - - -class QuotaReserveSqlAlchemyTestCase(QuotaSqlAlchemyBase): - # nova.db.sqlalchemy.api.quota_reserve is so complex it needs its - # own test case, and since it's a quota manipulator, this is the - # best place to put it... - - def test_quota_reserve_create_usages(self): - context = FakeContext('test_project', 'test_class') - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 0, 0) - - self.assertEqual(self.sync_called, set(['instances', 'cores', - 'ram'])) - self.usages_list[0]["in_use"] = 0 - self.usages_list[1]["in_use"] = 0 - self.usages_list[2]["in_use"] = 0 - self.compare_usage(self.usages_created, self.usages_list) - reservations_list = self._update_reservations_list(True) - self.compare_reservation(result, reservations_list) - - def test_quota_reserve_negative_in_use(self): - context = self._init_usages(-1, -1, -1, -1, until_refresh=1) - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 5, 0) - - self.assertEqual(self.sync_called, set(['instances', 'cores', - 'ram'])) - self.usages_list[0]["until_refresh"] = 5 - self.usages_list[1]["until_refresh"] = 5 - self.usages_list[2]["until_refresh"] = 5 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.compare_reservation(result, self._update_reservations_list()) - - def test_quota_reserve_until_refresh(self): - context = self._init_usages(3, 3, 3, 3, until_refresh=1) - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 5, 0) - - self.assertEqual(self.sync_called, set(['instances', 'cores', - 'ram'])) - self.usages_list[0]["until_refresh"] = 5 - self.usages_list[1]["until_refresh"] = 5 - self.usages_list[2]["until_refresh"] = 5 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.compare_reservation(result, self._update_reservations_list()) - - def test_quota_reserve_max_age(self): - max_age = 3600 - record_created = (timeutils.utcnow() - - datetime.timedelta(seconds=max_age)) - context = self._init_usages(3, 3, 3, 3, created_at=record_created, - updated_at=record_created) - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 0, max_age) - - self.assertEqual(self.sync_called, set(['instances', 'cores', - 'ram'])) - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.compare_reservation(result, self._update_reservations_list()) - - def test_quota_reserve_no_refresh(self): - context = self._init_usages(3, 3, 3, 3) - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 0, 0) - - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 3 - self.usages_list[1]["in_use"] = 3 - self.usages_list[2]["in_use"] = 3 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.compare_reservation(result, self._update_reservations_list()) - - def test_quota_reserve_unders(self): - context = self._init_usages(1, 3, 1 * 1024, 1) - self.deltas["instances"] = -2 - self.deltas["cores"] = -4 - self.deltas["ram"] = -2 * 1024 - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 0, 0) - - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 1 - self.usages_list[0]["reserved"] = 0 - self.usages_list[1]["in_use"] = 3 - self.usages_list[1]["reserved"] = 0 - self.usages_list[2]["in_use"] = 1 * 1024 - self.usages_list[2]["reserved"] = 0 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - reservations_list = self._update_reservations_list(False, True) - self.compare_reservation(result, reservations_list) - - def test_quota_reserve_overs(self): - context = self._init_usages(4, 8, 10 * 1024, 4) - try: - sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, 0, 0) - except exception.OverQuota as e: - expected_kwargs = {'code': 500, - 'usages': {'instances': {'reserved': 0, 'in_use': 4}, - 'ram': {'reserved': 0, 'in_use': 10240}, - 'cores': {'reserved': 0, 'in_use': 8}}, - 'overs': ['cores', 'instances', 'ram'], - 'quotas': {'cores': 10, 'ram': 10240, - 'instances': 5}} - self.assertEqual(e.kwargs, expected_kwargs) - else: - self.fail('Expected OverQuota failure') - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 4 - self.usages_list[0]["reserved"] = 0 - self.usages_list[1]["in_use"] = 8 - self.usages_list[1]["reserved"] = 0 - self.usages_list[2]["in_use"] = 10 * 1024 - self.usages_list[2]["reserved"] = 0 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.assertEqual(self.reservations_created, {}) - - def test_quota_reserve_cores_unlimited(self): - # Requesting 8 cores, quota_cores set to unlimited: - self.flags(cores=-1, group='quota') - self._init_usages(1, 8, 1 * 1024, 1) - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 1 - self.usages_list[0]["reserved"] = 0 - self.usages_list[1]["in_use"] = 8 - self.usages_list[1]["reserved"] = 0 - self.usages_list[2]["in_use"] = 1 * 1024 - self.usages_list[2]["reserved"] = 0 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.assertEqual(self.reservations_created, {}) - - def test_quota_reserve_ram_unlimited(self): - # Requesting 10*1024 ram, quota_ram set to unlimited: - self.flags(ram=-1, group='quota') - self._init_usages(1, 1, 10 * 1024, 1) - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 1 - self.usages_list[0]["reserved"] = 0 - self.usages_list[1]["in_use"] = 1 - self.usages_list[1]["reserved"] = 0 - self.usages_list[2]["in_use"] = 10 * 1024 - self.usages_list[2]["reserved"] = 0 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - self.assertEqual(self.reservations_created, {}) - - def test_quota_reserve_reduction(self): - context = self._init_usages(10, 20, 20 * 1024, 10) - self.deltas["instances"] = -2 - self.deltas["cores"] = -4 - self.deltas["ram"] = -2 * 1024 - result = sqa_api.quota_reserve(context, self.resources, self.quotas, - self.quotas, self.deltas, self.expire, - 0, 0) - - self.assertEqual(self.sync_called, set([])) - self.usages_list[0]["in_use"] = 10 - self.usages_list[0]["reserved"] = 0 - self.usages_list[1]["in_use"] = 20 - self.usages_list[1]["reserved"] = 0 - self.usages_list[2]["in_use"] = 20 * 1024 - self.usages_list[2]["reserved"] = 0 - self.compare_usage(self.usages, self.usages_list) - self.assertEqual(self.usages_created, {}) - reservations_list = self._update_reservations_list(False, True) - self.compare_reservation(result, reservations_list) - - -class QuotaEngineUsageRefreshTestCase(QuotaSqlAlchemyBase): - def _init_usages(self, *in_use, **kwargs): - for i, option in enumerate(('instances', 'cores', 'ram')): - self.init_usage('test_project', 'fake_user', - option, in_use[i], **kwargs) - return FakeContext('test_project', 'test_class') - - def setUp(self): - super(QuotaEngineUsageRefreshTestCase, self).setUp() - - # The usages_list are the expected usages (in_use) values after - # the test has run. - # The pattern is that the test will initialize the actual in_use - # to 3 for all the resources, then the refresh will sync - # the actual in_use to 2 for the resources whose names are in the keys - # list and are scoped to project or user. - - # The usages are indexed as follows: - # Index Resource name Scope - # 0 instances user - # 1 cores user - # 2 ram user - - # None of the usage refresh tests should add a reservation. - self.usages_list[0]['reserved'] = 0 - self.usages_list[1]['reserved'] = 0 - self.usages_list[2]['reserved'] = 0 - - def fake_quota_get_all_by_project_and_user(context, project_id, - user_id): - return self.quotas - - def fake_quota_get_all_by_project(context, project_id): - return self.quotas - - self.stub_out('nova.db.sqlalchemy.api.quota_get_all_by_project', - fake_quota_get_all_by_project) - self.stub_out( - 'nova.db.sqlalchemy.api.quota_get_all_by_project_and_user', - fake_quota_get_all_by_project_and_user) - - # The actual sync function for instances, ram, and cores, is - # _sync_instances, so override the function here. - def make_instances_sync(): - def sync(context, project_id, user_id): - updates = {} - self.sync_called.add('instances') - - for res_name in ('instances', 'cores', 'ram'): - if res_name not in self.usages: - # Usage doesn't exist yet, initialize - # the in_use to 0. - updates[res_name] = 0 - elif self.usages[res_name].in_use < 0: - updates[res_name] = 2 - else: - # Simulate as if the actual usage - # is one less than the recorded usage. - updates[res_name] = \ - self.usages[res_name].in_use - 1 - return updates - return sync - - sqa_api.QUOTA_SYNC_FUNCTIONS['_sync_instances'] = make_instances_sync() - - def test_usage_refresh_user_all_keys(self): - self._init_usages(3, 3, 3, 3, 3, 3, 3, until_refresh = 5) - # Let the parameters determine the project_id and user_id, - # not the context. - ctxt = context.get_admin_context() - quota.QUOTAS.usage_refresh(ctxt, 'test_project', 'fake_user') - - self.assertEqual(self.sync_called, set(['instances'])) - self.compare_usage(self.usages, self.usages_list) - - # No usages were created. - self.assertEqual(self.usages_created, {}) - - def test_usage_refresh_user_one_key(self): - context = self._init_usages(3, 3, 3, 3, 3, 3, 3, - until_refresh = 5) - keys = ['ram'] - # Let the context determine the project_id and user_id - quota.QUOTAS.usage_refresh(context, None, None, keys) - - self.assertEqual(self.sync_called, set(['instances'])) - - # Compare the expected usages with the actual usages. - self.compare_usage(self.usages, self.usages_list) - - # No usages were created. - self.assertEqual(self.usages_created, {}) - - def test_usage_refresh_create_user_usage(self): - context = FakeContext('test_project', 'test_class') - - # Create per-user ram usage - keys = ['ram'] - quota.QUOTAS.usage_refresh(context, 'test_project', 'fake_user', keys) - - self.assertEqual(self.sync_called, set(['instances'])) - - # Compare the expected usages with the created usages. - # Expect instances to be created and initialized to 0 - self.usages_list[0]['in_use'] = 0 - # Expect cores to be created and initialized to 0 - self.usages_list[1]['in_use'] = 0 - # Expect ram to be created and initialized to 0 - self.usages_list[2]['in_use'] = 0 - self.compare_usage(self.usages_created, self.usages_list[0:3]) - - self.assertEqual(len(self.usages_created), 3) - - def test_usage_refresh_project_all_keys(self): - self._init_usages(3, 3, 3, 3, 3, 3, 3, until_refresh = 5) - # Let the parameter determine the project_id, not the context. - ctxt = context.get_admin_context() - quota.QUOTAS.usage_refresh(ctxt, 'test_project') - - # No sync functions will be called because there are no more - # project-scoped ReservableResources - self.assertEqual(self.sync_called, set([])) - - # Compare the expected usages with the actual usages. - # Expect instances not to change since it is user scoped. - self.usages_list[0]['in_use'] = 3 - self.usages_list[0]['until_refresh'] = 5 - # Expect cores not to change since it is user scoped. - self.usages_list[1]['in_use'] = 3 - self.usages_list[1]['until_refresh'] = 5 - # Expect ram not to change since it is user scoped. - self.usages_list[2]['in_use'] = 3 - self.usages_list[2]['until_refresh'] = 5 - self.compare_usage(self.usages, self.usages_list) - - self.assertEqual(self.usages_created, {}) - - def _test_exception(self, context, project_id, user_id, keys): - try: - quota.QUOTAS.usage_refresh(context, project_id, user_id, keys) - except exception.QuotaUsageRefreshNotAllowed as e: - self.assertIn(keys[0], e.format_message()) - else: - self.fail('Expected QuotaUsageRefreshNotAllowed failure') - - def test_usage_refresh_non_syncable_user_key(self): - # security_group_rules is a valid user key, but not syncable - context = FakeContext('test_project', 'test_class') - self._test_exception(context, 'test_project', 'fake_user', - ['security_group_rules']) - - def test_usage_refresh_invalid_project_key(self): - ctxt = context.get_admin_context() - # ram is a valid syncable user key, but not a valid project key - self._test_exception(ctxt, "test_project", None, ['ram']) - - def test_usage_refresh_non_syncable_project_key(self): - # injected_files is a valid project key, but not syncable - ctxt = context.get_admin_context() - self._test_exception(ctxt, 'test_project', None, ['injected_files']) - class NoopQuotaDriverTestCase(test.TestCase): def setUp(self):