From f08d0ccf844e127f693cfc5498a205b13c873833 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Mon, 21 Oct 2019 16:17:17 +0000 Subject: [PATCH] Disable NUMATopologyFilter on rebuild This change leverages the new NUMA constraint checking added in in I0322d872bdff68936033a6f5a54e8296a6fb3434 to allow the NUMATopologyFilter to be skipped on rebuild. As the new behavior of rebuild enfroces that no changes to the numa constraints are allowed on rebuild we no longer need to execute the NUMATopologyFilter. Previously the NUMATopologyFilter would process the rebuild request as if it was a request to spawn a new instnace as the numa_fit_instance_to_host function is not rebuild aware. As such prior to this change a rebuild would only succeed if a host had enough additional capacity for a second instance on the same host meeting the requirement of the new image and existing flavor. This behavior was incorrect on two counts as a rebuild uses a noop claim. First the resouce usage cannot change so it was incorrect to require the addtional capacity to rebuild an instance. Secondly it was incorrect not to assert the resouce usage remained the same. I0322d872bdff68936033a6f5a54e8296a6fb3434 adressed guarding the rebuild against altering the resouce usage and this change allows in place rebuild. This change found a latent bug that will be adressed in a follow up change and updated the functional tests to note the incorrect behavior. Conflicts: nova/tests/functional/libvirt/test_numa_servers.py NOTE(sean-k-mooney): Trivial import conflicts Change-Id: I48bccc4b9adcac3c7a3e42769c11fdeb8f6fd132 Closes-Bug: #1804502 Implements: blueprint inplace-rebuild-of-numa-instances (cherry picked from commit 3f9411071d4c1a04ab0b68fd635597bf6959c0ca) (cherry picked from commit 94c0362918169a1fa06aa6cf5a483e9285d7b91f) (cherry picked from commit 4a691c33d13611714135b9390cb53de726fc901d) --- .../scheduler/filters/numa_topology_filter.py | 6 +++- .../functional/libvirt/test_numa_servers.py | 28 ++++++++++++++----- .../notes/numa-rebuild-b75f9a1966f576ea.yaml | 11 ++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/nova/scheduler/filters/numa_topology_filter.py b/nova/scheduler/filters/numa_topology_filter.py index 6fc74d083d44..5e3d6eb5936c 100644 --- a/nova/scheduler/filters/numa_topology_filter.py +++ b/nova/scheduler/filters/numa_topology_filter.py @@ -23,7 +23,11 @@ LOG = logging.getLogger(__name__) class NUMATopologyFilter(filters.BaseHostFilter): """Filter on requested NUMA topology.""" - RUN_ON_REBUILD = True + # 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 + # request and therefore do not need to run this filter on rebuild. + RUN_ON_REBUILD = False def _satisfies_cpu_policy(self, host_state, extra_specs, image_props): """Check that the host_state provided satisfies any available diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py index 220573b22c7f..54784b9f6bb3 100644 --- a/nova/tests/functional/libvirt/test_numa_servers.py +++ b/nova/tests/functional/libvirt/test_numa_servers.py @@ -17,6 +17,8 @@ import fixtures import mock import six +from testtools import skip + from oslo_config import cfg from oslo_log import log as logging @@ -31,6 +33,7 @@ from nova.tests.unit import fake_notifier from nova.tests.unit.virt.libvirt import fake_imagebackend from nova.tests.unit.virt.libvirt import fake_libvirt_utils from nova.tests.unit.virt.libvirt import fakelibvirt + CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -456,7 +459,17 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): self.assertTrue(filter_mock.called) self.assertEqual('ACTIVE', status) - def test_rebuild_server_with_network_affinity(self): + # FIXME(sean-k-mooney): The logic of this test is incorrect. + # The test was written to assert that we failed to rebuild + # because the NUMA constraints were violated due to the attachment + # of an interface from a second host NUMA node to an instance with + # a NUMA topology of 1 that is affined to a different NUMA node. + # Nova should reject the interface attachment if the NUMA constraints + # 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") + def test_attach_interface_with_network_affinity_violation(self): extra_spec = {'hw:numa_nodes': '1'} flavor_id = self._create_flavor(extra_spec=extra_spec) networks = [ @@ -491,10 +504,15 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): 'net_id': NUMAAffinityNeutronFixture.network_2['id'], } } + # FIXME(sean-k-mooney): This should raise an exception as this + # interface attachment would violate the NUMA constraints. self.api.attach_interface(server['id'], post) post = {'rebuild': { 'imageRef': 'a2459075-d96c-40d5-893e-577ff92e721c', }} + # NOTE(sean-k-mooney): the rest of the test is incorrect but + # is left to show the currently broken behavior. + # Now this should fail because we've violated the NUMA requirements # with the latest attachment ex = self.assertRaises(client.OpenStackApiException, @@ -659,12 +677,8 @@ class NUMAServersRebuildTests(NUMAServersTestBase): server = self._create_active_server( server_args={"flavorRef": flavor_id}) - # TODO(sean-k-mooney): this should pass but i currently expect it to - # fail because the NUMA topology filter does not support in place - # rebuild and we have used all the resources on the compute node. - self.assertRaises( - client.OpenStackApiException, self._rebuild_server, - server, self.image_ref_1) + # This should succeed as the numa constraints do not change. + self._rebuild_server(server, self.image_ref_1) def test_rebuild_server_with_different_numa_topology_fails(self): """Create a NUMA instance and ensure inplace rebuild fails. diff --git a/releasenotes/notes/numa-rebuild-b75f9a1966f576ea.yaml b/releasenotes/notes/numa-rebuild-b75f9a1966f576ea.yaml index 99777e4df6bf..646bb69d04da 100644 --- a/releasenotes/notes/numa-rebuild-b75f9a1966f576ea.yaml +++ b/releasenotes/notes/numa-rebuild-b75f9a1966f576ea.yaml @@ -14,3 +14,14 @@ fixes: and rejects the rebuild. .. _`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 + 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. +