openstack/nova
Code Issues Proposed changes

FUP for in-place numa rebuild

Browse Source 
This patch addresses a number of typos and minor
issues raised during review of [1][2]. A summary
of the changes are corrections to typos in comments,
a correction to the exception message, an update to
the release note and the addition of debug logging.

[1] I0322d872bdff68936033a6f5a54e8296a6fb3434
[2] I48bccc4b9adcac3c7a3e42769c11fdeb8f6fd132

Related-Bug: #1804502
Related-Bug: #1763766

Change-Id: I8975e524cd5a9c7dfb065bb2dc8ceb03f1b89e7b
(cherry picked from commit f6060ab6b5)
This commit is contained in:
Sean Mooney 2019-12-10 14:20:33 +00:00 committed by Stephen Finucane
parent 94c0362918
commit 48bb9a9663
6 changed files with 41 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
nova/compute/api.py
View File

@ -3405,8 +3405,6 @@ class API(base.Base):
:param instance: nova.objects.instance.Instance object
:param image: the new image the instance will be rebuilt with.
:param flavor: the flavor of the current instance.
:returns: True or raises on failure.
:raises: nova.exception.ImageNUMATopologyRebuildConflict
"""
@ -3421,25 +3419,28 @@ class API(base.Base):
# early out for non NUMA instances
if old_constraints is None and new_constraints is None:
# return true for easy unit testing
return True
return
# if only one of the constrains are non-None (or 'set') then the
# if only one of the constraints are non-None (or 'set') then the
# constraints changed so raise an exception.
if old_constraints is None or new_constraints is None:
action = "removing" if old_constraints else "introducing"
LOG.debug("NUMA rebuild validation failed. The requested image "
"would alter the NUMA constraints by %s a NUMA "
"topology.", action, instance=instance)
raise exception.ImageNUMATopologyRebuildConflict()
# otherwise since both the old a new constrains are non none compare
# otherwise since both the old a new constraints are non none compare
# them as dictionaries.
old = old_constraints.obj_to_primitive()
new = new_constraints.obj_to_primitive()
if old != new:
LOG.debug("NUMA rebuild validation failed. The requested image "
"conflicts with the existing NUMA constraints.",
instance=instance)
raise exception.ImageNUMATopologyRebuildConflict()
# TODO(sean-k-mooney): add PCI NUMA affinity policy check.
# return true for easy unit testing
return True
@staticmethod
def _check_quota_for_upsize(context, instance, current_flavor, new_flavor):
project_id, user_id = quotas_obj.ids_from_instance(context,

2
nova/exception.py
View File

@ -1917,7 +1917,7 @@ class ImageNUMATopologyForbidden(Forbidden):
class ImageNUMATopologyRebuildConflict(Invalid):
msg_fmt = _(
"An instance's NUMA typology cannot be changed as part of a rebuild. "
"An instance's NUMA topology cannot be changed as part of a rebuild. "
"The image provided is invalid for this instance.")

2
nova/scheduler/filters/numa_topology_filter.py
View File

@ -25,7 +25,7 @@ class NUMATopologyFilter(filters.BaseHostFilter):
# NOTE(sean-k-mooney): In change I0322d872bdff68936033a6f5a54e8296a6fb343
# we validate that the NUMA topology does not change in the api. If the
# requested image would alter the NUMA constrains we reject the rebuild
# requested image would alter the NUMA constraints we reject the rebuild
# request and therefore do not need to run this filter on rebuild.
RUN_ON_REBUILD = False

19
nova/tests/functional/libvirt/test_numa_servers.py
View File

@ -15,8 +15,7 @@
import mock
import six
from testtools import skip
import testtools
from oslo_config import cfg
from oslo_log import log as logging
@ -905,7 +904,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase):
# would be violated and it should fail at that point not when the
# instance is rebuilt. This is a latent bug which will be addressed
# in a separate patch.
@skip("bug 1855332")
@testtools.skip("bug 1855332")
def test_attach_interface_with_network_affinity_violation(self):
extra_spec = {'hw:numa_nodes': '1'}
flavor_id = self._create_flavor(extra_spec=extra_spec)
@ -1030,7 +1029,7 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
def setUp(self):
super(NUMAServersRebuildTests, self).setUp()
images = self.api.get_images()
# save references to first two image for server create and rebuild
# save references to first two images for server create and rebuild
self.image_ref_0 = images[0]['id']
self.image_ref_1 = images[1]['id']
@ -1094,9 +1093,9 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
extra_spec = {'hw:cpu_policy': 'dedicated'}
flavor_id = self._create_flavor(extra_spec=extra_spec)
# cpu_cores is set to 2 to ensure that
# we have enough space to boot the vm but not enough space to rebuild
# by doubling the resource use during scheduling.
# cpu_cores is set to 2 to ensure that we have enough space
# to boot the vm but not enough space to rebuild
# by doubling the resource use during scheduling.
host_info = fakelibvirt.HostInfo(
cpu_nodes=1, cpu_sockets=1, cpu_cores=2, kB_mem=15740000)
fake_connection = self._get_connection(host_info=host_info)
@ -1127,7 +1126,7 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
server = self._create_active_server(
server_args={"flavorRef": flavor_id})
# the original vm had an implicit numa topology of 1 virtual numa nodes
# The original vm had an implicit numa topology of 1 virtual numa node
# so we alter the requested numa topology in image_ref_1 to request
# 2 virtual numa nodes.
ctx = nova_context.get_admin_context()
@ -1135,11 +1134,11 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
self.fake_image_service.update(ctx, self.image_ref_1, image_meta)
# NOTE(sean-k-mooney): this should fail because rebuild uses noop
# claims therefor it is not allowed for the numa topology or resource
# claims therefore it is not allowed for the NUMA topology or resource
# usage to change during a rebuild.
ex = self.assertRaises(
client.OpenStackApiException, self._rebuild_server,
server, self.image_ref_1)
self.assertEqual(400, ex.response.status_code)
self.assertIn("An instance's NUMA typology cannot be changed",
self.assertIn("An instance's NUMA topology cannot be changed",
six.text_type(ex))

13
nova/tests/unit/compute/test_compute_api.py
View File

@ -6225,7 +6225,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
cells)
def test__validate_numa_rebuild_non_numa(self):
"""This test asserts that a rebuild of an instance without a NUMA
"""Assert that a rebuild of an instance without a NUMA
topology passes validation.
"""
flavor = objects.Flavor(
@ -6239,7 +6239,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
self.compute_api._validate_numa_rebuild(instance, image, flavor)
def test__validate_numa_rebuild_no_conflict(self):
"""This test asserts that a rebuild of an instance without a change
"""Assert that a rebuild of an instance without a change
in NUMA topology passes validation.
"""
flavor = objects.Flavor(
@ -6257,7 +6257,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
self.compute_api._validate_numa_rebuild(instance, image, flavor)
def test__validate_numa_rebuild_add_numa_toplogy(self):
"""This test asserts that a rebuild of an instance with a new image
"""Assert that a rebuild of an instance with a new image
that requests a NUMA topology when the original instance did not
have a NUMA topology is invalid.
"""
@ -6265,6 +6265,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
flavor = objects.Flavor(
id=42, vcpus=1, memory_mb=512, root_gb=1,
extra_specs={})
# _create_instance_obj results in the instance.image_meta being None.
instance = self._create_instance_obj(flavor=flavor)
# we use a dict instead of image metadata object as
# _validate_numa_rebuild constructs the object internally
@ -6279,7 +6280,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
self.compute_api._validate_numa_rebuild, instance, image, flavor)
def test__validate_numa_rebuild_remove_numa_toplogy(self):
"""This test asserts that a rebuild of an instance with a new image
"""Assert that a rebuild of an instance with a new image
that does not request a NUMA topology when the original image did
is invalid if it would alter the instances topology as a result.
"""
@ -6287,6 +6288,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
flavor = objects.Flavor(
id=42, vcpus=1, memory_mb=512, root_gb=1,
extra_specs={})
# _create_instance_obj results in the instance.image_meta being None.
instance = self._create_instance_obj(flavor=flavor)
# we use a dict instead of image metadata object as
# _validate_numa_rebuild constructs the object internally
@ -6309,7 +6311,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
image, flavor)
def test__validate_numa_rebuild_alter_numa_toplogy(self):
"""This test asserts that a rebuild of an instance with a new image
"""Assert that a rebuild of an instance with a new image
that requests a different NUMA topology than the original image
is invalid.
"""
@ -6321,6 +6323,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
flavor = objects.Flavor(
id=42, vcpus=2, memory_mb=512, root_gb=1,
extra_specs={})
# _create_instance_obj results in the instance.image_meta being None.
instance = self._create_instance_obj(flavor=flavor)
# we use a dict instead of image metadata object as
# _validate_numa_rebuild constructs the object internally

