From 34fed58c1cfcb64eff9c631699e58a01c64804a9 Mon Sep 17 00:00:00 2001
From: Steve Baker <sbaker@redhat.com>
Date: Fri, 1 Nov 2019 14:31:10 +1300
Subject: [PATCH] Clean up instance defaults setting

This change fixes a problem where an instance entry was not being
populated with an image specified in the defaults dict. This change
sets up a baseline defaults dict before validating the roles, and
populates any instances with the defaults in-place.

This is a low-risk backport as the provision workflow is new and not
documented to be used in stable/train.

Change-Id: I1929e1b135ed539a6da09b1ec5d2b64b6281dc47
Blueprint: nova-less-deploy
Closes-Bug: 1851575
(cherry picked from commit 82f3a35fdce9a9ab9a63295afabd91fa7761f60a)
---
 tripleo_common/actions/baremetal_deploy.py    | 16 ++++------
 .../tests/actions/test_baremetal_deploy.py    | 32 +++++++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/tripleo_common/actions/baremetal_deploy.py b/tripleo_common/actions/baremetal_deploy.py
index 543eabcd6..d1ad2c06f 100644
--- a/tripleo_common/actions/baremetal_deploy.py
+++ b/tripleo_common/actions/baremetal_deploy.py
@@ -363,10 +363,12 @@ class ExpandRolesAction(base.TripleOAction):
 
     def run(self, context):
         for role in self.roles:
+            role.setdefault('defaults', {})
+            role['defaults'].setdefault(
+                'image', {'href': self.default_image})
             for inst in role.get('instances', []):
-                # Set the default image so that the
-                # source validation can succeed.
-                inst.setdefault('image', {'href': self.default_image})
+                for k, v in role['defaults'].items():
+                    inst.setdefault(k, v)
 
                 # Set the default hostname now for duplicate hostname
                 # detection during validation
@@ -401,12 +403,7 @@ class ExpandRolesAction(base.TripleOAction):
             role_instances = []
             for instance in role.get('instances', []):
                 inst = {}
-                inst.update(role.get('defaults', {}))
                 inst.update(instance)
-                inst.setdefault('image', {'image': self.default_image})
-
-                if 'name' in inst and 'hostname' not in inst:
-                    inst['hostname'] = inst['name']
 
                 # create a hostname map entry now if the specified hostname
                 # is a valid generated name
@@ -420,8 +417,7 @@ class ExpandRolesAction(base.TripleOAction):
             while len([i for i in role_instances
                        if i.get('provisioned', True)]) < count:
                 inst = {}
-                inst.update(role.get('defaults', {}))
-                inst.setdefault('image', {'href': self.default_image})
+                inst.update(role['defaults'])
                 role_instances.append(inst)
 
             # NOTE(dtantsur): our hostname format may differ from THT defaults,
diff --git a/tripleo_common/tests/actions/test_baremetal_deploy.py b/tripleo_common/tests/actions/test_baremetal_deploy.py
index 6e911d5b0..de3a4c195 100644
--- a/tripleo_common/tests/actions/test_baremetal_deploy.py
+++ b/tripleo_common/tests/actions/test_baremetal_deploy.py
@@ -677,6 +677,38 @@ class TestExpandRoles(base.TestCase):
             },
             result['environment']['parameter_defaults'])
 
+    def test_image_in_defaults(self):
+        roles = [{
+            'name': 'Controller',
+            'defaults': {
+                'image': {
+                    'href': 'file:///tmp/foo.qcow2',
+                    'checksum': '12345678'
+                }
+            },
+            'count': 3,
+            'instances': [{
+                'hostname': 'overcloud-controller-0',
+                'image': {'href': 'overcloud-full'}
+            }, {
+                'hostname': 'overcloud-controller-1',
+            }]
+        }]
+        action = baremetal_deploy.ExpandRolesAction(roles)
+        result = action.run(mock.Mock())
+        self.assertEqual(
+            [
+                {'hostname': 'overcloud-controller-0',
+                 'image': {'href': 'overcloud-full'}},
+                {'hostname': 'overcloud-controller-1',
+                 'image': {'href': 'file:///tmp/foo.qcow2',
+                           'checksum': '12345678'}},
+                {'hostname': 'overcloud-controller-2',
+                 'image': {'href': 'file:///tmp/foo.qcow2',
+                           'checksum': '12345678'}},
+            ],
+            result['instances'])
+
     def test_with_parameters(self):
         roles = [{
             'name': 'Compute',