Skip to content

Commit

Permalink
Keep updating allocations for Ironic
Browse files Browse the repository at this point in the history
When ironic updates the instance.flavor to require the new custom
resource class, we really need the allocations to get updated. Easiest
way to do that is to make the resource tracker keep updating allocations
for the ironic virt driver. This can be dropped once the transition to
custom resource classes is complete.

If we were not to claim the extra resources, placement will pick nodes
that already have instances running on them when you boot an instance
with a flavor that only requests the custom resource class. This should
be what all ironic flavors do, before the upgrade to queens is
performed.

Closes-Bug: #1724589

Change-Id: Ibbf65a8d817d359786abcdffa6358089ed1107f6
(cherry picked from commit 5c2b867)
  • Loading branch information
JohnGarbutt authored and mriedem committed Oct 18, 2017
1 parent 8110ab1 commit 842641c
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 6 deletions.
17 changes: 11 additions & 6 deletions nova/compute/resource_tracker.py
Expand Up @@ -1010,7 +1010,7 @@ def _update_usage_from_migrations(self, context, migrations, nodename):
continue continue


def _update_usage_from_instance(self, context, instance, nodename, def _update_usage_from_instance(self, context, instance, nodename,
is_removed=False, has_ocata_computes=False): is_removed=False, require_allocation_refresh=False):
"""Update usage for a single instance.""" """Update usage for a single instance."""


uuid = instance['uuid'] uuid = instance['uuid']
Expand Down Expand Up @@ -1038,10 +1038,9 @@ def _update_usage_from_instance(self, context, instance, nodename,
self.pci_tracker.update_pci_for_instance(context, self.pci_tracker.update_pci_for_instance(context,
instance, instance,
sign=sign) sign=sign)
if has_ocata_computes: if require_allocation_refresh:
LOG.debug("We're on a Pike compute host in a deployment " LOG.debug("Auto-correcting allocations to handle Ocata "
"with Ocata compute hosts. Auto-correcting " "assumptions.")
"allocations to handle Ocata-style assumptions.")
self.reportclient.update_instance_allocation(cn, instance, self.reportclient.update_instance_allocation(cn, instance,
sign) sign)
else: else:
Expand Down Expand Up @@ -1135,10 +1134,16 @@ def _update_usage_from_instances(self, context, instances, nodename):
context, 'nova-compute') context, 'nova-compute')
has_ocata_computes = compute_version < 22 has_ocata_computes = compute_version < 22


# Some drivers (ironic) still need the allocations to be
# fixed up, as they transition the way their inventory is reported.
require_allocation_refresh = (
has_ocata_computes or
self.driver.requires_allocation_refresh)

for instance in instances: for instance in instances:
if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL: if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL:
self._update_usage_from_instance(context, instance, nodename, self._update_usage_from_instance(context, instance, nodename,
has_ocata_computes=has_ocata_computes) require_allocation_refresh=require_allocation_refresh)


self._remove_deleted_instances_allocations(context, cn) self._remove_deleted_instances_allocations(context, cn)


Expand Down
34 changes: 34 additions & 0 deletions nova/tests/unit/compute/test_resource_tracker.py
Expand Up @@ -2571,6 +2571,40 @@ def test(version_mock, uufi, rdia):
self.assertEqual(-1024, cn.free_ram_mb) self.assertEqual(-1024, cn.free_ram_mb)
self.assertEqual(-1, cn.free_disk_gb) self.assertEqual(-1, cn.free_disk_gb)


def test_update_usage_from_instances_refreshes_ironic(self):
self.rt.driver.requires_allocation_refresh = True

@mock.patch.object(self.rt,
'_remove_deleted_instances_allocations')
@mock.patch.object(self.rt, '_update_usage_from_instance')
@mock.patch('nova.objects.Service.get_minimum_version',
return_value=22)
def test(version_mock, uufi, rdia):
self.rt._update_usage_from_instances('ctxt', [self.instance],
_NODENAME)

uufi.assert_called_once_with('ctxt', self.instance, _NODENAME,
require_allocation_refresh=True)

test()

def test_update_usage_from_instances_no_refresh(self):
self.rt.driver.requires_allocation_refresh = False

@mock.patch.object(self.rt,
'_remove_deleted_instances_allocations')
@mock.patch.object(self.rt, '_update_usage_from_instance')
@mock.patch('nova.objects.Service.get_minimum_version',
return_value=22)
def test(version_mock, uufi, rdia):
self.rt._update_usage_from_instances('ctxt', [self.instance],
_NODENAME)

uufi.assert_called_once_with('ctxt', self.instance, _NODENAME,
require_allocation_refresh=False)

test()

@mock.patch('nova.scheduler.utils.resources_from_flavor') @mock.patch('nova.scheduler.utils.resources_from_flavor')
def test_delete_allocation_for_evacuated_instance( def test_delete_allocation_for_evacuated_instance(
self, mock_resource_from_flavor): self, mock_resource_from_flavor):
Expand Down
2 changes: 2 additions & 0 deletions nova/tests/unit/virt/ironic/test_driver.py
Expand Up @@ -134,6 +134,8 @@ def test_driver_capabilities(self):
'supports_migrate_to_same_host'], 'supports_migrate_to_same_host'],
'Driver capabilities for ' 'Driver capabilities for '
'\'supports_migrate_to_same_host\' is invalid') '\'supports_migrate_to_same_host\' is invalid')
self.assertTrue(self.driver.requires_allocation_refresh,
'Driver requires allocation refresh')


def test__get_hypervisor_type(self): def test__get_hypervisor_type(self):
self.assertEqual('ironic', self.driver._get_hypervisor_type()) self.assertEqual('ironic', self.driver._get_hypervisor_type())
Expand Down
2 changes: 2 additions & 0 deletions nova/tests/unit/virt/libvirt/test_driver.py
Expand Up @@ -804,6 +804,8 @@ def test_driver_capabilities(self):
'Driver capabilities for ' 'Driver capabilities for '
'\'supports_extend_volume\' ' '\'supports_extend_volume\' '
'is invalid') 'is invalid')
self.assertFalse(drvr.requires_allocation_refresh,
'Driver does not need allocation refresh')


def create_fake_libvirt_mock(self, **kwargs): def create_fake_libvirt_mock(self, **kwargs):
"""Defining mocks for LibvirtDriver(libvirt is not used).""" """Defining mocks for LibvirtDriver(libvirt is not used)."""
Expand Down
2 changes: 2 additions & 0 deletions nova/virt/driver.py
Expand Up @@ -132,6 +132,8 @@ class ComputeDriver(object):
"supports_extend_volume": False, "supports_extend_volume": False,
} }


requires_allocation_refresh = False

def __init__(self, virtapi): def __init__(self, virtapi):
self.virtapi = virtapi self.virtapi = virtapi
self._compute_event_callback = None self._compute_event_callback = None
Expand Down
6 changes: 6 additions & 0 deletions nova/virt/ironic/driver.py
Expand Up @@ -135,6 +135,12 @@ class IronicDriver(virt_driver.ComputeDriver):
"supports_attach_interface": True "supports_attach_interface": True
} }


# Needed for exiting instances to have allocations for custom resource
# class resources
# TODO(johngarbutt) we should remove this once the resource class
# migration has been completed.
requires_allocation_refresh = True

def __init__(self, virtapi, read_only=False): def __init__(self, virtapi, read_only=False):
super(IronicDriver, self).__init__(virtapi) super(IronicDriver, self).__init__(virtapi)
global ironic global ironic
Expand Down

0 comments on commit 842641c

Please sign in to comment.