Improve Amphora image registration logic#2258
Conversation
Group the image rename and registration tasks in a block to ensure both are skipped if the correct image is already present. Use a more robust check using selectattr and checksum to determine if the image needs to be updated: with the old check there was no guarantee that the latest image would be the first element in the list.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Octavia Amphora image registration process by grouping the rename and registration tasks into a conditional block. The review feedback suggests simplifying the conditions by removing redundant name filters, as the image information is already filtered by name in a preceding task.
| changed_when: true | ||
| environment: "{{ openstack_auth_env }}" | ||
| delegate_to: "{{ groups['controllers'][0] }}" | ||
| - when: image_info.images | selectattr("name", "equalto", "amphora-x64-haproxy") | selectattr("checksum", "equalto", image_checksum.stat.checksum) | list | length == 0 |
There was a problem hiding this comment.
The selectattr filter for the image name is redundant here because the openstack.cloud.image_info task at line 79 already filters the results by the name amphora-x64-haproxy. Removing it makes the condition more concise and easier to read.
- when: image_info.images | selectattr("checksum", "equalto", image_checksum.stat.checksum) | list | length == 0| changed_when: true | ||
| environment: "{{ openstack_auth_env }}" | ||
| delegate_to: "{{ groups['controllers'][0] }}" | ||
| when: image_info.images | selectattr("name", "equalto", "amphora-x64-haproxy") | list | length == 1 |
There was a problem hiding this comment.
The name filter is redundant here as well, as the image_info results are already filtered by name. You can simplify this to check the length of the images list directly.
when: image_info.images | length == 1
Alex-Welsh
added
the
waiting-author-response
Alex-Welsh
left a comment
Alex-Welsh
left a comment
There was a problem hiding this comment.
Looks alright but needs linter fixes
|
Happy Friday @priteau, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
2 similar comments
|
Happy Friday @priteau, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
|
Happy Friday @priteau, this is a friendly reminder that this PR is waiting for your changes or response. Please take a look when you have a moment! Note: Once your changes are ready, remove the |
2 participants
Group the image rename and registration tasks in a block to ensure both are skipped if the correct image is already present.
Use a more robust check using selectattr and checksum to determine if the image needs to be updated: with the old check there was no guarantee that the latest image would be the first element in the list.