← Back to team overview

openstack team mailing list archive

[RFC] Add more host checks to the compute filter

 

Hi Daniel,

Attached is a patch that implements filtering on (architecture,
hypervisor_type, vm_mode) tuple as was discussed in this previous patch

https://review.openstack.org/#/c/9110/

CC'ing Chuck since he is the author of the ArchFilter patch.

Feedback appreciated before sending this off to gerrit.

Regards,
Jim
>From bc96fdf618a2b9426f4c5db59fc087f849ac9873 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@xxxxxxxx>
Date: Mon, 25 Jun 2012 15:54:43 -0600
Subject: [PATCH] Add more host checks to the compute filter

As discussed in a previous version of this patch [1], this change adds
checks in the ComputeFilter to verify hosts can support the
(architecture, hypervisor_type, vm_mode) tuple specified in the instance
properties.

Adding these checks to the compute filter seems consistent with its
definition [2]:

"ComputeFilter - checks that the capabilities provided by the compute service
satisfy the extra specifications, associated with the instance type."

[1] https://review.openstack.org/#/c/9110/
[2] https://github.com/openstack/nova/blob/master/doc/source/devref/filter_scheduler.rst

Change-Id: I1fcd7f9c706184701ca02f7d1672541d26c07f31
---
 nova/compute/api.py                                |    4 +-
 .../versions/108_instance_hypervisor_type.py       |   46 ++++++
 nova/db/sqlalchemy/models.py                       |    1 +
 nova/scheduler/filters/arch_filter.py              |   44 ------
 nova/scheduler/filters/compute_filter.py           |   56 ++++++-
 nova/tests/scheduler/test_host_filters.py          |  160 +++++++++++++-------
 6 files changed, 211 insertions(+), 100 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 1e3ebf1..008bdd6 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -323,7 +323,9 @@ class API(base.Base):
             return value
 
         options_from_image = {'os_type': prop('os_type'),
-                              'vm_mode': prop('vm_mode')}
+                              'architecture': prop('architecture'),
+                              'vm_mode': prop('vm_mode'),
+                              'hypervisor_type': prop('hypervisor_type')}
 
         # If instance doesn't have auto_disk_config overridden by request, use
         # whatever the image indicates
diff --git a/nova/db/sqlalchemy/migrate_repo/versions/108_instance_hypervisor_type.py b/nova/db/sqlalchemy/migrate_repo/versions/108_instance_hypervisor_type.py
new file mode 100644
index 0000000..f68a6a4
--- /dev/null
+++ b/nova/db/sqlalchemy/migrate_repo/versions/108_instance_hypervisor_type.py
@@ -0,0 +1,46 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 OpenStack LLC.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import Column, Integer, MetaData, String, Table
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    # add column:
+    instances = Table('instances', meta,
+                      Column('id', Integer(), primary_key=True, nullable=False)
+                      )
+    hypervisor_type = Column('hypervisor_type',
+                             String(length=255, convert_unicode=False,
+                                    assert_unicode=None, unicode_error=None,
+                                    _warn_on_bytestring=False),
+                             nullable=True)
+
+    instances.create_column(hypervisor_type)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    # drop column:
+    instances = Table('instances', meta,
+                      Column('id', Integer(), primary_key=True, nullable=False)
+                      )
+
+    instances.drop_column('hypervisor_type')
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 3359891..30f23e6 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -253,6 +253,7 @@ class Instance(BASE, NovaBase):
 
     os_type = Column(String(255))
     architecture = Column(String(255))
+    hypervisor_type = Column(String(255))
     vm_mode = Column(String(255))
     uuid = Column(String(36))
 
