From 11120aa5bbdff7d61ff9418cf6e5526e5bcba180 Mon Sep 17 00:00:00 2001 From: Chris Yeoh Date: Mon, 12 Nov 2012 14:22:26 +1030 Subject: Removes fixed_ip_get_network Removes fixed_ip_get_network since it doesn't actually work anymore (at some point in the past the network must have been stored along with the network_uuid but this is no longer the case) and nothing else uses it anyway. Also cleans up fake db test model to remove the fake_fixed_ip_get_network and underlying network reference which makes it closer in sync to what is actually implemented now Change-Id: I01dc0f88a8bc7cfef01c4e7f328bcdba75a8e0bc --- nova/db/api.py | 5 ----- nova/db/sqlalchemy/api.py | 6 ------ nova/tests/db/fakes.py | 11 ----------- 3 files changed, 22 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index 8349c7c25..2ae9522bf 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -480,11 +480,6 @@ def fixed_ips_by_virtual_interface(context, vif_id): return IMPL.fixed_ips_by_virtual_interface(context, vif_id) -def fixed_ip_get_network(context, address): - """Get a network for a fixed ip by address.""" - return IMPL.fixed_ip_get_network(context, address) - - def fixed_ip_update(context, address, values): """Create a fixed ip from the values dictionary.""" return IMPL.fixed_ip_update(context, address, values) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index ba5a7150e..c85d3e9c7 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1187,12 +1187,6 @@ def fixed_ips_by_virtual_interface(context, vif_id): return result -@require_admin_context -def fixed_ip_get_network(context, address): - fixed_ip_ref = fixed_ip_get_by_address(context, address) - return fixed_ip_ref.network - - @require_context def fixed_ip_update(context, address, values): session = get_session() diff --git a/nova/tests/db/fakes.py b/nova/tests/db/fakes.py index a78fd2e12..653edf58a 100644 --- a/nova/tests/db/fakes.py +++ b/nova/tests/db/fakes.py @@ -68,7 +68,6 @@ def stub_out_db_network_api(stubs): fixed_ip_fields = {'id': 0, 'network_id': 0, - 'network': FakeModel(network_fields), 'address': '192.168.0.100', 'instance': False, 'instance_id': 0, @@ -208,15 +207,6 @@ def stub_out_db_network_api(stubs): if ips: return FakeModel(ips[0]) - def fake_fixed_ip_get_network(context, address): - ips = filter(lambda i: i['address'] == address, - fixed_ips) - if ips: - nets = filter(lambda n: n['id'] == ips[0]['network_id'], - networks) - if nets: - return FakeModel(nets[0]) - def fake_fixed_ip_update(context, address, values): ips = filter(lambda i: i['address'] == address, fixed_ips) @@ -318,7 +308,6 @@ def stub_out_db_network_api(stubs): fake_fixed_ip_disassociate_all_by_timeout, fake_fixed_ip_get_by_instance, fake_fixed_ip_get_by_address, - fake_fixed_ip_get_network, fake_fixed_ip_update, fake_instance_type_get, fake_virtual_interface_create, -- cgit From 6443cfeecb9f7f9ac4ab84de27e2e0c4decf71d6 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 12 Nov 2012 15:36:32 -0500 Subject: Update vol mount smoketest to wait for volume. Updates the test_003_can_mount_volume smoke test so that it waits for the volume to show up in /proc/partitions for up to 60 seconds before proceeding. This eliminates the need for a hard coded sleep value which can cause intermittent test failures. Change-Id: Ibdc56a00e141211e40068c22fc268efeba6affff --- smoketests/test_sysadmin.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/smoketests/test_sysadmin.py b/smoketests/test_sysadmin.py index d6491c9d4..b05f0ac4b 100644 --- a/smoketests/test_sysadmin.py +++ b/smoketests/test_sysadmin.py @@ -249,12 +249,24 @@ class VolumeTests(base.UserSmokeTestCase): self.assertTrue(volume.status.startswith('in-use')) - # Give instance time to recognize volume. - time.sleep(5) - def test_003_can_mount_volume(self): ip = self.data['instance'].private_ip_address conn = self.connect_ssh(ip, TEST_KEY) + + # NOTE(dprince): give some time for volume to show up in partitions + stdin, stdout, stderr = conn.exec_command( + 'COUNT="0";' + 'until cat /proc/partitions | grep "%s\\$"; do ' + '[ "$COUNT" -eq "60" ] && exit 1;' + 'COUNT=$(( $COUNT + 1 ));' + 'sleep 1; ' + 'done' + % self.device.rpartition('/')[2]) + out = stdout.read() + if not out.strip().endswith(self.device.rpartition('/')[2]): + self.fail('Timeout waiting for volume partition in instance. %s %s' + % (out, stderr.read())) + # NOTE(vish): this will create a dev for images that don't have # udev rules stdin, stdout, stderr = conn.exec_command( -- cgit From c4de05e7c755870cb8447cc3005c67b59fe9f6b2 Mon Sep 17 00:00:00 2001 From: Joe Gordon Date: Thu, 8 Nov 2012 22:54:04 +0000 Subject: Remove volume.driver and volume.iscsi Remaining flags moved to nova.virt.libvirt.volume Part of bp delete-nova-volume Change-Id: I1b96c773f0b5df73b04215a4afb350a5583b0101 --- nova/tests/api/ec2/test_cloud.py | 4 +- nova/tests/fake_flags.py | 2 - nova/tests/test_libvirt.py | 154 +++++-- nova/virt/libvirt/volume.py | 17 +- nova/volume/driver.py | 954 --------------------------------------- nova/volume/iscsi.py | 235 ---------- 6 files changed, 138 insertions(+), 1228 deletions(-) delete mode 100644 nova/volume/driver.py delete mode 100644 nova/volume/iscsi.py diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index b378ef98d..23b7cc4dc 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -94,10 +94,8 @@ def get_instances_with_cached_ips(orig_func, *args, **kwargs): class CloudTestCase(test.TestCase): def setUp(self): super(CloudTestCase, self).setUp() - vol_tmpdir = tempfile.mkdtemp() self.flags(compute_driver='nova.virt.fake.FakeDriver', - volume_api_class='nova.tests.fake_volume.API', - volumes_dir=vol_tmpdir) + volume_api_class='nova.tests.fake_volume.API') def fake_show(meh, context, id): return {'id': id, diff --git a/nova/tests/fake_flags.py b/nova/tests/fake_flags.py index f8661e434..6ded0c5be 100644 --- a/nova/tests/fake_flags.py +++ b/nova/tests/fake_flags.py @@ -23,7 +23,6 @@ CONF = config.CONF CONF.import_opt('scheduler_driver', 'nova.scheduler.manager') CONF.import_opt('fake_network', 'nova.network.manager') -CONF.import_opt('iscsi_num_targets', 'nova.volume.driver') CONF.import_opt('network_size', 'nova.network.manager') CONF.import_opt('num_networks', 'nova.network.manager') CONF.import_opt('policy_file', 'nova.policy') @@ -35,7 +34,6 @@ def set_defaults(conf): conf.set_default('fake_network', True) conf.set_default('fake_rabbit', True) conf.set_default('flat_network_bridge', 'br100') - conf.set_default('iscsi_num_targets', 8) conf.set_default('network_size', 8) conf.set_default('num_networks', 2) conf.set_default('vlan_interface', 'eth0') diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 36c2bba7c..0005b8a88 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -60,7 +60,6 @@ from nova.virt.libvirt import snapshots from nova.virt.libvirt import utils as libvirt_utils from nova.virt.libvirt import volume from nova.virt.libvirt import volume_nfs -from nova.volume import driver as volume_driver try: @@ -173,10 +172,96 @@ class LibvirtVolumeTestCase(test.TestCase): self.assertEqual(tree.get('type'), 'block') self.assertEqual(tree.find('./serial').text, 'fake_serial') + def _get_iscsi_properties(self, volume): + """Gets iscsi configuration + + We ideally get saved information in the volume entity, but fall back + to discovery if need be. Discovery may be completely removed in future + The properties are: + + :target_discovered: boolean indicating whether discovery was used + + :target_iqn: the IQN of the iSCSI target + + :target_portal: the portal of the iSCSI target + + :target_lun: the lun of the iSCSI target + + :volume_id: the id of the volume (currently used by xen) + + :auth_method:, :auth_username:, :auth_password: + + the authentication details. Right now, either auth_method is not + present meaning no authentication, or auth_method == `CHAP` + meaning use CHAP with the specified credentials. + """ + + properties = {} + + location = volume['provider_location'] + + if location: + # provider_location is the same format as iSCSI discovery output + properties['target_discovered'] = False + else: + location = "stub" + + if not location: + raise exception.InvalidVolume(_("Could not find iSCSI export " + " for volume %s") % + (volume['name'])) + + LOG.debug(_("ISCSI Discovery: Found %s") % (location)) + properties['target_discovered'] = True + + results = location.split(" ") + properties['target_portal'] = results[0].split(",")[0] + properties['target_iqn'] = results[1] + try: + properties['target_lun'] = int(results[2]) + except (IndexError, ValueError): + properties['target_lun'] = 1 + + properties['volume_id'] = volume['id'] + + auth = volume['provider_auth'] + if auth: + (auth_method, auth_username, auth_secret) = auth.split() + + properties['auth_method'] = auth_method + properties['auth_username'] = auth_username + properties['auth_password'] = auth_secret + + return properties + + def iscsi_connection(self, volume): + """Initializes the connection and returns connection info. + + The iscsi driver returns a driver_volume_type of 'iscsi'. + The format of the driver data is defined in _get_iscsi_properties. + Example return value:: + + { + 'driver_volume_type': 'iscsi' + 'data': { + 'target_discovered': True, + 'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001', + 'target_portal': '127.0.0.0.1:3260', + 'volume_id': 1, + } + } + + """ + + iscsi_properties = self._get_iscsi_properties(volume) + return { + 'driver_volume_type': 'iscsi', + 'data': iscsi_properties + } + def test_libvirt_iscsi_driver(self): # NOTE(vish) exists is to make driver assume connecting worked self.stubs.Set(os.path, 'exists', lambda x: True) - vol_driver = volume_driver.ISCSIDriver() libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn) location = '10.0.2.15:3260' name = 'volume-00000001' @@ -185,7 +270,7 @@ class LibvirtVolumeTestCase(test.TestCase): 'name': name, 'provider_auth': None, 'provider_location': '%s,fake %s' % (location, iqn)} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.iscsi_connection(vol) mount_device = "vde" conf = libvirt_driver.connect_volume(connection_info, mount_device) tree = conf.format_dom() @@ -193,7 +278,6 @@ class LibvirtVolumeTestCase(test.TestCase): self.assertEqual(tree.get('type'), 'block') self.assertEqual(tree.find('./source').get('dev'), dev_str) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) expected_commands = [('iscsiadm', '-m', 'node', '-T', iqn, '-p', location), ('iscsiadm', '-m', 'node', '-T', iqn, @@ -213,7 +297,6 @@ class LibvirtVolumeTestCase(test.TestCase): def test_libvirt_iscsi_driver_still_in_use(self): # NOTE(vish) exists is to make driver assume connecting worked self.stubs.Set(os.path, 'exists', lambda x: True) - vol_driver = volume_driver.ISCSIDriver() libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn) location = '10.0.2.15:3260' name = 'volume-00000001' @@ -224,7 +307,7 @@ class LibvirtVolumeTestCase(test.TestCase): 'name': name, 'provider_auth': None, 'provider_location': '%s,fake %s' % (location, iqn)} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.iscsi_connection(vol) mount_device = "vde" conf = libvirt_driver.connect_volume(connection_info, mount_device) tree = conf.format_dom() @@ -232,7 +315,6 @@ class LibvirtVolumeTestCase(test.TestCase): self.assertEqual(tree.get('type'), 'block') self.assertEqual(tree.find('./source').get('dev'), dev_str) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) expected_commands = [('iscsiadm', '-m', 'node', '-T', iqn, '-p', location), ('iscsiadm', '-m', 'node', '-T', iqn, @@ -242,12 +324,19 @@ class LibvirtVolumeTestCase(test.TestCase): '-n', 'node.startup', '-v', 'automatic')] self.assertEqual(self.executes, expected_commands) + def sheepdog_connection(self, volume): + return { + 'driver_volume_type': 'sheepdog', + 'data': { + 'name': volume['name'] + } + } + def test_libvirt_sheepdog_driver(self): - vol_driver = volume_driver.SheepdogDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.sheepdog_connection(vol) mount_device = "vde" conf = libvirt_driver.connect_volume(connection_info, mount_device) tree = conf.format_dom() @@ -255,31 +344,39 @@ class LibvirtVolumeTestCase(test.TestCase): self.assertEqual(tree.find('./source').get('protocol'), 'sheepdog') self.assertEqual(tree.find('./source').get('name'), name) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) + + def rbd_connection(self, volume): + return { + 'driver_volume_type': 'rbd', + 'data': { + 'name': '%s/%s' % ('rbd', volume['name']), + 'auth_enabled': CONF.rbd_secret_uuid is not None, + 'auth_username': CONF.rbd_user, + 'secret_type': 'ceph', + 'secret_uuid': CONF.rbd_secret_uuid, + } + } def test_libvirt_rbd_driver(self): - vol_driver = volume_driver.RBDDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.rbd_connection(vol) mount_device = "vde" conf = libvirt_driver.connect_volume(connection_info, mount_device) tree = conf.format_dom() self.assertEqual(tree.get('type'), 'network') self.assertEqual(tree.find('./source').get('protocol'), 'rbd') - rbd_name = '%s/%s' % (CONF.rbd_pool, name) + rbd_name = '%s/%s' % ('rbd', name) self.assertEqual(tree.find('./source').get('name'), rbd_name) self.assertEqual(tree.find('./source/auth'), None) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_rbd_driver_auth_enabled(self): - vol_driver = volume_driver.RBDDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.rbd_connection(vol) uuid = '875a8070-d0b9-4949-8b31-104d125c9a64' user = 'foo' secret_type = 'ceph' @@ -293,20 +390,18 @@ class LibvirtVolumeTestCase(test.TestCase): tree = conf.format_dom() self.assertEqual(tree.get('type'), 'network') self.assertEqual(tree.find('./source').get('protocol'), 'rbd') - rbd_name = '%s/%s' % (CONF.rbd_pool, name) + rbd_name = '%s/%s' % ('rbd', name) self.assertEqual(tree.find('./source').get('name'), rbd_name) self.assertEqual(tree.find('./auth').get('username'), user) self.assertEqual(tree.find('./auth/secret').get('type'), secret_type) self.assertEqual(tree.find('./auth/secret').get('uuid'), uuid) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_rbd_driver_auth_enabled_flags_override(self): - vol_driver = volume_driver.RBDDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.rbd_connection(vol) uuid = '875a8070-d0b9-4949-8b31-104d125c9a64' user = 'foo' secret_type = 'ceph' @@ -325,20 +420,18 @@ class LibvirtVolumeTestCase(test.TestCase): tree = conf.format_dom() self.assertEqual(tree.get('type'), 'network') self.assertEqual(tree.find('./source').get('protocol'), 'rbd') - rbd_name = '%s/%s' % (CONF.rbd_pool, name) + rbd_name = '%s/%s' % ('rbd', name) self.assertEqual(tree.find('./source').get('name'), rbd_name) self.assertEqual(tree.find('./auth').get('username'), flags_user) self.assertEqual(tree.find('./auth/secret').get('type'), secret_type) self.assertEqual(tree.find('./auth/secret').get('uuid'), flags_uuid) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_rbd_driver_auth_disabled(self): - vol_driver = volume_driver.RBDDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.rbd_connection(vol) uuid = '875a8070-d0b9-4949-8b31-104d125c9a64' user = 'foo' secret_type = 'ceph' @@ -352,18 +445,16 @@ class LibvirtVolumeTestCase(test.TestCase): tree = conf.format_dom() self.assertEqual(tree.get('type'), 'network') self.assertEqual(tree.find('./source').get('protocol'), 'rbd') - rbd_name = '%s/%s' % (CONF.rbd_pool, name) + rbd_name = '%s/%s' % ('rbd', name) self.assertEqual(tree.find('./source').get('name'), rbd_name) self.assertEqual(tree.find('./auth'), None) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_rbd_driver_auth_disabled_flags_override(self): - vol_driver = volume_driver.RBDDriver() libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn) name = 'volume-00000001' vol = {'id': 1, 'name': name} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.rbd_connection(vol) uuid = '875a8070-d0b9-4949-8b31-104d125c9a64' user = 'foo' secret_type = 'ceph' @@ -384,17 +475,15 @@ class LibvirtVolumeTestCase(test.TestCase): tree = conf.format_dom() self.assertEqual(tree.get('type'), 'network') self.assertEqual(tree.find('./source').get('protocol'), 'rbd') - rbd_name = '%s/%s' % (CONF.rbd_pool, name) + rbd_name = '%s/%s' % ('rbd', name) self.assertEqual(tree.find('./source').get('name'), rbd_name) self.assertEqual(tree.find('./auth').get('username'), flags_user) self.assertEqual(tree.find('./auth/secret').get('type'), secret_type) self.assertEqual(tree.find('./auth/secret').get('uuid'), flags_uuid) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_lxc_volume(self): self.stubs.Set(os.path, 'exists', lambda x: True) - vol_driver = volume_driver.ISCSIDriver() libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn) location = '10.0.2.15:3260' name = 'volume-00000001' @@ -403,7 +492,7 @@ class LibvirtVolumeTestCase(test.TestCase): 'name': name, 'provider_auth': None, 'provider_location': '%s,fake %s' % (location, iqn)} - connection_info = vol_driver.initialize_connection(vol, self.connr) + connection_info = self.iscsi_connection(vol) mount_device = "vde" conf = libvirt_driver.connect_volume(connection_info, mount_device) tree = conf.format_dom() @@ -411,7 +500,6 @@ class LibvirtVolumeTestCase(test.TestCase): self.assertEqual(tree.get('type'), 'block') self.assertEqual(tree.find('./source').get('dev'), dev_str) libvirt_driver.disconnect_volume(connection_info, mount_device) - connection_info = vol_driver.terminate_connection(vol, self.connr) def test_libvirt_nfs_driver(self): # NOTE(vish) exists is to make driver assume connecting worked diff --git a/nova/virt/libvirt/volume.py b/nova/virt/libvirt/volume.py index 03c335fa0..66ffc2efa 100644 --- a/nova/virt/libvirt/volume.py +++ b/nova/virt/libvirt/volume.py @@ -23,6 +23,7 @@ import time from nova import config from nova import exception from nova import flags +from nova.openstack.common import cfg from nova.openstack.common import lockutils from nova.openstack.common import log as logging from nova import utils @@ -30,8 +31,22 @@ from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import utils as virtutils LOG = logging.getLogger(__name__) + +volume_opts = [ + cfg.IntOpt('num_iscsi_scan_tries', + default=3, + help='number of times to rescan iSCSI target to find volume'), + cfg.StrOpt('rbd_user', + default=None, + help='the RADOS client name for accessing rbd volumes'), + cfg.StrOpt('rbd_secret_uuid', + default=None, + help='the libvirt uuid of the secret for the rbd_user' + 'volumes') + ] + CONF = config.CONF -CONF.import_opt('num_iscsi_scan_tries', 'nova.volume.driver') +CONF.register_opts(volume_opts) class LibvirtVolumeDriver(object): diff --git a/nova/volume/driver.py b/nova/volume/driver.py deleted file mode 100644 index 44e688161..000000000 --- a/nova/volume/driver.py +++ /dev/null @@ -1,954 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright 2010 United States Government as represented by the -# Administrator of the National Aeronautics and Space Administration. -# 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. -""" -Drivers for volumes. - -""" - -import os -import tempfile -import time -import urllib - -from nova import config -from nova import exception -from nova import flags -from nova.openstack.common import cfg -from nova.openstack.common import log as logging -from nova import utils -from nova.volume import iscsi - - -LOG = logging.getLogger(__name__) - -volume_opts = [ - cfg.StrOpt('volume_group', - default='nova-volumes', - help='Name for the VG that will contain exported volumes'), - cfg.IntOpt('num_shell_tries', - default=3, - help='number of times to attempt to run flakey shell commands'), - cfg.IntOpt('num_iscsi_scan_tries', - default=3, - help='number of times to rescan iSCSI target to find volume'), - cfg.IntOpt('iscsi_num_targets', - default=100, - help='Number of iscsi target ids per host'), - cfg.StrOpt('iscsi_target_prefix', - default='iqn.2010-10.org.openstack:', - help='prefix for iscsi volumes'), - cfg.StrOpt('iscsi_ip_address', - default='$my_ip', - help='use this ip for iscsi'), - cfg.IntOpt('iscsi_port', - default=3260, - help='The port that the iSCSI daemon is listening on'), - cfg.StrOpt('rbd_pool', - default='rbd', - help='the RADOS pool in which rbd volumes are stored'), - cfg.StrOpt('rbd_user', - default=None, - help='the RADOS client name for accessing rbd volumes'), - cfg.StrOpt('rbd_secret_uuid', - default=None, - help='the libvirt uuid of the secret for the rbd_user' - 'volumes'), - cfg.StrOpt('volume_tmp_dir', - default=None, - help='where to store temporary image files if the volume ' - 'driver does not write them directly to the volume'), - ] - -CONF = config.CONF -CONF.register_opts(volume_opts) - - -class VolumeDriver(object): - """Executes commands relating to Volumes.""" - def __init__(self, execute=utils.execute, *args, **kwargs): - # NOTE(vish): db is set by Manager - self.db = None - self.set_execute(execute) - - def set_execute(self, execute): - self._execute = execute - - def _try_execute(self, *command, **kwargs): - # NOTE(vish): Volume commands can partially fail due to timing, but - # running them a second time on failure will usually - # recover nicely. - tries = 0 - while True: - try: - self._execute(*command, **kwargs) - return True - except exception.ProcessExecutionError: - tries = tries + 1 - if tries >= CONF.num_shell_tries: - raise - LOG.exception(_("Recovering from a failed execute. " - "Try number %s"), tries) - time.sleep(tries ** 2) - - def check_for_setup_error(self): - """Returns an error if prerequisites aren't met""" - out, err = self._execute('vgs', '--noheadings', '-o', 'name', - run_as_root=True) - volume_groups = out.split() - if not CONF.volume_group in volume_groups: - exception_message = (_("volume group %s doesn't exist") - % CONF.volume_group) - raise exception.VolumeBackendAPIException(data=exception_message) - - def _create_volume(self, volume_name, sizestr): - self._try_execute('lvcreate', '-L', sizestr, '-n', - volume_name, CONF.volume_group, run_as_root=True) - - def _copy_volume(self, srcstr, deststr, size_in_g): - # Use O_DIRECT to avoid thrashing the system buffer cache - direct_flags = ('iflag=direct', 'oflag=direct') - - # Check whether O_DIRECT is supported - try: - self._execute('dd', 'count=0', 'if=%s' % srcstr, 'of=%s' % deststr, - *direct_flags, run_as_root=True) - except exception.ProcessExecutionError: - direct_flags = () - - # Perform the copy - self._execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, - 'count=%d' % (size_in_g * 1024), 'bs=1M', - *direct_flags, run_as_root=True) - - def _volume_not_present(self, volume_name): - path_name = '%s/%s' % (CONF.volume_group, volume_name) - try: - self._try_execute('lvdisplay', path_name, run_as_root=True) - except Exception as e: - # If the volume isn't present - return True - return False - - def _delete_volume(self, volume, size_in_g): - """Deletes a logical volume.""" - # zero out old volumes to prevent data leaking between users - # TODO(ja): reclaiming space should be done lazy and low priority - self._copy_volume('/dev/zero', self.local_path(volume), size_in_g) - dev_path = self.local_path(volume) - if os.path.exists(dev_path): - self._try_execute('dmsetup', 'remove', '-f', dev_path, - run_as_root=True) - self._try_execute('lvremove', '-f', "%s/%s" % - (CONF.volume_group, - self._escape_snapshot(volume['name'])), - run_as_root=True) - - def _sizestr(self, size_in_g): - if int(size_in_g) == 0: - return '100M' - return '%sG' % size_in_g - - # Linux LVM reserves name that starts with snapshot, so that - # such volume name can't be created. Mangle it. - def _escape_snapshot(self, snapshot_name): - if not snapshot_name.startswith('snapshot'): - return snapshot_name - return '_' + snapshot_name - - def create_volume(self, volume): - """Creates a logical volume. Can optionally return a Dictionary of - changes to the volume object to be persisted.""" - self._create_volume(volume['name'], self._sizestr(volume['size'])) - - def create_volume_from_snapshot(self, volume, snapshot): - """Creates a volume from a snapshot.""" - self._create_volume(volume['name'], self._sizestr(volume['size'])) - self._copy_volume(self.local_path(snapshot), self.local_path(volume), - snapshot['volume_size']) - - def delete_volume(self, volume): - """Deletes a logical volume.""" - if self._volume_not_present(volume['name']): - # If the volume isn't present, then don't attempt to delete - return True - - # TODO(yamahata): lvm can't delete origin volume only without - # deleting derived snapshots. Can we do something fancy? - out, err = self._execute('lvdisplay', '--noheading', - '-C', '-o', 'Attr', - '%s/%s' % (CONF.volume_group, - volume['name']), - run_as_root=True) - # fake_execute returns None resulting unit test error - if out: - out = out.strip() - if (out[0] == 'o') or (out[0] == 'O'): - raise exception.VolumeIsBusy(volume_name=volume['name']) - - self._delete_volume(volume, volume['size']) - - def create_snapshot(self, snapshot): - """Creates a snapshot.""" - orig_lv_name = "%s/%s" % (CONF.volume_group, snapshot['volume_name']) - self._try_execute('lvcreate', '-L', - self._sizestr(snapshot['volume_size']), - '--name', self._escape_snapshot(snapshot['name']), - '--snapshot', orig_lv_name, run_as_root=True) - - def delete_snapshot(self, snapshot): - """Deletes a snapshot.""" - if self._volume_not_present(self._escape_snapshot(snapshot['name'])): - # If the snapshot isn't present, then don't attempt to delete - return True - - # TODO(yamahata): zeroing out the whole snapshot triggers COW. - # it's quite slow. - self._delete_volume(snapshot, snapshot['volume_size']) - - def local_path(self, volume): - # NOTE(vish): stops deprecation warning - escaped_group = CONF.volume_group.replace('-', '--') - escaped_name = self._escape_snapshot(volume['name']).replace('-', '--') - return "/dev/mapper/%s-%s" % (escaped_group, escaped_name) - - def ensure_export(self, context, volume): - """Synchronously recreates an export for a logical volume.""" - raise NotImplementedError() - - def create_export(self, context, volume): - """Exports the volume. Can optionally return a Dictionary of changes - to the volume object to be persisted.""" - raise NotImplementedError() - - def remove_export(self, context, volume): - """Removes an export for a logical volume.""" - raise NotImplementedError() - - def check_for_export(self, context, volume_id): - """Make sure volume is exported.""" - raise NotImplementedError() - - def initialize_connection(self, volume, connector): - """Allow connection to connector and return connection info.""" - raise NotImplementedError() - - def terminate_connection(self, volume, connector): - """Disallow connection from connector""" - raise NotImplementedError() - - def attach_volume(self, context, volume_id, instance_uuid, mountpoint): - """ Callback for volume attached to instance.""" - pass - - def detach_volume(self, context, volume_id): - """ Callback for volume detached.""" - pass - - def get_volume_stats(self, refresh=False): - """Return the current state of the volume service. If 'refresh' is - True, run the update first.""" - return None - - def do_setup(self, context): - """Any initialization the volume driver does while starting""" - pass - - def copy_image_to_volume(self, context, volume, image_service, image_id): - """Fetch the image from image_service and write it to the volume.""" - raise NotImplementedError() - - def copy_volume_to_image(self, context, volume, image_service, image_id): - """Copy the volume to the specified image.""" - raise NotImplementedError() - - def clone_image(self, volume, image_location): - """Create a volume efficiently from an existing image. - - image_location is a string whose format depends on the - image service backend in use. The driver should use it - to determine whether cloning is possible. - - Returns a boolean indicating whether cloning occurred - """ - return False - - -class ISCSIDriver(VolumeDriver): - """Executes commands relating to ISCSI volumes. - - We make use of model provider properties as follows: - - ``provider_location`` - if present, contains the iSCSI target information in the same - format as an ietadm discovery - i.e. ':, ' - - ``provider_auth`` - if present, contains a space-separated triple: - ' '. - `CHAP` is the only auth_method in use at the moment. - """ - - def __init__(self, *args, **kwargs): - self.tgtadm = iscsi.get_target_admin() - super(ISCSIDriver, self).__init__(*args, **kwargs) - - def set_execute(self, execute): - super(ISCSIDriver, self).set_execute(execute) - self.tgtadm.set_execute(execute) - - def ensure_export(self, context, volume): - """Synchronously recreates an export for a logical volume.""" - # NOTE(jdg): tgtadm doesn't use the iscsi_targets table - # TODO(jdg): In the future move all of the dependent stuff into the - # cooresponding target admin class - if not isinstance(self.tgtadm, iscsi.TgtAdm): - try: - iscsi_target = self.db.volume_get_iscsi_target_num(context, - volume['id']) - except exception.NotFound: - LOG.info(_("Skipping ensure_export. No iscsi_target " - "provisioned for volume: %s"), volume['id']) - return - else: - iscsi_target = 1 # dummy value when using TgtAdm - - iscsi_name = "%s%s" % (CONF.iscsi_target_prefix, volume['name']) - volume_path = "/dev/%s/%s" % (CONF.volume_group, volume['name']) - - # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need - # should clean this all up at some point in the future - self.tgtadm.create_iscsi_target(iscsi_name, iscsi_target, - 0, volume_path, - check_exit_code=False) - - def _ensure_iscsi_targets(self, context, host): - """Ensure that target ids have been created in datastore.""" - # NOTE(jdg): tgtadm doesn't use the iscsi_targets table - # TODO(jdg): In the future move all of the dependent stuff into the - # cooresponding target admin class - if not isinstance(self.tgtadm, iscsi.TgtAdm): - host_iscsi_targets = self.db.iscsi_target_count_by_host(context, - host) - if host_iscsi_targets >= CONF.iscsi_num_targets: - return - - # NOTE(vish): Target ids start at 1, not 0. - for target_num in xrange(1, CONF.iscsi_num_targets + 1): - target = {'host': host, 'target_num': target_num} - self.db.iscsi_target_create_safe(context, target) - - def create_export(self, context, volume): - """Creates an export for a logical volume.""" - #BOOKMARK(jdg) - - iscsi_name = "%s%s" % (CONF.iscsi_target_prefix, volume['name']) - volume_path = "/dev/%s/%s" % (CONF.volume_group, volume['name']) - - model_update = {} - - # TODO(jdg): In the future move all of the dependent stuff into the - # cooresponding target admin class - if not isinstance(self.tgtadm, iscsi.TgtAdm): - lun = 0 - self._ensure_iscsi_targets(context, volume['host']) - iscsi_target = self.db.volume_allocate_iscsi_target(context, - volume['id'], - volume['host']) - else: - lun = 1 # For tgtadm the controller is lun 0, dev starts at lun 1 - iscsi_target = 0 # NOTE(jdg): Not used by tgtadm - - # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need - # should clean this all up at some point in the future - tid = self.tgtadm.create_iscsi_target(iscsi_name, - iscsi_target, - 0, - volume_path) - model_update['provider_location'] = _iscsi_location( - CONF.iscsi_ip_address, tid, iscsi_name, lun) - return model_update - - def remove_export(self, context, volume): - """Removes an export for a logical volume.""" - - # NOTE(jdg): tgtadm doesn't use the iscsi_targets table - # TODO(jdg): In the future move all of the dependent stuff into the - # cooresponding target admin class - if not isinstance(self.tgtadm, iscsi.TgtAdm): - try: - iscsi_target = self.db.volume_get_iscsi_target_num(context, - volume['id']) - except exception.NotFound: - LOG.info(_("Skipping remove_export. No iscsi_target " - "provisioned for volume: %s"), volume['id']) - return - else: - iscsi_target = 0 - - try: - - # NOTE: provider_location may be unset if the volume hasn't - # been exported - location = volume['provider_location'].split(' ') - iqn = location[1] - - # ietadm show will exit with an error - # this export has already been removed - self.tgtadm.show_target(iscsi_target, iqn=iqn) - except Exception as e: - LOG.info(_("Skipping remove_export. No iscsi_target " - "is presently exported for volume: %s"), volume['id']) - return - - self.tgtadm.remove_iscsi_target(iscsi_target, 0, volume['id']) - - def _do_iscsi_discovery(self, volume): - #TODO(justinsb): Deprecate discovery and use stored info - #NOTE(justinsb): Discovery won't work with CHAP-secured targets (?) - LOG.warn(_("ISCSI provider_location not stored, using discovery")) - - volume_name = volume['name'] - - (out, _err) = self._execute('iscsiadm', '-m', 'discovery', - '-t', 'sendtargets', '-p', volume['host'], - run_as_root=True) - for target in out.splitlines(): - if CONF.iscsi_ip_address in target and volume_name in target: - return target - return None - - def _get_iscsi_properties(self, volume): - """Gets iscsi configuration - - We ideally get saved information in the volume entity, but fall back - to discovery if need be. Discovery may be completely removed in future - The properties are: - - :target_discovered: boolean indicating whether discovery was used - - :target_iqn: the IQN of the iSCSI target - - :target_portal: the portal of the iSCSI target - - :target_lun: the lun of the iSCSI target - - :volume_id: the id of the volume (currently used by xen) - - :auth_method:, :auth_username:, :auth_password: - - the authentication details. Right now, either auth_method is not - present meaning no authentication, or auth_method == `CHAP` - meaning use CHAP with the specified credentials. - """ - - properties = {} - - location = volume['provider_location'] - - if location: - # provider_location is the same format as iSCSI discovery output - properties['target_discovered'] = False - else: - location = self._do_iscsi_discovery(volume) - - if not location: - raise exception.InvalidVolume(_("Could not find iSCSI export " - " for volume %s") % - (volume['name'])) - - LOG.debug(_("ISCSI Discovery: Found %s") % (location)) - properties['target_discovered'] = True - - results = location.split(" ") - properties['target_portal'] = results[0].split(",")[0] - properties['target_iqn'] = results[1] - try: - properties['target_lun'] = int(results[2]) - except (IndexError, ValueError): - if CONF.iscsi_helper == 'tgtadm': - properties['target_lun'] = 1 - else: - properties['target_lun'] = 0 - - properties['volume_id'] = volume['id'] - - auth = volume['provider_auth'] - if auth: - (auth_method, auth_username, auth_secret) = auth.split() - - properties['auth_method'] = auth_method - properties['auth_username'] = auth_username - properties['auth_password'] = auth_secret - - return properties - - def _run_iscsiadm(self, iscsi_properties, iscsi_command): - (out, err) = self._execute('iscsiadm', '-m', 'node', '-T', - iscsi_properties['target_iqn'], - '-p', iscsi_properties['target_portal'], - *iscsi_command, run_as_root=True) - LOG.debug("iscsiadm %s: stdout=%s stderr=%s" % - (iscsi_command, out, err)) - return (out, err) - - def _iscsiadm_update(self, iscsi_properties, property_key, property_value): - iscsi_command = ('--op', 'update', '-n', property_key, - '-v', property_value) - return self._run_iscsiadm(iscsi_properties, iscsi_command) - - def initialize_connection(self, volume, connector): - """Initializes the connection and returns connection info. - - The iscsi driver returns a driver_volume_type of 'iscsi'. - The format of the driver data is defined in _get_iscsi_properties. - Example return value:: - - { - 'driver_volume_type': 'iscsi' - 'data': { - 'target_discovered': True, - 'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001', - 'target_portal': '127.0.0.0.1:3260', - 'volume_id': 1, - } - } - - """ - - iscsi_properties = self._get_iscsi_properties(volume) - return { - 'driver_volume_type': 'iscsi', - 'data': iscsi_properties - } - - def terminate_connection(self, volume, connector): - pass - - def check_for_export(self, context, volume_id): - """Make sure volume is exported.""" - vol_uuid_file = 'volume-%s' % volume_id - volume_path = os.path.join(CONF.volumes_dir, vol_uuid_file) - if os.path.isfile(volume_path): - iqn = '%s%s' % (CONF.iscsi_target_prefix, - vol_uuid_file) - else: - raise exception.PersistentVolumeFileNotFound(volume_id=volume_id) - - # TODO(jdg): In the future move all of the dependent stuff into the - # cooresponding target admin class - if not isinstance(self.tgtadm, iscsi.TgtAdm): - tid = self.db.volume_get_iscsi_target_num(context, volume_id) - else: - tid = 0 - - try: - self.tgtadm.show_target(tid, iqn=iqn) - except exception.ProcessExecutionError, e: - # Instances remount read-only in this case. - # /etc/init.d/iscsitarget restart and rebooting nova-volume - # is better since ensure_export() works at boot time. - LOG.error(_("Cannot confirm exported volume " - "id:%(volume_id)s.") % locals()) - raise - - def copy_image_to_volume(self, context, volume, image_service, image_id): - """Fetch the image from image_service and write it to the volume.""" - volume_path = self.local_path(volume) - with utils.temporary_chown(volume_path): - with utils.file_open(volume_path, "wb") as image_file: - image_service.download(context, image_id, image_file) - - def copy_volume_to_image(self, context, volume, image_service, image_id): - """Copy the volume to the specified image.""" - volume_path = self.local_path(volume) - with utils.temporary_chown(volume_path): - with utils.file_open(volume_path) as volume_file: - image_service.update(context, image_id, {}, volume_file) - - -class FakeISCSIDriver(ISCSIDriver): - """Logs calls instead of executing.""" - def __init__(self, *args, **kwargs): - super(FakeISCSIDriver, self).__init__(execute=self.fake_execute, - *args, **kwargs) - - def check_for_setup_error(self): - """No setup necessary in fake mode.""" - pass - - def initialize_connection(self, volume, connector): - return { - 'driver_volume_type': 'iscsi', - 'data': {} - } - - def terminate_connection(self, volume, connector): - pass - - @staticmethod - def fake_execute(cmd, *_args, **_kwargs): - """Execute that simply logs the command.""" - LOG.debug(_("FAKE ISCSI: %s"), cmd) - return (None, None) - - -class RBDDriver(VolumeDriver): - """Implements RADOS block device (RBD) volume commands""" - - def check_for_setup_error(self): - """Returns an error if prerequisites aren't met""" - (stdout, stderr) = self._execute('rados', 'lspools') - pools = stdout.split("\n") - if not CONF.rbd_pool in pools: - exception_message = (_("rbd has no pool %s") % - CONF.rbd_pool) - raise exception.VolumeBackendAPIException(data=exception_message) - - def _supports_layering(self): - stdout, _ = self._execute('rbd', '--help') - return 'clone' in stdout - - def create_volume(self, volume): - """Creates a logical volume.""" - if int(volume['size']) == 0: - size = 100 - else: - size = int(volume['size']) * 1024 - args = ['rbd', 'create', - '--pool', CONF.rbd_pool, - '--size', size, - volume['name']] - if self._supports_layering(): - args += ['--new-format'] - self._try_execute(*args) - - def _clone(self, volume, src_pool, src_image, src_snap): - self._try_execute('rbd', 'clone', - '--pool', src_pool, - '--image', src_image, - '--snap', src_snap, - '--dest-pool', CONF.rbd_pool, - '--dest', volume['name']) - - def _resize(self, volume): - size = int(volume['size']) * 1024 - self._try_execute('rbd', 'resize', - '--pool', CONF.rbd_pool, - '--image', volume['name'], - '--size', size) - - def create_volume_from_snapshot(self, volume, snapshot): - """Creates a volume from a snapshot.""" - self._clone(volume, CONF.rbd_pool, - snapshot['volume_name'], snapshot['name']) - if int(volume['size']): - self._resize(volume) - - def delete_volume(self, volume): - """Deletes a logical volume.""" - stdout, _ = self._execute('rbd', 'snap', 'ls', - '--pool', CONF.rbd_pool, - volume['name']) - if stdout.count('\n') > 1: - raise exception.VolumeIsBusy(volume_name=volume['name']) - self._try_execute('rbd', 'rm', - '--pool', CONF.rbd_pool, - volume['name']) - - def create_snapshot(self, snapshot): - """Creates an rbd snapshot""" - self._try_execute('rbd', 'snap', 'create', - '--pool', CONF.rbd_pool, - '--snap', snapshot['name'], - snapshot['volume_name']) - if self._supports_layering(): - self._try_execute('rbd', 'snap', 'protect', - '--pool', CONF.rbd_pool, - '--snap', snapshot['name'], - snapshot['volume_name']) - - def delete_snapshot(self, snapshot): - """Deletes an rbd snapshot""" - if self._supports_layering(): - try: - self._try_execute('rbd', 'snap', 'unprotect', - '--pool', CONF.rbd_pool, - '--snap', snapshot['name'], - snapshot['volume_name']) - except exception.ProcessExecutionError: - raise exception.SnapshotIsBusy(snapshot_name=snapshot['name']) - self._try_execute('rbd', 'snap', 'rm', - '--pool', CONF.rbd_pool, - '--snap', snapshot['name'], - snapshot['volume_name']) - - def local_path(self, volume): - """Returns the path of the rbd volume.""" - # This is the same as the remote path - # since qemu accesses it directly. - return "rbd:%s/%s" % (CONF.rbd_pool, volume['name']) - - def ensure_export(self, context, volume): - """Synchronously recreates an export for a logical volume.""" - pass - - def create_export(self, context, volume): - """Exports the volume""" - pass - - def remove_export(self, context, volume): - """Removes an export for a logical volume""" - pass - - def check_for_export(self, context, volume_id): - """Make sure volume is exported.""" - pass - - def initialize_connection(self, volume, connector): - return { - 'driver_volume_type': 'rbd', - 'data': { - 'name': '%s/%s' % (CONF.rbd_pool, volume['name']), - 'auth_enabled': CONF.rbd_secret_uuid is not None, - 'auth_username': CONF.rbd_user, - 'secret_type': 'ceph', - 'secret_uuid': CONF.rbd_secret_uuid, - } - } - - def terminate_connection(self, volume, connector): - pass - - def _parse_location(self, location): - prefix = 'rbd://' - if not location.startswith(prefix): - reason = _('Image %s is not stored in rbd') % location - raise exception.ImageUnacceptable(reason) - pieces = map(urllib.unquote, location[len(prefix):].split('/')) - if any(map(lambda p: p == '', pieces)): - reason = _('Image %s has blank components') % location - raise exception.ImageUnacceptable(reason) - if len(pieces) != 4: - reason = _('Image %s is not an rbd snapshot') % location - raise exception.ImageUnacceptable(reason) - return pieces - - def _get_fsid(self): - stdout, _ = self._execute('ceph', 'fsid') - return stdout.rstrip('\n') - - def _is_cloneable(self, image_location): - try: - fsid, pool, image, snapshot = self._parse_location(image_location) - except exception.ImageUnacceptable: - return False - - if self._get_fsid() != fsid: - reason = _('%s is in a different ceph cluster') % image_location - LOG.debug(reason) - return False - - # check that we can read the image - try: - self._execute('rbd', 'info', - '--pool', pool, - '--image', image, - '--snap', snapshot) - except exception.ProcessExecutionError: - LOG.debug(_('Unable to read image %s') % image_location) - return False - - return True - - def clone_image(self, volume, image_location): - if image_location is None or not self._is_cloneable(image_location): - return False - _, pool, image, snapshot = self._parse_location(image_location) - self._clone(volume, pool, image, snapshot) - self._resize(volume) - return True - - def copy_image_to_volume(self, context, volume, image_service, image_id): - # TODO(jdurgin): replace with librbd - # this is a temporary hack, since rewriting this driver - # to use librbd would take too long - if CONF.volume_tmp_dir and not os.path.exists(CONF.volume_tmp_dir): - os.makedirs(CONF.volume_tmp_dir) - - with tempfile.NamedTemporaryFile(dir=CONF.volume_tmp_dir) as tmp: - image_service.download(context, image_id, tmp) - # import creates the image, so we must remove it first - self._try_execute('rbd', 'rm', - '--pool', CONF.rbd_pool, - volume['name']) - self._try_execute('rbd', 'import', - '--pool', CONF.rbd_pool, - tmp.name, volume['name']) - - -class SheepdogDriver(VolumeDriver): - """Executes commands relating to Sheepdog Volumes""" - - def check_for_setup_error(self): - """Returns an error if prerequisites aren't met""" - try: - #NOTE(francois-charlier) Since 0.24 'collie cluster info -r' - # gives short output, but for compatibility reason we won't - # use it and just check if 'running' is in the output. - (out, err) = self._execute('collie', 'cluster', 'info') - if not 'running' in out.split(): - exception_message = _("Sheepdog is not working: %s") % out - raise exception.VolumeBackendAPIException( - data=exception_message) - - except exception.ProcessExecutionError: - exception_message = _("Sheepdog is not working") - raise exception.NovaException(data=exception_message) - - def create_volume(self, volume): - """Creates a sheepdog volume""" - self._try_execute('qemu-img', 'create', - "sheepdog:%s" % volume['name'], - self._sizestr(volume['size'])) - - def create_volume_from_snapshot(self, volume, snapshot): - """Creates a sheepdog volume from a snapshot.""" - self._try_execute('qemu-img', 'create', '-b', - "sheepdog:%s:%s" % (snapshot['volume_name'], - snapshot['name']), - "sheepdog:%s" % volume['name']) - - def delete_volume(self, volume): - """Deletes a logical volume""" - self._try_execute('collie', 'vdi', 'delete', volume['name']) - - def create_snapshot(self, snapshot): - """Creates a sheepdog snapshot""" - self._try_execute('qemu-img', 'snapshot', '-c', snapshot['name'], - "sheepdog:%s" % snapshot['volume_name']) - - def delete_snapshot(self, snapshot): - """Deletes a sheepdog snapshot""" - self._try_execute('collie', 'vdi', 'delete', snapshot['volume_name'], - '-s', snapshot['name']) - - def local_path(self, volume): - return "sheepdog:%s" % volume['name'] - - def ensure_export(self, context, volume): - """Safely and synchronously recreates an export for a logical volume""" - pass - - def create_export(self, context, volume): - """Exports the volume""" - pass - - def remove_export(self, context, volume): - """Removes an export for a logical volume""" - pass - - def check_for_export(self, context, volume_id): - """Make sure volume is exported.""" - pass - - def initialize_connection(self, volume, connector): - return { - 'driver_volume_type': 'sheepdog', - 'data': { - 'name': volume['name'] - } - } - - def terminate_connection(self, volume, connector): - pass - - -class LoggingVolumeDriver(VolumeDriver): - """Logs and records calls, for unit tests.""" - - def check_for_setup_error(self): - pass - - def create_volume(self, volume): - self.log_action('create_volume', volume) - - def delete_volume(self, volume): - self.log_action('delete_volume', volume) - - def local_path(self, volume): - print "local_path not implemented" - raise NotImplementedError() - - def ensure_export(self, context, volume): - self.log_action('ensure_export', volume) - - def create_export(self, context, volume): - self.log_action('create_export', volume) - - def remove_export(self, context, volume): - self.log_action('remove_export', volume) - - def initialize_connection(self, volume, connector): - self.log_action('initialize_connection', volume) - - def terminate_connection(self, volume, connector): - self.log_action('terminate_connection', volume) - - def check_for_export(self, context, volume_id): - self.log_action('check_for_export', volume_id) - - _LOGS = [] - - @staticmethod - def clear_logs(): - LoggingVolumeDriver._LOGS = [] - - @staticmethod - def log_action(action, parameters): - """Logs the command.""" - LOG.debug(_("LoggingVolumeDriver: %s") % (action)) - log_dictionary = {} - if parameters: - log_dictionary = dict(parameters) - log_dictionary['action'] = action - LOG.debug(_("LoggingVolumeDriver: %s") % (log_dictionary)) - LoggingVolumeDriver._LOGS.append(log_dictionary) - - @staticmethod - def all_logs(): - return LoggingVolumeDriver._LOGS - - @staticmethod - def logs_like(action, **kwargs): - matches = [] - for entry in LoggingVolumeDriver._LOGS: - if entry['action'] != action: - continue - match = True - for k, v in kwargs.iteritems(): - if entry.get(k) != v: - match = False - break - if match: - matches.append(entry) - return matches - - -def _iscsi_location(ip, target, iqn, lun=None): - return "%s:%s,%s %s %s" % (ip, CONF.iscsi_port, target, iqn, lun) diff --git a/nova/volume/iscsi.py b/nova/volume/iscsi.py deleted file mode 100644 index ce2776920..000000000 --- a/nova/volume/iscsi.py +++ /dev/null @@ -1,235 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright 2010 United States Government as represented by the -# Administrator of the National Aeronautics and Space Administration. -# 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. -""" -Helper code for the iSCSI volume driver. - -""" -import os - -from nova import config -from nova import exception -from nova import flags -from nova.openstack.common import cfg -from nova.openstack.common import fileutils -from nova.openstack.common import log as logging -from nova import utils - -LOG = logging.getLogger(__name__) - -iscsi_helper_opt = [ - cfg.StrOpt('iscsi_helper', - default='tgtadm', - help='iscsi target user-land tool to use'), - cfg.StrOpt('volumes_dir', - default='$state_path/volumes', - help='Volume configuration file storage directory'), -] - -CONF = config.CONF -CONF.register_opts(iscsi_helper_opt) - - -class TargetAdmin(object): - """iSCSI target administration. - - Base class for iSCSI target admin helpers. - """ - - def __init__(self, cmd, execute): - self._cmd = cmd - self.set_execute(execute) - - def set_execute(self, execute): - """Set the function to be used to execute commands.""" - self._execute = execute - - def _run(self, *args, **kwargs): - self._execute(self._cmd, *args, run_as_root=True, **kwargs) - - def create_iscsi_target(self, name, tid, lun, path, **kwargs): - """Create a iSCSI target and logical unit""" - raise NotImplementedError() - - def remove_iscsi_target(self, tid, lun, vol_id, **kwargs): - """Remove a iSCSI target and logical unit""" - raise NotImplementedError() - - def _new_target(self, name, tid, **kwargs): - """Create a new iSCSI target.""" - raise NotImplementedError() - - def _delete_target(self, tid, **kwargs): - """Delete a target.""" - raise NotImplementedError() - - def show_target(self, tid, iqn=None, **kwargs): - """Query the given target ID.""" - raise NotImplementedError() - - def _new_logicalunit(self, tid, lun, path, **kwargs): - """Create a new LUN on a target using the supplied path.""" - raise NotImplementedError() - - def _delete_logicalunit(self, tid, lun, **kwargs): - """Delete a logical unit from a target.""" - raise NotImplementedError() - - -class TgtAdm(TargetAdmin): - """iSCSI target administration using tgtadm.""" - - def __init__(self, execute=utils.execute): - super(TgtAdm, self).__init__('tgtadm', execute) - - def _get_target(self, iqn): - (out, err) = self._execute('tgt-admin', '--show', run_as_root=True) - lines = out.split('\n') - for line in lines: - if iqn in line: - parsed = line.split() - tid = parsed[1] - return tid[:-1] - - return None - - def create_iscsi_target(self, name, tid, lun, path, **kwargs): - # Note(jdg) tid and lun aren't used by TgtAdm but remain for - # compatibility - - fileutils.ensure_tree(CONF.volumes_dir) - - vol_id = name.split(':')[1] - volume_conf = """ - - backing-store %s - - """ % (name, path) - - LOG.info(_('Creating volume: %s') % vol_id) - volumes_dir = CONF.volumes_dir - volume_path = os.path.join(volumes_dir, vol_id) - - f = open(volume_path, 'w+') - f.write(volume_conf) - f.close() - - try: - (out, err) = self._execute('tgt-admin', - '--update', - name, - run_as_root=True) - except exception.ProcessExecutionError, e: - LOG.error(_("Failed to create iscsi target for volume " - "id:%(vol_id)s.") % locals()) - - #Don't forget to remove the persistent file we created - os.unlink(volume_path) - raise exception.ISCSITargetCreateFailed(volume_id=vol_id) - - iqn = '%s%s' % (CONF.iscsi_target_prefix, vol_id) - tid = self._get_target(iqn) - if tid is None: - LOG.error(_("Failed to create iscsi target for volume " - "id:%(vol_id)s. Please ensure your tgtd config file " - "contains 'include %(volumes_dir)s/*'") % locals()) - raise exception.NotFound() - - return tid - - def remove_iscsi_target(self, tid, lun, vol_id, **kwargs): - LOG.info(_('Removing volume: %s') % vol_id) - vol_uuid_file = 'volume-%s' % vol_id - volume_path = os.path.join(CONF.volumes_dir, vol_uuid_file) - if os.path.isfile(volume_path): - iqn = '%s%s' % (CONF.iscsi_target_prefix, - vol_uuid_file) - else: - raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) - try: - self._execute('tgt-admin', - '--delete', - iqn, - run_as_root=True) - except exception.ProcessExecutionError, e: - LOG.error(_("Failed to create iscsi target for volume " - "id:%(volume_id)s.") % locals()) - raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) - - os.unlink(volume_path) - - def show_target(self, tid, iqn=None, **kwargs): - if iqn is None: - raise exception.InvalidParameterValue( - err=_('valid iqn needed for show_target')) - - tid = self._get_target(iqn) - if tid is None: - raise exception.NotFound() - - -class IetAdm(TargetAdmin): - """iSCSI target administration using ietadm.""" - - def __init__(self, execute=utils.execute): - super(IetAdm, self).__init__('ietadm', execute) - - def create_iscsi_target(self, name, tid, lun, path, **kwargs): - self._new_target(name, tid, **kwargs) - self._new_logicalunit(tid, lun, path, **kwargs) - return tid - - def remove_iscsi_target(self, tid, lun, vol_id, **kwargs): - LOG.info(_('Removing volume: %s') % vol_id) - self._delete_logicalunit(tid, lun, **kwargs) - self._delete_target(tid, **kwargs) - - def _new_target(self, name, tid, **kwargs): - self._run('--op', 'new', - '--tid=%s' % tid, - '--params', 'Name=%s' % name, - **kwargs) - - def _delete_target(self, tid, **kwargs): - self._run('--op', 'delete', - '--tid=%s' % tid, - **kwargs) - - def show_target(self, tid, iqn=None, **kwargs): - self._run('--op', 'show', - '--tid=%s' % tid, - **kwargs) - - def _new_logicalunit(self, tid, lun, path, **kwargs): - self._run('--op', 'new', - '--tid=%s' % tid, - '--lun=%d' % lun, - '--params', 'Path=%s,Type=fileio' % path, - **kwargs) - - def _delete_logicalunit(self, tid, lun, **kwargs): - self._run('--op', 'delete', - '--tid=%s' % tid, - '--lun=%d' % lun, - **kwargs) - - -def get_target_admin(): - if CONF.iscsi_helper == 'tgtadm': - return TgtAdm() - else: - return IetAdm() -- cgit From ca1282ad7c577398b8a4c47481fa0c94b62893b4 Mon Sep 17 00:00:00 2001 From: Jian Wen Date: Tue, 13 Nov 2012 17:02:36 +0800 Subject: Fixes usage of migrate_instance_start Because migrate_instance_start is used to remove floating ips on the source compute node, we need send the RPC to the source compute node instead of the dest compute node. Fixes bug 1078207 Change-Id: Ie993548944268e1ab3af0b89e74e1b54d8137802 --- nova/network/api.py | 2 +- nova/tests/network/test_api.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/network/api.py b/nova/network/api.py index e4fd38b13..4b803482a 100644 --- a/nova/network/api.py +++ b/nova/network/api.py @@ -351,7 +351,7 @@ class API(base.Base): if self._is_multi_host(context, instance): args['floating_addresses'] = \ self._get_floating_ip_addresses(context, instance) - args['host'] = migration['dest_compute'] + args['host'] = migration['source_compute'] self.network_rpcapi.migrate_instance_start(context, **args) diff --git a/nova/tests/network/test_api.py b/nova/tests/network/test_api.py index bfc9dc878..15ead5337 100644 --- a/nova/tests/network/test_api.py +++ b/nova/tests/network/test_api.py @@ -117,7 +117,6 @@ class ApiTestCase(test.TestCase): 'project_id': 'fake_project_id', 'floating_addresses': None} if multi_host: - expected['host'] = 'fake_compute_dest' expected['floating_addresses'] = ['fake_float1', 'fake_float2'] return fake_instance, fake_migration, expected @@ -125,6 +124,7 @@ class ApiTestCase(test.TestCase): info = {'kwargs': {}} arg1, arg2, expected = self._stub_migrate_instance_calls( 'migrate_instance_start', True, info) + expected['host'] = 'fake_compute_source' self.network_api.migrate_instance_start(self.context, arg1, arg2) self.assertEqual(info['kwargs'], expected) @@ -139,6 +139,7 @@ class ApiTestCase(test.TestCase): info = {'kwargs': {}} arg1, arg2, expected = self._stub_migrate_instance_calls( 'migrate_instance_finish', True, info) + expected['host'] = 'fake_compute_dest' self.network_api.migrate_instance_finish(self.context, arg1, arg2) self.assertEqual(info['kwargs'], expected) -- cgit From 2f82f39ed7b10f052f686310a9e7e4d6d2a6ae58 Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Mon, 12 Nov 2012 17:06:05 -0500 Subject: Try hard shutdown if clean fails on resize down In the xenapi driver, a clean shutdown is attempted before a resize down. If the Xenapi raises an exception it is logged and ignored and the resize will fail later in the process. This adds a hard shutdown attempt if the clean shutdown fails. Change-Id: I458931a19dd2375ad7142d60cb8b6ed274de9483 --- nova/virt/xenapi/vmops.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 19077c7df..11ee7b04b 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -692,7 +692,10 @@ class VMOps(object): instance=instance) # 2. Power down the instance before resizing - vm_utils.clean_shutdown_vm(self._session, instance, vm_ref) + if not vm_utils.clean_shutdown_vm(self._session, instance, vm_ref): + LOG.debug(_("Clean shutdown did not complete successfully, " + "trying hard shutdown."), instance=instance) + vm_utils.hard_shutdown_vm(self._session, instance, vm_ref) self._update_instance_progress(context, instance, step=2, total_steps=RESIZE_TOTAL_STEPS) -- cgit From 3e25d9bc235381081210ca160bb1188ed05274ea Mon Sep 17 00:00:00 2001 From: Brian Elliott Date: Thu, 8 Nov 2012 17:09:34 +0000 Subject: Add DB query to get in-progress migrations. Adds DB query to get in-progress migrations. This will be needed by resource tracker to get more accurate usage values around inbound and outbound resizes. bug 1065267 Change-Id: Ib111002c70aa16e404c6e2c3fcb4ad234177ce91 --- nova/db/api.py | 7 ++++ nova/db/sqlalchemy/api.py | 11 +++++ .../versions/137_add_indexes_to_migrations.py | 46 ++++++++++++++++++++ nova/db/sqlalchemy/models.py | 5 +++ nova/tests/test_db_api.py | 49 ++++++++++++++++++++++ 5 files changed, 118 insertions(+) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/137_add_indexes_to_migrations.py diff --git a/nova/db/api.py b/nova/db/api.py index 04786a1fb..ea47e5f43 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -406,6 +406,13 @@ def migration_get_unconfirmed_by_dest_compute(context, confirm_window, confirm_window, dest_compute) +def migration_get_in_progress_by_host(context, host): + """Finds all migrations for the given host that are not yet confirmed or + reverted. + """ + return IMPL.migration_get_in_progress_by_host(context, host) + + #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 72caad74d..ec78a87e6 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -3351,6 +3351,17 @@ def migration_get_unconfirmed_by_dest_compute(context, confirm_window, all() +@require_admin_context +def migration_get_in_progress_by_host(context, host, session=None): + + return model_query(context, models.Migration, session=session).\ + filter(or_(models.Migration.source_compute == host, + models.Migration.dest_compute == host)).\ + filter(~models.Migration.status.in_(['confirmed', 'reverted'])).\ + options(joinedload('instance')).\ + all() + + ################## diff --git a/nova/db/sqlalchemy/migrate_repo/versions/137_add_indexes_to_migrations.py b/nova/db/sqlalchemy/migrate_repo/versions/137_add_indexes_to_migrations.py new file mode 100644 index 000000000..1499bd351 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/137_add_indexes_to_migrations.py @@ -0,0 +1,46 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2012 OpenStack LLC. +# 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 sqlalchemy import Index, MetaData, Table +from sqlalchemy.exc import IntegrityError + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + t = Table('migrations', meta, autoload=True) + + # Based on migration_get_in_progress_by_host + # from: nova/db/sqlalchemy/api.py + i = Index('migrations_by_host_and_status_idx', t.c.deleted, + t.c.source_compute, t.c.dest_compute, t.c.status) + try: + i.create(migrate_engine) + except IntegrityError: + pass + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + t = Table('migrations', meta, autoload=True) + + i = Index('migrations_by_host_and_status_idx', t.c.deleted, + t.c.source_compute, t.c.dest_compute, t.c.status) + i.drop(migrate_engine) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 194b80f2d..d7c638ff8 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -641,6 +641,11 @@ class Migration(BASE, NovaBase): #TODO(_cerberus_): enum status = Column(String(255)) + instance = relationship("Instance", foreign_keys=instance_uuid, + primaryjoin='and_(Migration.instance_uuid == ' + 'Instance.uuid, Instance.deleted == ' + 'False)') + class Network(BASE, NovaBase): """Represents a network.""" diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index ac66a25c1..8bb4a0035 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -1009,6 +1009,55 @@ class CapacityTestCase(test.TestCase): self.assertEqual(1, int(stat['value'])) +class MigrationTestCase(test.TestCase): + + def setUp(self): + super(MigrationTestCase, self).setUp() + self.ctxt = context.get_admin_context() + + self._create() + self._create() + self._create(status='reverted') + self._create(status='confirmed') + self._create(source_compute='host2', dest_compute='host1') + self._create(source_compute='host2', dest_compute='host3') + self._create(source_compute='host3', dest_compute='host4') + + def _create(self, status='migrating', source_compute='host1', + dest_compute='host2'): + + values = {'host': source_compute} + instance = db.instance_create(self.ctxt, values) + + values = {'status': status, 'source_compute': source_compute, + 'dest_compute': dest_compute, + 'instance_uuid': instance['uuid']} + db.migration_create(self.ctxt, values) + + def _assert_in_progress(self, migrations): + for migration in migrations: + self.assertNotEqual('confirmed', migration.status) + self.assertNotEqual('reverted', migration.status) + + def test_in_progress_host1(self): + migrations = db.migration_get_in_progress_by_host(self.ctxt, 'host1') + # 2 as source + 1 as dest + self.assertEqual(3, len(migrations)) + self._assert_in_progress(migrations) + + def test_in_progress_host2(self): + migrations = db.migration_get_in_progress_by_host(self.ctxt, 'host2') + # 2 as dest, 2 as source + self.assertEqual(4, len(migrations)) + self._assert_in_progress(migrations) + + def test_instance_join(self): + migrations = db.migration_get_in_progress_by_host(self.ctxt, 'host2') + for migration in migrations: + instance = migration['instance'] + self.assertEqual(migration['instance_uuid'], instance['uuid']) + + class TestIpAllocation(test.TestCase): def setUp(self): -- cgit From bb897502e0a1f30376c94ce4e42f9baae3011583 Mon Sep 17 00:00:00 2001 From: Yaguang Tang Date: Wed, 14 Nov 2012 10:51:08 +0800 Subject: fix flag type define error. Change-Id: Ieac6bfaf3bf8f967e76873c933cf131d7d7a91b1 --- nova/virt/xenapi/agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/xenapi/agent.py b/nova/virt/xenapi/agent.py index 9fad07898..dac6061a9 100644 --- a/nova/virt/xenapi/agent.py +++ b/nova/virt/xenapi/agent.py @@ -50,7 +50,7 @@ xenapi_agent_opts = [ 'configuration is not injected into the image. ' 'Used if compute_driver=xenapi.XenAPIDriver and ' ' flat_injected=True'), - cfg.StrOpt('xenapi_disable_agent', + cfg.BoolOpt('xenapi_disable_agent', default=False, help='Disable XenAPI agent. Reduces the amount of time ' 'it takes nova to detect that a VM has started, when ' -- cgit From 18b9c7f29b8925aadeb6ac1ea95c1e1d74e9fa50 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Wed, 14 Nov 2012 06:18:49 +0000 Subject: Isolate tests from the environment variable http_proxy. This fixes bug 1078548. This is done by adding a dependency on the fixtures library, which has dedicated code for just this sort of thing, and using it in the base class. Change-Id: I841fbf912b1a3ab43ca8c52c779d930aaf4a0e96 --- nova/test.py | 2 ++ tools/test-requires | 1 + 2 files changed, 3 insertions(+) diff --git a/nova/test.py b/nova/test.py index 73956a35c..d8a8ea082 100644 --- a/nova/test.py +++ b/nova/test.py @@ -26,6 +26,7 @@ inline callbacks. import sys import uuid +from fixtures import EnvironmentVariable import mox import stubout import testtools @@ -82,6 +83,7 @@ class TestCase(testtools.TestCase): self.injected = [] self._services = [] self._modules = {} + self.useFixture(EnvironmentVariable('http_proxy')) def tearDown(self): """Runs after each test method to tear down test environment.""" diff --git a/tools/test-requires b/tools/test-requires index fc56d3c87..b3d4b5a22 100644 --- a/tools/test-requires +++ b/tools/test-requires @@ -2,6 +2,7 @@ distribute>=0.6.24 coverage +fixtures mox==0.5.3 nose testtools -- cgit From 5beeed884753c3fb196cb66684d761dfc424b0db Mon Sep 17 00:00:00 2001 From: Zhongyue Luo Date: Wed, 14 Nov 2012 18:25:59 +0800 Subject: Remove gen_uuid() Removed gen_uuid and uuid related unittests Replaced utils.gen_uuid() with uuid.uuid4() Change-Id: I3020a0d67525eb73cfb8873d2570c93e2022bb76 --- nova/compute/api.py | 3 ++- nova/compute/instance_types.py | 3 ++- nova/compute/manager.py | 3 ++- nova/context.py | 4 +-- nova/db/sqlalchemy/api.py | 8 +++--- .../versions/089_add_volume_id_mappings.py | 8 +++--- nova/network/manager.py | 3 ++- .../compute/contrib/test_admin_actions.py | 9 ++++--- .../openstack/compute/contrib/test_floating_ips.py | 6 +++-- nova/tests/api/openstack/compute/test_consoles.py | 7 ++--- .../api/openstack/compute/test_server_actions.py | 5 ++-- .../api/openstack/compute/test_server_metadata.py | 6 +++-- nova/tests/api/openstack/compute/test_servers.py | 31 +++++++++++----------- nova/tests/api/openstack/compute/test_versions.py | 5 ++-- nova/tests/api/openstack/fakes.py | 4 +-- nova/tests/compute/test_compute.py | 11 ++++---- nova/tests/fake_volume.py | 8 +++--- nova/tests/hyperv/db_fakes.py | 5 ++-- nova/tests/image/fake.py | 5 ++-- nova/tests/integrated/integrated_helpers.py | 4 +-- nova/tests/network/test_quantumv2.py | 6 +++-- nova/tests/test_db_api.py | 7 ++--- nova/tests/test_utils.py | 26 ------------------ nova/tests/vmwareapi/db_fakes.py | 3 ++- nova/utils.py | 5 ---- 25 files changed, 89 insertions(+), 96 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 092ea6b39..5a8bcad2f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -27,6 +27,7 @@ import re import string import time import urllib +import uuid from nova import block_device from nova.compute import instance_types @@ -678,7 +679,7 @@ class API(base.Base): if not instance.get('uuid'): # Generate the instance_uuid here so we can use it # for additional setup before creating the DB entry. - instance['uuid'] = str(utils.gen_uuid()) + instance['uuid'] = str(uuid.uuid4()) instance['launch_index'] = 0 instance['vm_state'] = vm_states.BUILDING diff --git a/nova/compute/instance_types.py b/nova/compute/instance_types.py index 5efa3d97c..4be4ab71d 100644 --- a/nova/compute/instance_types.py +++ b/nova/compute/instance_types.py @@ -21,6 +21,7 @@ """Built-in instance properties.""" import re +import uuid from nova import config from nova import context @@ -41,7 +42,7 @@ def create(name, memory, vcpus, root_gb, ephemeral_gb=None, flavorid=None, """Creates instance types.""" if flavorid is None: - flavorid = utils.gen_uuid() + flavorid = uuid.uuid4() if swap is None: swap = 0 if rxtx_factor is None: diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9011196b2..be20c7a40 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -40,6 +40,7 @@ import socket import sys import time import traceback +import uuid from eventlet import greenthread @@ -2180,7 +2181,7 @@ class ComputeManager(manager.SchedulerDependentManager): """Return connection information for a vnc console.""" context = context.elevated() LOG.debug(_("Getting vnc console"), instance=instance) - token = str(utils.gen_uuid()) + token = str(uuid.uuid4()) if console_type == 'novnc': # For essex, novncproxy_base_url must include the full path diff --git a/nova/context.py b/nova/context.py index 74f7a3c23..094e2bffb 100644 --- a/nova/context.py +++ b/nova/context.py @@ -20,19 +20,19 @@ """RequestContext: context for requests that persist through all of nova.""" import copy +import uuid from nova.openstack.common import local from nova.openstack.common import log as logging from nova.openstack.common import timeutils from nova import policy -from nova import utils LOG = logging.getLogger(__name__) def generate_request_id(): - return 'req-' + str(utils.gen_uuid()) + return 'req-' + str(uuid.uuid4()) class RequestContext(object): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 42cedc4b2..007e83cbe 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -23,6 +23,7 @@ import collections import copy import datetime import functools +import uuid from sqlalchemy import and_ from sqlalchemy.exc import IntegrityError @@ -46,7 +47,6 @@ from nova import flags from nova.openstack.common import log as logging from nova.openstack.common import timeutils from nova.openstack.common import uuidutils -from nova import utils CONF = config.CONF @@ -1344,7 +1344,7 @@ def instance_create(context, values): instance_ref = models.Instance() if not values.get('uuid'): - values['uuid'] = str(utils.gen_uuid()) + values['uuid'] = str(uuid.uuid4()) instance_ref['info_cache'] = models.InstanceInfoCache() info_cache = values.pop('info_cache', None) if info_cache is not None: @@ -2042,7 +2042,7 @@ def network_create_safe(context, values): raise exception.DuplicateVlan(vlan=values['vlan']) network_ref = models.Network() - network_ref['uuid'] = str(utils.gen_uuid()) + network_ref['uuid'] = str(uuid.uuid4()) network_ref.update(values) try: @@ -2643,7 +2643,7 @@ def quota_reserve(context, resources, quotas, deltas, expire, reservations = [] for resource, delta in deltas.items(): reservation = reservation_create(elevated, - str(utils.gen_uuid()), + str(uuid.uuid4()), usages[resource], context.project_id, resource, delta, expire, diff --git a/nova/db/sqlalchemy/migrate_repo/versions/089_add_volume_id_mappings.py b/nova/db/sqlalchemy/migrate_repo/versions/089_add_volume_id_mappings.py index d878e250b..a4cd06704 100644 --- a/nova/db/sqlalchemy/migrate_repo/versions/089_add_volume_id_mappings.py +++ b/nova/db/sqlalchemy/migrate_repo/versions/089_add_volume_id_mappings.py @@ -15,11 +15,13 @@ # License for the specific language governing permissions and limitations # under the License. +import uuid + from sqlalchemy import Boolean, Column, DateTime, Integer from sqlalchemy import MetaData, String, Table from nova.openstack.common import log as logging -from nova import utils + LOG = logging.getLogger(__name__) @@ -93,7 +95,7 @@ def upgrade(migrate_engine): volume_list = list(volumes.select().execute()) for v in volume_list: old_id = v['id'] - new_id = utils.gen_uuid() + new_id = uuid.uuid4() row = volume_id_mappings.insert() row.execute({'id': old_id, 'uuid': str(new_id)}) @@ -101,7 +103,7 @@ def upgrade(migrate_engine): snapshot_list = list(snapshots.select().execute()) for s in snapshot_list: old_id = s['id'] - new_id = utils.gen_uuid() + new_id = uuid.uuid4() row = snapshot_id_mappings.insert() row.execute({'id': old_id, 'uuid': str(new_id)}) diff --git a/nova/network/manager.py b/nova/network/manager.py index cf7b6109d..dbd82a6b2 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -49,6 +49,7 @@ import itertools import math import re import socket +import uuid from eventlet import greenpool import netaddr @@ -1263,7 +1264,7 @@ class NetworkManager(manager.SchedulerDependentManager): vif = {'address': utils.generate_mac_address(), 'instance_uuid': instance_uuid, 'network_id': network_id, - 'uuid': str(utils.gen_uuid())} + 'uuid': str(uuid.uuid4())} # try FLAG times to create a vif record with a unique mac_address for i in xrange(CONF.create_unique_mac_address_attempts): try: diff --git a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py index 337e1e4e7..f6337f035 100644 --- a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py +++ b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py @@ -13,6 +13,7 @@ # under the License. import datetime +import uuid import webob @@ -28,7 +29,7 @@ from nova.openstack.common import jsonutils from nova.scheduler import rpcapi as scheduler_rpcapi from nova import test from nova.tests.api.openstack import fakes -from nova import utils + CONF = config.CONF @@ -88,7 +89,7 @@ class AdminActionsTest(test.TestCase): def setUp(self): super(AdminActionsTest, self).setUp() self.stubs.Set(compute_api.API, 'get', fake_compute_api_get) - self.UUID = utils.gen_uuid() + self.UUID = uuid.uuid4() for _method in self._methods: self.stubs.Set(compute_api.API, _method, fake_compute_api) self.stubs.Set(scheduler_rpcapi.SchedulerAPI, @@ -181,7 +182,7 @@ class CreateBackupTests(test.TestCase): self.stubs.Set(compute_api.API, 'get', fake_compute_api_get) self.backup_stubs = fakes.stub_out_compute_api_backup(self.stubs) self.app = compute.APIRouter(init_only=('servers',)) - self.uuid = utils.gen_uuid() + self.uuid = uuid.uuid4() def _get_request(self, body): url = '/fake/servers/%s/action' % self.uuid @@ -307,7 +308,7 @@ class ResetStateTests(test.TestCase): self.exists = True self.kwargs = None - self.uuid = utils.gen_uuid() + self.uuid = uuid.uuid4() def fake_get(inst, context, instance_id): if self.exists: diff --git a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py index 171b0900e..d67682a4f 100644 --- a/nova/tests/api/openstack/compute/contrib/test_floating_ips.py +++ b/nova/tests/api/openstack/compute/contrib/test_floating_ips.py @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +import uuid + from lxml import etree import webob @@ -29,7 +31,7 @@ from nova.openstack.common import rpc from nova import test from nova.tests.api.openstack import fakes from nova.tests import fake_network -from nova import utils + FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' @@ -88,7 +90,7 @@ def network_api_disassociate(self, context, instance, floating_address): def fake_instance_get(context, instance_id): return { "id": 1, - "uuid": utils.gen_uuid(), + "uuid": uuid.uuid4(), "name": 'fake', "user_id": 'fakeuser', "project_id": '123'} diff --git a/nova/tests/api/openstack/compute/test_consoles.py b/nova/tests/api/openstack/compute/test_consoles.py index 8a701588c..413015a4d 100644 --- a/nova/tests/api/openstack/compute/test_consoles.py +++ b/nova/tests/api/openstack/compute/test_consoles.py @@ -17,6 +17,7 @@ # under the License. import datetime +import uuid as uuidutils from lxml import etree import webob @@ -31,7 +32,7 @@ from nova.openstack.common import timeutils from nova import test from nova.tests.api.openstack import fakes from nova.tests import matchers -from nova import utils + FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' @@ -57,7 +58,7 @@ class FakeInstanceDB(object): if id is None: id = self.max_id + 1 if uuid is None: - uuid = str(utils.gen_uuid()) + uuid = str(uuidutils.uuid4()) instance = stub_instance(id, uuid=uuid) self.instances_by_id[id] = instance self.ids_by_uuid[uuid] = id @@ -133,7 +134,7 @@ class ConsolesControllerTest(test.TestCase): self.instance_db.return_server_by_id) self.stubs.Set(db, 'instance_get_by_uuid', self.instance_db.return_server_by_uuid) - self.uuid = str(utils.gen_uuid()) + self.uuid = str(uuidutils.uuid4()) self.url = '/v2/fake/servers/%s/consoles' % self.uuid self.controller = consoles.Controller() diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index ae2fde791..415313d9a 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -14,6 +14,7 @@ # under the License. import base64 +import uuid import mox import webob @@ -32,7 +33,7 @@ from nova import test from nova.tests.api.openstack import fakes from nova.tests.image import fake from nova.tests import matchers -from nova import utils + CONF = config.CONF FAKE_UUID = fakes.FAKE_UUID @@ -177,7 +178,7 @@ class ServerActionsControllerTest(test.TestCase): req = fakes.HTTPRequest.blank(self.url) self.assertRaises(webob.exc.HTTPNotFound, self.controller._action_reboot, - req, str(utils.gen_uuid()), body) + req, str(uuid.uuid4()), body) def test_reboot_raises_conflict_on_invalid_state(self): body = dict(reboot=dict(type="HARD")) diff --git a/nova/tests/api/openstack/compute/test_server_metadata.py b/nova/tests/api/openstack/compute/test_server_metadata.py index 093c1efda..82b7df74c 100644 --- a/nova/tests/api/openstack/compute/test_server_metadata.py +++ b/nova/tests/api/openstack/compute/test_server_metadata.py @@ -15,6 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +import uuid + import webob from nova.api.openstack.compute import server_metadata @@ -26,7 +28,7 @@ from nova import flags from nova.openstack.common import jsonutils from nova import test from nova.tests.api.openstack import fakes -from nova import utils + CONF = config.CONF @@ -108,7 +110,7 @@ class ServerMetaDataTest(test.TestCase): fake_change_instance_metadata) self.controller = server_metadata.Controller() - self.uuid = str(utils.gen_uuid()) + self.uuid = str(uuid.uuid4()) self.url = '/v1.1/fake/servers/%s/metadata' % self.uuid def test_index(self): diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index e329a5f7b..06f6e9ba7 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -19,6 +19,7 @@ import base64 import datetime import urlparse +import uuid import iso8601 from lxml import etree @@ -49,7 +50,7 @@ from nova.tests.api.openstack import fakes from nova.tests import fake_network from nova.tests.image import fake from nova.tests import matchers -from nova import utils + CONF = config.CONF @@ -238,7 +239,7 @@ class ServersControllerTest(test.TestCase): """Create two servers with the same host and different project_ids and check that the hostId's are unique""" def return_instance_with_host(self, *args): - project_id = str(utils.gen_uuid()) + project_id = str(uuid.uuid4()) return fakes.stub_instance(id=1, uuid=FAKE_UUID, project_id=project_id, host='fake_host') @@ -517,7 +518,7 @@ class ServersControllerTest(test.TestCase): self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get) - server_id = str(utils.gen_uuid()) + server_id = str(uuid.uuid4()) req = fakes.HTTPRequest.blank('/v2/fake/servers/%s/ips' % server_id) self.assertRaises(webob.exc.HTTPNotFound, self.ips_controller.index, req, server_id) @@ -674,7 +675,7 @@ class ServersControllerTest(test.TestCase): self.controller.index, req) def test_get_servers_with_bad_option(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -690,7 +691,7 @@ class ServersControllerTest(test.TestCase): self.assertEqual(servers[0]['id'], server_uuid) def test_get_servers_allows_image(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -773,7 +774,7 @@ class ServersControllerTest(test.TestCase): self.assertTrue('servers' in res) def test_get_servers_allows_flavor(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -793,7 +794,7 @@ class ServersControllerTest(test.TestCase): self.assertEqual(servers[0]['id'], server_uuid) def test_get_servers_allows_status(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -825,7 +826,7 @@ class ServersControllerTest(test.TestCase): self.controller.detail, req) def test_get_servers_deleted_status_as_admin(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -845,7 +846,7 @@ class ServersControllerTest(test.TestCase): self.assertEqual(servers[0]['id'], server_uuid) def test_get_servers_allows_name(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -864,7 +865,7 @@ class ServersControllerTest(test.TestCase): self.assertEqual(servers[0]['id'], server_uuid) def test_get_servers_allows_changes_since(self): - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -896,7 +897,7 @@ class ServersControllerTest(test.TestCase): context is not admin. Make sure the admin and unknown options are stripped before they get to compute_api.get_all() """ - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -925,7 +926,7 @@ class ServersControllerTest(test.TestCase): """Test getting servers by admin-only or unknown options when context is admin. All options should be passed """ - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -954,7 +955,7 @@ class ServersControllerTest(test.TestCase): """Test getting servers by ip with admin_api enabled and admin context """ - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -977,7 +978,7 @@ class ServersControllerTest(test.TestCase): """Test getting servers by ip6 with admin_api enabled and admin context """ - server_uuid = str(utils.gen_uuid()) + server_uuid = str(uuid.uuid4()) def fake_get_all(compute_self, context, search_opts=None, sort_key=None, sort_dir='desc', @@ -1700,7 +1701,7 @@ class ServersControllerCreateTest(test.TestCase): fakes.stub_out_key_pair_funcs(self.stubs) fake.stub_out_image_service(self.stubs) fakes.stub_out_nw_api(self.stubs) - self.stubs.Set(utils, 'gen_uuid', fake_gen_uuid) + self.stubs.Set(uuid, 'uuid4', fake_gen_uuid) self.stubs.Set(db, 'instance_add_security_group', return_security_group) self.stubs.Set(db, 'project_get_networks', diff --git a/nova/tests/api/openstack/compute/test_versions.py b/nova/tests/api/openstack/compute/test_versions.py index d59270bd6..2c05d6d8a 100644 --- a/nova/tests/api/openstack/compute/test_versions.py +++ b/nova/tests/api/openstack/compute/test_versions.py @@ -15,6 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +import uuid as uuidutils + import feedparser from lxml import etree import webob @@ -27,7 +29,6 @@ from nova import test from nova.tests.api.openstack import common from nova.tests.api.openstack import fakes from nova.tests import matchers -from nova import utils NS = { @@ -385,7 +386,7 @@ class VersionsTest(test.TestCase): self.assertEqual(res.content_type, "application/json") def test_multi_choice_server(self): - uuid = str(utils.gen_uuid()) + uuid = str(uuidutils.uuid4()) req = webob.Request.blank('/servers/' + uuid) req.accept = "application/json" res = req.get_response(fakes.wsgi_app()) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 4f39e569e..89a78deaa 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -16,6 +16,7 @@ # under the License. import datetime +import uuid import glanceclient.v1.images import routes @@ -44,7 +45,6 @@ from nova.openstack.common import timeutils from nova import quota from nova.tests import fake_network from nova.tests.glance import stubs as glance_stubs -from nova import utils from nova import wsgi @@ -373,7 +373,7 @@ def create_info_cache(nw_cache): def get_fake_uuid(token=0): if not token in FAKE_UUIDS: - FAKE_UUIDS[token] = str(utils.gen_uuid()) + FAKE_UUIDS[token] = str(uuid.uuid4()) return FAKE_UUIDS[token] diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 3b2c00b32..e84fc9175 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -23,6 +23,7 @@ import copy import datetime import sys import time +import uuid import mox @@ -2397,7 +2398,7 @@ class ComputeTestCase(BaseTestCase): def test_add_instance_fault(self): exc_info = None - instance_uuid = str(utils.gen_uuid()) + instance_uuid = str(uuid.uuid4()) def fake_db_fault_create(ctxt, values): self.assertTrue(values['details'].startswith('test')) @@ -2425,7 +2426,7 @@ class ComputeTestCase(BaseTestCase): def test_add_instance_fault_with_remote_error(self): exc_info = None - instance_uuid = str(utils.gen_uuid()) + instance_uuid = str(uuid.uuid4()) def fake_db_fault_create(ctxt, values): self.assertTrue(values['details'].startswith('Remote error')) @@ -2454,7 +2455,7 @@ class ComputeTestCase(BaseTestCase): def test_add_instance_fault_user_error(self): exc_info = None - instance_uuid = str(utils.gen_uuid()) + instance_uuid = str(uuid.uuid4()) def fake_db_fault_create(ctxt, values): @@ -2480,7 +2481,7 @@ class ComputeTestCase(BaseTestCase): user_exc, exc_info) def test_add_instance_fault_no_exc_info(self): - instance_uuid = str(utils.gen_uuid()) + instance_uuid = str(uuid.uuid4()) def fake_db_fault_create(ctxt, values): expected = { @@ -3036,7 +3037,7 @@ class ComputeAPITestCase(BaseTestCase): db.instance_destroy(self.context, refs[0]['uuid']) def test_default_hostname_generator(self): - fake_uuids = [str(utils.gen_uuid()) for x in xrange(4)] + fake_uuids = [str(uuid.uuid4()) for x in xrange(4)] orig_populate = self.compute_api._populate_instance_for_create diff --git a/nova/tests/fake_volume.py b/nova/tests/fake_volume.py index 54fd85fe5..f490b6705 100644 --- a/nova/tests/fake_volume.py +++ b/nova/tests/fake_volume.py @@ -14,10 +14,12 @@ """Implementation of a fake volume API""" +import uuid + from nova import exception from nova.openstack.common import log as logging from nova.openstack.common import timeutils -from nova import utils + LOG = logging.getLogger(__name__) @@ -34,7 +36,7 @@ class fake_volume(): if snapshot is not None: snapshot_id = snapshot['id'] if volume_id is None: - volume_id = str(utils.gen_uuid()) + volume_id = str(uuid.uuid4()) self.vol = { 'created_at': timeutils.utcnow(), 'deleted_at': None, @@ -79,7 +81,7 @@ class fake_snapshot(): def __init__(self, volume_id, size, name, desc, id=None): if id is None: - id = str(utils.gen_uuid()) + id = str(uuid.uuid4()) self.snap = { 'created_at': timeutils.utcnow(), 'deleted_at': None, diff --git a/nova/tests/hyperv/db_fakes.py b/nova/tests/hyperv/db_fakes.py index 9f5572fd1..e240483ea 100644 --- a/nova/tests/hyperv/db_fakes.py +++ b/nova/tests/hyperv/db_fakes.py @@ -19,6 +19,7 @@ Stubouts, mocks and fixtures for the test suite """ import time +import uuid from nova.compute import task_states from nova.compute import vm_states @@ -29,7 +30,7 @@ from nova import utils def get_fake_instance_data(name, project_id, user_id): return {'name': name, 'id': 1, - 'uuid': utils.gen_uuid(), + 'uuid': uuid.uuid4(), 'project_id': project_id, 'user_id': user_id, 'image_ref': "1", @@ -124,7 +125,7 @@ def stub_out_db_instance_api(stubs): base_options = { 'name': values['name'], 'id': values['id'], - 'uuid': utils.gen_uuid(), + 'uuid': uuid.uuid4(), 'reservation_id': utils.generate_uid('r'), 'image_ref': values['image_ref'], 'kernel_id': values['kernel_id'], diff --git a/nova/tests/image/fake.py b/nova/tests/image/fake.py index 1c61bee35..b2953526f 100644 --- a/nova/tests/image/fake.py +++ b/nova/tests/image/fake.py @@ -20,13 +20,14 @@ import copy import datetime +import uuid from nova import config from nova import exception from nova import flags import nova.image.glance from nova.openstack.common import log as logging -from nova import utils + CONF = config.CONF LOG = logging.getLogger(__name__) @@ -176,7 +177,7 @@ class _FakeImageService(object): :raises: Duplicate if the image already exist. """ - image_id = str(metadata.get('id', utils.gen_uuid())) + image_id = str(metadata.get('id', uuid.uuid4())) metadata['id'] = image_id if image_id in self.images: raise exception.Duplicate() diff --git a/nova/tests/integrated/integrated_helpers.py b/nova/tests/integrated/integrated_helpers.py index b1b2c076e..825881137 100644 --- a/nova/tests/integrated/integrated_helpers.py +++ b/nova/tests/integrated/integrated_helpers.py @@ -21,6 +21,7 @@ Provides common functionality for integrated unit tests import random import string +import uuid import nova.image.glance from nova.openstack.common.log import logging @@ -29,7 +30,6 @@ from nova import test # For the flags from nova.tests import fake_crypto import nova.tests.image.fake from nova.tests.integrated.api import client -from nova import utils LOG = logging.getLogger(__name__) @@ -116,7 +116,7 @@ class _IntegratedTestBase(test.TestCase): return generate_new_element(server_names, 'server') def get_invalid_image(self): - return str(utils.gen_uuid()) + return str(uuid.uuid4()) def _build_minimal_create_server_request(self): server = {} diff --git a/nova/tests/network/test_quantumv2.py b/nova/tests/network/test_quantumv2.py index e69058cc0..7c19698fb 100644 --- a/nova/tests/network/test_quantumv2.py +++ b/nova/tests/network/test_quantumv2.py @@ -15,6 +15,8 @@ # # vim: tabstop=4 shiftwidth=4 softtabstop=4 +import uuid + import mox from nova import config @@ -24,9 +26,9 @@ from nova.network import model from nova.network import quantumv2 from nova.network.quantumv2 import api as quantumapi from nova import test -from nova import utils from quantumclient.v2_0 import client + CONF = config.CONF #NOTE: Quantum client raises Exception which is discouraged by HACKING. @@ -133,7 +135,7 @@ class TestQuantumv2(test.TestCase): 'auth_token', 'bff4a5a6b9eb4ea2a6efec6eefb77936') self.instance = {'project_id': '9d049e4b60b64716978ab415e6fbd5c0', - 'uuid': str(utils.gen_uuid()), + 'uuid': str(uuid.uuid4()), 'display_name': 'test_instance', 'security_groups': []} self.nets1 = [{'id': 'my_netid1', diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index 001880483..39d3e1b1f 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -20,6 +20,7 @@ """Unit tests for the DB API""" import datetime +import uuid as uuidutils from nova import config from nova import context @@ -29,7 +30,7 @@ from nova import flags from nova.openstack.common import timeutils from nova import test from nova.tests import matchers -from nova import utils + CONF = config.CONF CONF.import_opt('reserved_host_memory_mb', 'nova.compute.resource_tracker') @@ -145,7 +146,7 @@ class DbApiTestCase(test.TestCase): self.assertRaises(exception.MarkerNotFound, db.instance_get_all_by_filters, self.context, {'display_name': '%test%'}, - marker=str(utils.gen_uuid())) + marker=str(uuidutils.uuid4())) def test_migration_get_unconfirmed_by_dest_compute(self): ctxt = context.get_admin_context() @@ -286,7 +287,7 @@ class DbApiTestCase(test.TestCase): def test_instance_fault_create(self): """Ensure we can create an instance fault""" ctxt = context.get_admin_context() - uuid = str(utils.gen_uuid()) + uuid = str(uuidutils.uuid4()) # Create a fault fault_values = { diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 0fc14c1c7..68e6a6a76 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -31,7 +31,6 @@ from nova import config from nova import exception from nova import flags from nova.openstack.common import timeutils -from nova.openstack.common import uuidutils from nova import test from nova import utils @@ -509,31 +508,6 @@ class GenericUtilsTestCase(test.TestCase): self.assertEquals(h1, h2) -class IsUUIDLikeTestCase(test.TestCase): - def assertUUIDLike(self, val, expected): - result = uuidutils.is_uuid_like(val) - self.assertEqual(result, expected) - - def test_good_uuid(self): - val = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' - self.assertUUIDLike(val, True) - - def test_integer_passed(self): - val = 1 - self.assertUUIDLike(val, False) - - def test_non_uuid_string_passed(self): - val = 'foo-fooo' - self.assertUUIDLike(val, False) - - def test_non_uuid_string_passed2(self): - val = 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' - self.assertUUIDLike(val, False) - - def test_gen_valid_uuid(self): - self.assertUUIDLike(str(utils.gen_uuid()), True) - - class MonkeyPatchTestCase(test.TestCase): """Unit test for utils.monkey_patch().""" def setUp(self): diff --git a/nova/tests/vmwareapi/db_fakes.py b/nova/tests/vmwareapi/db_fakes.py index a469c4706..dd19f4929 100644 --- a/nova/tests/vmwareapi/db_fakes.py +++ b/nova/tests/vmwareapi/db_fakes.py @@ -20,6 +20,7 @@ Stubouts, mocks and fixtures for the test suite """ import time +import uuid from nova.compute import task_states from nova.compute import vm_states @@ -62,7 +63,7 @@ def stub_out_db_instance_api(stubs): base_options = { 'name': values['name'], 'id': values['id'], - 'uuid': utils.gen_uuid(), + 'uuid': uuid.uuid4(), 'reservation_id': utils.generate_uid('r'), 'image_ref': values['image_ref'], 'kernel_id': values['kernel_id'], diff --git a/nova/utils.py b/nova/utils.py index 5f78b93da..7ee58bdd8 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -37,7 +37,6 @@ import struct import sys import tempfile import time -import uuid import weakref from xml.sax import saxutils @@ -776,10 +775,6 @@ def parse_server_string(server_str): return ('', '') -def gen_uuid(): - return uuid.uuid4() - - def bool_from_str(val): """Convert a string representation of a bool into a bool value""" -- cgit From 9fdf7552779d518af9cda4e366bf81fddb0cb6f2 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 14 Nov 2012 17:25:43 +0000 Subject: Make sure instance data is always refreshed Fixes bug 1078793 When updating an instance's instance_type_id, the 'instance_type' joined to the instance is not updated. This updates that. A DB API test is added that failed before and passes now. Also: Some cases of passing stale instance data within resize_instance() and finish_resize() were found and addressed. Change-Id: If335cc286a71597d3100425080e51c75aeec7a50 --- nova/compute/manager.py | 18 +++++++++--------- nova/db/sqlalchemy/api.py | 7 +++++++ nova/tests/test_db_api.py | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index be20c7a40..a5084d6fe 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1837,9 +1837,9 @@ class ComputeManager(manager.SchedulerDependentManager): migration['id'], {'status': 'migrating'}) - self._instance_update(context, instance['uuid'], - task_state=task_states.RESIZE_MIGRATING, - expected_task_state=task_states.RESIZE_PREP) + instance = self._instance_update(context, instance['uuid'], + task_state=task_states.RESIZE_MIGRATING, + expected_task_state=task_states.RESIZE_PREP) self._notify_about_instance_usage( context, instance, "resize.start", network_info=network_info) @@ -1861,11 +1861,11 @@ class ComputeManager(manager.SchedulerDependentManager): migration['id'], {'status': 'post-migrating'}) - self._instance_update(context, instance['uuid'], - host=migration['dest_compute'], - task_state=task_states.RESIZE_MIGRATED, - expected_task_state=task_states. - RESIZE_MIGRATING) + instance = self._instance_update(context, instance['uuid'], + host=migration['dest_compute'], + task_state=task_states.RESIZE_MIGRATED, + expected_task_state=task_states. + RESIZE_MIGRATING) self.compute_rpcapi.finish_resize(context, instance, migration, image, disk_info, @@ -1910,7 +1910,7 @@ class ComputeManager(manager.SchedulerDependentManager): network_info = self._get_instance_nw_info(context, instance) - self._instance_update(context, instance['uuid'], + instance = self._instance_update(context, instance['uuid'], task_state=task_states.RESIZE_FINISH, expected_task_state=task_states.RESIZE_MIGRATED) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 007e83cbe..3cba27c1e 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1813,6 +1813,13 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False): instance_ref.update(values) instance_ref.save(session=session) + if 'instance_type_id' in values: + # NOTE(comstud): It appears that sqlalchemy doesn't refresh + # the instance_type model after you update the ID. You end + # up with an instance_type model that only has 'id' updated, + # but the rest of the model has the data from the old + # instance_type. + session.refresh(instance_ref['instance_type']) return (old_instance_ref, instance_ref) diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index 39d3e1b1f..31d1b01b2 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -272,6 +272,26 @@ class DbApiTestCase(test.TestCase): system_meta = db.instance_system_metadata_get(ctxt, instance.uuid) self.assertEqual('baz', system_meta['original_image_ref']) + def test_instance_update_of_instance_type_id(self): + ctxt = context.get_admin_context() + + inst_type1 = db.instance_type_get_by_name(ctxt, 'm1.tiny') + inst_type2 = db.instance_type_get_by_name(ctxt, 'm1.small') + + values = {'instance_type_id': inst_type1['id']} + instance = db.instance_create(ctxt, values) + + self.assertEqual(instance['instance_type']['id'], inst_type1['id']) + self.assertEqual(instance['instance_type']['name'], + inst_type1['name']) + + values = {'instance_type_id': inst_type2['id']} + instance = db.instance_update(ctxt, instance['uuid'], values) + + self.assertEqual(instance['instance_type']['id'], inst_type2['id']) + self.assertEqual(instance['instance_type']['name'], + inst_type2['name']) + def test_instance_update_with_and_get_original(self): ctxt = context.get_admin_context() -- cgit From 56778928303b74112a83e9208f107b9fa06f12e7 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 12 Nov 2012 23:33:01 +0000 Subject: Add module for loading specific classes This adds nova/loadables.py which contains code that is to be shared by host and cell scheduling filtering and weighing. Most of this code originated from nova/scheduler/filters/__init__.py (which was copied from nova/api extension loading). This makes it more generic so that it can be shared. Note that this functionality is quite different than the generic plugin manager that exists in openstack-common. That code could not be used here without some significant changes. It also seems pretty overkill for the functionality required by scheduling filtering and weighing. Change-Id: I1b217dc2bc2d1dc2235c8251318d06b46597e8f4 --- nova/loadables.py | 116 ++++++++++++++++++++++++++++ nova/tests/fake_loadables/__init__.py | 27 +++++++ nova/tests/fake_loadables/fake_loadable1.py | 44 +++++++++++ nova/tests/fake_loadables/fake_loadable2.py | 39 ++++++++++ nova/tests/test_loadables.py | 113 +++++++++++++++++++++++++++ 5 files changed, 339 insertions(+) create mode 100644 nova/loadables.py create mode 100644 nova/tests/fake_loadables/__init__.py create mode 100644 nova/tests/fake_loadables/fake_loadable1.py create mode 100644 nova/tests/fake_loadables/fake_loadable2.py create mode 100644 nova/tests/test_loadables.py diff --git a/nova/loadables.py b/nova/loadables.py new file mode 100644 index 000000000..0c930267e --- /dev/null +++ b/nova/loadables.py @@ -0,0 +1,116 @@ +# Copyright (c) 2011-2012 OpenStack, LLC. +# 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. + +""" +Generic Loadable class support. + +Meant to be used by such things as scheduler filters and weights where we +want to load modules from certain directories and find certain types of +classes within those modules. Note that this is quite different than +generic plugins and the pluginmanager code that exists elsewhere. + +Usage: + +Create a directory with an __init__.py with code such as: + +class SomeLoadableClass(object): + pass + + +class MyLoader(nova.loadables.BaseLoader) + def __init__(self): + super(MyLoader, self).__init__(SomeLoadableClass) + +If you create modules in the same directory and subclass SomeLoadableClass +within them, MyLoader().get_all_classes() will return a list +of such classes. +""" + +import inspect +import os +import sys + +from nova import exception +from nova.openstack.common import importutils + + +class BaseLoader(object): + def __init__(self, loadable_cls_type): + mod = sys.modules[self.__class__.__module__] + self.path = mod.__path__[0] + self.package = mod.__package__ + self.loadable_cls_type = loadable_cls_type + + def _is_correct_class(self, obj): + """Return whether an object is a class of the correct type and + is not prefixed with an underscore. + """ + return (inspect.isclass(obj) and + (not obj.__name__.startswith('_')) and + issubclass(obj, self.loadable_cls_type)) + + def _get_classes_from_module(self, module_name): + """Get the classes from a module that match the type we want.""" + classes = [] + module = importutils.import_module(module_name) + for obj_name in dir(module): + # Skip objects that are meant to be private. + if obj_name.startswith('_'): + continue + itm = getattr(module, obj_name) + if self._is_correct_class(itm): + classes.append(itm) + return classes + + def get_all_classes(self): + """Get the classes of the type we want from all modules found + in the directory that defines this class. + """ + classes = [] + for dirpath, dirnames, filenames in os.walk(self.path): + relpath = os.path.relpath(dirpath, self.path) + if relpath == '.': + relpkg = '' + else: + relpkg = '.%s' % '.'.join(relpath.split(os.sep)) + for fname in filenames: + root, ext = os.path.splitext(fname) + if ext != '.py' or root == '__init__': + continue + module_name = "%s%s.%s" % (self.package, relpkg, root) + mod_classes = self._get_classes_from_module(module_name) + classes.extend(mod_classes) + return classes + + def get_matching_classes(self, loadable_class_names): + """Get loadable classes from a list of names. Each name can be + a full module path or the full path to a method that returns + classes to use. The latter behavior is useful to specify a method + that returns a list of classes to use in a default case. + """ + classes = [] + for cls_name in loadable_class_names: + obj = importutils.import_class(cls_name) + if self._is_correct_class(obj): + classes.append(obj) + elif inspect.isfunction(obj): + # Get list of classes from a function + for cls in obj(): + classes.append(cls) + else: + error_str = 'Not a class of the correct type' + raise exception.ClassNotFound(class_name=cls_name, + exception=error_str) + return classes diff --git a/nova/tests/fake_loadables/__init__.py b/nova/tests/fake_loadables/__init__.py new file mode 100644 index 000000000..824243347 --- /dev/null +++ b/nova/tests/fake_loadables/__init__.py @@ -0,0 +1,27 @@ +# Copyright 2012 OpenStack LLC. # 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. +""" +Fakes For Loadable class handling. +""" + +from nova import loadables + + +class FakeLoadable(object): + pass + + +class FakeLoader(loadables.BaseLoader): + def __init__(self): + super(FakeLoader, self).__init__(FakeLoadable) diff --git a/nova/tests/fake_loadables/fake_loadable1.py b/nova/tests/fake_loadables/fake_loadable1.py new file mode 100644 index 000000000..58f9704b3 --- /dev/null +++ b/nova/tests/fake_loadables/fake_loadable1.py @@ -0,0 +1,44 @@ +# Copyright 2012 OpenStack LLC. # 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. +""" +Fake Loadable subclasses module #1 +""" + +from nova.tests import fake_loadables + + +class FakeLoadableSubClass1(fake_loadables.FakeLoadable): + pass + + +class FakeLoadableSubClass2(fake_loadables.FakeLoadable): + pass + + +class _FakeLoadableSubClass3(fake_loadables.FakeLoadable): + """Classes beginning with '_' will be ignored.""" + pass + + +class FakeLoadableSubClass4(object): + """Not a correct subclass.""" + + +def return_valid_classes(): + return [FakeLoadableSubClass1, FakeLoadableSubClass2] + + +def return_invalid_classes(): + return [FakeLoadableSubClass1, _FakeLoadableSubClass3, + FakeLoadableSubClass4] diff --git a/nova/tests/fake_loadables/fake_loadable2.py b/nova/tests/fake_loadables/fake_loadable2.py new file mode 100644 index 000000000..3e365effc --- /dev/null +++ b/nova/tests/fake_loadables/fake_loadable2.py @@ -0,0 +1,39 @@ +# Copyright 2012 OpenStack LLC. # 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. +""" +Fake Loadable subclasses module #2 +""" + +from nova.tests import fake_loadables + + +class FakeLoadableSubClass5(fake_loadables.FakeLoadable): + pass + + +class FakeLoadableSubClass6(fake_loadables.FakeLoadable): + pass + + +class _FakeLoadableSubClass7(fake_loadables.FakeLoadable): + """Classes beginning with '_' will be ignored.""" + pass + + +class FakeLoadableSubClass8(BaseException): + """Not a correct subclass.""" + + +def return_valid_class(): + return [FakeLoadableSubClass6] diff --git a/nova/tests/test_loadables.py b/nova/tests/test_loadables.py new file mode 100644 index 000000000..6d16b9fa8 --- /dev/null +++ b/nova/tests/test_loadables.py @@ -0,0 +1,113 @@ +# Copyright 2012 OpenStack LLC. # 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. +""" +Tests For Loadable class handling. +""" + +from nova import exception +from nova import test +from nova.tests import fake_loadables + + +class LoadablesTestCase(test.TestCase): + def setUp(self): + super(LoadablesTestCase, self).setUp() + self.fake_loader = fake_loadables.FakeLoader() + # The name that we imported above for testing + self.test_package = 'nova.tests.fake_loadables' + + def test_loader_init(self): + self.assertEqual(self.fake_loader.package, self.test_package) + # Test the path of the module + ending_path = '/' + self.test_package.replace('.', '/') + self.assertTrue(self.fake_loader.path.endswith(ending_path)) + self.assertEqual(self.fake_loader.loadable_cls_type, + fake_loadables.FakeLoadable) + + def _compare_classes(self, classes, expected): + class_names = [cls.__name__ for cls in classes] + self.assertEqual(set(class_names), set(expected)) + + def test_get_all_classes(self): + classes = self.fake_loader.get_all_classes() + expected_class_names = ['FakeLoadableSubClass1', + 'FakeLoadableSubClass2', + 'FakeLoadableSubClass5', + 'FakeLoadableSubClass6'] + self._compare_classes(classes, expected_class_names) + + def test_get_matching_classes(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.FakeLoadableSubClass1', + prefix + '.fake_loadable2.FakeLoadableSubClass5'] + classes = self.fake_loader.get_matching_classes(test_classes) + expected_class_names = ['FakeLoadableSubClass1', + 'FakeLoadableSubClass5'] + self._compare_classes(classes, expected_class_names) + + def test_get_matching_classes_with_underscore(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.FakeLoadableSubClass1', + prefix + '.fake_loadable2._FakeLoadableSubClass7'] + self.assertRaises(exception.ClassNotFound, + self.fake_loader.get_matching_classes, + test_classes) + + def test_get_matching_classes_with_wrong_type1(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.FakeLoadableSubClass4', + prefix + '.fake_loadable2.FakeLoadableSubClass5'] + self.assertRaises(exception.ClassNotFound, + self.fake_loader.get_matching_classes, + test_classes) + + def test_get_matching_classes_with_wrong_type2(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.FakeLoadableSubClass1', + prefix + '.fake_loadable2.FakeLoadableSubClass8'] + self.assertRaises(exception.ClassNotFound, + self.fake_loader.get_matching_classes, + test_classes) + + def test_get_matching_classes_with_one_function(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.return_valid_classes', + prefix + '.fake_loadable2.FakeLoadableSubClass5'] + classes = self.fake_loader.get_matching_classes(test_classes) + expected_class_names = ['FakeLoadableSubClass1', + 'FakeLoadableSubClass2', + 'FakeLoadableSubClass5'] + self._compare_classes(classes, expected_class_names) + + def test_get_matching_classes_with_two_functions(self): + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.return_valid_classes', + prefix + '.fake_loadable2.return_valid_class'] + classes = self.fake_loader.get_matching_classes(test_classes) + expected_class_names = ['FakeLoadableSubClass1', + 'FakeLoadableSubClass2', + 'FakeLoadableSubClass6'] + self._compare_classes(classes, expected_class_names) + + def test_get_matching_classes_with_function_including_invalids(self): + # When using a method, no checking is done on valid classes. + prefix = self.test_package + test_classes = [prefix + '.fake_loadable1.return_invalid_classes', + prefix + '.fake_loadable2.return_valid_class'] + classes = self.fake_loader.get_matching_classes(test_classes) + expected_class_names = ['FakeLoadableSubClass1', + '_FakeLoadableSubClass3', + 'FakeLoadableSubClass4', + 'FakeLoadableSubClass6'] + self._compare_classes(classes, expected_class_names) -- cgit From 0d9ce8319d75bfbd69fe4e0759aeed38e6054e56 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Fri, 9 Nov 2012 01:57:42 +0000 Subject: Refactor scheduling filters This changes the scheduling filters to use the code in nova/loadables.py to locate filtering classes. This also creates some base functionality in nova/filters.py to be used by both the host scheduler and the cells scheduler. The scheduler_available_filters default has changed to 'nova.scheduler.filters.all_filters' which is better named compared to the old setting of 'standard_filters'. The old method is still supported for those that have put it explicitly in their configs, but it's marked as deprecated. DocImpact Change-Id: I84fdeafdba0275ab4b25f8857563bd7b1494bb69 --- nova/filters.py | 53 ++++++ nova/scheduler/filter_scheduler.py | 2 +- nova/scheduler/filters/__init__.py | 76 +++----- nova/scheduler/host_manager.py | 86 ++++----- nova/tests/scheduler/test_filter_scheduler.py | 6 +- nova/tests/scheduler/test_host_filters.py | 45 ++--- nova/tests/scheduler/test_host_manager.py | 249 ++++++++++++++------------ nova/tests/test_filters.py | 125 +++++++++++++ 8 files changed, 405 insertions(+), 237 deletions(-) create mode 100644 nova/filters.py create mode 100644 nova/tests/test_filters.py diff --git a/nova/filters.py b/nova/filters.py new file mode 100644 index 000000000..a3339eff8 --- /dev/null +++ b/nova/filters.py @@ -0,0 +1,53 @@ +# Copyright (c) 2011-2012 OpenStack, LLC. +# 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. + +""" +Filter support +""" + +from nova import loadables + + +class BaseFilter(object): + """Base class for all filter classes.""" + def _filter_one(self, obj, filter_properties): + """Return True if it passes the filter, False otherwise. + Override this in a subclass. + """ + return True + + def filter_all(self, filter_obj_list, filter_properties): + """Yield objects that pass the filter. + + Can be overriden in a subclass, if you need to base filtering + decisions on all objects. Otherwise, one can just override + _filter_one() to filter a single object. + """ + for obj in filter_obj_list: + if self._filter_one(obj, filter_properties): + yield obj + + +class BaseFilterHandler(loadables.BaseLoader): + """Base class to handle loading filter classes. + + This class should be subclassed where one needs to use filters. + """ + + def get_filtered_objects(self, filter_classes, objs, + filter_properties): + for filter_cls in filter_classes: + objs = filter_cls().filter_all(objs, filter_properties) + return list(objs) diff --git a/nova/scheduler/filter_scheduler.py b/nova/scheduler/filter_scheduler.py index 3fdea1d59..10dc7e37c 100644 --- a/nova/scheduler/filter_scheduler.py +++ b/nova/scheduler/filter_scheduler.py @@ -268,7 +268,7 @@ class FilterScheduler(driver.Scheduler): num_instances = request_spec.get('num_instances', 1) for num in xrange(num_instances): # Filter local hosts based on requirements ... - hosts = self.host_manager.filter_hosts(hosts, + hosts = self.host_manager.get_filtered_hosts(hosts, filter_properties) if not hosts: # Can't get any more locally. diff --git a/nova/scheduler/filters/__init__.py b/nova/scheduler/filters/__init__.py index 2056f968e..6e8e7ea7b 100644 --- a/nova/scheduler/filters/__init__.py +++ b/nova/scheduler/filters/__init__.py @@ -17,71 +17,41 @@ Scheduler host filters """ -import os -import types +from nova import filters +from nova.openstack.common import log as logging -from nova import exception -from nova.openstack.common import importutils +LOG = logging.getLogger(__name__) -class BaseHostFilter(object): +class BaseHostFilter(filters.BaseFilter): """Base class for host filters.""" + def _filter_one(self, obj, filter_properties): + """Return True if the object passes the filter, otherwise False.""" + return self.host_passes(obj, filter_properties) def host_passes(self, host_state, filter_properties): + """Return True if the HostState passes the filter, otherwise False. + Override this in a subclass. + """ raise NotImplementedError() - def _full_name(self): - """module.classname of the filter.""" - return "%s.%s" % (self.__module__, self.__class__.__name__) +class HostFilterHandler(filters.BaseFilterHandler): + def __init__(self): + super(HostFilterHandler, self).__init__(BaseHostFilter) -def _is_filter_class(cls): - """Return whether a class is a valid Host Filter class.""" - return type(cls) is types.TypeType and issubclass(cls, BaseHostFilter) +def all_filters(): + """Return a list of filter classes found in this directory. -def _get_filter_classes_from_module(module_name): - """Get all filter classes from a module.""" - classes = [] - module = importutils.import_module(module_name) - for obj_name in dir(module): - itm = getattr(module, obj_name) - if _is_filter_class(itm): - classes.append(itm) - return classes + This method is used as the default for available scheduler filters + and should return a list of all filter classes available. + """ + return HostFilterHandler().get_all_classes() def standard_filters(): - """Return a list of filter classes found in this directory.""" - classes = [] - filters_dir = __path__[0] - for dirpath, dirnames, filenames in os.walk(filters_dir): - relpath = os.path.relpath(dirpath, filters_dir) - if relpath == '.': - relpkg = '' - else: - relpkg = '.%s' % '.'.join(relpath.split(os.sep)) - for fname in filenames: - root, ext = os.path.splitext(fname) - if ext != '.py' or root == '__init__': - continue - module_name = "%s%s.%s" % (__package__, relpkg, root) - mod_classes = _get_filter_classes_from_module(module_name) - classes.extend(mod_classes) - return classes - - -def get_filter_classes(filter_class_names): - """Get filter classes from class names.""" - classes = [] - for cls_name in filter_class_names: - obj = importutils.import_class(cls_name) - if _is_filter_class(obj): - classes.append(obj) - elif type(obj) is types.FunctionType: - # Get list of classes from a function - classes.extend(obj()) - else: - raise exception.ClassNotFound(class_name=cls_name, - exception='Not a valid scheduler filter') - return classes + """Deprecated. Configs should change to use all_filters().""" + LOG.deprecated(_("Use 'nova.scheduler.filters.all_filters' instead " + "of 'nova.scheduler.filters.standard_filters'")) + return all_filters() diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py index 2d6c25e9b..e93b529f9 100644 --- a/nova/scheduler/host_manager.py +++ b/nova/scheduler/host_manager.py @@ -31,7 +31,7 @@ from nova.scheduler import filters host_manager_opts = [ cfg.MultiStrOpt('scheduler_available_filters', - default=['nova.scheduler.filters.standard_filters'], + default=['nova.scheduler.filters.all_filters'], help='Filter classes available to the scheduler which may ' 'be specified more than once. An entry of ' '"nova.scheduler.filters.standard_filters" ' @@ -239,32 +239,6 @@ class HostState(object): def _statmap(self, stats): return dict((st['key'], st['value']) for st in stats) - def passes_filters(self, filter_fns, filter_properties): - """Return whether or not this host passes filters.""" - - if self.host in filter_properties.get('ignore_hosts', []): - LOG.debug(_('Host filter fails for ignored host %(host)s'), - {'host': self.host}) - return False - - force_hosts = filter_properties.get('force_hosts', []) - if force_hosts: - if not self.host in force_hosts: - LOG.debug(_('Host filter fails for non-forced host %(host)s'), - {'host': self.host}) - return self.host in force_hosts - - for filter_fn in filter_fns: - if not filter_fn(self, filter_properties): - LOG.debug(_('Host filter function %(func)s failed for ' - '%(host)s'), - {'func': repr(filter_fn), - 'host': self.host}) - return False - - LOG.debug(_('Host filter passes for %(host)s'), {'host': self.host}) - return True - def __repr__(self): return ("(%s, %s) ram:%s disk:%s io_ops:%s instances:%s vm_type:%s" % (self.host, self.nodename, self.free_ram_mb, self.free_disk_mb, @@ -281,32 +255,28 @@ class HostManager(object): # { (host, hypervisor_hostname) : { : { cap k : v }}} self.service_states = {} self.host_state_map = {} - self.filter_classes = filters.get_filter_classes( + self.filter_handler = filters.HostFilterHandler() + self.filter_classes = self.filter_handler.get_matching_classes( CONF.scheduler_available_filters) - def _choose_host_filters(self, filters): + def _choose_host_filters(self, filter_cls_names): """Since the caller may specify which filters to use we need to have an authoritative list of what is permissible. This function checks the filter names against a predefined set of acceptable filters. """ - if filters is None: - filters = CONF.scheduler_default_filters - if not isinstance(filters, (list, tuple)): - filters = [filters] + if filter_cls_names is None: + filter_cls_names = CONF.scheduler_default_filters + if not isinstance(filter_cls_names, (list, tuple)): + filter_cls_names = [filter_cls_names] good_filters = [] bad_filters = [] - for filter_name in filters: + for filter_name in filter_cls_names: found_class = False for cls in self.filter_classes: if cls.__name__ == filter_name: + good_filters.append(cls) found_class = True - filter_instance = cls() - # Get the filter function - filter_func = getattr(filter_instance, - 'host_passes', None) - if filter_func: - good_filters.append(filter_func) break if not found_class: bad_filters.append(filter_name) @@ -315,14 +285,36 @@ class HostManager(object): raise exception.SchedulerHostFilterNotFound(filter_name=msg) return good_filters - def filter_hosts(self, hosts, filter_properties, filters=None): + def get_filtered_hosts(self, hosts, filter_properties, + filter_class_names=None): """Filter hosts and return only ones passing all filters""" - filtered_hosts = [] - filter_fns = self._choose_host_filters(filters) - for host in hosts: - if host.passes_filters(filter_fns, filter_properties): - filtered_hosts.append(host) - return filtered_hosts + filter_classes = self._choose_host_filters(filter_class_names) + + hosts = set(hosts) + ignore_hosts = set(filter_properties.get('ignore_hosts', [])) + ignore_hosts = hosts & ignore_hosts + if ignore_hosts: + ignored_hosts = ', '.join(ignore_hosts) + msg = _('Host filter ignoring hosts: %(ignored_hosts)s') + LOG.debug(msg, locals()) + hosts = hosts - ignore_hosts + + force_hosts = set(filter_properties.get('force_hosts', [])) + if force_hosts: + matching_force_hosts = hosts & force_hosts + if not matching_force_hosts: + forced_hosts = ', '.join(force_hosts) + msg = _("No hosts matched due to not matching 'force_hosts'" + "value of '%(forced_hosts)s'") + LOG.debug(msg, locals()) + return [] + forced_hosts = ', '.join(matching_force_hosts) + msg = _('Host filter forcing available hosts to %(forced_hosts)s') + LOG.debug(msg, locals()) + hosts = matching_force_hosts + + return self.filter_handler.get_filtered_objects(filter_classes, + hosts, filter_properties) def update_service_capabilities(self, service_name, host, capabilities): """Update the per-service capabilities based on this notification.""" diff --git a/nova/tests/scheduler/test_filter_scheduler.py b/nova/tests/scheduler/test_filter_scheduler.py index 6add77cbe..b8ab45bc2 100644 --- a/nova/tests/scheduler/test_filter_scheduler.py +++ b/nova/tests/scheduler/test_filter_scheduler.py @@ -32,7 +32,7 @@ from nova.tests.scheduler import fakes from nova.tests.scheduler import test_scheduler -def fake_filter_hosts(hosts, filter_properties): +def fake_get_filtered_hosts(hosts, filter_properties): return list(hosts) @@ -155,8 +155,8 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context = context.RequestContext('user', 'project', is_admin=True) - self.stubs.Set(sched.host_manager, 'filter_hosts', - fake_filter_hosts) + self.stubs.Set(sched.host_manager, 'get_filtered_hosts', + fake_get_filtered_hosts) self.stubs.Set(least_cost, 'weighted_sum', _fake_weighted_sum) fakes.mox_host_manager_db_calls(self.mox, fake_context) diff --git a/nova/tests/scheduler/test_host_filters.py b/nova/tests/scheduler/test_host_filters.py index f19755b82..1f35ede9b 100644 --- a/nova/tests/scheduler/test_host_filters.py +++ b/nova/tests/scheduler/test_host_filters.py @@ -263,32 +263,33 @@ class HostFiltersTestCase(test.TestCase): self.json_query = jsonutils.dumps( ['and', ['>=', '$free_ram_mb', 1024], ['>=', '$free_disk_mb', 200 * 1024]]) - # This has a side effect of testing 'get_filter_classes' - # when specifying a method (in this case, our standard filters) - classes = filters.get_filter_classes( - ['nova.scheduler.filters.standard_filters']) + filter_handler = filters.HostFilterHandler() + classes = filter_handler.get_matching_classes( + ['nova.scheduler.filters.all_filters']) self.class_map = {} for cls in classes: self.class_map[cls.__name__] = cls - def test_get_filter_classes(self): - classes = filters.get_filter_classes( - ['nova.tests.scheduler.test_host_filters.TestFilter']) - self.assertEqual(len(classes), 1) - self.assertEqual(classes[0].__name__, 'TestFilter') - # Test a specific class along with our standard filters - classes = filters.get_filter_classes( - ['nova.tests.scheduler.test_host_filters.TestFilter', - 'nova.scheduler.filters.standard_filters']) - self.assertEqual(len(classes), 1 + len(self.class_map)) - - def test_get_filter_classes_raises_on_invalid_classes(self): - self.assertRaises(ImportError, - filters.get_filter_classes, - ['nova.tests.scheduler.test_host_filters.NoExist']) - self.assertRaises(exception.ClassNotFound, - filters.get_filter_classes, - ['nova.tests.scheduler.test_host_filters.TestBogusFilter']) + def test_standard_filters_is_deprecated(self): + info = {'called': False} + + def _fake_deprecated(*args, **kwargs): + info['called'] = True + + self.stubs.Set(filters.LOG, 'deprecated', _fake_deprecated) + + filter_handler = filters.HostFilterHandler() + filter_handler.get_matching_classes( + ['nova.scheduler.filters.standard_filters']) + + self.assertTrue(info['called']) + self.assertIn('AllHostsFilter', self.class_map) + self.assertIn('ComputeFilter', self.class_map) + + def test_all_filters(self): + # Double check at least a couple of known filters exist + self.assertIn('AllHostsFilter', self.class_map) + self.assertIn('ComputeFilter', self.class_map) def test_all_host_filter(self): filt_cls = self.class_map['AllHostsFilter']() diff --git a/nova/tests/scheduler/test_host_manager.py b/nova/tests/scheduler/test_host_manager.py index 0984cbf80..d12f1dea5 100644 --- a/nova/tests/scheduler/test_host_manager.py +++ b/nova/tests/scheduler/test_host_manager.py @@ -62,34 +62,146 @@ class HostManagerTestCase(test.TestCase): ComputeFilterClass2] # Test we returns 1 correct function - filter_fns = self.host_manager._choose_host_filters(None) - self.assertEqual(len(filter_fns), 1) - self.assertEqual(filter_fns[0].__func__, - ComputeFilterClass2.host_passes.__func__) - - def test_filter_hosts(self): - filters = ['fake-filter1', 'fake-filter2'] - fake_host1 = host_manager.HostState('host1', 'node1') - fake_host2 = host_manager.HostState('host2', 'node2') - hosts = [fake_host1, fake_host2] - filter_properties = {'fake_prop': 'fake_val'} - - self.mox.StubOutWithMock(self.host_manager, - '_choose_host_filters') - self.mox.StubOutWithMock(fake_host1, 'passes_filters') - self.mox.StubOutWithMock(fake_host2, 'passes_filters') - - self.host_manager._choose_host_filters(None).AndReturn(filters) - fake_host1.passes_filters(filters, filter_properties).AndReturn( - False) - fake_host2.passes_filters(filters, filter_properties).AndReturn( - True) + filter_classes = self.host_manager._choose_host_filters(None) + self.assertEqual(len(filter_classes), 1) + self.assertEqual(filter_classes[0].__name__, 'ComputeFilterClass2') + + def test_get_filtered_hosts(self): + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1'] + fake_classes = 'fake_classes' + fake_properties = {'moo': 1, 'cow': 2} + expected_hosts = set(fake_hosts) + fake_result = 'fake_result' + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + + self.host_manager._choose_host_filters(None).AndReturn(fake_classes) + self.host_manager.filter_handler.get_filtered_objects(fake_classes, + expected_hosts, fake_properties).AndReturn(fake_result) self.mox.ReplayAll() - filtered_hosts = self.host_manager.filter_hosts(hosts, - filter_properties, filters=None) - self.assertEqual(len(filtered_hosts), 1) - self.assertEqual(filtered_hosts[0], fake_host2) + + result = self.host_manager. get_filtered_hosts(fake_hosts, + fake_properties) + self.assertEqual(result, fake_result) + + def test_get_filtered_hosts_with_specificed_filters(self): + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1'] + fake_classes = 'fake_classes' + fake_properties = {'moo': 1, 'cow': 2} + fake_filters = 'fake_filters' + expected_hosts = set(fake_hosts) + fake_result = 'fake_result' + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + + self.host_manager._choose_host_filters(fake_filters).AndReturn( + fake_classes) + self.host_manager.filter_handler.get_filtered_objects(fake_classes, + expected_hosts, fake_properties).AndReturn(fake_result) + + self.mox.ReplayAll() + + result = self.host_manager.get_filtered_hosts(fake_hosts, + fake_properties, filter_class_names=fake_filters) + self.assertEqual(result, fake_result) + + def test_get_filtered_hosts_with_ignore(self): + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1', 'fake_host3', 'fake_host4'] + fake_classes = 'fake_classes' + fake_properties = {'ignore_hosts': ['fake_host1', 'fake_host3', + 'fake_host5']} + expected_hosts = set(['fake_host2', 'fake_host4']) + fake_result = 'fake_result' + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + + self.host_manager._choose_host_filters(None).AndReturn(fake_classes) + self.host_manager.filter_handler.get_filtered_objects(fake_classes, + expected_hosts, fake_properties).AndReturn(fake_result) + + self.mox.ReplayAll() + + result = self.host_manager.get_filtered_hosts(fake_hosts, + fake_properties) + self.assertEqual(result, fake_result) + + def test_get_filtered_hosts_with_force_hosts(self): + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1', 'fake_host3', 'fake_host4'] + fake_classes = 'fake_classes' + fake_properties = {'force_hosts': ['fake_host1', 'fake_host3', + 'fake_host5']} + expected_hosts = set(['fake_host1', 'fake_host3']) + fake_result = 'fake_result' + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + + self.host_manager._choose_host_filters(None).AndReturn(fake_classes) + self.host_manager.filter_handler.get_filtered_objects(fake_classes, + expected_hosts, fake_properties).AndReturn(fake_result) + + self.mox.ReplayAll() + + result = self.host_manager.get_filtered_hosts(fake_hosts, + fake_properties) + self.assertEqual(result, fake_result) + + def test_get_filtered_hosts_with_no_matching_force_hosts(self): + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1', 'fake_host3', 'fake_host4'] + fake_classes = 'fake_classes' + fake_properties = {'force_hosts': ['fake_host5', 'fake_host6']} + expected_result = [] + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + # Make sure this is not called. + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + + self.host_manager._choose_host_filters(None).AndReturn(fake_classes) + + self.mox.ReplayAll() + + result = self.host_manager.get_filtered_hosts(fake_hosts, + fake_properties) + self.assertEqual(result, expected_result) + + def test_get_filtered_hosts_with_ignore_and_force(self): + """Ensure ignore_hosts processed before force_hosts in host filters""" + fake_hosts = ['fake_host1', 'fake_host2', 'fake_host1', + 'fake_host1', 'fake_host3', 'fake_host4'] + fake_classes = 'fake_classes' + fake_properties = {'force_hosts': ['fake_host3', 'fake_host1'], + 'ignore_hosts': ['fake_host1']} + expected_hosts = set(['fake_host3']) + fake_result = 'fake_result' + + self.mox.StubOutWithMock(self.host_manager, '_choose_host_filters') + # Make sure this is not called. + self.mox.StubOutWithMock(self.host_manager.filter_handler, + 'get_filtered_objects') + self.host_manager.filter_handler.get_filtered_objects(fake_classes, + expected_hosts, fake_properties).AndReturn(fake_result) + + self.host_manager._choose_host_filters(None).AndReturn(fake_classes) + + self.mox.ReplayAll() + + result = self.host_manager.get_filtered_hosts(fake_hosts, + fake_properties) + self.assertEqual(result, fake_result) def test_update_service_capabilities(self): service_states = self.host_manager.service_states @@ -190,91 +302,6 @@ class HostStateTestCase(test.TestCase): # update_from_compute_node() and consume_from_instance() are tested # in HostManagerTestCase.test_get_all_host_states() - def test_host_state_passes_filters_passes(self): - fake_host = host_manager.HostState('host1', 'node1') - filter_properties = {} - - cls1 = ComputeFilterClass1() - cls2 = ComputeFilterClass2() - self.mox.StubOutWithMock(cls1, 'host_passes') - self.mox.StubOutWithMock(cls2, 'host_passes') - filter_fns = [cls1.host_passes, cls2.host_passes] - - cls1.host_passes(fake_host, filter_properties).AndReturn(True) - cls2.host_passes(fake_host, filter_properties).AndReturn(True) - - self.mox.ReplayAll() - result = fake_host.passes_filters(filter_fns, filter_properties) - self.assertTrue(result) - - def test_host_state_passes_filters_passes_with_ignore(self): - fake_host = host_manager.HostState('host1', 'node1') - filter_properties = {'ignore_hosts': ['host2']} - - cls1 = ComputeFilterClass1() - cls2 = ComputeFilterClass2() - self.mox.StubOutWithMock(cls1, 'host_passes') - self.mox.StubOutWithMock(cls2, 'host_passes') - filter_fns = [cls1.host_passes, cls2.host_passes] - - cls1.host_passes(fake_host, filter_properties).AndReturn(True) - cls2.host_passes(fake_host, filter_properties).AndReturn(True) - - self.mox.ReplayAll() - result = fake_host.passes_filters(filter_fns, filter_properties) - self.assertTrue(result) - - def test_host_state_passes_filters_fails(self): - fake_host = host_manager.HostState('host1', 'node1') - filter_properties = {} - - cls1 = ComputeFilterClass1() - cls2 = ComputeFilterClass2() - self.mox.StubOutWithMock(cls1, 'host_passes') - self.mox.StubOutWithMock(cls2, 'host_passes') - filter_fns = [cls1.host_passes, cls2.host_passes] - - cls1.host_passes(fake_host, filter_properties).AndReturn(False) - # cls2.host_passes() not called because of short circuit - - self.mox.ReplayAll() - result = fake_host.passes_filters(filter_fns, filter_properties) - self.assertFalse(result) - - def test_host_state_passes_filters_fails_from_ignore(self): - fake_host = host_manager.HostState('host1', 'node1') - filter_properties = {'ignore_hosts': ['host1']} - - cls1 = ComputeFilterClass1() - cls2 = ComputeFilterClass2() - self.mox.StubOutWithMock(cls1, 'host_passes') - self.mox.StubOutWithMock(cls2, 'host_passes') - filter_fns = [cls1.host_passes, cls2.host_passes] - - # cls[12].host_passes() not called because of short circuit - # with matching host to ignore - - self.mox.ReplayAll() - result = fake_host.passes_filters(filter_fns, filter_properties) - self.assertFalse(result) - - def test_host_state_passes_filters_skipped_from_force(self): - fake_host = host_manager.HostState('host1', 'node1') - filter_properties = {'force_hosts': ['host1']} - - cls1 = ComputeFilterClass1() - cls2 = ComputeFilterClass2() - self.mox.StubOutWithMock(cls1, 'host_passes') - self.mox.StubOutWithMock(cls2, 'host_passes') - filter_fns = [cls1.host_passes, cls2.host_passes] - - # cls[12].host_passes() not called because of short circuit - # with matching host to force - - self.mox.ReplayAll() - result = fake_host.passes_filters(filter_fns, filter_properties) - self.assertTrue(result) - def test_stat_consumption_from_compute_node(self): stats = [ dict(key='num_instances', value='5'), diff --git a/nova/tests/test_filters.py b/nova/tests/test_filters.py new file mode 100644 index 000000000..546b13180 --- /dev/null +++ b/nova/tests/test_filters.py @@ -0,0 +1,125 @@ +# Copyright 2012 OpenStack LLC. # 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. +""" +Tests For Scheduler Host Filters. +""" + +import inspect +import sys + +from nova import filters +from nova import loadables +from nova import test + + +class Filter1(filters.BaseFilter): + """Test Filter class #1.""" + pass + + +class Filter2(filters.BaseFilter): + """Test Filter class #2.""" + pass + + +class FiltersTestCase(test.TestCase): + def test_filter_all(self): + filter_obj_list = ['obj1', 'obj2', 'obj3'] + filter_properties = 'fake_filter_properties' + base_filter = filters.BaseFilter() + + self.mox.StubOutWithMock(base_filter, '_filter_one') + + base_filter._filter_one('obj1', filter_properties).AndReturn(True) + base_filter._filter_one('obj2', filter_properties).AndReturn(False) + base_filter._filter_one('obj3', filter_properties).AndReturn(True) + + self.mox.ReplayAll() + + result = base_filter.filter_all(filter_obj_list, filter_properties) + self.assertTrue(inspect.isgenerator(result)) + self.assertEqual(list(result), ['obj1', 'obj3']) + + def test_filter_all_recursive_yields(self): + """Test filter_all() allows generators from previous filter_all()s.""" + # filter_all() yields results. We want to make sure that we can + # call filter_all() with generators returned from previous calls + # to filter_all(). + filter_obj_list = ['obj1', 'obj2', 'obj3'] + filter_properties = 'fake_filter_properties' + base_filter = filters.BaseFilter() + + self.mox.StubOutWithMock(base_filter, '_filter_one') + + total_iterations = 200 + + # The order that _filter_one is going to get called gets + # confusing because we will be recursively yielding things.. + # We are going to simulate the first call to filter_all() + # returning False for 'obj2'. So, 'obj1' will get yielded + # 'total_iterations' number of times before the first filter_all() + # call gets to processing 'obj2'. We then return 'False' for it. + # After that, 'obj3' gets yielded 'total_iterations' number of + # times. + for x in xrange(total_iterations): + base_filter._filter_one('obj1', filter_properties).AndReturn(True) + base_filter._filter_one('obj2', filter_properties).AndReturn(False) + for x in xrange(total_iterations): + base_filter._filter_one('obj3', filter_properties).AndReturn(True) + self.mox.ReplayAll() + + objs = iter(filter_obj_list) + for x in xrange(total_iterations): + # Pass in generators returned from previous calls. + objs = base_filter.filter_all(objs, filter_properties) + self.assertTrue(inspect.isgenerator(objs)) + self.assertEqual(list(objs), ['obj1', 'obj3']) + + def test_get_filtered_objects(self): + filter_objs_initial = ['initial', 'filter1', 'objects1'] + filter_objs_second = ['second', 'filter2', 'objects2'] + filter_objs_last = ['last', 'filter3', 'objects3'] + filter_properties = 'fake_filter_properties' + + def _fake_base_loader_init(*args, **kwargs): + pass + + self.stubs.Set(loadables.BaseLoader, '__init__', + _fake_base_loader_init) + + filt1_mock = self.mox.CreateMock(Filter1) + filt2_mock = self.mox.CreateMock(Filter2) + + self.mox.StubOutWithMock(sys.modules[__name__], 'Filter1', + use_mock_anything=True) + self.mox.StubOutWithMock(filt1_mock, 'filter_all') + self.mox.StubOutWithMock(sys.modules[__name__], 'Filter2', + use_mock_anything=True) + self.mox.StubOutWithMock(filt2_mock, 'filter_all') + + Filter1().AndReturn(filt1_mock) + filt1_mock.filter_all(filter_objs_initial, + filter_properties).AndReturn(filter_objs_second) + Filter2().AndReturn(filt2_mock) + filt2_mock.filter_all(filter_objs_second, + filter_properties).AndReturn(filter_objs_last) + + self.mox.ReplayAll() + + filter_handler = filters.BaseFilterHandler(filters.BaseFilter) + filter_classes = [Filter1, Filter2] + result = filter_handler.get_filtered_objects(filter_classes, + filter_objs_initial, + filter_properties) + self.assertEqual(result, filter_objs_last) -- cgit From 45402ff2ddf772e744bdb117d4c2edd4c74d8e7e Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Wed, 7 Nov 2012 15:57:12 -0800 Subject: create_db_entry_for_new_instance did not call sgh for default When an instance is launched create_db_entry_for_new_instance() is called which calls self.db.instance_create(). Then, db.security_group_ensure_default() is called which creates the default security group entry in the nova db. Though the sgh handler was not called. Alternatively, if any nova secgroup-* call is made the sgh is called which creates the default security group. This patch adds a call to ensure_default() before db.instance_create() so the sgh is called. Fixes a case missed in bug 1050982 Change-Id: I9649b5ef026ca18a55abfb66e87fd0e358eba6f2 --- nova/compute/api.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5a8bcad2f..8c611f2fd 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -732,6 +732,10 @@ class API(base.Base): self._populate_instance_shutdown_terminate(instance, image, block_device_mapping) + # ensure_default security group is called before the instance + # is created so the creation of the default security group is + # proxied to the sgh. + self.security_group_api.ensure_default(context) instance = self.db.instance_create(context, instance) self._populate_instance_for_bdm(context, instance, -- cgit