From 76dc23fd06809b412a10af8273ad530db2d3e5f8 Mon Sep 17 00:00:00 2001
From: Stuart Grace <stuart.grace@bbc.co.uk>
Date: Fri, 28 Apr 2023 17:36:31 +0100
Subject: [PATCH] Fix venv_build_targets when used with multi-arch

Change venv_build_targets data structure to a single-level dict
with keys of the form  <distro_ver>_<arch> instead of nested dicts.
When adding new architectures to the old structure, previous
entries for other architectures were overwritten, leaving only
the last seen architecture for each distro version. This could
result in a "Dict object has no attribute ..." error when trying
to build a venv for any other architure.

Closes-Bug: #2018012
Change-Id: I8ddabf996559b5300b52cad1649d8657889337cd
---
 defaults/main.yml |  2 +-
 vars/main.yml     | 38 +++++++++++++++++++-------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/defaults/main.yml b/defaults/main.yml
index b32a329..c4f1fa3 100644
--- a/defaults/main.yml
+++ b/defaults/main.yml
@@ -130,7 +130,7 @@ venv_wheel_build_enable: True
 # subsequent venv builds on this host and others. When
 # this is the same as the target host, then we will not
 # bother building wheels.
-venv_build_host: "{{ venv_build_targets[ansible_facts['distribution_version']][ansible_facts['architecture']] }}"
+venv_build_host: "{{ venv_build_targets[(ansible_facts['distribution'] | lower) | replace(' ', '_') + '_' + ansible_facts['distribution_version'] + '_' + ansible_facts['architecture']] }}"
 
 # The owner of directories and files held on the build host.
 # venv_build_host_user_name: "root"
diff --git a/vars/main.yml b/vars/main.yml
index 1742ba0..b0427fb 100644
--- a/vars/main.yml
+++ b/vars/main.yml
@@ -20,17 +20,18 @@
 # This is the data structure used to determine the build host.
 # venv_build_targets:
 #   {
-#     ansible_facts['distribution_version']: {
-#       ansible_facts['architecture']: inventory_hostname
+#     ansible_facts['distribution'] + '_' +
+#       ansible_facts['distribution_version'] + '_' +
+#            ansible_facts['architecture']: inventory_hostname
 #     }
 #   }
 #
 # Auto generation process:
-# * The automatic build targets will iterate over the group name
-#   "repo_all" and if any target is found it will catagorize it
-#   using the distro and cpu architecture criteria.
-# * If no group named "repo_all" is found the current inventory
-#   hostname will be used as the only available build target.
+# * The current inventory hostname is inserted as the first build target
+#   and will be used if no other suitable targets are found.
+# * It then iterates over the group name "repo_all" and if any targets
+#   are found it will catagorize them by distro and cpu architecture
+#   and add them to the list of targets.
 # * If no build target is found for matching the distro and cpu
 #   criteria of the active inventory item, the generator will fall
 #   back to using the active inventory host as the build target.
@@ -42,17 +43,15 @@
 #                    deployers should be using a shared file system
 #                    for the repo servers.
 venv_build_targets: |-
-  {% set targets = {
-      (ansible_facts['distribution_version'] | string): {
-          (ansible_facts['architecture'] | string): (inventory_hostname | string)
-      }
-    }
-  %}
-  {% for item in ((groups['repo_all'] | default([inventory_hostname])) | reverse) %}
-  {%   set distro = hostvars[item]['ansible_facts']['distribution_version'] %}
+  {% set targets = {} %}
+  {% for item in ((groups['repo_all'] | default([])) | reverse) %}
+  {%   set distro = (hostvars[item]['ansible_facts']['distribution'] | lower) | replace(' ', '_') %}
+  {%   set distro_ver = hostvars[item]['ansible_facts']['distribution_version'] %}
   {%   set arch = hostvars[item]['ansible_facts']['architecture'] %}
-  {%   set target_item = {(arch | string): (item | string)} %}
-  {%   set _ = targets.__setitem__(distro, target_item) %}
+  {%   set distro_arch = [distro, distro_ver, arch] | join('_') %}
+  {%   if distro_arch not in targets %}
+  {%     set _ = targets.update({distro_arch: item | string}) %}
+  {%   endif %}
   {% endfor %}
   {{ targets }}
 
@@ -60,8 +59,9 @@ _venv_wheels_play_hosts: |
   {% set wheel_groups = {} %}
   {% for host in ansible_play_hosts %}
   {%   set arch = hostvars[host]['ansible_facts']['architecture'] %}
-  {%   set distro = hostvars[host]['ansible_facts']['distribution_version'] %}
-  {%   set distro_arch = [distro, arch] | join('_') %}
+  {%   set distro = (hostvars[host]['ansible_facts']['distribution'] | lower) | replace(' ', '_') %}
+  {%   set distro_ver = hostvars[host]['ansible_facts']['distribution_version'] %}
+  {%   set distro_arch = [distro, distro_ver, arch] | join('_') %}
   {%   if distro_arch not in wheel_groups %}
   {%     set _ = wheel_groups.update({distro_arch: [host]}) %}
   {%   else %}