From 45662d77a2da77714f8e792e86ebd64a52270ef5 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 24 Jul 2018 15:31:33 +0100 Subject: [PATCH] Add additional functional tests for NUMA networks Cover the move and move-like operations through cold migration and rebuild tests. This requires extensive filling of gaps in the NeutronFixture and fake libvirt driver. Part of blueprint numa-aware-vswitches Change-Id: I18acbda74dd8ac4e9a5d439f1393765a9a6a60f3 Co-Authored-By: Sean Mooney Co-Authored-By: Dan Smith --- nova/tests/fixtures.py | 7 +- .../functional/libvirt/test_numa_servers.py | 306 +++++++++++++++++- nova/tests/unit/virt/libvirt/fakelibvirt.py | 36 ++- 3 files changed, 332 insertions(+), 17 deletions(-) diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index a735f18a71..be0dc961c3 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1299,7 +1299,10 @@ class NeutronFixture(fixtures.Fixture): return {'network': network} def list_networks(self, retrieve_all=True, **_params): - return copy.deepcopy({'networks': self._networks}) + networks = copy.deepcopy(self._networks) + if 'id' in _params: + networks = [x for x in networks if x['id'] in _params['id']] + return {'networks': networks} def list_ports(self, retrieve_all=True, **_params): return self._filter_ports(**_params) @@ -1310,7 +1313,7 @@ class NeutronFixture(fixtures.Fixture): def list_floatingips(self, retrieve_all=True, **_params): return copy.deepcopy({'floatingips': self._floatingips}) - def create_port(self, *args, **kwargs): + def create_port(self, body=None): self._ports.append(copy.deepcopy(NeutronFixture.port_2)) return copy.deepcopy({'port': NeutronFixture.port_2}) diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py index 815656b495..bedba9436d 100644 --- a/nova/tests/functional/libvirt/test_numa_servers.py +++ b/nova/tests/functional/libvirt/test_numa_servers.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import six + import fixtures import mock from oslo_config import cfg @@ -23,12 +25,14 @@ from nova import context as nova_context from nova import objects from nova import test from nova.tests import fixtures as nova_fixtures +from nova.tests.functional.api import client from nova.tests.functional.test_servers import ServersTestBase from nova.tests.unit import fake_network 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 +import os_vif CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -198,7 +202,7 @@ class NUMAAffinityNeutronFixture(nova_fixtures.NeutronFixture): 'id': '3cb9bc59-5699-4588-a4b1-b87f96708bc6', 'status': 'ACTIVE', 'subnets': [], - 'name': 'private-network', + 'name': 'physical-network-foo', 'admin_state_up': True, 'tenant_id': nova_fixtures.NeutronFixture.tenant_id, 'provider:physical_network': 'foo', @@ -208,6 +212,7 @@ class NUMAAffinityNeutronFixture(nova_fixtures.NeutronFixture): network_2 = network_1.copy() network_2.update({ 'id': 'a252b8cd-2d99-4e82-9a97-ec1217c496f5', + 'name': 'physical-network-bar', 'provider:physical_network': 'bar', 'provider:network_type': 'vlan', 'provider:segmentation_id': 123, @@ -215,14 +220,115 @@ class NUMAAffinityNeutronFixture(nova_fixtures.NeutronFixture): network_3 = network_1.copy() network_3.update({ 'id': '877a79cc-295b-4b80-9606-092bf132931e', + 'name': 'tunneled-network', 'provider:physical_network': None, 'provider:network_type': 'vxlan', 'provider:segmentation_id': 69, }) + subnet_1 = nova_fixtures.NeutronFixture.subnet_1.copy() + subnet_1.update({ + 'name': 'physical-subnet-foo', + }) + subnet_2 = nova_fixtures.NeutronFixture.subnet_1.copy() + subnet_2.update({ + 'id': 'b4c13749-c002-47ed-bf42-8b1d44fa9ff2', + 'name': 'physical-subnet-bar', + 'network_id': network_2['id'], + }) + subnet_3 = nova_fixtures.NeutronFixture.subnet_1.copy() + subnet_3.update({ + 'id': '4dacb20b-917f-4275-aa75-825894553442', + 'name': 'tunneled-subnet', + 'network_id': network_3['id'], + }) + network_1['subnets'] = [subnet_1] + network_2['subnets'] = [subnet_2] + network_3['subnets'] = [subnet_3] + + network_1_port_2 = { + 'id': 'f32582b5-8694-4be8-9a52-c5732f601c9d', + 'network_id': network_1['id'], + 'status': 'ACTIVE', + 'mac_address': '71:ce:c7:8b:cd:dc', + 'fixed_ips': [ + { + 'ip_address': '192.168.1.10', + 'subnet_id': subnet_1['id'] + } + ], + 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', + } + network_1_port_3 = { + 'id': '9c7580a0-8b01-41f3-ba07-a114709a4b74', + 'network_id': network_1['id'], + 'status': 'ACTIVE', + 'mac_address': '71:ce:c7:2b:cd:dc', + 'fixed_ips': [ + { + 'ip_address': '192.168.1.11', + 'subnet_id': subnet_1['id'] + } + ], + 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', + } + network_2_port_1 = { + 'id': '67d36444-6353-40f5-9e92-59346cf0dfda', + 'network_id': network_2['id'], + 'status': 'ACTIVE', + 'mac_address': 'd2:0b:fd:d7:89:9b', + 'fixed_ips': [ + { + 'ip_address': '192.168.1.6', + 'subnet_id': subnet_2['id'] + } + ], + 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', + } + network_3_port_1 = { + 'id': '4bfa1dc4-4354-4840-b0b4-f06196fa1344', + 'network_id': network_3['id'], + 'status': 'ACTIVE', + 'mac_address': 'd2:0b:fd:99:89:9b', + 'fixed_ips': [ + { + 'ip_address': '192.168.2.6', + 'subnet_id': subnet_3['id'] + } + ], + 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', + } + def __init__(self, test): super(NUMAAffinityNeutronFixture, self).__init__(test) self._networks = [self.network_1, self.network_2, self.network_3] + self._net1_ports = [self.network_1_port_2, self.network_1_port_3] + + def create_port(self, body=None): + if not body: + # even though 'body' is apparently nullable, body will always be + # set here + assert('Body should not be None') + + network_id = body['port']['network_id'] + assert network_id in ([n['id'] for n in self._networks]), ( + 'Network %s not in fixture' % network_id) + + if network_id == self.network_1['id']: + port = self._net1_ports.pop(0) + elif network_id == self.network_2['id']: + port = self.network_2_port_1 + elif network_id == self.network_3['id']: + port = self.network_3_port_1 + + port = port.copy() + port.update(body['port']) + self._ports.append(port) + return {'port': port} class NUMAServersWithNetworksTest(NUMAServersTestBase): @@ -239,16 +345,25 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): super(NUMAServersWithNetworksTest, self).setUp() - self.useFixture(NUMAAffinityNeutronFixture(self)) + # NOTE(mriedem): Unset the stub methods so we actually run our + # neutronv2/api code and populate the net attributes on the + # network model. + fake_network.unset_stub_network_methods(self) - @mock.patch('nova.virt.libvirt.host.Host.get_connection') - def _test_create_server_with_networks(self, flavor_id, networks, - mock_conn): + self.neutron_fixture = self.useFixture( + NUMAAffinityNeutronFixture(self)) + + _p = mock.patch('nova.virt.libvirt.host.Host.get_connection') + self.mock_conn = _p.start() + self.addCleanup(_p.stop) + os_vif.initialize() + + def _test_create_server_with_networks(self, flavor_id, networks): host_info = fakelibvirt.NUMAHostInfo(cpu_nodes=2, cpu_sockets=1, cpu_cores=2, cpu_threads=2, kB_mem=15740000) fake_connection = self._get_connection(host_info=host_info) - mock_conn.return_value = fake_connection + self.mock_conn.return_value = fake_connection self.compute = self.start_service('compute', host='test_compute0') @@ -262,7 +377,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): found_server = self.api.get_server(created_server['id']) - return self._wait_for_state_change(found_server, 'BUILD')['status'] + return self._wait_for_state_change(found_server, 'BUILD') def test_create_server_with_single_physnet(self): extra_spec = {'hw:numa_nodes': '1'} @@ -276,7 +391,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): '.numa_topology_filter.NUMATopologyFilter.host_passes', side_effect=host_pass_mock) as filter_mock: status = self._test_create_server_with_networks( - flavor_id, networks) + flavor_id, networks)['status'] self.assertTrue(filter_mock.called) self.assertEqual('ACTIVE', status) @@ -300,7 +415,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): '.numa_topology_filter.NUMATopologyFilter.host_passes', side_effect=host_pass_mock) as filter_mock: status = self._test_create_server_with_networks( - flavor_id, networks) + flavor_id, networks)['status'] self.assertTrue(filter_mock.called) self.assertEqual('ACTIVE', status) @@ -323,7 +438,7 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): '.numa_topology_filter.NUMATopologyFilter.host_passes', side_effect=host_pass_mock) as filter_mock: status = self._test_create_server_with_networks( - flavor_id, networks) + flavor_id, networks)['status'] self.assertTrue(filter_mock.called) self.assertEqual('ERROR', status) @@ -346,7 +461,176 @@ class NUMAServersWithNetworksTest(NUMAServersTestBase): '.numa_topology_filter.NUMATopologyFilter.host_passes', side_effect=host_pass_mock) as filter_mock: status = self._test_create_server_with_networks( - flavor_id, networks) + flavor_id, networks)['status'] self.assertTrue(filter_mock.called) self.assertEqual('ACTIVE', status) + + def test_rebuild_server_with_network_affinity(self): + extra_spec = {'hw:numa_nodes': '1'} + flavor_id = self._create_flavor(extra_spec=extra_spec) + networks = [ + {'uuid': NUMAAffinityNeutronFixture.network_1['id']}, + ] + + server = self._test_create_server_with_networks(flavor_id, networks) + + self.assertEqual('ACTIVE', server['status']) + + # attach an interface from the **same** network + post = { + 'interfaceAttachment': { + 'net_id': NUMAAffinityNeutronFixture.network_1['id'], + } + } + self.api.attach_interface(server['id'], post) + + post = {'rebuild': { + 'imageRef': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6', + }} + + # This should succeed since we haven't changed the NUMA affinity + # requirements + self.api.post_server_action(server['id'], post) + found_server = self._wait_for_state_change(server, 'BUILD') + self.assertEqual('ACTIVE', found_server['status']) + + # attach an interface from a **different** network + post = { + 'interfaceAttachment': { + 'net_id': NUMAAffinityNeutronFixture.network_2['id'], + } + } + self.api.attach_interface(server['id'], post) + post = {'rebuild': { + 'imageRef': 'a2459075-d96c-40d5-893e-577ff92e721c', + }} + # Now this should fail because we've violated the NUMA requirements + # with the latest attachment + ex = self.assertRaises(client.OpenStackApiException, + self.api.post_server_action, server['id'], post) + # NOTE(danms): This wouldn't happen in a real deployment since rebuild + # is a cast, but since we are using CastAsCall this will bubble to the + # API. + self.assertEqual(500, ex.response.status_code) + self.assertIn('NoValidHost', six.text_type(ex)) + + def test_cold_migrate_with_physnet(self): + host_info = fakelibvirt.NUMAHostInfo(cpu_nodes=2, cpu_sockets=1, + cpu_cores=2, cpu_threads=2, + kB_mem=15740000) + + # Start services + self.computes = {} + for host in ['test_compute0', 'test_compute1']: + fake_connection = self._get_connection(host_info=host_info) + fake_connection.getHostname = lambda: host + + # This is fun. Firstly we need to do a global'ish mock so we can + # actually start the service. + with mock.patch('nova.virt.libvirt.host.Host.get_connection', + return_value=fake_connection): + compute = self.start_service('compute', host=host) + + # Once that's done, we need to do some tweaks to each individual + # compute "service" to make sure they return unique objects + compute.driver._host.get_connection = lambda: fake_connection + self.computes[host] = compute + + # Create server + extra_spec = {'hw:numa_nodes': '1'} + flavor_id = self._create_flavor(extra_spec=extra_spec) + networks = [ + {'uuid': NUMAAffinityNeutronFixture.network_1['id']}, + ] + + good_server = self._build_server(flavor_id) + good_server['networks'] = networks + post = {'server': good_server} + + created_server = self.api.post_server(post) + server = self._wait_for_state_change(created_server, 'BUILD') + + self.assertEqual('ACTIVE', server['status']) + original_host = server['OS-EXT-SRV-ATTR:host'] + + # TODO(stephenfin): The mock of 'migrate_disk_and_power_off' should + # probably be less...dumb + host_pass_mock = self._get_topology_filter_spy() + with mock.patch('nova.scheduler.filters' + '.numa_topology_filter.NUMATopologyFilter.host_passes', + side_effect=host_pass_mock) as filter_mock, \ + mock.patch('nova.virt.libvirt.driver.LibvirtDriver' + '.migrate_disk_and_power_off', return_value='{}'): + self.api.post_server_action(server['id'], {'migrate': None}) + + server = self._wait_for_state_change(created_server, 'VERIFY_RESIZE') + + # We don't bother confirming the resize as we expect this to have + # landed and all we want to know is whether the filter was correct + self.assertNotEqual(original_host, server['OS-EXT-SRV-ATTR:host']) + + self.assertEqual(1, len(filter_mock.call_args_list)) + args, kwargs = filter_mock.call_args_list[0] + self.assertEqual(2, len(args)) + self.assertEqual({}, kwargs) + network_metadata = args[1].network_metadata + self.assertIsNotNone(network_metadata) + self.assertEqual(set(['foo']), network_metadata.physnets) + + def test_cold_migrate_with_physnet_fails(self): + host_infos = [ + # host 1 has room on both nodes + fakelibvirt.NUMAHostInfo(cpu_nodes=2, cpu_sockets=1, + cpu_cores=2, cpu_threads=2, + kB_mem=15740000), + # host 2 has no second node, where the desired physnet is + # reported to be attached + fakelibvirt.NUMAHostInfo(cpu_nodes=1, cpu_sockets=1, + cpu_cores=1, cpu_threads=1, + kB_mem=15740000), + ] + + # Start services + self.computes = {} + for host in ['test_compute0', 'test_compute1']: + host_info = host_infos.pop(0) + fake_connection = self._get_connection(host_info=host_info) + fake_connection.getHostname = lambda: host + + # This is fun. Firstly we need to do a global'ish mock so we can + # actually start the service. + with mock.patch('nova.virt.libvirt.host.Host.get_connection', + return_value=fake_connection): + compute = self.start_service('compute', host=host) + + # Once that's done, we need to do some tweaks to each individual + # compute "service" to make sure they return unique objects + compute.driver._host.get_connection = lambda: fake_connection + self.computes[host] = compute + + # Create server + extra_spec = {'hw:numa_nodes': '1'} + flavor_id = self._create_flavor(extra_spec=extra_spec) + networks = [ + {'uuid': NUMAAffinityNeutronFixture.network_1['id']}, + ] + + good_server = self._build_server(flavor_id) + good_server['networks'] = networks + post = {'server': good_server} + + created_server = self.api.post_server(post) + server = self._wait_for_state_change(created_server, 'BUILD') + + self.assertEqual('ACTIVE', server['status']) + + # TODO(stephenfin): The mock of 'migrate_disk_and_power_off' should + # probably be less...dumb + with mock.patch('nova.virt.libvirt.driver.LibvirtDriver' + '.migrate_disk_and_power_off', return_value='{}'): + ex = self.assertRaises(client.OpenStackApiException, + self.api.post_server_action, + server['id'], {'migrate': None}) + self.assertEqual(400, ex.response.status_code) + self.assertIn('No valid host', six.text_type(ex)) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index 938f9a4c67..28e1f4440e 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -441,6 +441,25 @@ def _parse_disk_info(element): return disk_info +def _parse_nic_info(element): + nic_info = {} + nic_info['type'] = element.get('type', 'bridge') + + driver = element.find('./mac') + if driver is not None: + nic_info['mac'] = driver.get('address') + + source = element.find('./source') + if source is not None: + nic_info['source'] = source.get('bridge') + + target = element.find('./target') + if target is not None: + nic_info['target_dev'] = target.get('dev') + + return nic_info + + def disable_event_thread(self): """Disable nova libvirt driver event thread. @@ -781,10 +800,19 @@ class Domain(object): pass def attachDevice(self, xml): - disk_info = _parse_disk_info(etree.fromstring(xml)) - disk_info['_attached'] = True - self._def['devices']['disks'] += [disk_info] - return True + result = False + if xml.startswith("