Re-use existing ComputeNode on ironic rebalance
When a nova-compute service dies that is one of several ironic based nova-compute services running, a node rebalance occurs to ensure there is still an active nova-compute service dealing with requests for the given instance that is running. Today, when this occurs, we create a new ComputeNode entry. This change alters that logic to detect the case of the ironic node rebalance and in that case we re-use the existing ComputeNode entry, simply updating the host field to match the new host it has been rebalanced onto. Previously we hit problems with placement when we get a new ComputeNode.uuid for the same ironic_node.uuid. This reusing of the existing entry keeps the ComputeNode.uuid the same when the rebalance of the ComputeNode occurs. Without keeping the same ComputeNode.uuid placement errors out with a 409 because we attempt to create a ResourceProvider that has the same name as an existing ResourceProvdier. Had that worked, we would have noticed the race that occurs after we create the ResourceProvider but before we add back the existing allocations for existing instances. Keeping the ComputeNode.uuid the same means we simply look up the existing ResourceProvider in placement, avoiding all this pain and tears. Closes-Bug: #1714248 Co-Authored-By: Dmitry Tantsur <dtantsur@redhat.com> Change-Id: I4253cffca3dbf558c875eed7e77711a31e9e3406
This commit is contained in:
parent
3cec0cb584
commit
e3c5e22d1f
|
@ -492,6 +492,50 @@ class ResourceTracker(object):
|
||||||
return (nodename not in self.compute_nodes or
|
return (nodename not in self.compute_nodes or
|
||||||
not self.driver.node_is_available(nodename))
|
not self.driver.node_is_available(nodename))
|
||||||
|
|
||||||
|
def _check_for_nodes_rebalance(self, context, resources, nodename):
|
||||||
|
"""Check if nodes rebalance has happened.
|
||||||
|
|
||||||
|
The ironic driver maintains a hash ring mapping bare metal nodes
|
||||||
|
to compute nodes. If a compute dies, the hash ring is rebuilt, and
|
||||||
|
some of its bare metal nodes (more precisely, those not in ACTIVE
|
||||||
|
state) are assigned to other computes.
|
||||||
|
|
||||||
|
This method checks for this condition and adjusts the database
|
||||||
|
accordingly.
|
||||||
|
|
||||||
|
:param context: security context
|
||||||
|
:param resources: initial values
|
||||||
|
:param nodename: node name
|
||||||
|
:returns: True if a suitable compute node record was found, else False
|
||||||
|
"""
|
||||||
|
if not self.driver.rebalances_nodes:
|
||||||
|
return False
|
||||||
|
|
||||||
|
# Its possible ironic just did a node re-balance, so let's
|
||||||
|
# check if there is a compute node that already has the correct
|
||||||
|
# hypervisor_hostname. We can re-use that rather than create a
|
||||||
|
# new one and have to move existing placement allocations
|
||||||
|
cn_candidates = objects.ComputeNodeList.get_by_hypervisor(
|
||||||
|
context, nodename)
|
||||||
|
|
||||||
|
if len(cn_candidates) == 1:
|
||||||
|
cn = cn_candidates[0]
|
||||||
|
LOG.info("ComputeNode %(name)s moving from %(old)s to %(new)s",
|
||||||
|
{"name": nodename, "old": cn.host, "new": self.host})
|
||||||
|
cn.host = self.host
|
||||||
|
self.compute_nodes[nodename] = cn
|
||||||
|
self._copy_resources(cn, resources)
|
||||||
|
self._setup_pci_tracker(context, cn, resources)
|
||||||
|
self._update(context, cn)
|
||||||
|
return True
|
||||||
|
elif len(cn_candidates) > 1:
|
||||||
|
LOG.error(
|
||||||
|
"Found more than one ComputeNode for nodename %s. "
|
||||||
|
"Please clean up the orphaned ComputeNode records in your DB.",
|
||||||
|
nodename)
|
||||||
|
|
||||||
|
return False
|
||||||
|
|
||||||
def _init_compute_node(self, context, resources):
|
def _init_compute_node(self, context, resources):
|
||||||
"""Initialize the compute node if it does not already exist.
|
"""Initialize the compute node if it does not already exist.
|
||||||
|
|
||||||
|
@ -527,6 +571,9 @@ class ResourceTracker(object):
|
||||||
self._update(context, cn)
|
self._update(context, cn)
|
||||||
return
|
return
|
||||||
|
|
||||||
|
if self._check_for_nodes_rebalance(context, resources, nodename):
|
||||||
|
return
|
||||||
|
|
||||||
# there was no local copy and none in the database
|
# there was no local copy and none in the database
|
||||||
# so we need to create a new compute node. This needs
|
# so we need to create a new compute node. This needs
|
||||||
# to be initialized with resource values.
|
# to be initialized with resource values.
|
||||||
|
|
|
@ -435,6 +435,7 @@ def setup_rt(hostname, virt_resources=_VIRT_DRIVER_AVAIL_RESOURCES,
|
||||||
vd.get_inventory.side_effect = NotImplementedError
|
vd.get_inventory.side_effect = NotImplementedError
|
||||||
vd.get_host_ip_addr.return_value = _NODENAME
|
vd.get_host_ip_addr.return_value = _NODENAME
|
||||||
vd.estimate_instance_overhead.side_effect = estimate_overhead
|
vd.estimate_instance_overhead.side_effect = estimate_overhead
|
||||||
|
vd.rebalances_nodes = False
|
||||||
|
|
||||||
with test.nested(
|
with test.nested(
|
||||||
mock.patch('nova.scheduler.client.SchedulerClient',
|
mock.patch('nova.scheduler.client.SchedulerClient',
|
||||||
|
@ -1012,6 +1013,39 @@ class TestInitComputeNode(BaseTestCase):
|
||||||
self.assertFalse(create_mock.called)
|
self.assertFalse(create_mock.called)
|
||||||
self.assertTrue(update_mock.called)
|
self.assertTrue(update_mock.called)
|
||||||
|
|
||||||
|
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||||
|
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||||
|
return_value=objects.PciDeviceList())
|
||||||
|
@mock.patch('nova.objects.ComputeNode.create')
|
||||||
|
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||||
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
|
'_update')
|
||||||
|
def test_compute_node_rebalanced(self, update_mock, get_mock, create_mock,
|
||||||
|
pci_mock, get_by_hypervisor_mock):
|
||||||
|
self._setup_rt()
|
||||||
|
self.driver_mock.rebalances_nodes = True
|
||||||
|
cn = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
|
||||||
|
cn.host = "old-host"
|
||||||
|
|
||||||
|
def fake_get_all(_ctx, nodename):
|
||||||
|
return [cn]
|
||||||
|
|
||||||
|
get_mock.side_effect = exc.NotFound
|
||||||
|
get_by_hypervisor_mock.side_effect = fake_get_all
|
||||||
|
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
|
||||||
|
|
||||||
|
self.rt._init_compute_node(mock.sentinel.ctx, resources)
|
||||||
|
|
||||||
|
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
|
||||||
|
_NODENAME)
|
||||||
|
get_by_hypervisor_mock.assert_called_once_with(mock.sentinel.ctx,
|
||||||
|
_NODENAME)
|
||||||
|
create_mock.assert_not_called()
|
||||||
|
update_mock.assert_called_once_with(mock.sentinel.ctx, cn)
|
||||||
|
|
||||||
|
self.assertEqual(_HOSTNAME, self.rt.compute_nodes[_NODENAME].host)
|
||||||
|
|
||||||
|
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||||
return_value=objects.PciDeviceList(objects=[]))
|
return_value=objects.PciDeviceList(objects=[]))
|
||||||
@mock.patch('nova.objects.ComputeNode.create')
|
@mock.patch('nova.objects.ComputeNode.create')
|
||||||
|
@ -1019,10 +1053,55 @@ class TestInitComputeNode(BaseTestCase):
|
||||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
'_update')
|
'_update')
|
||||||
def test_compute_node_created_on_empty(self, update_mock, get_mock,
|
def test_compute_node_created_on_empty(self, update_mock, get_mock,
|
||||||
create_mock, pci_tracker_mock):
|
create_mock, pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock):
|
||||||
|
get_by_hypervisor_mock.return_value = []
|
||||||
|
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||||
|
pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock)
|
||||||
|
|
||||||
|
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||||
|
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||||
|
return_value=objects.PciDeviceList(objects=[]))
|
||||||
|
@mock.patch('nova.objects.ComputeNode.create')
|
||||||
|
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||||
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
|
'_update')
|
||||||
|
def test_compute_node_created_on_empty_rebalance(self, update_mock,
|
||||||
|
get_mock,
|
||||||
|
create_mock,
|
||||||
|
pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock):
|
||||||
|
get_by_hypervisor_mock.return_value = []
|
||||||
|
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||||
|
pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock,
|
||||||
|
rebalances_nodes=True)
|
||||||
|
|
||||||
|
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||||
|
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||||
|
return_value=objects.PciDeviceList(objects=[]))
|
||||||
|
@mock.patch('nova.objects.ComputeNode.create')
|
||||||
|
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||||
|
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||||
|
'_update')
|
||||||
|
def test_compute_node_created_too_many(self, update_mock, get_mock,
|
||||||
|
create_mock, pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock):
|
||||||
|
get_by_hypervisor_mock.return_value = ["fake_node_1", "fake_node_2"]
|
||||||
|
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||||
|
pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock,
|
||||||
|
rebalances_nodes=True)
|
||||||
|
|
||||||
|
def _test_compute_node_created(self, update_mock, get_mock,
|
||||||
|
create_mock, pci_tracker_mock,
|
||||||
|
get_by_hypervisor_mock,
|
||||||
|
rebalances_nodes=False):
|
||||||
self.flags(cpu_allocation_ratio=1.0, ram_allocation_ratio=1.0,
|
self.flags(cpu_allocation_ratio=1.0, ram_allocation_ratio=1.0,
|
||||||
disk_allocation_ratio=1.0)
|
disk_allocation_ratio=1.0)
|
||||||
self._setup_rt()
|
self._setup_rt()
|
||||||
|
self.driver_mock.rebalances_nodes = rebalances_nodes
|
||||||
|
|
||||||
get_mock.side_effect = exc.NotFound
|
get_mock.side_effect = exc.NotFound
|
||||||
|
|
||||||
|
@ -1088,6 +1167,11 @@ class TestInitComputeNode(BaseTestCase):
|
||||||
cn = self.rt.compute_nodes[_NODENAME]
|
cn = self.rt.compute_nodes[_NODENAME]
|
||||||
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
|
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
|
||||||
_NODENAME)
|
_NODENAME)
|
||||||
|
if rebalances_nodes:
|
||||||
|
get_by_hypervisor_mock.assert_called_once_with(
|
||||||
|
mock.sentinel.ctx, _NODENAME)
|
||||||
|
else:
|
||||||
|
get_by_hypervisor_mock.assert_not_called()
|
||||||
create_mock.assert_called_once_with()
|
create_mock.assert_called_once_with()
|
||||||
self.assertTrue(obj_base.obj_equal_prims(expected_compute, cn))
|
self.assertTrue(obj_base.obj_equal_prims(expected_compute, cn))
|
||||||
pci_tracker_mock.assert_called_once_with(mock.sentinel.ctx,
|
pci_tracker_mock.assert_called_once_with(mock.sentinel.ctx,
|
||||||
|
|
|
@ -134,6 +134,11 @@ class ComputeDriver(object):
|
||||||
|
|
||||||
requires_allocation_refresh = False
|
requires_allocation_refresh = False
|
||||||
|
|
||||||
|
# Indicates if this driver will rebalance nodes among compute service
|
||||||
|
# hosts. This is really here for ironic and should not be used by any
|
||||||
|
# other driver.
|
||||||
|
rebalances_nodes = 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
|
||||||
|
|
|
@ -161,9 +161,12 @@ class FakeDriver(driver.ComputeDriver):
|
||||||
self._mounts = {}
|
self._mounts = {}
|
||||||
self._interfaces = {}
|
self._interfaces = {}
|
||||||
self.active_migrations = {}
|
self.active_migrations = {}
|
||||||
|
self._nodes = self._init_nodes()
|
||||||
|
|
||||||
|
def _init_nodes(self):
|
||||||
if not _FAKE_NODES:
|
if not _FAKE_NODES:
|
||||||
set_nodes([CONF.host])
|
set_nodes([CONF.host])
|
||||||
self._nodes = copy.copy(_FAKE_NODES)
|
return copy.copy(_FAKE_NODES)
|
||||||
|
|
||||||
def init_host(self, host):
|
def init_host(self, host):
|
||||||
return
|
return
|
||||||
|
|
|
@ -141,6 +141,9 @@ class IronicDriver(virt_driver.ComputeDriver):
|
||||||
# migration has been completed.
|
# migration has been completed.
|
||||||
requires_allocation_refresh = True
|
requires_allocation_refresh = True
|
||||||
|
|
||||||
|
# This driver is capable of rebalancing nodes between computes.
|
||||||
|
rebalances_nodes = 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
|
||||||
|
|
Loading…
Reference in New Issue