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

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
@@ -202,75 +202,47 @@ def data(TEST):
     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': [],
         }, {