Add secret store unseal playbook for action runners #1969
Conversation
When CI action runners are rebooted, their secret store needs to be unsealed. Added unsealing playbook for them.
![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 introduces a new Ansible playbook for unsealing the secret store on CI action runners, which is a necessary step after they are rebooted. The playbook is clear and follows existing patterns within the project. My review includes a couple of suggestions to improve maintainability and idempotency. One suggestion is to change the pip package state from latest to present to ensure predictable behavior. The other is a recommendation to refactor common tasks into a shared role to avoid code duplication with other similar playbooks.
| - name: Set a fact about the virtualenv on the remote system | ||
| ansible.builtin.set_fact: | ||
| virtualenv: "{{ ansible_python_interpreter | dirname | dirname }}" | ||
| when: | ||
| - ansible_python_interpreter is defined | ||
| - not ansible_python_interpreter.startswith('/bin/') | ||
| - not ansible_python_interpreter.startswith('/usr/bin/') | ||
|
|
||
| - name: Ensure Python hvac module is installed | ||
| ansible.builtin.pip: | ||
| name: hvac | ||
| state: latest | ||
| extra_args: "{% if pip_upper_constraints_file %}-c {{ pip_upper_constraints_file }}{% endif %}" | ||
| virtualenv: "{{ virtualenv is defined | ternary(virtualenv, omit) }}" | ||
| become: "{{ virtualenv is not defined }}" |
There was a problem hiding this comment.
These first two tasks, for setting up the virtualenv fact and installing the hvac package, are also present in secret-store-unseal-seed.yml and secret-store-unseal-overcloud.yml. To improve maintainability and reduce code duplication, consider extracting these common tasks into a new Ansible role (e.g., vault-client-prereqs). This new role could then be included in all three playbooks, making the codebase easier to manage in the long run.
| - name: Ensure Python hvac module is installed | ||
| ansible.builtin.pip: | ||
| name: hvac | ||
| state: latest |
There was a problem hiding this comment.
For better idempotency and predictability, it's recommended to use state: present instead of state: latest. Using latest can cause the hvac package to be reinstalled on every playbook run if a newer version is available, which might not be the desired behavior and can lead to unexpected changes. present will ensure the package is installed and will not attempt to upgrade it if it's already present, while still respecting the version specified in pip_upper_constraints_file if provided.
state: present
When CI action runners are rebooted, their secret store needs to be unsealed.
Added unsealing playbook for them.