From 8de02b82b4fb9cd4912d31fa3c9359820545b384 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Wed, 19 Jun 2024 17:24:37 +0100
Subject: [PATCH] Avoid shared IPA image cache on Ansible control host

When using overcloud Ironic, IPA images for Ironic inspector are
downloaded to the Ansible control host to a cache directory, by default
/opt/kayobe/images/ipa/. They are later copied into the local Kolla
Ansible configuration under
etc/kolla/config/ironic/ironic-agent.{initramfs,kernel}.

The use of a shared cache directory results in problems when multiple
users share a single Ansible control host, since the cache is created as
writeable only for the user that created it. Other users sharing the
same Ansible control host will be unable to write to the cache.

We may also see issues if multiple Kayobe environments using different
IPA images are deployed from one Ansible control host.

The cache is not strictly necessary, since we can download the images
directly to the kayobe-config repo. This change avoids the use of the
cache. The performance impact should be minimal, only requiring an
additional download when a fresh kayobe-config is used.

Change-Id: I022c53afc0f64ccc79eeff4a220ade4c9216edfc
Closes-Bug: #2069845
---
 ansible/kolla-openstack.yml                   | 38 +++---------------
 .../roles/image-download/defaults/main.yml    |  4 ++
 ansible/roles/image-download/tasks/main.yml   | 40 ++++++++++++-------
 .../roles/kolla-openstack/defaults/main.yml   |  5 +++
 .../roles/kolla-openstack/tasks/config.yml    |  1 +
 ...move-local-ipa-cache-1fe125360f5d3385.yaml |  7 ++++
 6 files changed, 48 insertions(+), 47 deletions(-)
 create mode 100644 releasenotes/notes/remove-local-ipa-cache-1fe125360f5d3385.yaml

diff --git a/ansible/kolla-openstack.yml b/ansible/kolla-openstack.yml
index d351cd1b..5891f188 100644
--- a/ansible/kolla-openstack.yml
+++ b/ansible/kolla-openstack.yml
@@ -46,37 +46,6 @@
               when: not item.stat.exists
           tags:
             - config-validation
-
-        - name: Check whether the image cache directory exists
-          local_action:
-            module: stat
-            path: "{{ hostvars.localhost.image_cache_path }}"
-            get_checksum: False
-            mime: False
-          register: image_cache_stat
-
-        - name: Ensure the image cache directory exists
-          local_action:
-            module: file
-            path: "{{ hostvars.localhost.image_cache_path }}"
-            state: directory
-            owner: "{{ lookup('env', 'USER') }}"
-            group: "{{ lookup('env', 'USER') }}"
-          become: True
-          when: >-
-            not image_cache_stat.stat.exists or
-            not image_cache_stat.stat.writeable
-
-        - name: Ensure Ironic Python Agent images are copied onto the local machine
-          fetch:
-            src: "{{ image_cache_path }}/{{ ipa_image_name }}/{{ item.src }}"
-            dest: "{{ hostvars.localhost.image_cache_path }}/{{ ipa_image_name }}/{{ item.dest }}"
-            flat: True
-          with_items:
-            - src: "{{ ipa_images[0] }}"
-              dest: "{{ ipa_images_kernel_name }}"
-            - src: "{{ ipa_images[1] }}"
-              dest: "{{ ipa_images_ramdisk_name }}"
       when:
         - kolla_enable_ironic | bool
         - ipa_build_images | bool
@@ -149,9 +118,11 @@
 
             - name: Set facts containing IPA kernel and ramdisk paths
               set_fact:
-                kolla_inspector_ipa_kernel_path: "{{ image_cache_path }}/{{ ipa_image_name }}/{{ ipa_images_kernel_name }}"
-                kolla_inspector_ipa_ramdisk_path: "{{ image_cache_path }}/{{ ipa_image_name }}/{{ ipa_images_ramdisk_name }}"
+                kolla_inspector_ipa_kernel_path: "{{ hostvars[inspector_ipa_host].image_cache_path }}/{{ ipa_image_name }}/{{ ipa_images_kernel_name }}"
+                kolla_inspector_ipa_ramdisk_path: "{{ hostvars[inspector_ipa_host].image_cache_path }}/{{ ipa_image_name }}/{{ ipa_images_ramdisk_name }}"
               when: ipa_build_images | bool
+              vars:
+                inspector_ipa_host: "{{ groups['controllers_with_ironic_enabled_True'][0] }}"
           when: kolla_enable_ironic | bool
       tags:
         - config
@@ -169,6 +140,7 @@
       kolla_inspector_swift_auth:
         auth_type: none
         endpoint_override: "http://{% raw %}{{ api_interface_address }}{% endraw %}:{{ inspector_store_port }}"
+      kolla_inspector_ipa_host: "{{ groups['controllers_with_ironic_enabled_True'][0] }}"
       kolla_openstack_custom_config_paths_extra_multi_env_static:
         - "{{ kayobe_config_path }}"
       kolla_openstack_custom_config_paths_extra_multi_env: "{{ kolla_openstack_custom_config_paths_extra_multi_env_static + kayobe_env_search_paths }}"
