From 5c79e69521f1bf8b33feadfa1b1ba9f841ac3a10 Mon Sep 17 00:00:00 2001 From: Dmitry Bogun Date: Tue, 13 Dec 2016 17:43:19 +0200 Subject: [PATCH] FIX dashes usage in image names Image names stored into deployment config are looses dash characters during preparation of deployment config on node boot. This is not desired behaviour. And now it is fixed. Change-Id: If188b53160ffbad664b1505373edf3ee0fb14bd7 --- bareon_ironic/modules/bareon_base.py | 25 +++- bareon_ironic/tests/base.py | 9 +- bareon_ironic/tests/modules/__init__.py | 0 .../tests/modules/test_bareon_base.py | 111 ++++++++++++++++++ 4 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 bareon_ironic/tests/modules/__init__.py create mode 100644 bareon_ironic/tests/modules/test_bareon_base.py diff --git a/bareon_ironic/modules/bareon_base.py b/bareon_ironic/modules/bareon_base.py index a3cb24e..79b5f7c 100644 --- a/bareon_ironic/modules/bareon_base.py +++ b/bareon_ironic/modules/bareon_base.py @@ -492,7 +492,19 @@ class BareonDeploy(base.DeployInterface): ] user_images = provision_config.get('images', default_user_images) - for image in user_images: + invalid_images = [] + origin_names = [None] * len(user_images) + for idx, image in enumerate(user_images): + try: + bareon_utils.validate_json(('name', 'url'), image) + except exception.MissingParameterValue as e: + invalid_images.append( + 'Invalid "image" record - there is no key {key} (#{idx}: ' + '{payload})'.format( + key=e, idx=idx, payload=json.dumps(image))) + continue + + origin_names[idx] = image['name'] image_uuid, image_name = image_service.get_glance_image_uuid_name( task, image['url']) image['boot'] = (boot_image == image_uuid) @@ -501,18 +513,25 @@ class BareonDeploy(base.DeployInterface): image['image_uuid'] = image_uuid image['image_name'] = image_name + if invalid_images: + raise exception.InvalidParameterValue( + err='\n'.join(invalid_images)) + fetched_image_resources = self._fetch_images(task, user_images) image_deployment_config = [ { - 'name': image.name, + # Grab name from source data to keep it untouched, because + # "resources" subsystem replace all not alphanumeric symbols + # to underscores in 'name' field. + 'name': name, 'image_pull_url': image.pull_url, 'target': image.target, 'boot': image.boot, 'image_uuid': image.image_uuid, 'image_name': image.image_name } - for image in fetched_image_resources + for name, image in zip(origin_names, fetched_image_resources) ] bareon_utils.change_node_dict( diff --git a/bareon_ironic/tests/base.py b/bareon_ironic/tests/base.py index 79cbbee..6ab9deb 100644 --- a/bareon_ironic/tests/base.py +++ b/bareon_ironic/tests/base.py @@ -13,8 +13,13 @@ # License for the specific language governing permissions and limitations # under the License. -import testtools +from ironic.tests import base +from ironic.tests.unit.db import base as db_base -class AbstractTestCase(testtools.TestCase): +class AbstractTestCase(base.TestCase): + pass + + +class AbstractDBTestCase(db_base.DbTestCase): pass diff --git a/bareon_ironic/tests/modules/__init__.py b/bareon_ironic/tests/modules/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bareon_ironic/tests/modules/test_bareon_base.py b/bareon_ironic/tests/modules/test_bareon_base.py new file mode 100644 index 0000000..173dd58 --- /dev/null +++ b/bareon_ironic/tests/modules/test_bareon_base.py @@ -0,0 +1,111 @@ +# +# Copyright 2016 Cray Inc., 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. + +import fixtures +import mock +from ironic.common import exception +from ironic.conductor import task_manager +from ironic.tests.unit.objects import utils as test_utils + +from bareon_ironic.modules import bareon_base +from bareon_ironic.modules.resources import image_service +from bareon_ironic.modules.resources import resources +from bareon_ironic.tests import base + + +SWIFT_DEPLOY_IMAGE_MODE = resources.PullSwiftTempurlResource.MODE + + +class BareonBaseTestCase(base.AbstractDBTestCase): + def setUp(self): + super(BareonBaseTestCase, self).setUp() + + self.config(enabled_drivers=['bare_swift_ssh']) + + self.temp_dir = self.useFixture(fixtures.TempHomeDir()) + + @mock.patch.object(bareon_base.BareonDeploy, + "_get_deploy_driver", + mock.Mock(return_value="test_driver")) + @mock.patch.object(bareon_base.BareonDeploy, + "_get_image_resource_mode", + mock.Mock(return_value="test_mode")) + @mock.patch.object(image_service, "get_glance_image_uuid_name", + mock.Mock(return_value=('uuid1', 'name1'))) + @mock.patch.object(bareon_base.BareonDeploy, '_fetch_images', + mock.Mock()) + def test__add_image_deployment_config__missing_parameter(self): + origin_image = { + 'name': 'dashes-in-name', + 'url': 'name1', + 'target': '/' + } + + node = test_utils.create_test_node( + self.context, + driver_info={}, + instance_info={'image_source': 'uuid1'}) + + for field in ('name', 'url'): + image = origin_image.copy() + image.pop(field) + provision_conf = {'images': [image]} + + with task_manager.acquire(self.context, node.uuid, + driver_name='bare_swift_ssh') as task: + task.node = node + deploy = bareon_base.BareonDeploy() + self.assertRaises( + exception.InvalidParameterValue, + deploy._add_image_deployment_config, task, provision_conf) + + @mock.patch.object(bareon_base.BareonDeploy, + "_get_deploy_driver", + mock.Mock(return_value="test_driver")) + @mock.patch.object(bareon_base.BareonDeploy, + "_get_image_resource_mode", + mock.Mock(return_value=SWIFT_DEPLOY_IMAGE_MODE)) + @mock.patch.object(image_service, "get_glance_image_uuid_name", + mock.Mock(return_value=('uuid1', 'name1'))) + @mock.patch.object(resources.ResourceList, "fetch_resources", mock.Mock()) + def test__add_image_deployment_config_not_alphanum_name(self): + origin_image = { + 'name': 'dashes-in-name', + 'url': 'name1', + 'target': '/' + } + provision_conf = {'images': [origin_image.copy()]} + + node = test_utils.create_test_node( + self.context, + driver_info={}, + instance_info={'image_source': 'uuid1'}) + + with task_manager.acquire(self.context, node.uuid, + driver_name='bare_swift_ssh') as task: + task.node = node + deploy = bareon_base.BareonDeploy() + with mock.patch.object( + bareon_base, 'get_tenant_images_json_path') as path: + tenant_images_path = self.temp_dir.join('tenant_images.json') + path.side_effect = lambda node: tenant_images_path + + result = deploy._add_image_deployment_config( + task, provision_conf) + + result_image = result['images'] + result_image = result_image[0] + + self.assertEqual(origin_image['name'], result_image['name'])