FUP for in-place numa rebuild
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 Change-Id: I8975e524cd5a9c7dfb065bb2dc8ceb03f1b89e7b Related-Bug: #1804502 Related-Bug: #1763766 (cherry picked from commitf6060ab6b5
) (cherry picked from commit48bb9a9663
) (cherry picked from commit8346c527b3
)
This commit is contained in:
parent
f08d0ccf84
commit
84c6381660
|
@ -3348,8 +3348,6 @@ class API(base.Base):
|
||||||
:param instance: nova.objects.instance.Instance object
|
:param instance: nova.objects.instance.Instance object
|
||||||
:param image: the new image the instance will be rebuilt with.
|
:param image: the new image the instance will be rebuilt with.
|
||||||
:param flavor: the flavor of the current instance.
|
:param flavor: the flavor of the current instance.
|
||||||
|
|
||||||
:returns: True or raises on failure.
|
|
||||||
:raises: nova.exception.ImageNUMATopologyRebuildConflict
|
:raises: nova.exception.ImageNUMATopologyRebuildConflict
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
@ -3364,25 +3362,28 @@ class API(base.Base):
|
||||||
|
|
||||||
# early out for non NUMA instances
|
# early out for non NUMA instances
|
||||||
if old_constraints is None and new_constraints is None:
|
if old_constraints is None and new_constraints is None:
|
||||||
# return true for easy unit testing
|
return
|
||||||
return True
|
|
||||||
|
|
||||||
# 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.
|
# constraints changed so raise an exception.
|
||||||
if old_constraints is None or new_constraints is None:
|
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()
|
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.
|
# them as dictionaries.
|
||||||
old = old_constraints.obj_to_primitive()
|
old = old_constraints.obj_to_primitive()
|
||||||
new = new_constraints.obj_to_primitive()
|
new = new_constraints.obj_to_primitive()
|
||||||
if old != new:
|
if old != new:
|
||||||
|
LOG.debug("NUMA rebuild validation failed. The requested image "
|
||||||
|
"conflicts with the existing NUMA constraints.",
|
||||||
|
instance=instance)
|
||||||
raise exception.ImageNUMATopologyRebuildConflict()
|
raise exception.ImageNUMATopologyRebuildConflict()
|
||||||
# TODO(sean-k-mooney): add PCI NUMA affinity policy check.
|
# TODO(sean-k-mooney): add PCI NUMA affinity policy check.
|
||||||
|
|
||||||
# return true for easy unit testing
|
|
||||||
return True
|
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _check_quota_for_upsize(context, instance, current_flavor, new_flavor):
|
def _check_quota_for_upsize(context, instance, current_flavor, new_flavor):
|
||||||
project_id, user_id = quotas_obj.ids_from_instance(context,
|
project_id, user_id = quotas_obj.ids_from_instance(context,
|
||||||
|
|
|
@ -1913,7 +1913,7 @@ class ImageNUMATopologyForbidden(Forbidden):
|
||||||
|
|
||||||
class ImageNUMATopologyRebuildConflict(Invalid):
|
class ImageNUMATopologyRebuildConflict(Invalid):
|
||||||
msg_fmt = _(
|
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.")
|
"The image provided is invalid for this instance.")
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -25,7 +25,7 @@ class NUMATopologyFilter(filters.BaseHostFilter):
|
||||||
|
|
||||||
# NOTE(sean-k-mooney): In change I0322d872bdff68936033a6f5a54e8296a6fb343
|
# NOTE(sean-k-mooney): In change I0322d872bdff68936033a6f5a54e8296a6fb343
|
||||||
# we validate that the NUMA topology does not change in the api. If the
|
# 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.
|
# request and therefore do not need to run this filter on rebuild.
|
||||||
RUN_ON_REBUILD = False
|
RUN_ON_REBUILD = False
|
||||||
|
|
||||||
|
|
|
@ -16,8 +16,7 @@
|
||||||
import fixtures
|
import fixtures
|
||||||
import mock
|
import mock
|
||||||
import six
|
import six
|
||||||
|
import testtools
|
||||||
from testtools import skip
|
|
||||||
|
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
|
@ -468,7 +467,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase):
|
||||||
# would be violated and it should fail at that point not when the
|
# 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
|
# instance is rebuilt. This is a latent bug which will be addressed
|
||||||
# in a separate patch.
|
# in a separate patch.
|
||||||
@skip("bug 1855332")
|
@testtools.skip("bug 1855332")
|
||||||
def test_attach_interface_with_network_affinity_violation(self):
|
def test_attach_interface_with_network_affinity_violation(self):
|
||||||
extra_spec = {'hw:numa_nodes': '1'}
|
extra_spec = {'hw:numa_nodes': '1'}
|
||||||
flavor_id = self._create_flavor(extra_spec=extra_spec)
|
flavor_id = self._create_flavor(extra_spec=extra_spec)
|
||||||
|
@ -592,7 +591,7 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
super(NUMAServersRebuildTests, self).setUp()
|
super(NUMAServersRebuildTests, self).setUp()
|
||||||
images = self.api.get_images()
|
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_0 = images[0]['id']
|
||||||
self.image_ref_1 = images[1]['id']
|
self.image_ref_1 = images[1]['id']
|
||||||
|
|
||||||
|
@ -665,9 +664,9 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
|
||||||
extra_spec = {'hw:cpu_policy': 'dedicated'}
|
extra_spec = {'hw:cpu_policy': 'dedicated'}
|
||||||
flavor_id = self._create_flavor(extra_spec=extra_spec)
|
flavor_id = self._create_flavor(extra_spec=extra_spec)
|
||||||
|
|
||||||
# cpu_cores is set to 2 to ensure that
|
# cpu_cores is set to 2 to ensure that we have enough space
|
||||||
# we have enough space to boot the vm but not enough space to rebuild
|
# to boot the vm but not enough space to rebuild
|
||||||
# by doubling the resource use during scheduling.
|
# by doubling the resource use during scheduling.
|
||||||
host_info = fakelibvirt.NUMAHostInfo(
|
host_info = fakelibvirt.NUMAHostInfo(
|
||||||
cpu_nodes=1, cpu_sockets=1, cpu_cores=2, kB_mem=15740000)
|
cpu_nodes=1, cpu_sockets=1, cpu_cores=2, kB_mem=15740000)
|
||||||
fake_connection = self._get_connection(host_info=host_info)
|
fake_connection = self._get_connection(host_info=host_info)
|
||||||
|
@ -698,7 +697,7 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
|
||||||
server = self._create_active_server(
|
server = self._create_active_server(
|
||||||
server_args={"flavorRef": flavor_id})
|
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
|
# so we alter the requested numa topology in image_ref_1 to request
|
||||||
# 2 virtual numa nodes.
|
# 2 virtual numa nodes.
|
||||||
ctx = nova_context.get_admin_context()
|
ctx = nova_context.get_admin_context()
|
||||||
|
@ -706,11 +705,11 @@ class NUMAServersRebuildTests(NUMAServersTestBase):
|
||||||
self.fake_image_service.update(ctx, self.image_ref_1, image_meta)
|
self.fake_image_service.update(ctx, self.image_ref_1, image_meta)
|
||||||
|
|
||||||
# NOTE(sean-k-mooney): this should fail because rebuild uses noop
|
# 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.
|
# usage to change during a rebuild.
|
||||||
ex = self.assertRaises(
|
ex = self.assertRaises(
|
||||||
client.OpenStackApiException, self._rebuild_server,
|
client.OpenStackApiException, self._rebuild_server,
|
||||||
server, self.image_ref_1)
|
server, self.image_ref_1)
|
||||||
self.assertEqual(400, ex.response.status_code)
|
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))
|
six.text_type(ex))
|
||||||
|
|
|
@ -6199,7 +6199,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
cells)
|
cells)
|
||||||
|
|
||||||
def test__validate_numa_rebuild_non_numa(self):
|
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.
|
topology passes validation.
|
||||||
"""
|
"""
|
||||||
flavor = objects.Flavor(
|
flavor = objects.Flavor(
|
||||||
|
@ -6213,7 +6213,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
self.compute_api._validate_numa_rebuild(instance, image, flavor)
|
self.compute_api._validate_numa_rebuild(instance, image, flavor)
|
||||||
|
|
||||||
def test__validate_numa_rebuild_no_conflict(self):
|
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.
|
in NUMA topology passes validation.
|
||||||
"""
|
"""
|
||||||
flavor = objects.Flavor(
|
flavor = objects.Flavor(
|
||||||
|
@ -6231,7 +6231,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
self.compute_api._validate_numa_rebuild(instance, image, flavor)
|
self.compute_api._validate_numa_rebuild(instance, image, flavor)
|
||||||
|
|
||||||
def test__validate_numa_rebuild_add_numa_toplogy(self):
|
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
|
that requests a NUMA topology when the original instance did not
|
||||||
have a NUMA topology is invalid.
|
have a NUMA topology is invalid.
|
||||||
"""
|
"""
|
||||||
|
@ -6239,6 +6239,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
flavor = objects.Flavor(
|
flavor = objects.Flavor(
|
||||||
id=42, vcpus=1, memory_mb=512, root_gb=1,
|
id=42, vcpus=1, memory_mb=512, root_gb=1,
|
||||||
extra_specs={})
|
extra_specs={})
|
||||||
|
# _create_instance_obj results in the instance.image_meta being None.
|
||||||
instance = self._create_instance_obj(flavor=flavor)
|
instance = self._create_instance_obj(flavor=flavor)
|
||||||
# we use a dict instead of image metadata object as
|
# we use a dict instead of image metadata object as
|
||||||
# _validate_numa_rebuild constructs the object internally
|
# _validate_numa_rebuild constructs the object internally
|
||||||
|
@ -6253,7 +6254,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
self.compute_api._validate_numa_rebuild, instance, image, flavor)
|
self.compute_api._validate_numa_rebuild, instance, image, flavor)
|
||||||
|
|
||||||
def test__validate_numa_rebuild_remove_numa_toplogy(self):
|
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
|
that does not request a NUMA topology when the original image did
|
||||||
is invalid if it would alter the instances topology as a result.
|
is invalid if it would alter the instances topology as a result.
|
||||||
"""
|
"""
|
||||||
|
@ -6261,6 +6262,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
flavor = objects.Flavor(
|
flavor = objects.Flavor(
|
||||||
id=42, vcpus=1, memory_mb=512, root_gb=1,
|
id=42, vcpus=1, memory_mb=512, root_gb=1,
|
||||||
extra_specs={})
|
extra_specs={})
|
||||||
|
# _create_instance_obj results in the instance.image_meta being None.
|
||||||
instance = self._create_instance_obj(flavor=flavor)
|
instance = self._create_instance_obj(flavor=flavor)
|
||||||
# we use a dict instead of image metadata object as
|
# we use a dict instead of image metadata object as
|
||||||
# _validate_numa_rebuild constructs the object internally
|
# _validate_numa_rebuild constructs the object internally
|
||||||
|
@ -6283,7 +6285,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
image, flavor)
|
image, flavor)
|
||||||
|
|
||||||
def test__validate_numa_rebuild_alter_numa_toplogy(self):
|
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
|
that requests a different NUMA topology than the original image
|
||||||
is invalid.
|
is invalid.
|
||||||
"""
|
"""
|
||||||
|
@ -6295,6 +6297,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
||||||
flavor = objects.Flavor(
|
flavor = objects.Flavor(
|
||||||
id=42, vcpus=2, memory_mb=512, root_gb=1,
|
id=42, vcpus=2, memory_mb=512, root_gb=1,
|
||||||
extra_specs={})
|
extra_specs={})
|
||||||
|
# _create_instance_obj results in the instance.image_meta being None.
|
||||||
instance = self._create_instance_obj(flavor=flavor)
|
instance = self._create_instance_obj(flavor=flavor)
|
||||||
# we use a dict instead of image metadata object as
|
# we use a dict instead of image metadata object as
|
||||||
# _validate_numa_rebuild constructs the object internally
|
# _validate_numa_rebuild constructs the object internally
|
||||||
|
|
|
@ -6,22 +6,24 @@ fixes:
|
||||||
Previously Nova would have ignored the NUMA topology of the new image
|
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
|
continuing to use the NUMA topology of the existing instance until a move
|
||||||
operation was performed. As Nova did not explicitly guard against
|
operation was performed. As Nova did not explicitly guard against
|
||||||
inadvertent changes in resource request contained in a new image,
|
inadvertent changes to resource requests contained in a new image,
|
||||||
it was possible to rebuild with an image that would violate this requirement
|
it was possible to rebuild with an image that would violate this
|
||||||
`bug #1763766`_. This resulted in an inconsistent state as the instance that
|
requirement; see `bug #1763766`_ for details. This resulted in an
|
||||||
was running did not match the instance that was requested. Nova now explicitly
|
inconsistent state as the instance that was running did not match the
|
||||||
checks if a rebuild would alter the requested NUMA topology of an instance
|
instance that was requested. Nova now explicitly checks if a rebuild would
|
||||||
and rejects the rebuild.
|
alter the requested NUMA topology of an instance and rejects the rebuild
|
||||||
|
if so.
|
||||||
|
|
||||||
.. _`bug #1763766`: https://bugs.launchpad.net/nova/+bug/1763766
|
.. _`bug #1763766`: https://bugs.launchpad.net/nova/+bug/1763766
|
||||||
|
|
||||||
features:
|
|
||||||
- |
|
- |
|
||||||
With the changes introduced to address `bug #1763766`_, Nova now guards
|
With the changes introduced to address `bug #1763766`_, Nova now guards
|
||||||
against NUMA constraint changes on rebuild. As a result the
|
against NUMA constraint changes on rebuild. As a result the
|
||||||
``NUMATopologyFilter`` is no longer required to run on rebuild since
|
``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
|
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
|
rebuild of an instance with a NUMA topology even if the image changes
|
||||||
provided the new image does not alter the topology.
|
provided the new image does not alter the topology which addresses
|
||||||
|
`bug #1804502`_.
|
||||||
|
|
||||||
|
.. _`bug #1804502`: https://bugs.launchpad.net/nova/+bug/1804502
|
||||||
|
|
Loading…
Reference in New Issue