From 9e96f64126d0098b26a16da549860db2d6ac8b34 Mon Sep 17 00:00:00 2001 From: Sylvain Bauza Date: Wed, 3 Feb 2021 15:19:58 +0100 Subject: [PATCH] Rename ensure_network_metadata to amend requested_networks As we don't persist (fortunately) the requested networks when booting an instance, we need a way to implement the value of the RequestSpec field during any create or move operation so we would know in a later change which port or network was asked. Partially-Implements: blueprint routed-networks-scheduling Change-Id: I0c7e32f6088a8fc1625a0655af824dee2df4a12c --- nova/compute/api.py | 4 +++ nova/conductor/manager.py | 4 +-- nova/conductor/tasks/live_migrate.py | 2 +- nova/conductor/tasks/migrate.py | 2 +- nova/objects/request_spec.py | 22 ++++++++++++++-- nova/tests/unit/compute/test_compute.py | 26 +++++++++++++++++++ .../unit/conductor/tasks/test_live_migrate.py | 10 +++---- .../unit/conductor/tasks/test_migrate.py | 6 ++--- nova/tests/unit/conductor/test_conductor.py | 12 ++++----- nova/tests/unit/objects/test_request_spec.py | 15 ++++++++--- 10 files changed, 79 insertions(+), 24 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 077a848f4350..7a94107bc444 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1545,6 +1545,10 @@ class API(base.Base): build_requests.append(build_request) instance = build_request.get_new_instance(context) instances.append(instance) + # NOTE(sbauza): Add the requested networks so the related scheduler + # pre-filter can verify them + if requested_networks is not None: + rs.requested_networks = requested_networks request_specs.append(rs) self.compute_task_api.schedule_and_build_instances( diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 605608e79f49..c74f0b67406c 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -983,7 +983,7 @@ class ComputeTaskManager(base.Base): context, instance, request_spec) request_spec.ensure_project_and_user_id(instance) - request_spec.ensure_network_metadata(instance) + request_spec.ensure_network_information(instance) compute_utils.heal_reqspec_is_bfv( context, request_spec, instance) host_lists = self._schedule_instances(context, @@ -1186,7 +1186,7 @@ class ComputeTaskManager(base.Base): self._restrict_request_spec_to_cell( context, instance, request_spec) request_spec.ensure_project_and_user_id(instance) - request_spec.ensure_network_metadata(instance) + request_spec.ensure_network_information(instance) compute_utils.heal_reqspec_is_bfv( context, request_spec, instance) diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index 275a49cee1bc..9613c136a41a 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -502,7 +502,7 @@ class LiveMigrationTask(base.TaskBase): cell=cell_mapping) request_spec.ensure_project_and_user_id(self.instance) - request_spec.ensure_network_metadata(self.instance) + request_spec.ensure_network_information(self.instance) compute_utils.heal_reqspec_is_bfv( self.context, request_spec, self.instance) diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 3dec41940d81..af1ef5af6a64 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -270,7 +270,7 @@ class MigrationTask(base.TaskBase): migration = self._preallocate_migration() self.request_spec.ensure_project_and_user_id(self.instance) - self.request_spec.ensure_network_metadata(self.instance) + self.request_spec.ensure_network_information(self.instance) compute_utils.heal_reqspec_is_bfv( self.context, self.request_spec, self.instance) # On an initial call to migrate, 'self.host_list' will be None, so we diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index 19c96c478059..11334335e0ea 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -110,7 +110,8 @@ class RequestSpec(base.NovaObject): default=None), # NOTE(efried): This field won't be persisted. 'request_level_params': fields.ObjectField('RequestLevelParams'), - # NOTE(sbauza); This field won't be persisted. + # NOTE(sbauza); This field won't be persisted. For move operations, we + # reevaluate it using the network-related instance info_cache. 'requested_networks': fields.ObjectField('NetworkRequestList') } @@ -544,12 +545,16 @@ class RequestSpec(base.NovaObject): if 'user_id' not in self or self.user_id is None: self.user_id = instance.user_id - def ensure_network_metadata(self, instance): + def ensure_network_information(self, instance): if not (instance.info_cache and instance.info_cache.network_info): + # NOTE(sbauza): On create, the network_info field is null but we + # directly set the RequestSpec nested network_requests field, so we + # are fine returning here. return physnets = set([]) tunneled = True + network_requests = [] # physical_network and tunneled might not be in the cache for old # instances that haven't had their info_cache healed yet @@ -561,8 +566,21 @@ class RequestSpec(base.NovaObject): tunneled |= vif.get('network', {}).get('meta', {}).get( 'tunneled', False) + # We also want to recreate the original NetworkRequests + # TODO(sbauza): We miss tag and pci_request_id information that is + # not stored in the VIF model to fully provide all fields + # FIXME(sbauza): We can't also guess whether the user provided us + # a specific IP address to use for create, and which one. + nr_args = { + 'network_id': vif['network']['id'], + 'port_id': vif['id'], + } + network_request = objects.NetworkRequest(**nr_args) + network_requests.append(network_request) self.network_metadata = objects.NetworkMetadata( physnets=physnets, tunneled=tunneled) + self.requested_networks = objects.NetworkRequestList( + objects=network_requests) @staticmethod def _from_db_object(context, spec, db_spec): diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 4dd97c0416e3..e5f2d07158d5 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8668,6 +8668,32 @@ class ComputeAPITestCase(BaseTestCase): len(db.instance_get_all(self.context))) mock_secgroups.assert_called_once_with(mock.ANY, 'invalid_sec_group') + def test_create_instance_associates_requested_networks(self): + # Make sure create adds the requested networks to the RequestSpec + + requested_networks = objects.NetworkRequestList( + objects=[objects.NetworkRequest(port_id=uuids.port_instance)]) + + with test.nested( + mock.patch.object(self.compute_api.compute_task_api, + 'schedule_and_build_instances'), + mock.patch.object(self.compute_api.network_api, + 'create_resource_requests', + return_value=(None, [])), + ) as (mock_sbi, _mock_create_resreqs): + self.compute_api.create( + self.context, + instance_type=self.default_flavor, + image_href=uuids.image_href_id, + requested_networks=requested_networks) + + build_call = mock_sbi.call_args_list[0] + reqspec = build_call[1]['request_spec'][0] + + self.assertEqual(1, len(reqspec.requested_networks)) + self.assertEqual(uuids.port_instance, + reqspec.requested_networks[0].port_id) + def test_create_with_malformed_user_data(self): # Test an instance type with malformed user data. diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index 0c3852eab734..e3bbcdc67867 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -71,8 +71,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self.heal_reqspec_is_bfv_mock = _p.start() self.addCleanup(_p.stop) - _p = mock.patch('nova.objects.RequestSpec.ensure_network_metadata') - self.ensure_network_metadata_mock = _p.start() + _p = mock.patch('nova.objects.RequestSpec.ensure_network_information') + self.ensure_network_information_mock = _p.start() self.addCleanup(_p.stop) _p = mock.patch( @@ -154,7 +154,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): # When the task is executed with a destination it means the host is # being forced and we don't call the scheduler, so we don't need to # modify the request spec - self.ensure_network_metadata_mock.assert_not_called() + self.ensure_network_information_mock.assert_not_called() @mock.patch('nova.availability_zones.get_host_availability_zone', return_value='nova') @@ -415,7 +415,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): mock_setup.assert_called_once_with(self.context, self.fake_spec) mock_reset.assert_called_once_with() - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( self.instance) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, self.fake_spec, self.instance) @@ -699,7 +699,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): mock_setup.assert_called_once_with(self.context, self.fake_spec) mock_reset.assert_called_once_with() - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( self.instance) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, self.fake_spec, self.instance) diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index bfee6fef0327..72423610ea32 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -61,8 +61,8 @@ class MigrationTaskTestCase(test.NoDBTestCase): self.heal_reqspec_is_bfv_mock = _p.start() self.addCleanup(_p.stop) - _p = mock.patch('nova.objects.RequestSpec.ensure_network_metadata') - self.ensure_network_metadata_mock = _p.start() + _p = mock.patch('nova.objects.RequestSpec.ensure_network_information') + self.ensure_network_information_mock = _p.start() self.addCleanup(_p.stop) self.mock_network_api = mock.Mock() @@ -130,7 +130,7 @@ class MigrationTaskTestCase(test.NoDBTestCase): task.execute() _is_source_cell_mock.assert_called_once_with(selection) - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( self.instance) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, self.request_spec, self.instance) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 485ae166e0e4..30c1f9861d2a 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -362,8 +362,8 @@ class _BaseTaskTestCase(object): self.heal_reqspec_is_bfv_mock = _p.start() self.addCleanup(_p.stop) - _p = mock.patch('nova.objects.RequestSpec.ensure_network_metadata') - self.ensure_network_metadata_mock = _p.start() + _p = mock.patch('nova.objects.RequestSpec.ensure_network_information') + self.ensure_network_information_mock = _p.start() self.addCleanup(_p.stop) def _prepare_rebuild_args(self, update_args=None): @@ -1385,7 +1385,7 @@ class _BaseTaskTestCase(object): # ComputeTaskManager. if isinstance(self.conductor, conductor_manager.ComputeTaskManager): - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( test.MatchType(objects.Instance)) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, fake_spec, instance) @@ -1397,7 +1397,7 @@ class _BaseTaskTestCase(object): else: # RPC API tests won't have the same request spec or instance # since they go over the wire. - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( test.MatchType(objects.Instance)) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, test.MatchType(objects.RequestSpec), @@ -1770,7 +1770,7 @@ class _BaseTaskTestCase(object): self.conductor_manager.rebuild_instance(context=self.context, instance=inst_obj, **rebuild_args) - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( inst_obj) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, fake_spec, inst_obj) @@ -1820,7 +1820,7 @@ class _BaseTaskTestCase(object): self.conductor_manager.rebuild_instance(context=self.context, instance=inst_obj, **rebuild_args) - self.ensure_network_metadata_mock.assert_called_once_with( + self.ensure_network_information_mock.assert_called_once_with( inst_obj) self.heal_reqspec_is_bfv_mock.assert_called_once_with( self.context, fake_spec, inst_obj) diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index fe1bb723d5fd..f31fa832e76e 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -532,7 +532,7 @@ class _TestRequestSpecObject(object): spec = objects.RequestSpec() self.assertEqual({}, spec.to_legacy_filter_properties_dict()) - def test_ensure_network_metadata(self): + def test_ensure_network_information(self): network_a = fake_network_cache_model.new_network({ 'physical_network': 'foo', 'tunneled': False}) vif_a = fake_network_cache_model.new_vif({'network': network_a}) @@ -553,14 +553,19 @@ class _TestRequestSpecObject(object): spec = objects.RequestSpec() self.assertNotIn('network_metadata', spec) + self.assertNotIn('requested_networks', spec) - spec.ensure_network_metadata(instance) + spec.ensure_network_information(instance) self.assertIn('network_metadata', spec) self.assertIsInstance(spec.network_metadata, objects.NetworkMetadata) self.assertEqual(spec.network_metadata.physnets, set(['foo', 'bar'])) self.assertTrue(spec.network_metadata.tunneled) + self.assertEqual(4, len(spec.requested_networks)) + for idx, reqnet in enumerate(spec.requested_networks): + self.assertEqual(nw_info[idx]['network']['id'], reqnet.network_id) + self.assertEqual(nw_info[idx]['id'], reqnet.port_id) - def test_ensure_network_metadata_missing(self): + def test_ensure_network_information_missing(self): nw_info = network_model.NetworkInfo([]) info_cache = objects.InstanceInfoCache(network_info=nw_info, instance_uuid=uuids.instance) @@ -569,9 +574,11 @@ class _TestRequestSpecObject(object): spec = objects.RequestSpec() self.assertNotIn('network_metadata', spec) + self.assertNotIn('requested_networks', spec) - spec.ensure_network_metadata(instance) + spec.ensure_network_information(instance) self.assertNotIn('network_metadata', spec) + self.assertNotIn('requested_networks', spec) @mock.patch.object(request_spec.RequestSpec, '_get_by_instance_uuid_from_db')