Create filter_properties earlier in boot request

In order to prep for cellsv2 and its need to call the scheduler earlier
in the boot process this creates the filter_properties dict earlier.  A
request_spec object will need to be created and persisted before the
instance is written to the db and the request_spec depends on having
filter_properties.

In _create_instance() a check was removed that set instance_type to the
default flavor if instance_type was passed in as None.  There is no
place currently in the code that passes in None for instance_type,
except the test that I fixed, so this is safe to remove.  And since
instance_type is set on filter_properties earlier than this check if
instance_type was set to the default flavor at this point that would
mean that filter_properties and instance would have different values for
instance_type.  With the check removed if somehow a None instance_type
was passed in it would fail later in the method when it is accessed as a
dict.

Change-Id: I42afda9643e4c24c9a12ace8e88dd1a9c13aa121

nova/compute/api.py

@@ -74,6 +74,7 @@
 import nova.policy
 from nova import rpc
 from nova.scheduler import client as scheduler_client
+from nova.scheduler import utils as scheduler_utils
 from nova import servicegroup
 from nova import utils
 from nova.virt import hardware
@@ -901,16 +902,6 @@ def _validate_and_build_base_options(self, context, instance_type,
         # by the network quotas
         return base_options, max_network_count
 
-    def _build_filter_properties(self, context, scheduler_hints, forced_host,
-            forced_node, instance_type):
-        filter_properties = dict(scheduler_hints=scheduler_hints)
-        filter_properties['instance_type'] = instance_type
-        if forced_host:
-            filter_properties['force_hosts'] = [forced_host]
-        if forced_node:
-            filter_properties['force_nodes'] = [forced_node]
-        return filter_properties
-
     def _provision_instances(self, context, instance_type, min_count,
             max_count, base_options, boot_meta, security_groups,
             block_device_mapping, shutdown_terminate,
@@ -1018,12 +1009,13 @@ def _get_bdm_image_metadata(self, context, block_device_mapping,
         return {}
 
     @staticmethod
-    def _get_requested_instance_group(context, scheduler_hints,
+    def _get_requested_instance_group(context, filter_properties,
                                       check_quota):
-        if not scheduler_hints:
+        if (not filter_properties or
+                not filter_properties.get('scheduler_hints')):
             return
 
-        group_hint = scheduler_hints.get('group')
+        group_hint = filter_properties.get('scheduler_hints').get('group')
         if not group_hint:
             return
 
@@ -1038,13 +1030,11 @@ def _create_instance(self, context, instance_type,
                min_count, max_count,
                display_name, display_description,
                key_name, key_data, security_groups,
-               availability_zone, forced_host, forced_node, user_data,
-               metadata, injected_files, admin_password,
-               access_ip_v4, access_ip_v6,
+               availability_zone, user_data, metadata, injected_files,
+               admin_password, access_ip_v4, access_ip_v6,
                requested_networks, config_drive,
-               block_device_mapping, auto_disk_config,
-               reservation_id=None, scheduler_hints=None,
-               legacy_bdm=True, shutdown_terminate=False,
+               block_device_mapping, auto_disk_config, filter_properties,
+               reservation_id=None, legacy_bdm=True, shutdown_terminate=False,
                check_server_group_quota=False):
         """Verify all the input parameters regardless of the provisioning
         strategy being performed and schedule the instance(s) for
@@ -1058,8 +1048,6 @@ def _create_instance(self, context, instance_type,
         min_count = min_count or 1
         max_count = max_count or min_count
         block_device_mapping = block_device_mapping or []
-        if not instance_type:
-            instance_type = flavors.get_default_flavor()
 
         if image_href:
             image_id, boot_meta = self._get_image(context, image_href)
@@ -1102,17 +1090,13 @@ def _create_instance(self, context, instance_type,
                 block_device_mapping.root_bdm())
 
         instance_group = self._get_requested_instance_group(context,
-                                   scheduler_hints, check_server_group_quota)
+                                   filter_properties, check_server_group_quota)
 
         instances = self._provision_instances(context, instance_type,
                 min_count, max_count, base_options, boot_meta, security_groups,
                 block_device_mapping, shutdown_terminate,
                 instance_group, check_server_group_quota)
 
-        filter_properties = self._build_filter_properties(context,
-                scheduler_hints, forced_host,
-                forced_node, instance_type)
-
         for instance in instances:
             self._record_action_start(context, instance,
                                       instance_actions.CREATE)
@@ -1484,19 +1468,21 @@ def create(self, context, instance_type,
                 msg = _('The requested availability zone is not available')
                 raise exception.InvalidRequest(msg)
 
+        filter_properties = scheduler_utils.build_filter_properties(
+                scheduler_hints, forced_host, forced_node, instance_type)
+
         return self._create_instance(
                        context, instance_type,
                        image_href, kernel_id, ramdisk_id,
                        min_count, max_count,
                        display_name, display_description,
                        key_name, key_data, security_group,
-                       availability_zone, forced_host, forced_node,
-                       user_data, metadata,
+                       availability_zone, user_data, metadata,
                        injected_files, admin_password,
                        access_ip_v4, access_ip_v6,
                        requested_networks, config_drive,
                        block_device_mapping, auto_disk_config,
-                       scheduler_hints=scheduler_hints,
+                       filter_properties=filter_properties,
                        legacy_bdm=legacy_bdm,
                        shutdown_terminate=shutdown_terminate,
                        check_server_group_quota=check_server_group_quota)

nova/scheduler/utils.py

@@ -117,6 +117,20 @@ def set_vm_state_and_notify(context, instance_uuid, service, method, updates,
     notifier.error(context, event_type, payload)
 
 
+def build_filter_properties(scheduler_hints, forced_host,
+        forced_node, instance_type):
+    """Build the filter_properties dict from data in the boot request."""
+    filter_properties = dict(scheduler_hints=scheduler_hints)
+    filter_properties['instance_type'] = instance_type
+    # TODO(alaski): It doesn't seem necessary that these are conditionally
+    # added.  Let's just add empty lists if not forced_host/node.
+    if forced_host:
+        filter_properties['force_hosts'] = [forced_host]
+    if forced_node:
+        filter_properties['force_nodes'] = [forced_node]
+    return filter_properties
+
+
 def populate_filter_properties(filter_properties, host_state):
     """Add additional information to the filter properties after a node has
     been selected by the scheduling process.

nova/tests/unit/compute/test_compute.py

@@ -10974,9 +10974,11 @@ def test_force_host_pass(self):
                  "network:validate_networks": []}
         self.policy.set_rules(rules)
 
-        self.compute_api.create(self.context, None,
-                                image_href=uuids.host_instance,
-                                availability_zone='1', forced_host='1')
+        self.compute_api.create(self.context,
+                objects.Flavor(id=1, disabled=False, memory_mb=256, vcpus=1,
+                    root_gb=1, ephemeral_gb=1, swap=0),
+                image_href=uuids.host_instance, availability_zone='1',
+                forced_host='1')
 
 
 class DisabledInstanceTypesTestCase(BaseTestCase):

nova/tests/unit/scheduler/test_scheduler_utils.py

@@ -107,6 +107,30 @@ def test_set_vm_state_and_notify(self, mock_save, mock_add, mock_get):
                                                             event_type,
                                                             payload)
 
+    def test_build_filter_properties(self):
+        sched_hints = {'hint': ['over-there']}
+        forced_host = 'forced-host1'
+        forced_node = 'forced-node1'
+        instance_type = objects.Flavor()
+        filt_props = scheduler_utils.build_filter_properties(sched_hints,
+                forced_host, forced_node, instance_type)
+        self.assertEqual(sched_hints, filt_props['scheduler_hints'])
+        self.assertEqual([forced_host], filt_props['force_hosts'])
+        self.assertEqual([forced_node], filt_props['force_nodes'])
+        self.assertEqual(instance_type, filt_props['instance_type'])
+
+    def test_build_filter_properties_no_forced_host_no_force_node(self):
+        sched_hints = {'hint': ['over-there']}
+        forced_host = None
+        forced_node = None
+        instance_type = objects.Flavor()
+        filt_props = scheduler_utils.build_filter_properties(sched_hints,
+                forced_host, forced_node, instance_type)
+        self.assertEqual(sched_hints, filt_props['scheduler_hints'])
+        self.assertEqual(instance_type, filt_props['instance_type'])
+        self.assertNotIn('forced_host', filt_props)
+        self.assertNotIn('forced_node', filt_props)
+
     def _test_populate_filter_props(self, host_state_obj=True,
                                     with_retry=True,
                                     force_hosts=None,