Merge "Properly handle provisioning images"

11 files changed, 69 insertions, 89 deletions

diff --git a/tuskar_ui/api/tuskar.py b/tuskar_ui/api/tuskar.py
index 85d293eb..89244591 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]
@@ -389,9 +389,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"))
@@ -411,11 +419,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
@@ -445,7 +453,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
@@ -453,12 +461,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': [],
         }, {