From 9164b8d224ae6629cdac00248b98fad762bdfc10 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 14 Mar 2011 10:46:26 +0100 Subject: Make utils.execute not overwrite std{in,out,err} args to Popen on retries. Make utils.execute reject unknown kwargs. Add a couple of unit tests for utils.execute. --- nova/tests/test_utils.py | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ nova/utils.py | 25 +++++++++------- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 34a407f1a..51cd57c76 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -14,11 +14,87 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import tempfile + from nova import test from nova import utils from nova import exception +class ExecuteTestCase(test.TestCase): + def test_retry_on_failure(self): + fd, tmpfilename = tempfile.mkstemp() + _, tmpfilename2 = tempfile.mkstemp() + try: + fp = os.fdopen(fd, 'w+') + fp.write('''#!/bin/sh +# If stdin fails to get passed during one of the runs, make a note. +if ! grep -q foo +then + echo 'failure' > "$1" +fi +# If stdin has failed to get passed during this or a previous run, exit early. +if grep failure "$1" +then + exit 1 +fi +runs="$(cat $1)" +if [ -z "$runs" ] +then + runs=0 +fi +runs=$(($runs + 1)) +echo $runs > "$1" +exit 1 +''') + fp.close() + os.chmod(tmpfilename, 0755) + self.assertRaises(exception.ProcessExecutionError, + utils.execute, + tmpfilename, tmpfilename2, attempts=10, + process_input='foo', + delay_on_retry=False) + fp = open(tmpfilename2, 'r+') + runs = fp.read() + fp.close() + self.assertNotEquals(runs.strip(), 'failure', 'stdin did not ' + 'always get passed ' + 'correctly') + runs = int(runs.strip()) + self.assertEquals(runs, 10, 'Ran %d times instead of 10.' % (runs,)) + finally: + os.unlink(tmpfilename) + os.unlink(tmpfilename2) + + def test_unknown_kwargs_raises_error(self): + self.assertRaises(exception.Error, + utils.execute, + '/bin/true', this_is_not_a_valid_kwarg=True) + + def test_no_retry_on_success(self): + fd, tmpfilename = tempfile.mkstemp() + _, tmpfilename2 = tempfile.mkstemp() + try: + fp = os.fdopen(fd, 'w+') + fp.write('''#!/bin/sh +# If we've already run, bail out. +grep -q foo "$1" && exit 1 +# Mark that we've run before. +echo foo > "$1" +# Check that stdin gets passed correctly. +grep foo +''') + fp.close() + os.chmod(tmpfilename, 0755) + utils.execute(tmpfilename, + tmpfilename2, + process_input='foo', + attempts=2) + finally: + os.unlink(tmpfilename) + os.unlink(tmpfilename2) + class GetFromPathTestCase(test.TestCase): def test_tolerates_nones(self): f = utils.get_from_path diff --git a/nova/utils.py b/nova/utils.py index 87e726394..2a98411ea 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -133,13 +133,14 @@ def fetchfile(url, target): def execute(*cmd, **kwargs): - process_input = kwargs.get('process_input', None) - addl_env = kwargs.get('addl_env', None) - check_exit_code = kwargs.get('check_exit_code', 0) - stdin = kwargs.get('stdin', subprocess.PIPE) - stdout = kwargs.get('stdout', subprocess.PIPE) - stderr = kwargs.get('stderr', subprocess.PIPE) - attempts = kwargs.get('attempts', 1) + process_input = kwargs.pop('process_input', None) + addl_env = kwargs.pop('addl_env', None) + check_exit_code = kwargs.pop('check_exit_code', 0) + delay_on_retry = kwargs.pop('delay_on_retry', True) + attempts = kwargs.pop('attempts', 1) + if len(kwargs): + raise exception.Error(_('Got unknown keyword args ' + 'to utils.execute: %r') % kwargs) cmd = map(str, cmd) while attempts > 0: @@ -149,8 +150,11 @@ def execute(*cmd, **kwargs): env = os.environ.copy() if addl_env: env.update(addl_env) - obj = subprocess.Popen(cmd, stdin=stdin, - stdout=stdout, stderr=stderr, env=env) + obj = subprocess.Popen(cmd, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env) result = None if process_input != None: result = obj.communicate(process_input) @@ -176,7 +180,8 @@ def execute(*cmd, **kwargs): raise else: LOG.debug(_("%r failed. Retrying."), cmd) - greenthread.sleep(random.randint(20, 200) / 100.0) + if delay_on_retry: + greenthread.sleep(random.randint(20, 200) / 100.0) def ssh_execute(ssh, cmd, process_input=None, -- cgit From 3cf224b9e676b88d1990b13476095be6ec156e5d Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 14 Mar 2011 21:28:42 -0700 Subject: Fixed problem with metadata creation (backported fix) --- nova/db/sqlalchemy/api.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 56998ce05..d4dd82227 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -746,6 +746,15 @@ def instance_create(context, values): context - request context object values - dict containing column values. """ + metadata = values.get('metadata') + metadata_refs = [] + if metadata: + for metadata_item in metadata: + metadata_ref = models.InstanceMetadata() + metadata_ref.update(metadata_item) + metadata_refs.append(metadata_ref) + values['metadata'] = metadata_refs + instance_ref = models.Instance() instance_ref.update(values) -- cgit From a766b4111addad804e47b8be3e6dedb5f80a83c4 Mon Sep 17 00:00:00 2001 From: Monsyne Dragon Date: Thu, 17 Mar 2011 02:20:18 +0000 Subject: added in network qos support for xenserver. Pull qos settings from flavor, use when creating instance. --- nova/api/openstack/servers.py | 3 ++- nova/tests/db/fakes.py | 30 ++++++++++++++++++---- nova/tests/test_xenapi.py | 8 ++++++ nova/virt/xenapi/vm_utils.py | 9 ++++--- nova/virt/xenapi/vmops.py | 9 +++++-- plugins/xenserver/xenapi/etc/xapi.d/plugins/glance | 2 +- 6 files changed, 49 insertions(+), 12 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 3ecd4fb01..2f26fa873 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -32,6 +32,7 @@ from nova.api.openstack import faults from nova.auth import manager as auth_manager from nova.compute import instance_types from nova.compute import power_state +from nova.quota import QuotaError import nova.api.openstack @@ -189,7 +190,7 @@ class Controller(wsgi.Controller): metadata=metadata, injected_files=injected_files) except QuotaError as error: - self._handle_quota_error(error) + self._handle_quota_errors(error) server = _translate_keys(instances[0]) password = "%s%s" % (server['server']['name'][:4], diff --git a/nova/tests/db/fakes.py b/nova/tests/db/fakes.py index 5e9a3aa3b..2d25d5fc5 100644 --- a/nova/tests/db/fakes.py +++ b/nova/tests/db/fakes.py @@ -28,13 +28,33 @@ def stub_out_db_instance_api(stubs): """ Stubs out the db API for creating Instances """ INSTANCE_TYPES = { - 'm1.tiny': dict(memory_mb=512, vcpus=1, local_gb=0, flavorid=1), - 'm1.small': dict(memory_mb=2048, vcpus=1, local_gb=20, flavorid=2), + 'm1.tiny': dict(memory_mb=512, + vcpus=1, + local_gb=0, + flavorid=1, + rxtx_cap=1), + 'm1.small': dict(memory_mb=2048, + vcpus=1, + local_gb=20, + flavorid=2, + rxtx_cap=2), 'm1.medium': - dict(memory_mb=4096, vcpus=2, local_gb=40, flavorid=3), - 'm1.large': dict(memory_mb=8192, vcpus=4, local_gb=80, flavorid=4), + dict(memory_mb=4096, + vcpus=2, + local_gb=40, + flavorid=3, + rxtx_cap=3), + 'm1.large': dict(memory_mb=8192, + vcpus=4, + local_gb=80, + flavorid=4, + rxtx_cap=4), 'm1.xlarge': - dict(memory_mb=16384, vcpus=8, local_gb=160, flavorid=5)} + dict(memory_mb=16384, + vcpus=8, + local_gb=160, + flavorid=5, + rxtx_cap=5)} class FakeModel(object): """ Stubs out for model """ diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 8b0affd5c..66a973a78 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -361,6 +361,14 @@ class XenAPIVMTestCase(test.TestCase): glance_stubs.FakeGlance.IMAGE_RAMDISK) self.check_vm_params_for_linux_with_external_kernel() + def test_spawn_with_network_qos(self): + self._create_instance() + for vif_ref in xenapi_fake.get_all('VIF'): + vif_rec = xenapi_fake.get_record('VIF', vif_ref) + self.assertEquals(vif_rec['qos_algorithm_type'], 'ratelimit') + self.assertEquals(vif_rec['qos_algorithm_params']['kbps'], + str(4 * 1024)) + def tearDown(self): super(XenAPIVMTestCase, self).tearDown() self.manager.delete_project(self.project) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 763c5fe40..e0621f73a 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -233,7 +233,9 @@ class VMHelper(HelperBase): raise StorageError(_('Unable to destroy VBD %s') % vbd_ref) @classmethod - def create_vif(cls, session, vm_ref, network_ref, mac_address, dev="0"): + def create_vif(cls, session, vm_ref, network_ref, mac_address, + dev="0", + rxtx_cap=0): """Create a VIF record. Returns a Deferred that gives the new VIF reference.""" vif_rec = {} @@ -243,8 +245,9 @@ class VMHelper(HelperBase): vif_rec['MAC'] = mac_address vif_rec['MTU'] = '1500' vif_rec['other_config'] = {} - vif_rec['qos_algorithm_type'] = '' - vif_rec['qos_algorithm_params'] = {} + vif_rec['qos_algorithm_type'] = "ratelimit" if rxtx_cap else '' + vif_rec['qos_algorithm_params'] = \ + {"kbps": str(rxtx_cap * 1024)} if rxtx_cap else {} LOG.debug(_('Creating VIF for VM %(vm_ref)s,' ' network %(network_ref)s.') % locals()) vif_ref = session.call_xenapi('VIF.create', vif_rec) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 488a61e8e..29f162ad1 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -744,8 +744,12 @@ class VMOps(object): Creates vifs for an instance """ - vm_ref = self._get_vm_opaque_ref(instance.id) + vm_ref = self._get_vm_opaque_ref(instance['id']) + admin_context = context.get_admin_context() + flavor = db.instance_type_get_by_name(admin_context, + instance.instance_type) logging.debug(_("creating vif(s) for vm: |%s|"), vm_ref) + rxtx_cap = flavor['rxtx_cap'] if networks is None: networks = db.network_get_all_by_instance(admin_context, instance['id']) @@ -766,7 +770,8 @@ class VMOps(object): device = "0" VMHelper.create_vif(self._session, vm_ref, network_ref, - instance.mac_address, device) + instance.mac_address, device, + rxtx_cap=rxtx_cap) def reset_network(self, instance): """ diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance index c996f6ef4..db39cb0f4 100644 --- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance @@ -216,7 +216,7 @@ def _upload_tarball(staging_path, image_id, glance_host, glance_port, os_type): 'x-image-meta-status': 'queued', 'x-image-meta-disk-format': 'vhd', 'x-image-meta-container-format': 'ovf', - 'x-image-meta-property-os-type': os_type + 'x-image-meta-property-os-type': os_type, } for header, value in headers.iteritems(): -- cgit From 192d3ad7bb9cf49abbca98b0d8e9ae822b204365 Mon Sep 17 00:00:00 2001 From: Monsyne Dragon Date: Thu, 17 Mar 2011 19:34:45 +0000 Subject: fixed code formatting nit. --- nova/virt/xenapi/vm_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index e0621f73a..7dbca321f 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -234,8 +234,7 @@ class VMHelper(HelperBase): @classmethod def create_vif(cls, session, vm_ref, network_ref, mac_address, - dev="0", - rxtx_cap=0): + dev="0", rxtx_cap=0): """Create a VIF record. Returns a Deferred that gives the new VIF reference.""" vif_rec = {} -- cgit From 06c0b81e54adf3fb0635a7cd7679bcdb051e6263 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 17 Mar 2011 21:43:22 +0100 Subject: pep8 --- nova/tests/test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 51cd57c76..e08d229b0 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -62,7 +62,8 @@ exit 1 'always get passed ' 'correctly') runs = int(runs.strip()) - self.assertEquals(runs, 10, 'Ran %d times instead of 10.' % (runs,)) + self.assertEquals(runs, 10, + 'Ran %d times instead of 10.' % (runs,)) finally: os.unlink(tmpfilename) os.unlink(tmpfilename2) @@ -95,6 +96,7 @@ grep foo os.unlink(tmpfilename) os.unlink(tmpfilename2) + class GetFromPathTestCase(test.TestCase): def test_tolerates_nones(self): f = utils.get_from_path -- cgit