openstack
/
nova
Code Issues Proposed changes

ironic_host_manager: fix population of instances info on start 

IronicHostManager currently overrides the _init_instance_info()
method of the base class and unconditionally skips population of
instances information for all compute nodes, even if they are not
Ironic ones.

If there are compute nodes with the hypervisor_type different from
Ironic in the same cloud. the instances info will be missing in
nova-scheduler (if IronicHostManager is configured as a host manager
impl in nova.conf), which will effectively break instance affinity
filters like DifferentHostFilter or SameHostFilter, that check set
intersections of instances running on a particular host and the ones
passed as a hint for nova-scheduler in a boot request.

IronicHostManager should use the method implementation of the base
class for non-ironic compute nodes.

Ib1ddb44d71f7b085512c1f3fc0544f7b00c754fe fixed the problem with
scheduling, this change is needed to make sure we also populate the
instances info on start of nova-scheduler.

Closes-Bug: #1606496

Co-Authored-By: Timofei Durakov <tdurakov@mirantis.com>

(cherry-picked from cc64a45d98)
Change-Id: I9d8d2dc99773df4097c178d924d182a0d1971bcc
This commit is contained in:
Roman Podoliaka 2016-07-27 19:46:16 +03:00
parent 6f1151378b
commit 13513e6232
5 changed files with 87 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
nova/scheduler/host_manager.py
View File

@ -389,20 +389,27 @@ class HostManager(object):
if aggregate.id in self.host_aggregates_map[host]:
self.host_aggregates_map[host].remove(aggregate.id)
def _init_instance_info(self):
def _init_instance_info(self, compute_nodes=None):
"""Creates the initial view of instances for all hosts.
As this initial population of instance information may take some time,
we don't wish to block the scheduler's startup while this completes.
The async method allows us to simply mock out the _init_instance_info()
method in tests.
:param compute_nodes: a list of nodes to populate instances info for
if is None, compute_nodes will be looked up in database
"""
def _async_init_instance_info():
context = context_module.get_admin_context()
def _async_init_instance_info(compute_nodes):
context = context_module.RequestContext()
LOG.debug("START:_async_init_instance_info")
self._instance_info = {}
compute_nodes = objects.ComputeNodeList.get_all(context).objects
if not compute_nodes:
compute_nodes = objects.ComputeNodeList.get_all(
context).objects
LOG.debug("Total number of compute nodes: %s", len(compute_nodes))
# Break the queries into batches of 10 to reduce the total number
# of calls to the DB.
@ -416,8 +423,8 @@ class HostManager(object):
filters = {"host": [curr_node.host
for curr_node in curr_nodes],
"deleted": False}
result = objects.InstanceList.get_by_filters(context,
filters)
result = objects.InstanceList.get_by_filters(
context.elevated(), filters)
instances = result.objects
LOG.debug("Adding %s instances for hosts %s-%s",
len(instances), start_node, end_node)
@ -433,7 +440,7 @@ class HostManager(object):
LOG.debug("END:_async_init_instance_info")
# Run this async so that we don't block the scheduler start-up
utils.spawn_n(_async_init_instance_info)
utils.spawn_n(_async_init_instance_info, compute_nodes)
def _choose_host_filters(self, filter_cls_names):
"""Since the caller may specify which filters to use we need

12
nova/scheduler/ironic_host_manager.py
View File

@ -23,6 +23,8 @@ subdivided into multiple instances.
"""
from nova.compute import hv_type
import nova.conf
from nova import context as context_module
from nova import objects
from nova.scheduler import host_manager
CONF = nova.conf.CONF
@ -91,9 +93,15 @@ class IronicHostManager(host_manager.HostManager):
else:
return host_manager.HostState(host, node)
def _init_instance_info(self):
def _init_instance_info(self, compute_nodes=None):
"""Ironic hosts should not pass instance info."""
pass
context = context_module.RequestContext()
if not compute_nodes:
compute_nodes = objects.ComputeNodeList.get_all(context).objects
non_ironic_computes = [c for c in compute_nodes
if not self._is_ironic_compute(c)]
super(IronicHostManager, self)._init_instance_info(non_ironic_computes)
def _get_instance_info(self, context, compute):
"""Ironic hosts should not pass instance info."""

24
nova/tests/unit/scheduler/test_host_manager.py
View File

