From f203da383871aea195fd5a03ac3b5544176344cb Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 5 Feb 2020 16:16:57 +0000 Subject: [PATCH] objects: Add MigrationTypeField We use these things many places in the code and it would be good to have constants to reference. Do just that. Note that this results in a change in the object hash. However, there are no actual changes in the output object so that's okay. Change-Id: If02567ce0a3431dda5b2bf6d398bbf7cc954eed0 Signed-off-by: Stephen Finucane --- nova/api/openstack/compute/migrations.py | 5 +++- .../openstack/compute/server_migrations.py | 2 +- nova/compute/api.py | 10 +++---- nova/compute/claims.py | 7 +++-- nova/compute/manager.py | 9 +++--- nova/compute/resource_tracker.py | 23 ++++++++------- nova/conductor/manager.py | 2 +- nova/network/constants.py | 1 - nova/network/neutron.py | 8 +++--- nova/objects/fields.py | 14 ++++++++++ nova/objects/migration.py | 12 ++++++-- nova/tests/unit/network/test_neutron.py | 28 +++++++++++-------- nova/tests/unit/objects/test_objects.py | 2 +- .../unit/policies/test_server_migrations.py | 7 +++-- 14 files changed, 81 insertions(+), 49 deletions(-) diff --git a/nova/api/openstack/compute/migrations.py b/nova/api/openstack/compute/migrations.py index 49e5b4eaa376..f0770e8e7b15 100644 --- a/nova/api/openstack/compute/migrations.py +++ b/nova/api/openstack/compute/migrations.py @@ -23,6 +23,7 @@ from nova.compute import api as compute from nova import exception from nova.i18n import _ from nova.objects import base as obj_base +from nova.objects import fields from nova.policies import migrations as migrations_policies @@ -72,7 +73,9 @@ class MigrationsController(wsgi.Controller): # NOTE(Shaohe Feng) above version 2.23, add migration_type for all # kinds of migration, but we only add links just for in-progress # live-migration. - if add_link and obj['migration_type'] == "live-migration" and ( + if (add_link and + obj['migration_type'] == + fields.MigrationType.LIVE_MIGRATION and obj["status"] in live_migration_in_progress): obj["links"] = self._view_builder._get_links( req, obj["id"], diff --git a/nova/api/openstack/compute/server_migrations.py b/nova/api/openstack/compute/server_migrations.py index 6e8f3559ec06..230d6088fe72 100644 --- a/nova/api/openstack/compute/server_migrations.py +++ b/nova/api/openstack/compute/server_migrations.py @@ -132,7 +132,7 @@ class ServerMigrationsController(wsgi.Controller): " server %(uuid)s.") % {"id": id, "uuid": server_id} raise exc.HTTPNotFound(explanation=msg) - if migration.get("migration_type") != "live-migration": + if not migration.is_live_migration: msg = _("Migration %(id)s for server %(uuid)s is not" " live-migration.") % {"id": id, "uuid": server_id} raise exc.HTTPNotFound(explanation=msg) diff --git a/nova/compute/api.py b/nova/compute/api.py index 7f68070885c2..d28987b72f1b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -5036,12 +5036,10 @@ class API(base.Base): # NOTE(danms): Create this as a tombstone for the source compute # to find and cleanup. No need to pass it anywhere else. - migration = objects.Migration(context, - source_compute=instance.host, - source_node=instance.node, - instance_uuid=instance.uuid, - status='accepted', - migration_type='evacuation') + migration = objects.Migration( + context, source_compute=instance.host, source_node=instance.node, + instance_uuid=instance.uuid, status='accepted', + migration_type=fields_obj.MigrationType.EVACUATION) if host: migration.dest_compute = host migration.create() diff --git a/nova/compute/claims.py b/nova/compute/claims.py index 99c553db1ac3..bcc6f3088501 100644 --- a/nova/compute/claims.py +++ b/nova/compute/claims.py @@ -194,9 +194,10 @@ class MoveClaim(Claim): migration, all types of PCI requests are supported, so we just call up to normal Claim's _test_pci(). """ - if self.migration.migration_type != 'live-migration': + if not self.migration.is_live_migration: return super(MoveClaim, self)._test_pci() - elif self._pci_requests.requests: + + if self._pci_requests.requests: for pci_request in self._pci_requests.requests: if (pci_request.source != objects.InstancePCIRequest.NEUTRON_PORT): @@ -225,7 +226,7 @@ class MoveClaim(Claim): something we want to support, so fail the claim if the page sizes are different. """ - if (self.migration.migration_type == 'live-migration' and + if (self.migration.is_live_migration and self.instance.numa_topology and # NOTE(artom) We only support a single page size across all # cells, checking cell 0 is sufficient. diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 75ef023ce66a..894c0847237f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -731,7 +731,7 @@ class ComputeManager(manager.Manager): # the user can rebuild the instance (in ERROR state) on the source # host. 'status': ['accepted', 'pre-migrating', 'done'], - 'migration_type': 'evacuation', + 'migration_type': fields.MigrationType.EVACUATION, } with utils.temporary_mutation(context, read_deleted='yes'): evacuations = objects.MigrationList.get_by_filters(context, @@ -8552,9 +8552,10 @@ class ComputeManager(manager.Manager): # NOTE(mriedem): This is a no-op for neutron. self.network_api.setup_networks_on_host(context, instance, self.host) - migration = objects.Migration(source_compute=instance.host, - dest_compute=self.host, - migration_type='live-migration') + migration = objects.Migration( + source_compute=instance.host, + dest_compute=self.host, + migration_type=fields.MigrationType.LIVE_MIGRATION) self.network_api.migrate_instance_finish( context, instance, migration, provider_mappings=None) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 63496e7a0b4a..a3f63ede36c6 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -39,6 +39,7 @@ from nova import exception from nova.i18n import _ from nova import objects from nova.objects import base as obj_base +from nova.objects import fields from nova.objects import migration as migration_obj from nova.pci import manager as pci_manager from nova.pci import request as pci_request @@ -191,9 +192,10 @@ class ResourceTracker(object): limits=None, image_meta=None, migration=None): """Create a claim for a rebuild operation.""" instance_type = instance.flavor - return self._move_claim(context, instance, instance_type, nodename, - migration, allocations, move_type='evacuation', - limits=limits, image_meta=image_meta) + return self._move_claim( + context, instance, instance_type, nodename, migration, allocations, + move_type=fields.MigrationType.EVACUATION, + image_meta=image_meta, limits=limits) @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair=True) def resize_claim(self, context, instance, instance_type, nodename, @@ -225,9 +227,11 @@ class ResourceTracker(object): # Flavor and image cannot change during a live migration. instance_type = instance.flavor image_meta = instance.image_meta - return self._move_claim(context, instance, instance_type, nodename, - migration, allocs, move_type='live-migration', - image_meta=image_meta, limits=limits) + return self._move_claim( + context, instance, instance_type, nodename, migration, allocs, + move_type=fields.MigrationType.LIVE_MIGRATION, + image_meta=image_meta, limits=limits, + ) def _move_claim(self, context, instance, new_instance_type, nodename, migration, allocations, move_type=None, @@ -293,7 +297,7 @@ class ResourceTracker(object): # migration to avoid stepping on that code's toes. Ideally, # MoveClaim/this method would be used for all live migration resource # claims. - if self.pci_tracker and migration.migration_type != 'live-migration': + if self.pci_tracker and not migration.is_live_migration: # NOTE(jaypipes): ComputeNode.pci_device_pools is set below # in _update_usage_from_instance(). claimed_pci_devices_objs = self.pci_tracker.claim_instance( @@ -369,7 +373,7 @@ class ResourceTracker(object): # NOTE(artom) Migration objects for live migrations are created with # status 'accepted' by the conductor in live_migrate_instance() and do # not have a 'pre-migrating' status. - if migration.migration_type != 'live-migration': + if not migration.is_live_migration: migration.status = 'pre-migrating' migration.save() @@ -1637,8 +1641,7 @@ class ResourceTracker(object): def _get_instance_type(self, instance, prefix, migration): """Get the instance type from instance.""" - stashed_flavors = migration.migration_type in ('resize',) - if stashed_flavors: + if migration.is_resize: return getattr(instance, '%sflavor' % prefix) else: # NOTE(ndipanov): Certain migration types (all but resize) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 9f816cd5332e..5c6f50724562 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -456,7 +456,7 @@ class ComputeTaskManager(base.Base): migration.status = 'accepted' migration.instance_uuid = instance.uuid migration.source_compute = instance.host - migration.migration_type = 'live-migration' + migration.migration_type = fields.MigrationType.LIVE_MIGRATION if instance.obj_attr_is_set('flavor'): migration.old_instance_type_id = instance.flavor.id migration.new_instance_type_id = instance.flavor.id diff --git a/nova/network/constants.py b/nova/network/constants.py index cfa5b1b9d212..f3a7caaacdbc 100644 --- a/nova/network/constants.py +++ b/nova/network/constants.py @@ -21,7 +21,6 @@ MULTI_NET_EXT = 'Multi Provider Network' FIP_PORT_DETAILS = 'Floating IP Port Details Extension' SUBSTR_PORT_FILTERING = 'IP address substring filtering' PORT_BINDING_EXTENDED = 'Port Bindings Extended' -LIVE_MIGRATION = 'live-migration' DEFAULT_SECGROUP = 'default' BINDING_PROFILE = 'binding:profile' BINDING_HOST_ID = 'binding:host_id' diff --git a/nova/network/neutron.py b/nova/network/neutron.py index 745d522f8c4e..0aef485a8398 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -3415,7 +3415,7 @@ class API(base.Base): vnic_type = p.get('binding:vnic_type') if (vnic_type in network_model.VNIC_TYPES_SRIOV and migration is not None and - migration['migration_type'] != constants.LIVE_MIGRATION): + not migration.is_live_migration): # Note(adrianc): for live migration binding profile was already # updated in conductor when calling bind_ports_to_host() if not pci_mapping: @@ -3437,9 +3437,9 @@ class API(base.Base): # allocation key in the port binding. However during resize, cold # migrate, evacuate and unshelve we have to set the binding here. # Also note that during unshelve no migration object is created. - if (p.get('resource_request') and - (migration is None or - migration['migration_type'] != constants.LIVE_MIGRATION)): + if p.get('resource_request') and ( + migration is None or not migration.is_live_migration + ): if not provider_mappings: # TODO(gibi): Remove this check when compute RPC API is # bumped to 6.0 diff --git a/nova/objects/fields.py b/nova/objects/fields.py index 946dc4e9189d..0e7606aba939 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -464,6 +464,16 @@ class ImageSignatureKeyType(BaseNovaEnum): ) +class MigrationType(BaseNovaEnum): + + MIGRATION = 'migration' # cold migration + RESIZE = 'resize' + LIVE_MIGRATION = 'live-migration' + EVACUATION = 'evacuation' + + ALL = (MIGRATION, RESIZE, LIVE_MIGRATION, EVACUATION) + + class OSType(BaseNovaEnum): LINUX = "linux" @@ -1207,6 +1217,10 @@ class ImageSignatureKeyTypeField(BaseEnumField): AUTO_TYPE = ImageSignatureKeyType() +class MigrationTypeField(BaseEnumField): + AUTO_TYPE = MigrationType() + + class OSTypeField(BaseEnumField): AUTO_TYPE = OSType() diff --git a/nova/objects/migration.py b/nova/objects/migration.py index 4489f5c0bc6c..5c185f45e2f0 100644 --- a/nova/objects/migration.py +++ b/nova/objects/migration.py @@ -57,9 +57,7 @@ class Migration(base.NovaPersistentObject, base.NovaObject, 'new_instance_type_id': fields.IntegerField(nullable=True), 'instance_uuid': fields.StringField(nullable=True), 'status': fields.StringField(nullable=True), - 'migration_type': fields.EnumField(['migration', 'resize', - 'live-migration', 'evacuation'], - nullable=False), + 'migration_type': fields.MigrationTypeField(nullable=False), 'hidden': fields.BooleanField(nullable=False, default=False), 'memory_total': fields.IntegerField(nullable=True), 'memory_processed': fields.IntegerField(nullable=True), @@ -205,6 +203,14 @@ class Migration(base.NovaPersistentObject, base.NovaObject, def is_same_host(self): return self.source_compute == self.dest_compute + @property + def is_live_migration(self): + return self.migration_type == fields.MigrationType.LIVE_MIGRATION + + @property + def is_resize(self): + return self.migration_type == fields.MigrationType.RESIZE + @base.NovaObjectRegistry.register class MigrationList(base.ObjectListBase, base.NovaObject): diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 3e6dc6417bae..c64a1de50ea8 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -4382,8 +4382,8 @@ class TestAPI(TestAPIBase): 'pci_vendor_info': 'old_pci_vendor_info'}}, {'id': 'fake-port-2', constants.BINDING_HOST_ID: instance.host}]} - migration = {'status': 'confirmed', - 'migration_type': "migration"} + migration = objects.Migration( + status='confirmed', migration_type='migration') list_ports_mock = mock.Mock(return_value=fake_ports) get_client_mock.return_value.list_ports = list_ports_mock @@ -4439,8 +4439,8 @@ class TestAPI(TestAPIBase): {'pci_slot': '0000:0a:00.1', 'physical_network': 'old_phys_net', 'pci_vendor_info': 'old_pci_vendor_info'}}]} - migration = {'status': 'confirmed', - 'migration_type': "migration"} + migration = objects.Migration( + status='confirmed', migration_type='migration') list_ports_mock = mock.Mock(return_value=fake_ports) get_client_mock.return_value.list_ports = list_ports_mock @@ -4595,8 +4595,8 @@ class TestAPI(TestAPIBase): {'pci_slot': '0000:0a:00.1', 'physical_network': 'phys_net', 'pci_vendor_info': 'vendor_info'}}]} - migration = {'status': 'confirmed', - 'migration_type': "live-migration"} + migration = objects.Migration( + status='confirmed', migration_type='live-migration') list_ports_mock = mock.Mock(return_value=fake_ports) get_client_mock.return_value.list_ports = list_ports_mock update_port_mock = mock.Mock() @@ -4727,7 +4727,7 @@ class TestAPI(TestAPIBase): def test_get_pci_mapping_for_migration(self): instance = fake_instance.fake_instance_obj(self.context) instance.migration_context = objects.MigrationContext() - migration = {'status': 'confirmed'} + migration = objects.Migration(status='confirmed') with mock.patch.object(instance.migration_context, 'get_pci_mapping_for_migration') as map_func: @@ -4737,7 +4737,7 @@ class TestAPI(TestAPIBase): def test_get_pci_mapping_for_migration_reverted(self): instance = fake_instance.fake_instance_obj(self.context) instance.migration_context = objects.MigrationContext() - migration = {'status': 'reverted'} + migration = objects.Migration(status='reverted') with mock.patch.object(instance.migration_context, 'get_pci_mapping_for_migration') as map_func: @@ -6132,7 +6132,8 @@ class TestAPI(TestAPIBase): # Just create a simple instance with a single port. instance = objects.Instance(info_cache=objects.InstanceInfoCache( network_info=model.NetworkInfo([model.VIF(uuids.port_id)]))) - migration = {'source_compute': 'source', 'dest_compute': 'dest'} + migration = objects.Migration( + source_compute='source', dest_compute='dest') with mock.patch.object(self.api, 'activate_port_binding') as activate: with mock.patch.object(self.api, 'supports_port_binding_extension', return_value=True): @@ -6155,7 +6156,8 @@ class TestAPI(TestAPIBase): # Just create a simple instance with a single port. instance = objects.Instance(info_cache=objects.InstanceInfoCache( network_info=model.NetworkInfo([model.VIF(uuids.port_id)]))) - migration = {'source_compute': 'source', 'dest_compute': 'dest'} + migration = objects.Migration( + source_compute='source', dest_compute='dest') with mock.patch.object(self.api, 'activate_port_binding', new_callable=mock.NonCallableMock): with mock.patch.object(self.api, 'supports_port_binding_extension', @@ -6180,7 +6182,8 @@ class TestAPI(TestAPIBase): instance = objects.Instance(info_cache=objects.InstanceInfoCache( network_info=model.NetworkInfo([ model.VIF(uuids.port1), model.VIF(uuids.port2)]))) - migration = {'source_compute': 'source', 'dest_compute': 'dest'} + migration = objects.Migration( + source_compute='source', dest_compute='dest') with mock.patch.object(self.api, 'activate_port_binding', new_callable=mock.NonCallableMock): with mock.patch.object(self.api, 'supports_port_binding_extension', @@ -6202,7 +6205,8 @@ class TestAPI(TestAPIBase): instance = objects.Instance(info_cache=objects.InstanceInfoCache( network_info=model.NetworkInfo([ model.VIF(uuids.port1), model.VIF(uuids.port2)]))) - migration = {'source_compute': 'source', 'dest_compute': 'dest'} + migration = objects.Migration( + source_compute='source', dest_compute='dest') with mock.patch.object(self.api, 'activate_port_binding', new_callable=mock.NonCallableMock): with mock.patch.object(self.api, 'supports_port_binding_extension', diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index b5bd0f32414e..470a3c62c397 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1103,7 +1103,7 @@ object_data = { 'LibvirtLiveMigrateData': '1.10-348cf70ea44d3b985f45f64725d6f6a7', 'LibvirtLiveMigrateNUMAInfo': '1.0-0e777677f3459d0ed1634eabbdb6c22f', 'MemoryDiagnostics': '1.0-2c995ae0f2223bb0f8e523c5cc0b83da', - 'Migration': '1.7-b77066a88d08bdb0b05d7bc18780c55a', + 'Migration': '1.7-bd45b232fd7c95cd79ae9187e10ef582', 'MigrationContext': '1.2-89f10a83999f852a489962ae37d8a026', 'MigrationList': '1.4-983a9c29d4f1e747ce719dc9063b729b', 'MonitorMetric': '1.1-53b1db7c4ae2c531db79761e7acc52ba', diff --git a/nova/tests/unit/policies/test_server_migrations.py b/nova/tests/unit/policies/test_server_migrations.py index 6268f10dfe44..479bced83f72 100644 --- a/nova/tests/unit/policies/test_server_migrations.py +++ b/nova/tests/unit/policies/test_server_migrations.py @@ -17,6 +17,7 @@ from oslo_utils.fixture import uuidsentinel as uuids from nova.api.openstack.compute import server_migrations from nova.compute import vm_states +from nova import objects from nova.policies import base as base_policy from nova.policies import servers_migrations as policies from nova.tests.unit.api.openstack import fakes @@ -82,8 +83,10 @@ class ServerMigrationsPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') def test_show_server_migrations_policy(self, mock_show, mock_output): rule_name = policies.POLICY_ROOT % 'show' - mock_show.return_value = {"migration_type": "live-migration", - "status": "running"} + mock_show.return_value = objects.Migration( + migration_type='live-migration', + status='running', + ) self.common_policy_check(self.reader_authorized_contexts, self.reader_unauthorized_contexts, rule_name, self.controller.show,