objects: Add MigrationTypeField

stephenfin · stephenfin · commit f203da383871 · 2020-05-08T14:45:54.000+01:00
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 &lt;sfinucan@redhat.com&gt;
diff --git a/nova/api/openstack/compute/migrations.py b/nova/api/openstack/compute/migrations.py
@@ -23,6 +23,7 @@
 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 @@ def _output(self, req, migrations_obj, add_link=False,
             # 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
@@ -132,7 +132,7 @@ def show(self, req, server_id, id):
                     " 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
@@ -5036,12 +5036,10 @@ def evacuate(self, context, instance, host, on_shared_storage,
 
         # 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
@@ -194,9 +194,10 @@ def _test_pci(self):
         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 @@ def _test_live_migration_page_size(self):
         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
@@ -731,7 +731,7 @@ def _destroy_evacuated_instances(self, context, node_cache):
             # 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 @@ def post_live_migration_at_destination(self, context, instance,
         # 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
@@ -39,6 +39,7 @@
 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 @@ def rebuild_claim(self, context, instance, nodename, allocations,
                       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 @@ def live_migration_claim(self, context, instance, nodename, migration,
         # 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 @@ def _move_claim(self, context, instance, new_instance_type, nodename,
         # 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 @@ def _claim_existing_migration(self, migration, nodename):
         # 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 @@ def _verify_resources(self, resources):
 
     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
@@ -456,7 +456,7 @@ def _set_vm_state(context, instance, ex, vm_state=None,
         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
@@ -21,7 +21,6 @@
 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
@@ -3415,7 +3415,7 @@ def _update_port_binding_for_instance(
             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 @@ def _update_port_binding_for_instance(
             # 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
@@ -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
@@ -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 @@ def instance(self, instance):
     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
@@ -4382,8 +4382,8 @@ def test_update_port_bindings_for_instance_with_pci(self,
                              '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 @@ def test_update_port_bindings_for_instance_with_pci_fail(self,
                             {'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 @@ def test_update_port_bindings_for_instance_with_live_migration(
                  {'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 @@ def test_update_port_bindings_for_instance_with_resource_req_live_mig(
     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 @@ def test_get_pci_mapping_for_migration(self):
     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 @@ def test_migrate_instance_start_activate(self, get_client_mock):
         # 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 @@ def test_migrate_instance_start_already_active(self, get_client_mock):
         # 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 @@ def test_migrate_instance_start_no_bindings(self, get_client_mock):
         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 @@ def test_migrate_instance_start_get_error(self, get_client_mock):
         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
@@ -1103,7 +1103,7 @@ def obj_name(cls):
     '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
@@ -17,6 +17,7 @@
 
 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 @@ def test_list_server_migrations_policy(self, mock_get):
     @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,
