From a5c32aea36198ea2d15309cd8d7cd45df7f27571 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic Date: Fri, 3 Jul 2015 23:57:52 +0200 Subject: Properly handle provisioning images Handling of provisioning images has changed on the Tuskar side; this fix brings Tukskar-UI in line with these changes. * There used to be separate images for all the deployment roles. This is no longer the case and all the roles now use the same image, 'overcloud-full'. As the result, the way the relationship between images and roles is displayed on the Provisioning Images page needs to be updated. * Related to the previous point, test data needed updating to reflect that. * Images associated with roles are now saved by name in the Tuskar plan. This fix ensures that the UI saves and retrieves images by name and not by UUID, like it used to do. Change-Id: I11b73fcaed45200dfbdb72dc3ab9a27afb67a31d --- tuskar_ui/api/tuskar.py | 35 +++++++++++++-------- tuskar_ui/infrastructure/images/tables.py | 6 ++-- tuskar_ui/infrastructure/images/views.py | 2 +- tuskar_ui/infrastructure/nodes/tests.py | 32 +++++++++---------- tuskar_ui/infrastructure/roles/tests.py | 16 +++++----- tuskar_ui/infrastructure/roles/views.py | 2 +- tuskar_ui/infrastructure/roles/workflows.py | 4 +-- tuskar_ui/test/api_tests/node_tests.py | 2 +- tuskar_ui/test/api_tests/tuskar_tests.py | 4 +-- tuskar_ui/test/test_data/heat_data.py | 48 ++++++----------------------- tuskar_ui/test/test_data/tuskar_data.py | 7 ++--- 11 files changed, 69 insertions(+), 89 deletions(-) diff --git a/tuskar_ui/api/tuskar.py b/tuskar_ui/api/tuskar.py index c97f93e7..36773340 100644 --- a/tuskar_ui/api/tuskar.py +++ b/tuskar_ui/api/tuskar.py @@ -256,7 +256,7 @@ class Plan(base.APIResourceWrapper): key_params = [] for role in self.role_list: key_params.extend([role.node_count_parameter_name, - role.image_id_parameter_name, + role.image_parameter_name, role.flavor_parameter_name]) params = [p for p in params if p['name'] not in key_params] return [Parameter(p, plan=self) for p in params] @@ -387,9 +387,17 @@ class Role(base.APIResourceWrapper): @classmethod @memoized.memoized - def _roles_by_image_id(cls, request, plan): - return {plan.parameter_value(role.image_id_parameter_name): role - for role in Role.list(request)} + def _roles_by_image(cls, request, plan): + roles_by_image = {} + + for role in Role.list(request): + image = plan.parameter_value(role.image_parameter_name) + if image in roles_by_image: + roles_by_image[image].append(role) + else: + roles_by_image[image] = [role] + + return roles_by_image @classmethod @handle_errors(_("Unable to retrieve overcloud role")) @@ -409,11 +417,11 @@ class Role(base.APIResourceWrapper): Role can be found :rtype: tuskar_ui.api.tuskar.Role """ - roles = cls._roles_by_image_id(request, plan) + roles = cls._roles_by_image(request, plan) try: - return roles[image.id] + return roles[image.name] except KeyError: - return None + return [] @classmethod @memoized.memoized @@ -443,7 +451,7 @@ class Role(base.APIResourceWrapper): return self.parameter_prefix + 'count' @property - def image_id_parameter_name(self): + def image_parameter_name(self): return self.parameter_prefix + 'Image' @property @@ -451,12 +459,13 @@ class Role(base.APIResourceWrapper): return self.parameter_prefix + 'Flavor' def image(self, plan): - image_id = plan.parameter_value(self.image_id_parameter_name) - if image_id: + image_name = plan.parameter_value(self.image_parameter_name) + if image_name: try: - return glance.image_get(self._request, image_id) - except glance_exceptions.HTTPNotFound: - LOG.error("Couldn't obtain image with id %s" % image_id) + return glance.image_list_detailed( + self._request, filters={'name': image_name})[0][0] + except (glance_exceptions.HTTPNotFound, IndexError): + LOG.error("Couldn't obtain image with name %s" % image_name) return None def flavor(self, plan): diff --git a/tuskar_ui/infrastructure/images/tables.py b/tuskar_ui/infrastructure/images/tables.py index 8bd8bbf4..bbc8c0a9 100644 --- a/tuskar_ui/infrastructure/images/tables.py +++ b/tuskar_ui/infrastructure/images/tables.py @@ -61,9 +61,9 @@ class ImagesTable(tables.DataTable): verbose_name=_("Image Name")) disk_format = tables.Column('disk_format', verbose_name=_("Format")) - role = tables.Column(lambda image: - image.role.name if image.role else '-', - verbose_name=_("Deployment Role")) + roles = tables.Column(lambda image: + ', '.join([r.name for r in image.roles]), + verbose_name=_("Deployment Roles")) class Meta(object): name = "images" diff --git a/tuskar_ui/infrastructure/images/views.py b/tuskar_ui/infrastructure/images/views.py index b18e0634..198fdf43 100644 --- a/tuskar_ui/infrastructure/images/views.py +++ b/tuskar_ui/infrastructure/images/views.py @@ -57,7 +57,7 @@ class IndexView(infrastructure_views.ItemCountMixin, plan = tuskar_api.tuskar.Plan.get_the_plan(self.request) for image in images: - image.role = tuskar_api.tuskar.Role.get_by_image( + image.roles = tuskar_api.tuskar.Role.get_by_image( self.request, plan, image) return images diff --git a/tuskar_ui/infrastructure/nodes/tests.py b/tuskar_ui/infrastructure/nodes/tests.py index 4f91bb9e..485a31b4 100644 --- a/tuskar_ui/infrastructure/nodes/tests.py +++ b/tuskar_ui/infrastructure/nodes/tests.py @@ -162,8 +162,8 @@ class NodesTests(test.BaseAdminViewTests): 'register_nodes-0-cpus': '1', 'register_nodes-0-memory_mb': '2', 'register_nodes-0-local_gb': '3', - 'register_nodes-0-deployment_kernel': images[6].id, - 'register_nodes-0-deployment_ramdisk': images[7].id, + 'register_nodes-0-deployment_kernel': images[3].id, + 'register_nodes-0-deployment_ramdisk': images[4].id, 'register_nodes-1-driver': 'pxe_ipmitool', 'register_nodes-1-ipmi_address': '127.0.0.2', @@ -172,8 +172,8 @@ class NodesTests(test.BaseAdminViewTests): 'register_nodes-1-cpus': '4', 'register_nodes-1-memory_mb': '5', 'register_nodes-1-local_gb': '6', - 'register_nodes-1-deployment_kernel': images[6].id, - 'register_nodes-1-deployment_ramdisk': images[7].id, + 'register_nodes-1-deployment_kernel': images[3].id, + 'register_nodes-1-deployment_ramdisk': images[4].id, } with mock.patch('tuskar_ui.api.node.Node', **{ 'spec_set': ['create', 'get_all_mac_addresses'], @@ -198,8 +198,8 @@ class NodesTests(test.BaseAdminViewTests): ipmi_username=u'username', ipmi_password=u'password', driver='pxe_ipmitool', - deployment_kernel=images[6].id, - deployment_ramdisk=images[7].id, + deployment_kernel=images[3].id, + deployment_ramdisk=images[4].id, ), mock.call( mock.ANY, @@ -212,8 +212,8 @@ class NodesTests(test.BaseAdminViewTests): ipmi_username=None, ipmi_password=None, driver='pxe_ipmitool', - deployment_kernel=images[6].id, - deployment_ramdisk=images[7].id, + deployment_kernel=images[3].id, + deployment_ramdisk=images[4].id, ), ]) @@ -234,8 +234,8 @@ class NodesTests(test.BaseAdminViewTests): 'register_nodes-0-cpus': '1', 'register_nodes-0-memory_mb': '2', 'register_nodes-0-local_gb': '3', - 'register_nodes-0-deployment_kernel': images[6].id, - 'register_nodes-0-deployment_ramdisk': images[7].id, + 'register_nodes-0-deployment_kernel': images[3].id, + 'register_nodes-0-deployment_ramdisk': images[4].id, 'register_nodes-1-driver': 'pxe_ipmitool', 'register_nodes-1-ipmi_address': '127.0.0.2', @@ -244,8 +244,8 @@ class NodesTests(test.BaseAdminViewTests): 'register_nodes-1-cpus': '4', 'register_nodes-1-memory_mb': '5', 'register_nodes-1-local_gb': '6', - 'register_nodes-1-deployment_kernel': images[6].id, - 'register_nodes-1-deployment_ramdisk': images[7].id, + 'register_nodes-1-deployment_kernel': images[3].id, + 'register_nodes-1-deployment_ramdisk': images[4].id, } with mock.patch('tuskar_ui.api.node.Node', **{ 'spec_set': ['create', 'get_all_mac_addresses'], @@ -269,8 +269,8 @@ class NodesTests(test.BaseAdminViewTests): ipmi_username=u'username', ipmi_password=u'password', driver='pxe_ipmitool', - deployment_kernel=images[6].id, - deployment_ramdisk=images[7].id, + deployment_kernel=images[3].id, + deployment_ramdisk=images[4].id, ), mock.call( mock.ANY, @@ -283,8 +283,8 @@ class NodesTests(test.BaseAdminViewTests): ipmi_username=None, ipmi_password=None, driver='pxe_ipmitool', - deployment_kernel=images[6].id, - deployment_ramdisk=images[7].id, + deployment_kernel=images[3].id, + deployment_ramdisk=images[4].id, ), ]) self.assertTemplateUsed( diff --git a/tuskar_ui/infrastructure/roles/tests.py b/tuskar_ui/infrastructure/roles/tests.py index 55cba9d8..d9ee2057 100644 --- a/tuskar_ui/infrastructure/roles/tests.py +++ b/tuskar_ui/infrastructure/roles/tests.py @@ -48,15 +48,15 @@ class RolesTest(test.BaseAdminViewTests): plans = [api.tuskar.Plan(plan) for plan in self.tuskarclient_plans.list()] flavor = self.novaclient_flavors.first() - image = self.glanceclient_images.first() + images = self.glanceclient_images.list() with contextlib.nested( patch('tuskar_ui.api.tuskar.Plan.list', return_value=plans), patch('tuskar_ui.api.tuskar.Role.list', return_value=roles), - patch('openstack_dashboard.api.glance.image_get', - return_value=image), + patch('openstack_dashboard.api.glance.image_list_detailed', + return_value=[images]), patch('tuskar_ui.api.flavor.Flavor.get_by_name', return_value=flavor)): res = self.client.get(INDEX_URL) @@ -69,7 +69,7 @@ class RolesTest(test.BaseAdminViewTests): plans = [api.tuskar.Plan(plan) for plan in self.tuskarclient_plans.list()] flavor = self.novaclient_flavors.first() - image = self.glanceclient_images.first() + images = self.glanceclient_images.list() stack = api.heat.Stack(TEST_DATA.heatclient_stacks.first()) with contextlib.nested( @@ -83,8 +83,8 @@ class RolesTest(test.BaseAdminViewTests): return_value=[]), patch('tuskar_ui.api.tuskar.Plan.list', return_value=plans), - patch('openstack_dashboard.api.glance.image_get', - return_value=image), + patch('openstack_dashboard.api.glance.image_list_detailed', + return_value=[images]), patch('tuskar_ui.api.flavor.Flavor.get_by_name', return_value=flavor)): res = self.client.get(DETAIL_URL) @@ -137,7 +137,7 @@ class RolesTest(test.BaseAdminViewTests): 'name': 'controller', 'description': 'The controller node role.', 'flavor': self.novaclient_flavors.first().name, - 'image': self.glanceclient_images.first().id, + 'image': self.glanceclient_images.first().name, 'nodes': '0', } @@ -163,4 +163,4 @@ class RolesTest(test.BaseAdminViewTests): args = mock_patch.call_args_list[0][0] self.assertEqual(args[1], plan.id) self.assertEqual(args[2]['Controller-1::Flavor'], u'flavor-1') - self.assertEqual(args[2]['Controller-1::Image'], u'2') + self.assertEqual(args[2]['Controller-1::Image'], u'overcloud-full') diff --git a/tuskar_ui/infrastructure/roles/views.py b/tuskar_ui/infrastructure/roles/views.py index 78a73065..b5cb5801 100644 --- a/tuskar_ui/infrastructure/roles/views.py +++ b/tuskar_ui/infrastructure/roles/views.py @@ -143,7 +143,7 @@ class UpdateView(workflows.WorkflowView, views.StackMixin, views.RoleMixin): role_flavor = '' if role_flavor is None else role_flavor.name role_image = role.image(plan) - role_image = '' if role_image is None else role_image.id + role_image = '' if role_image is None else role_image.name free_nodes = len(api.node.Node.list(self.request, associated=False, maintenance=False)) diff --git a/tuskar_ui/infrastructure/roles/workflows.py b/tuskar_ui/infrastructure/roles/workflows.py index 2a138c10..b9786112 100644 --- a/tuskar_ui/infrastructure/roles/workflows.py +++ b/tuskar_ui/infrastructure/roles/workflows.py @@ -78,7 +78,7 @@ class UpdateRoleInfoAction(workflows.Action): images = [image for image in images if tuskar_utils.check_image_type(image, 'overcloud provisioning')] - choices = [(i.id, i.name) for i in images] + choices = [(i.name, i.name) for i in images] return [('', _('Unknown'))] + choices def clean_nodes(self): @@ -170,7 +170,7 @@ class UpdateRole(workflows.Workflow): redirect=reverse_lazy(self.index_url)) parameters = data['parameters'] - parameters[role.image_id_parameter_name] = data['image'] + parameters[role.image_parameter_name] = data['image'] parameters[role.node_count_parameter_name] = data['nodes'] if utils.matching_deployment_mode(): parameters[role.flavor_parameter_name] = data['flavor'] diff --git a/tuskar_ui/test/api_tests/node_tests.py b/tuskar_ui/test/api_tests/node_tests.py index 2589caab..7bde594b 100644 --- a/tuskar_ui/test/api_tests/node_tests.py +++ b/tuskar_ui/test/api_tests/node_tests.py @@ -158,4 +158,4 @@ class NodeAPITests(test.APITestCase): return_value=([instance], False), ): ret_val = api.node.Node(node).image_name - self.assertEqual(ret_val, 'overcloud-control') + self.assertEqual(ret_val, 'overcloud-full') diff --git a/tuskar_ui/test/api_tests/tuskar_tests.py b/tuskar_ui/test/api_tests/tuskar_tests.py index 16526a81..3979bd73 100644 --- a/tuskar_ui/test/api_tests/tuskar_tests.py +++ b/tuskar_ui/test/api_tests/tuskar_tests.py @@ -108,8 +108,8 @@ class TuskarAPITests(test.APITestCase): return_value=roles): ret_val = api.tuskar.Role.get_by_image( self.request, plan, image) - self.assertIsInstance(ret_val, api.tuskar.Role) - self.assertEqual(ret_val.name, 'Controller') + self.assertIsInstance(ret_val, list) + self.assertEqual(len(ret_val), 3) def test_parameter_stripped_name(self): plan = api.tuskar.Plan(self.tuskarclient_plans.first()) diff --git a/tuskar_ui/test/test_data/heat_data.py b/tuskar_ui/test/test_data/heat_data.py index d99908fc..303bcc0a 100644 --- a/tuskar_ui/test/test_data/heat_data.py +++ b/tuskar_ui/test/test_data/heat_data.py @@ -201,76 +201,48 @@ def data(TEST): # Image TEST.glanceclient_images = test_data_utils.TestDataContainer() image_1 = images.Image( - images.ImageManager(None), - {'id': '2', - 'name': 'overcloud-control', - 'is_public': True, - 'protected': False, - 'properties': { - 'type': 'overcloud provisioning' - }}) - image_2 = images.Image( images.ImageManager(None), {'id': '1', - 'name': 'overcloud-compute', - 'is_public': True, - 'protected': False, - 'properties': { - 'type': 'overcloud provisioning' - }}) - image_3 = images.Image( - images.ImageManager(None), - {'id': '3', - 'name': 'Object Storage Image', - 'is_public': True, - 'protected': False, - 'properties': { - 'type': 'overcloud provisioning' - }}) - image_4 = images.Image( - images.ImageManager(None), - {'id': '4', - 'name': 'Block Storage Image', + 'name': 'overcloud-full', 'is_public': True, 'protected': False, 'properties': { 'type': 'overcloud provisioning' }}) - image_5 = images.Image( + image_2 = images.Image( images.ImageManager(None), - {'id': '5', + {'id': '2', 'name': 'Discovery Ramdisk', 'is_public': True, 'protected': False, 'properties': { 'type': 'discovery ramdisk' }}) - image_6 = images.Image( + image_3 = images.Image( images.ImageManager(None), - {'id': '6', + {'id': '3', 'name': 'Discovery Kernel', 'is_public': True, 'protected': False, 'properties': { 'type': 'discovery kernel' }}) - image_7 = images.Image( + image_4 = images.Image( images.ImageManager(None), - {'id': '7', + {'id': '4', 'name': 'Baremetal Deployment Kernel', 'is_public': True, 'protected': False, 'properties': { 'type': 'deploy kernel' }}) - image_8 = images.Image( + image_5 = images.Image( images.ImageManager(None), - {'id': '8', + {'id': '5', 'name': 'Baremetal Deployment Ramdisk', 'is_public': True, 'protected': False, 'properties': { 'type': 'deploy ramdisk' }}) - TEST.glanceclient_images.add(image_1, image_2, image_3, image_4, - image_5, image_6, image_7, image_8) + TEST.glanceclient_images.add(image_1, image_2, image_3, image_4, image_5) diff --git a/tuskar_ui/test/test_data/tuskar_data.py b/tuskar_ui/test/test_data/tuskar_data.py index c1913180..dae72f1c 100644 --- a/tuskar_ui/test/test_data/tuskar_data.py +++ b/tuskar_ui/test/test_data/tuskar_data.py @@ -29,7 +29,6 @@ def data(TEST): 'template': '', 'created_at': '2014-05-27T21:11:09Z', 'modified_at': '2014-05-30T21:11:09Z', - 'uuid': '1234567890', 'roles': [ { 'uuid': 'role-1', @@ -108,7 +107,7 @@ def data(TEST): 'description': 'Controller image ID', 'hidden': False, 'default': '', - 'value': '2', + 'value': 'overcloud-full', 'parameter_type': 'string', 'constraints': [], }, { @@ -117,7 +116,7 @@ def data(TEST): 'description': 'Compute image ID', 'hidden': False, 'default': '', - 'value': '1', + 'value': 'overcloud-full', 'parameter_type': 'string', 'constraints': [], }, { @@ -126,7 +125,7 @@ def data(TEST): 'description': 'Block storage image ID', 'hidden': False, 'default': '', - 'value': '4', + 'value': 'overcloud-full', 'parameter_type': 'string', 'constraints': [], }, { -- cgit v1.2.1