openstack/nova
Code Issues Proposed changes

Address nits in I83a5f06ad

Browse Source 
Make functions a little more coherent by ensuring they do one thing and
one thing only. Similarly ensure we don't use "private" variables in
other files.

Change-Id: Ib916e01f655d59bcc7cd0932d909e56bb1d4af8a
Related-Bug: #1545675
This commit is contained in:
Stephen Finucane 2016-03-07 13:40:21 +00:00
parent 3d7e403cc7
commit 0469f71181
4 changed files with 29 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
nova/compute/resource_tracker.py
View File

@ -131,9 +131,6 @@ def _instance_in_resize_state(instance):
return False
_REMOVED_STATES = (vm_states.DELETED, vm_states.SHELVED_OFFLOADED)
class ResourceTracker(object):
"""Compute helper class for keeping track of resource usage as instances
are built and destroyed.
@ -326,31 +323,33 @@ class ResourceTracker(object):
migration.status = 'pre-migrating'
migration.save()
def _set_instance_host_and_node(self, instance, clear=False):
def _set_instance_host_and_node(self, instance):
"""Tag the instance as belonging to this host. This should be done
while the COMPUTE_RESOURCES_SEMAPHORE is held so the resource claim
will not be lost if the audit process starts.
"""
instance.host = None if clear else self.host
if not clear:
instance.launched_on = self.host
instance.node = None if clear else self.nodename
instance.host = self.host
instance.launched_on = self.host
instance.node = self.nodename
instance.save()
def _unset_instance_host_and_node(self, instance):
"""Untag the instance so it no longer belongs to the host.
This should be done while the COMPUTE_RESOURCES_SEMAPHORE is held so
the resource claim will not be lost if the audit process starts.
"""
instance.host = None
instance.node = None
instance.save()
@utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)
def abort_instance_claim(self, context, instance):
"""Remove usage from the given instance."""
# flag the instance as deleted to revert the resource usage
# and associated stats:
real_vm_state = instance.vm_state
instance.vm_state = vm_states.DELETED
try:
self._update_usage_from_instance(context, instance)
finally:
instance.vm_state = real_vm_state
self._update_usage_from_instance(context, instance, is_removed=True)
instance.clear_numa_topology()
self._set_instance_host_and_node(instance, clear=True)
self._unset_instance_host_and_node(instance)
self._update(context.elevated())
@ -868,12 +867,14 @@ class ResourceTracker(object):
"migration."), instance_uuid=uuid)
continue
def _update_usage_from_instance(self, context, instance):
def _update_usage_from_instance(self, context, instance, is_removed=False):
"""Update usage for a single instance."""
uuid = instance['uuid']
is_new_instance = uuid not in self.tracked_instances
is_removed_instance = instance['vm_state'] in _REMOVED_STATES
is_removed_instance = (
is_removed or
instance['vm_state'] in vm_states.ALLOW_RESOURCE_REMOVAL)
if is_new_instance:
self.tracked_instances[uuid] = obj_base.obj_to_primitive(instance)
@ -883,7 +884,7 @@ class ResourceTracker(object):
self.tracked_instances.pop(uuid)
sign = -1
self.stats.update_stats_for_instance(instance)
self.stats.update_stats_for_instance(instance, is_removed_instance)
# if it's a new or deleted instance:
if is_new_instance or is_removed_instance:
@ -925,7 +926,7 @@ class ResourceTracker(object):
self.driver)
for instance in instances:
if instance.vm_state not in _REMOVED_STATES:
if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL:
self._update_usage_from_instance(context, instance)
def _find_orphaned_instances(self):

6
nova/compute/stats.py
View File

@ -13,7 +13,6 @@
# License for the specific language governing permissions and limitations
# under the License.
from nova.compute import resource_tracker
from nova.compute import task_states
from nova.compute import vm_states
from nova.i18n import _
@ -84,7 +83,7 @@ class Stats(dict):
key = "num_os_type_%s" % os_type
return self.get(key, 0)
def update_stats_for_instance(self, instance):
def update_stats_for_instance(self, instance, is_removed=False):
"""Update stats after an instance is changed."""
uuid = instance['uuid']
@ -106,10 +105,9 @@ class Stats(dict):
(vm_state, task_state, os_type, project_id) = \
self._extract_state_from_instance(instance)
if vm_state in resource_tracker._REMOVED_STATES:
if is_removed or vm_state in vm_states.ALLOW_RESOURCE_REMOVAL:
self._decrement("num_instances")
self.states.pop(uuid)
else:
self._increment("num_vm_%s" % vm_state)
self._increment("num_task_%s" % task_state)

3
nova/compute/vm_states.py
View File

@ -53,3 +53,6 @@ ALLOW_HARD_REBOOT = ALLOW_SOFT_REBOOT + [STOPPED, PAUSED, SUSPENDED, ERROR]
ALLOW_TRIGGER_CRASH_DUMP = [ACTIVE, PAUSED, RESCUED, RESIZED, ERROR]
# states we allow to trigger crash dump
ALLOW_RESOURCE_REMOVAL = [DELETED, SHELVED_OFFLOADED]
# states we allow resources to be freed in

4
nova/tests/unit/compute/test_resource_tracker.py
View File

@ -741,11 +741,11 @@ class TrackerTestCase(BaseTrackerTestCase):
self.assertEqual(self.tracker.nodename, inst.node)
self.assertEqual(self.tracker.host, inst.launched_on)
def test_set_instance_host_and_node_clear(self):
def test_unset_instance_host_and_node(self):
inst = objects.Instance()
with mock.patch.object(inst, 'save') as mock_save:
self.tracker._set_instance_host_and_node(inst)
self.tracker._set_instance_host_and_node(inst, clear=True)
self.tracker._unset_instance_host_and_node(inst)
self.assertEqual(2, mock_save.call_count)
self.assertIsNone(inst.host)
self.assertIsNone(inst.node)