@ -114,6 +114,30 @@ class HostManagerTestCase(test.NoDBTestCase):
exp_filters = {'deleted': False, 'host': [u'host1', u'host2']}
mock_get_by_filters.assert_called_once_with(mock.ANY, exp_filters)
@mock.patch.object(nova.objects.InstanceList, 'get_by_filters')
@mock.patch.object(nova.objects.ComputeNodeList, 'get_all')
def test_init_instance_info_compute_nodes(self, mock_get_all,
mock_get_by_filters):
cn1 = objects.ComputeNode(host='host1')
cn2 = objects.ComputeNode(host='host2')
inst1 = objects.Instance(host='host1', uuid=uuids.instance_1)
inst2 = objects.Instance(host='host1', uuid=uuids.instance_2)
inst3 = objects.Instance(host='host2', uuid=uuids.instance_3)
mock_get_by_filters.return_value = objects.InstanceList(
objects=[inst1, inst2, inst3])
hm = self.host_manager
hm._instance_info = {}
hm._init_instance_info([cn1, cn2])
self.assertEqual(len(hm._instance_info), 2)
fake_info = hm._instance_info['host1']
self.assertIn(uuids.instance_1, fake_info['instances'])
self.assertIn(uuids.instance_2, fake_info['instances'])
self.assertNotIn(uuids.instance_3, fake_info['instances'])
exp_filters = {'deleted': False, 'host': [u'host1', u'host2']}
mock_get_by_filters.assert_called_once_with(mock.ANY, exp_filters)
# should not be called if the list of nodes was passed explicitly
self.assertFalse(mock_get_all.called)
def test_default_filters(self):
default_filters = self.host_manager.default_filters
self.assertEqual(1, len(default_filters))

39
nova/tests/unit/scheduler/test_ironic_host_manager.py
View File

@ -43,7 +43,8 @@ class FakeFilterClass2(filters.BaseHostFilter):
class IronicHostManagerTestCase(test.NoDBTestCase):
"""Test case for IronicHostManager class."""
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def setUp(self, mock_init_agg, mock_init_inst):
super(IronicHostManagerTestCase, self).setUp()
@ -127,11 +128,39 @@ class IronicHostManagerTestCase(test.NoDBTestCase):
# we return exactly what the base class implementation returned
self.assertIs(expected_rv, rv)
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(objects.ComputeNodeList, 'get_all')
def test_init_instance_info(self, mock_get_all,
mock_base_init_instance_info):
cn1 = objects.ComputeNode(**{'hypervisor_type': 'ironic'})
cn2 = objects.ComputeNode(**{'hypervisor_type': 'qemu'})
cn3 = objects.ComputeNode(**{'hypervisor_type': 'qemu'})
mock_get_all.return_value.objects = [cn1, cn2, cn3]
self.host_manager._init_instance_info()
# ensure we filter out ironic nodes before calling the base class impl
mock_base_init_instance_info.assert_called_once_with([cn2, cn3])
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(objects.ComputeNodeList, 'get_all')
def test_init_instance_info_compute_nodes(self, mock_get_all,
mock_base_init_instance_info):
cn1 = objects.ComputeNode(**{'hypervisor_type': 'ironic'})
cn2 = objects.ComputeNode(**{'hypervisor_type': 'qemu'})
self.host_manager._init_instance_info(compute_nodes=[cn1, cn2])
# check we don't try to get nodes list if it was passed explicitly
self.assertFalse(mock_get_all.called)
# ensure we filter out ironic nodes before calling the base class impl
mock_base_init_instance_info.assert_called_once_with([cn2])
class IronicHostManagerChangedNodesTestCase(test.NoDBTestCase):
"""Test case for IronicHostManager class."""
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def setUp(self, mock_init_agg, mock_init_inst):
super(IronicHostManagerChangedNodesTestCase, self).setUp()
@ -277,7 +306,8 @@ class IronicHostManagerChangedNodesTestCase(test.NoDBTestCase):
class IronicHostManagerTestFilters(test.NoDBTestCase):
"""Test filters work for IronicHostManager."""
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def setUp(self, mock_init_agg, mock_init_inst):
super(IronicHostManagerTestFilters, self).setUp()
@ -314,7 +344,8 @@ class IronicHostManagerTestFilters(test.NoDBTestCase):
self.assertEqual(1, len(default_filters))
self.assertIsInstance(default_filters[0], FakeFilterClass1)
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def test_host_manager_default_filters_uses_baremetal(self, mock_init_agg,
mock_init_inst):

6
nova/tests/unit/scheduler/test_scheduler.py
View File

@ -191,7 +191,8 @@ class SchedulerInitTestCase(test.NoDBTestCase):
manager = self.driver_cls().host_manager
self.assertIsInstance(manager, host_manager.HostManager)
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def test_init_using_ironic_hostmanager(self,
mock_init_agg,
@ -211,7 +212,8 @@ class SchedulerInitTestCase(test.NoDBTestCase):
# NOTE(Yingxin): Loading full class path is deprecated and should be
# removed in the N release.
@mock.patch.object(driver.LOG, 'warning')
@mock.patch.object(host_manager.HostManager, '_init_instance_info')
@mock.patch.object(ironic_host_manager.IronicHostManager,
'_init_instance_info')
@mock.patch.object(host_manager.HostManager, '_init_aggregates')
def test_init_using_classpath_to_hostmanager(self,
mock_init_agg,