diff --git a/nova/scheduler/filters/arch_filter.py b/nova/scheduler/filters/arch_filter.py
deleted file mode 100644
index 1f11d07..0000000
--- a/nova/scheduler/filters/arch_filter.py
+++ /dev/null
@@ -1,44 +0,0 @@
-# Copyright (c) 2011-2012 OpenStack, LLC
-# Copyright (c) 2012 Canonical Ltd
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-
-from nova import log as logging
-from nova.scheduler import filters
-from nova import utils
-
-
-LOG = logging.getLogger(__name__)
-
-
-class ArchFilter(filters.BaseHostFilter):
-    """Filter out hosts that can not support the guest architecture.
-    Note: This is supported for libvirt only now.
-    """
-
-    def host_passes(self, host_state, filter_properties):
-        spec = filter_properties.get('request_spec', {})
-        props = spec.get('instance_properties', {})
-
-        cpu_info = host_state.capabilities.get('cpu_info')
-        permitted_instances = cpu_info.get('permitted_instance_types', None)
-
-        instance_arch = utils.sys_platform_translate(props.get('architecture'))
-
-        if permitted_instances and instance_arch in permitted_instances:
-            return True
-
-        LOG.warn(_('%(host_state)s fails permitted_instance_types'), locals())
-        return False
diff --git a/nova/scheduler/filters/compute_filter.py b/nova/scheduler/filters/compute_filter.py
index 5409d3d..bf67b96 100644
--- a/nova/scheduler/filters/compute_filter.py
+++ b/nova/scheduler/filters/compute_filter.py
@@ -22,7 +22,8 @@ LOG = logging.getLogger(__name__)
 
 
 class ComputeFilter(filters.BaseHostFilter):