22
releasenotes/notes/numa-rebuild-b75f9a1966f576ea.yaml
View File

@ -6,22 +6,24 @@ fixes:
Previously Nova would have ignored the NUMA topology of the new image
continuing to use the NUMA topology of the existing instance until a move
operation was performed. As Nova did not explicitly guard against
inadvertent changes in resource request contained in a new image,
it was possible to rebuild with an image that would violate this requirement
`bug #1763766`_. This resulted in an inconsistent state as the instance that
was running did not match the instance that was requested. Nova now explicitly
checks if a rebuild would alter the requested NUMA topology of an instance
and rejects the rebuild.
inadvertent changes to resource requests contained in a new image,
it was possible to rebuild with an image that would violate this
requirement; see `bug #1763766`_ for details. This resulted in an
inconsistent state as the instance that was running did not match the
instance that was requested. Nova now explicitly checks if a rebuild would
alter the requested NUMA topology of an instance and rejects the rebuild
if so.
.. _`bug #1763766`: https://bugs.launchpad.net/nova/+bug/1763766
features:
- |
With the changes introduced to address `bug #1763766`_, Nova now guards
against NUMA constraint changes on rebuild. As a result the
``NUMATopologyFilter`` is no longer required to run on rebuild since
we already know the topology will not change and therefor the existing
we already know the topology will not change and therefore the existing
resource claim is still valid. As such it is now possible to do an in-place
rebuild of a instance with a NUMA topology even if the image changes
provided the new image does not alter the topology.
rebuild of an instance with a NUMA topology even if the image changes
provided the new image does not alter the topology which addresses
`bug #1804502`_.
.. _`bug #1804502`: https://bugs.launchpad.net/nova/+bug/1804502