From b79b0fc65d189d732958534104a41701c7357166 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Harald=20Jens=C3=A5s?= <hjensas@redhat.com>
Date: Fri, 29 Jan 2021 05:46:45 +0100
Subject: [PATCH] Fix uc install, Undercloud role must overwrite

The re-factoring in https://review.opendev.org/766109
broke the undercloud installer in case the undercloud
hostname is'nt 'undercloud'.

Due to the re-factoring making use of setdefault to
append missing keys, not replacing them the Undercloud
installer role got two host entries. One host 'undercloud'
added by the _undercloud_inventory() method and then
another host entry matching the hostname of the
undercloud being installed.

Special case the role 'Undercloud', ensure this entry
is always overwritten to avoid the "duplicate" host in
the 'Undercloud' group.

Note: CI did'nt pick up the issue because all underclouds
      deployed by CI uses 'undercloud.$DOMAN' as the hostname.

Closes-Bug: #1913551
Change-Id: I6091ebacadef1cbc72294ef54bc100f0509a53b1
---
 tripleo_common/inventory.py            |  8 ++-
 tripleo_common/tests/test_inventory.py | 73 ++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/tripleo_common/inventory.py b/tripleo_common/inventory.py
index 84d214607..3da6bc7bb 100644
--- a/tripleo_common/inventory.py
+++ b/tripleo_common/inventory.py
@@ -188,7 +188,13 @@ class TripleoInventory(object):
             role_networks = sorted([str(net) for net in net_ip_map])
             networks.update(role_networks)
 
-            role = ret.setdefault(role_name, {})
+            # Undercloud role in the stack should overwrite, not append.
+            # See bug: https://bugs.launchpad.net/tripleo/+bug/1913551
+            if role_name == 'Undercloud':
+                role = ret[role_name] = {}
+            else:
+                role = ret.setdefault(role_name, {})
+
             hosts = role.setdefault('hosts', {})
             role_vars = role.setdefault('vars', {})
 
diff --git a/tripleo_common/tests/test_inventory.py b/tripleo_common/tests/test_inventory.py
index 6a973f6e0..4bf8f5bc7 100644
--- a/tripleo_common/tests/test_inventory.py
+++ b/tripleo_common/tests/test_inventory.py
@@ -309,6 +309,79 @@ class TestInventory(base.TestCase):
         for k in expected:
             self.assertEqual(expected[k], inv_list[k])
 
+    def test_inventory_list_undercloud_installer(self):
+        outputs_data = {
+            'outputs': [
+                {'output_key': 'EnabledServices',
+                 'output_value': {'Undercloud': ['sa', 'sb']}},
+                {'output_key': 'KeystoneURL',
+                 'output_value': 'xyz://keystone'},
+                {'output_key': 'ServerIdData',
+                 'output_value': {'server_ids': {'Undercloud': ['a']},
+                                  'bootstrap_server_id': 'a'}},
+                {'output_key': 'RoleNetHostnameMap',
+                 'output_value': {'Undercloud': {
+                     'ctlplane': ['uc0.ctlplane.localdomain'],
+                     'external': ['uc0.external.localdomain'],
+                     'canonical': ['uc0.lab.example.com']}}},
+                {'output_key': 'RoleNetIpMap',
+                 'output_value': {'Undercloud': {'ctlplane': ['x.x.x.1'],
+                                                 'external': ['x.x.x.1']}}},
+                {'output_key': 'VipMap',
+                 'output_value': {'ctlplane': 'x.x.x.4', 'redis': 'x.x.x.6'}},
+                {'output_key': 'RoleData',
+                 'output_value': {'Undercloud': {'config_settings': 'foo1'}}}
+            ]
+        }
+        plan_name = 'undercloud'
+        hclient = MagicMock()
+        hclient.stacks.environment.return_value = {'parameter_defaults': {
+            'AdminPassword': 'theadminpw', 'ContainerCli': 'podman'}}
+        mock_stack = MagicMock()
+        mock_stack.outputs = outputs_data['outputs']
+        hclient.stacks.get.return_value = mock_stack
+
+        outputs = StackOutputs(mock_stack)
+        inventory = TripleoInventory(
+            hclient=hclient,
+            plan_name=plan_name,
+            auth_url='xyz://keystone.local',
+            cacert='acacert',
+            project_name='admin',
+            username='admin',
+            ansible_ssh_user='heat-admin')
+        inventory.stack_outputs = outputs
+        expected = {
+            'Undercloud': {
+                'hosts': {
+                    'uc0': {
+                        'ansible_host': 'x.x.x.1',
+                        'canonical_hostname': 'uc0.lab.example.com',
+                        'ctlplane_hostname': 'uc0.ctlplane.localdomain',
+                        'ctlplane_ip': 'x.x.x.1',
+                        'deploy_server_id': 'a',
+                        'external_hostname': 'uc0.external.localdomain',
+                        'external_ip': 'x.x.x.1'}},
+                'vars': {
+                    'ansible_ssh_user': 'heat-admin',
+                    'bootstrap_server_id': 'a',
+                    'serial': 1,
+                    'tripleo_role_name': 'Undercloud',
+                    'tripleo_role_networks': ['ctlplane', 'external']}},
+            'allovercloud': {
+                'children': {'Undercloud': {}},
+                'vars': {'container_cli': 'podman',
+                         'ctlplane_vip': 'x.x.x.4',
+                         'redis_vip': 'x.x.x.6'}},
+            'sb': {'children': {'Undercloud': {}},
+                   'vars': {'ansible_ssh_user': 'heat-admin'}},
+            'sa': {'children': {'Undercloud': {}},
+                   'vars': {'ansible_ssh_user': 'heat-admin'}}
+            }
+        inv_list = inventory.list(dynamic=False)
+        for k in expected:
+            self.assertEqual(expected[k], inv_list[k])
+
     def test_inventory_list_undercloud_only(self):
         self.inventory.plan_name = None
         self.inventory.undercloud_connection = 'local'