-    """HostFilter hard-coded to work with InstanceType records."""
+    """HostFilter hard-coded to work with InstanceType and
+    InstanceProperties records."""
 
     def _satisfies_extra_specs(self, capabilities, instance_type):
         """Check that the capabilities provided by the compute service
@@ -38,8 +39,49 @@ class ComputeFilter(filters.BaseHostFilter):
                 return False
         return True
 
+    def _satisfies_arch(self, capabilities, instance_props):
+        """Check that the compute service satisfies the requested
+        instance architecture."""
+        inst_arch = instance_props.get('architecture')
+        if not inst_arch:
+            return True
+
+        inst_arch = utils.sys_platform_translate(inst_arch)
+        cpu_info = capabilities.get('cpu_info')
+        if not cpu_info:
+            return False
+
+        permitted_instances = cpu_info.get('permitted_instance_types', None)
+        if permitted_instances and inst_arch in permitted_instances:
+            return True
+        return False
+
+    def _satisfies_hypervisor(self, capabilities, instance_props):
+        """Check that the compute service satisfies the requested
+        instance hypervisor."""
+        inst_h_type = instance_props.get('hypervisor_type', None)
+        if not inst_h_type:
+            return True
+
+        if inst_h_type == capabilities.get('hypervisor_type', None):
+            return True
+        return False
+
+    def _satisfies_vm_mode(self, capabilities, instance_props):
+        """Check that the compute service satisfies the requested
+        instance vm_mode."""
+        inst_vm_mode = instance_props.get('vm_mode', None)
+        if not inst_vm_mode:
+            return True
+
+        if capabilities.get(inst_vm_mode, None):
+            return True
+        return False
+
     def host_passes(self, host_state, filter_properties):
         """Return a list of hosts that can create instance_type."""
+        spec = filter_properties.get('request_spec', {})
+        instance_props = spec.get('instance_properties', {})
         instance_type = filter_properties.get('instance_type')
         if host_state.topic != 'compute' or not instance_type:
             return True
@@ -57,4 +99,16 @@ class ComputeFilter(filters.BaseHostFilter):
             LOG.debug(_("%(host_state)s fails instance_type extra_specs "
                     "requirements"), locals())
             return False
+        if not self._satisfies_arch(capabilities, instance_props):
+            LOG.debug(_("%(host_state)s fails instance_properties "
+                    "architecture requirement"), locals())
+            return False
+        if not self._satisfies_hypervisor(capabilities, instance_props):
+            LOG.debug(_("%(host_state)s fails instance_properties "
+                    "hypervisor requirement"), locals())
+            return False
+        if not self._satisfies_vm_mode(capabilities, instance_props):
+            LOG.debug(_("%(host_state)s fails instance_properties "
+                    "vm_mode requirement"), locals())
+            return False
         return True
diff --git a/nova/tests/scheduler/test_host_filters.py b/nova/tests/scheduler/test_host_filters.py
index c6fabc1..9dd3410 100644
--- a/nova/tests/scheduler/test_host_filters.py
+++ b/nova/tests/scheduler/test_host_filters.py
@@ -387,6 +387,112 @@ class HostFiltersTestCase(test.TestCase):
 
         self.assertFalse(filt_cls.host_passes(host, filter_properties))
 
+    def test_compute_filter_passes_same_arch(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'architecture': 'x86_64'}}
+        permitted_instances = ['x86_64']
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        capabilities = {'enabled': True,
+                            'cpu_info': {
+                                'permitted_instance_types': permitted_instances
+                            }
+                        }
+        service = {'disabled': False}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertTrue(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_fails_different_arch(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'architecture': 'x86_64'}}
+        permitted_instances = ['arm']
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        capabilities = {'enabled': True,
+                            'cpu_info': {
+                                'permitted_instance_types': permitted_instances
+                            }
+                        }
+        service = {'disabled': False}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertFalse(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_fails_without_requested_arch(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'architecture': 'x86_64'}}
+        permitted_instances = []
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        capabilities = {'enabled': True,
+                            'cpu_info': {
+                                'permitted_instance_types': permitted_instances
+                            }
+                        }
+        service = {'disabled': False}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertFalse(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_passes_same_hypervisor(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'hypervisor_type': 'KVM'}}
+        capabilities = {'enabled': True, 'hypervisor_type': 'KVM'}
+        service = {'disabled': False}
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertTrue(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_fails_different_hypervisor(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'hypervisor_type': 'KVM'}}
+        capabilities = {'enabled': True, 'hypervisor_type': 'Xen'}
+        service = {'disabled': False}
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertFalse(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_passes_same_vmmode(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'vm_mode': 'pv'}}
+        capabilities = {'enabled': True, 'pv': True, 'hvm': True}
+        service = {'disabled': False}
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertTrue(filt_cls.host_passes(host, filter_properties))
+
+    def test_compute_filter_fails_different_vmmode(self):
+        self._stub_service_is_up(True)
+        filt_cls = self.class_map['ComputeFilter']()
+        req_spec = {'instance_properties': {'vm_mode': 'pv'}}
+        capabilities = {'enabled': True}
+        service = {'disabled': False}
+        filter_properties = {'instance_type': {'memory_mb': 1024},
+                             'request_spec': req_spec}
+        host = fakes.FakeHostState('host1', 'compute',
+                {'free_ram_mb': 1024, 'capabilities': capabilities,
+                 'service': service})
+        self.assertFalse(filt_cls.host_passes(host, filter_properties))
+
     def test_isolated_hosts_fails_isolated_on_non_isolated(self):
         self.flags(isolated_images=['isolated'], isolated_hosts=['isolated'])
         filt_cls = self.class_map['IsolatedHostsFilter']()
@@ -831,57 +937,3 @@ class HostFiltersTestCase(test.TestCase):
         request = self._make_zone_request('bad')
         host = fakes.FakeHostState('host1', 'compute', {'service': service})
         self.assertFalse(filt_cls.host_passes(host, request))
-
-    def test_arch_filter_same(self):
-        permitted_instances = ['x86_64']
-        filt_cls = self.class_map['ArchFilter']()
-        filter_properties = {
-            'request_spec': {
-                'instance_properties': {'architecture': 'x86_64'}
-            }
-        }
-        capabilities = {'enabled': True,
-                            'cpu_info': {
-                                'permitted_instance_types': permitted_instances
-                            }
-                        }
-        service = {'disabled': False}
-        host = fakes.FakeHostState('host1', 'compute',
-            {'capabilities': capabilities, 'service': service})
-        self.assertTrue(filt_cls.host_passes(host, filter_properties))
-
-    def test_arch_filter_different(self):
-        permitted_instances = ['arm']
-        filt_cls = self.class_map['ArchFilter']()
-        filter_properties = {
-            'request_spec': {
-                    'instance_properties': {'architecture': 'x86_64'}
-                }
-            }
-        capabilities = {'enabled': True,
-                            'cpu_info': {
-                                'permitted_instance_types': permitted_instances
-                            }
-                        }
-        service = {'disabled': False}
-        host = fakes.FakeHostState('host1', 'compute',
-            {'capabilities': capabilities, 'service': service})
-        self.assertFalse(filt_cls.host_passes(host, filter_properties))
-
-    def test_arch_filter_without_permitted_instances(self):
-        permitted_instances = []
-        filt_cls = self.class_map['ArchFilter']()
-        filter_properties = {
-             'request_spec': {
-                'instance_properties': {'architecture': 'x86_64'}
-            }
-        }
-        capabilities = {'enabled': True,
-                            'cpu_info': {
-                                'permitted_instance_types': permitted_instances
-                            }
-                        }
-        service = {'disabled': False}
-        host = fakes.FakeHostState('host1', 'compute',
-            {'capabilities': capabilities, 'service': service})
-        self.assertFalse(filt_cls.host_passes(host, filter_properties))
-- 
1.7.10.4


Follow ups