diff --git a/ansible/roles/image-download/defaults/main.yml b/ansible/roles/image-download/defaults/main.yml
index 045c0be0..52bec6dd 100644
--- a/ansible/roles/image-download/defaults/main.yml
+++ b/ansible/roles/image-download/defaults/main.yml
@@ -17,3 +17,7 @@ image_download_path:
 
 # Path to the image's destination.
 image_download_dest:
+
+# Host from which to fetch the image.
+# Only used when image_download_path is set.
+image_download_host: "{{ inventory_hostname }}"
diff --git a/ansible/roles/image-download/tasks/main.yml b/ansible/roles/image-download/tasks/main.yml
index babc21db..3a4dc4ba 100644
--- a/ansible/roles/image-download/tasks/main.yml
+++ b/ansible/roles/image-download/tasks/main.yml
@@ -9,8 +9,7 @@
         - name: Fail if the checksum algorithm is not set
           fail:
             msg: "Checksum algorithm for image {{ image_download_url }} not set"
-          when: image_download_checksum_algorithm is none or
-                image_download_checksum_algorithm == ""
+          when: image_download_checksum_algorithm is falsy
 
         - name: Get the expected checksum
           uri:
@@ -21,8 +20,7 @@
           retries: 3
           delay: 5
       when:
-        - image_download_checksum_url is not none
-        - image_download_checksum_url != ""
+        - image_download_checksum_url is truthy
 
     - name: Ensure the image is downloaded
       vars:
@@ -42,14 +40,28 @@
       retries: 3
       delay: 5
   when:
-    - image_download_url is not none
-    - image_download_url != ""
+    - image_download_url is truthy
 
-- name: Ensure the local image is copied
-  copy:
-    src: "{{ image_download_path }}"
-    dest: "{{ image_download_dest }}"
-    mode: 0640
-  when:
-    - image_download_path is not none
-    - image_download_path != ""
+- when: image_download_path is truthy
+  block:
+    - name: Ensure the local image is copied
+      copy:
+        src: "{{ image_download_path }}"
+        dest: "{{ image_download_dest }}"
+        mode: 0640
+      when:
+        - image_download_host is falsy
+
+    - name: Ensure the remote image is fetched
+      fetch:
+        src: "{{ image_download_path }}"
+        dest: "{{ image_download_dest }}"
+        mode: 0640
+        flat: true
+      when:
+        - image_download_host is truthy
+      delegate_to: "{{ image_download_host | default('localhost', true) }}"
+      vars:
+        # NOTE: Without this, the hosts's ansible_host variable will not be
+        # respected when using delegate_to.
+        ansible_host: "{{ hostvars[image_download_host].ansible_host | default(image_download_host) }}"
diff --git a/ansible/roles/kolla-openstack/defaults/main.yml b/ansible/roles/kolla-openstack/defaults/main.yml
index b389e8eb..d5527f7d 100644
--- a/ansible/roles/kolla-openstack/defaults/main.yml
+++ b/ansible/roles/kolla-openstack/defaults/main.yml
@@ -617,6 +617,11 @@ kolla_inspector_ipa_kernel_path:
 # Mutually exclusive with kolla_inspector_ipa_ramdisk_upstream_url.
 kolla_inspector_ipa_ramdisk_path:
 
+# Host from which to fetch Ironic Python Agent (IPA) kernel and ramdisk images
+# for Ironic Inspector.  Only used when kolla_inspector_ipa_kernel_path or
+# kolla_inspector_ipa_ramdisk_path is set.
+kolla_inspector_ipa_host:
+
 # Whether to enable the Swift introspection data store.
 kolla_inspector_enable_swift:
 
diff --git a/ansible/roles/kolla-openstack/tasks/config.yml b/ansible/roles/kolla-openstack/tasks/config.yml
index ea999ad7..2aa8948a 100644
--- a/ansible/roles/kolla-openstack/tasks/config.yml
+++ b/ansible/roles/kolla-openstack/tasks/config.yml
@@ -6,6 +6,7 @@
     image_download_checksum_algorithm: "{{ item.checksum_algorithm }}"
     image_download_path: "{{ item.path }}"
     image_download_dest: "{{ item.dest }}"
+    image_download_host: "{{ kolla_inspector_ipa_host }}"
   include_role:
     name: image-download
   with_items:
diff --git a/releasenotes/notes/remove-local-ipa-cache-1fe125360f5d3385.yaml b/releasenotes/notes/remove-local-ipa-cache-1fe125360f5d3385.yaml
new file mode 100644
index 00000000..2d395c08
--- /dev/null
+++ b/releasenotes/notes/remove-local-ipa-cache-1fe125360f5d3385.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue when using overcloud Ironic with a shared Ansible control
+    host. The use of a shared cache directory could lead to a failure to
+    download Ironic Python Agent (IPA) images. `LP#2069845
+    <https://bugs.launchpad.net/kayobe/+bug/2069845>`__
-- 
GitLab