Skip to content

Fix Amphora register playbook failing when using TLS#2162

Open
seunghun1ee wants to merge 1 commit intostackhpc/2025.1from
fix-amphora-register
Open

Fix Amphora register playbook failing when using TLS#2162
seunghun1ee wants to merge 1 commit intostackhpc/2025.1from
fix-amphora-register

Conversation

@seunghun1ee
Copy link
Member

CA certificate was not injected correctly for this playbook.
Plus, using openstack.cloud.image Ansible module for renaming the existing old Amphora image.

CA certificate was not injected correctly for this playbook.
Plus, using openstack.cloud.image Ansible module for renaming the
existing old Amphora image.
@seunghun1ee seunghun1ee self-assigned this Feb 16, 2026
@seunghun1ee seunghun1ee requested a review from a team as a code owner February 16, 2026 11:42
@seunghun1ee
Copy link
Member Author

It would be nice to backport this to Caracal too.

@seunghun1ee seunghun1ee added bug Something isn't working ansible Ansible playbooks Epoxy labels Feb 16, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request correctly addresses the TLS issue by introducing the openstack_cacert variable and applying it to the OpenStack modules. It also transitions the image renaming task to use the openstack.cloud.image module. However, the implementation of the rename task contains a logic error: by including the filename parameter, the module will attempt to upload the new image data to the old image ID because their checksums differ. This results in an unnecessary and incorrect data upload during what should be a simple rename operation. I have provided a suggestion to simplify this task.

Comment on lines +93 to +107
openstack.cloud.image:
auth_type: password
auth: "{{ openstack_auth }}"
ca_cert: "{{ openstack_cacert }}"
interface: "{{ openstack_interface }}"
id: "{{ image_info.images[0].id }}"
name: "amphora-x64-haproxy-{{ ansible_facts.date_time.iso8601_basic_short }}" # Change the name
tags: ["amphora"]
container_format: bare
disk_format: qcow2
is_public: false
filename: "{{ image_path }}"
properties:
hw_architecture: x86_64
hw_rng_model: virtio
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The "rename" task should not include the filename parameter. When filename is provided along with an id, the openstack.cloud.image module compares the local file's checksum with the existing image in Glance. Since the when condition (line 110) ensures the checksums are different, the module will attempt to upload the file at image_path (the new image) to the image identified by id (the old image). This results in the old image being overwritten with the new data before it is renamed, which is redundant and incorrect for an archival/rename step. Additionally, parameters like container_format, disk_format, is_public, and properties are unnecessary for a simple rename and should be removed to avoid accidental metadata updates.

      openstack.cloud.image:
        auth_type: password
        auth: "{{ openstack_auth }}"
        ca_cert: "{{ openstack_cacert }}"
        interface: "{{ openstack_interface }}"
        id: "{{ image_info.images[0].id }}"
        name: "amphora-x64-haproxy-{{ ansible_facts.date_time.iso8601_basic_short }}"
        tags: ["amphora"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ansible Ansible playbooks bug Something isn't working Epoxy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant