Make edpm_libvirt role compliant with ansible-lint production pro…

…file - Capitalized first letter of task names - Fixed indentation issues - Fixed jinja templating spacing issues - Changed `mode` parameter to be a string and added when needed - Added `changed_when`/`failed_when` in `command`/`shell` tasks - Fixed few missing FQCN - Added a `set -o pipefail` in a `shell` task Signed-off-by: Roberto Alfieri <[email protected]>
openstack-k8s-operators · Oct 16, 2023 · d62d404 · d62d404
1 parent e43d6f5
commit d62d404
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 62 deletions.
diff --git a/roles/edpm_libvirt/handlers/main.yml b/roles/edpm_libvirt/handlers/main.yml
@@ -14,28 +14,28 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-- name: restart libvirt containers
+- name: Restart libvirt containers
   become: true
   ansible.builtin.systemd:
     state: restarted
     name: "edpm_{{ item }}.service"
   loop: "{{ edpm_libvirt_containers }}"
-  listen: "restart libvirt"
+  listen: "Restart libvirt"
 
 
-- name: restart edpm_libvirt.target
+- name: Restart edpm_libvirt.target
   become: true
   ansible.builtin.systemd:
     name: edpm_libvirt.target
     enabled: true
     state: restarted
     daemon_reload: true
-  listen: "restart edpm_libvirt.target"
+  listen: "Restart edpm_libvirt.target"
 
-- name: reload edpm_libvirt_guests
+- name: Reload edpm_libvirt_guests
   become: true
   ansible.builtin.systemd:
     name: edpm_libvirt_guests
     enabled: true
     daemon_reload: true
-  listen: "reload edpm_libvirt_guests"
+  listen: "Reload edpm_libvirt_guests"
diff --git a/roles/edpm_libvirt/tasks/configure.yml b/roles/edpm_libvirt/tasks/configure.yml
@@ -1,5 +1,6 @@
 ---
-- name: create libvirt config dir
+
+- name: Create libvirt config dir
   tags:
     - configure
     - libvirt
@@ -12,13 +13,13 @@
     group: "{{ item.group | default(ansible_user) }}"
     mode: "{{ item.mode | default(omit) }}"
   loop:
-  - { "path": "/var/lib/openstack/config/libvirt"}
-  - { "path": "/var/lib/openstack/config/containers"}
-  - { 'path': "/var/lib/nova"}
-  - { "path": "/etc/tmpfiles.d/", "owner": "root", "group": "root"}
-  - { "path": "/var/lib/edpm-config/firewall", "owner": "root", "group": "root"}
+    - { "path": "/var/lib/openstack/config/libvirt"}
+    - { "path": "/var/lib/openstack/config/containers"}
+    - { 'path': "/var/lib/nova"}
+    - { "path": "/etc/tmpfiles.d/", "owner": "root", "group": "root"}
+    - { "path": "/var/lib/edpm-config/firewall", "owner": "root", "group": "root"}
 
-- name: create container config dirs
+- name: Create container config dirs
   become: true
   tags:
     - configure
@@ -29,35 +30,37 @@
     setype: "{{ item.setype | default('container_file_t') }}"
     owner: "{{ item.owner | default(ansible_user) }}"
     group: "{{ item.group | default(ansible_user) }}"
-    mode: 0755
+    mode: "0755"
   loop: "{{ edpm_libvirt_containers }}"
-- name: render libvirt container config.json
+
+- name: Render libvirt container config.json
   tags:
     - configure
     - libvirt
   ansible.builtin.template:
     src: "{{ item }}/config.json.j2"
     dest: "/var/lib/openstack/config/libvirt/{{ item }}/config.json"
     setype: "container_file_t"
-    mode: 0644
+    mode: "0644"
   loop: "{{ edpm_libvirt_containers }}"
   notify:
-  - restart libvirt
-- name: render libvirt config files
+    - Restart libvirt
+
+- name: Render libvirt config files
   tags:
     - configure
     - libvirt
   ansible.builtin.template:
     src: "{{ item.src }}"
     dest: "/var/lib/openstack/config/libvirt/{{ item.dest }}"
     setype: "container_file_t"
-    mode: 0644
+    mode: "0644"
   loop:
-  - {"src": "libvirt_virtlogd/virtlogd.conf", "dest": "libvirt_virtlogd/virtlogd.conf"}
-  - {"src": "libvirt_virtnodedevd/virtnodedevd.conf", "dest": "libvirt_virtnodedevd/virtnodedevd.conf"}
-  - {"src": "libvirt_virtproxyd/virtproxyd.conf", "dest": "libvirt_virtproxyd/virtproxyd.conf"}
-  - {"src": "libvirt_virtqemud/virtqemud.conf", "dest": "libvirt_virtqemud/virtqemud.conf"}
-  - {"src": "libvirt_virtqemud/qemu.conf", "dest": "libvirt_virtqemud/qemu.conf"}
-  - {"src": "libvirt_virtsecretd/virtsecretd.conf", "dest": "libvirt_virtsecretd/virtsecretd.conf"}
+    - {"src": "libvirt_virtlogd/virtlogd.conf", "dest": "libvirt_virtlogd/virtlogd.conf"}
+    - {"src": "libvirt_virtnodedevd/virtnodedevd.conf", "dest": "libvirt_virtnodedevd/virtnodedevd.conf"}
+    - {"src": "libvirt_virtproxyd/virtproxyd.conf", "dest": "libvirt_virtproxyd/virtproxyd.conf"}
+    - {"src": "libvirt_virtqemud/virtqemud.conf", "dest": "libvirt_virtqemud/virtqemud.conf"}
+    - {"src": "libvirt_virtqemud/qemu.conf", "dest": "libvirt_virtqemud/qemu.conf"}
+    - {"src": "libvirt_virtsecretd/virtsecretd.conf", "dest": "libvirt_virtsecretd/virtsecretd.conf"}
   notify:
-  - restart libvirt
+    - Restart libvirt
diff --git a/roles/edpm_libvirt/tasks/install.yml b/roles/edpm_libvirt/tasks/install.yml
@@ -1,54 +1,62 @@
 ---
-- name: ensure /var/run/libvirt is present upon reboot
+
+- name: Ensure /var/run/libvirt is present upon reboot
   tags:
     - install
     - libvirt
   become: true
   ansible.builtin.copy:
-        dest: /etc/tmpfiles.d/run-libvirt.conf
-        content: |
-          d /run/libvirt 0755 root root - -
-- name: create tempfiles
+    dest: /etc/tmpfiles.d/run-libvirt.conf
+    mode: "0644"
+    content: |
+      d /run/libvirt 0755 root root - -
+
+- name: Create Tempfiles
   tags:
     - install
     - libvirt
   become: true
-  command: systemd-tmpfiles --create
-- name: create persistent directories
+  ansible.builtin.command: systemd-tmpfiles --create
+  register: _systemd_tmpfiles_command
+  changed_when: _systemd_tmpfiles_command.rc == 0
+  failed_when: _systemd_tmpfiles_command.rc != 0
+
+- name: Create persistent directories
   tags:
     - install
     - libvirt
   become: true
-  file:
+  ansible.builtin.file:
     path: "{{ item.path }}"
     setype: "{{ item.setype | default('container_file_t') }}"
     state: directory
     owner: "{{ item.owner | default(omit) }}"
     group: "{{ item.group | default(omit) }}"
     mode: "{{ item.mode | default(omit) }}"
   loop:
-  # qemu is not in the libvirt group in the container images so use
-  # mode 755 to make /var/lib/libvirt traversable.
-  - { "path": "/var/lib/libvirt", "mode": "0755" }
-  - { "path": "/var/log/containers/libvirt", "mode": "0750" }
-  - { "path": "/var/log/containers/qemu", "mode": "0750" }
-  # dont set owner/group or mode on these, as they are managed
-  # by other roles, just ensure they exist.
-  - { "path": "/var/log/containers/stdouts" }
-  - { "path": "/var/lib/openstack/config/ceph" }
-- name: render libvirt container
+    # qemu is not in the libvirt group in the container images so use
+    # mode 755 to make /var/lib/libvirt traversable.
+    - { "path": "/var/lib/libvirt", "mode": "0755" }
+    - { "path": "/var/log/containers/libvirt", "mode": "0750" }
+    - { "path": "/var/log/containers/qemu", "mode": "0750" }
+    # dont set owner/group or mode on these, as they are managed
+    # by other roles, just ensure they exist.
+    - { "path": "/var/log/containers/stdouts" }
+    - { "path": "/var/lib/openstack/config/ceph" }
+- name: Render libvirt container
   tags:
     - install
     - libvirt
   ansible.builtin.template:
     src: "{{ item }}/{{ item }}.json.j2"
     dest: "/var/lib/openstack/config/containers/{{ item }}.json"
     setype: "container_file_t"
-    mode: 0644
+    mode: "0644"
   loop: "{{ edpm_libvirt_containers }}"
   notify:
-  - restart libvirt
-- name: deploy libvirt containers
+    - Restart libvirt
+
+- name: Deploy libvirt containers
   tags:
     - install
     - libvirt
@@ -60,12 +68,13 @@
     edpm_container_manage_config_patterns: "libvirt*.json"
     edpm_container_manage_clean_orphans: false
   register: edpm_container_manage_result
-- name: notify on container changes
+
+- name: Notify on container changes
   tags:
     - install
     - libvirt
-  ansible.builtin.debug:
+  ansible.builtin.debug:  # noqa: no-handler
     msg: "container changes: {{ edpm_container_manage_result.changed }}"
   when: edpm_container_manage_result.changed
   changed_when: edpm_container_manage_result.changed
-  notify: restart libvirt
+  notify: Restart libvirt
diff --git a/roles/edpm_libvirt/tasks/main.yml b/roles/edpm_libvirt/tasks/main.yml
@@ -14,16 +14,16 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-- name: configure libvirt
+- name: Configure libvirt
   ansible.builtin.include_tasks: configure.yml
 
-- name: install libvirt
+- name: Install libvirt
   ansible.builtin.include_tasks: install.yml
 
 - name: Flush handlers
   ansible.builtin.meta: flush_handlers
 
-- name: post-install libvirt
+- name: Post-install libvirt
   ansible.builtin.include_tasks: post-install.yml
 
 - name: Flush post-install handlers

diff --git a/roles/edpm_libvirt/tasks/post-install.yml b/roles/edpm_libvirt/tasks/post-install.yml
@@ -1,4 +1,5 @@
 ---
+
 - name: Gather Ceph configuration files
   tags:
     - install
@@ -22,15 +23,18 @@
   block:
     - name: Extract FSIDs from Ceph configuration files
       ansible.builtin.shell: |
+        set -o pipefail;
         echo {{ (item | basename).split('.')[0] }}
         awk -F '=' '/fsid/ {print $2}' {{ item }} | xargs
       register: fsids
+      changed_when: fsids.rc == 0
+      failed_when: fsids.rc != 0
       loop: "{{ found_confs.files | map(attribute='path') }}"
 
     - name: Map Ceph clusters to FSIDs
       ansible.builtin.set_fact:
         cluster_fsid_map: "{{ cluster_fsid_map | default({}) |\
-          combine({item.stdout_lines[0] : item.stdout_lines[1] }) }}"
+          \ combine({item.stdout_lines[0]: item.stdout_lines[1]}) }}"
       loop: "{{ fsids.results }}"
 
     - name: Gather Ceph keyring files
@@ -42,7 +46,7 @@
     - name: Map Ceph clusters to cephx users
       ansible.builtin.set_fact:
         cluster_user_map: "{{ cluster_user_map | default({}) |\
-          combine({item.split('.')[0] : item.split('.')[2] }) }}"
+          \ combine({item.split('.')[0]: item.split('.')[2]}) }}"
       loop: "{{ found_keys.files | map(attribute='path') | map('basename') }}"
 
     - name: Use maps to create libvirt secrets
@@ -67,15 +71,15 @@
     - install
     - post-libvirt
   ansible.builtin.include_role:
-      name: osp.edpm.edpm_nftables
-      tasks_from: "configure.yml"
+    name: osp.edpm.edpm_nftables
+    tasks_from: "configure.yml"
 - name: Reload firewall for new vnc rule
   tags:
     - install
     - post-libvirt
   ansible.builtin.include_role:
-      name: osp.edpm.edpm_nftables
-      tasks_from: "run.yml"
+    name: osp.edpm.edpm_nftables
+    tasks_from: "run.yml"
 
 - name: Install edpm_nova_libvirt_guests systemd unit file (podman)
   become: true
@@ -88,16 +92,16 @@
         dest: "/etc/systemd/system/edpm_libvirt.target"
         src: "edpm_libvirt.target"
         mode: "0644"
-      notify: "restart edpm_libvirt.target"
+      notify: "Restart edpm_libvirt.target"
     - name: Install libvirt-guests unit to stop nova_compute container before shutdown VMs
       ansible.builtin.template:
         dest: "/etc/systemd/system/edpm_libvirt_guests.service"
         src: "edpm_libvirt_guests.service"
         mode: "0644"
-      notify: "reload edpm_libvirt_guests"
+      notify: "Reload edpm_libvirt_guests"
     - name: Making sure virt-guest-shutdown.target is present
       ansible.builtin.template:
         dest: "/etc/systemd/system/virt-guest-shutdown.target"
         src: "virt-guest-shutdown.target"
         mode: "0644"
-      notify: "reload edpm_libvirt_guests"
+      notify: "Reload edpm_libvirt_guests"
diff --git a/roles/edpm_libvirt/tasks/virsh-secret.yml b/roles/edpm_libvirt/tasks/virsh-secret.yml
@@ -1,4 +1,5 @@
 ---
+
 - name: Create XML file for virsh secret on container host
   ansible.builtin.template:
     src: libvirt_virtqemud/secret.xml.j2
@@ -7,6 +8,9 @@
 
 - name: Copy XML file into libvirt_virtqemud container
   ansible.builtin.command: podman cp /tmp/secret.xml libvirt_virtqemud:/tmp/secret.xml
+  register: _podman_cp
+  changed_when: _podman_cp.rc == 0
+  failed_when: _podman_cp.rc != 0
 
 - name: Delete XML secret file on host
   ansible.builtin.file:
@@ -26,6 +30,8 @@
   ansible.builtin.command: "awk '$1 == \"key\" {print $3}' {{ key_path }}"
   no_log: true
   register: cephx_key
+  changed_when: cephx_key.rc == 0
+  failed_when: cephx_key.rc != 0
   vars:
     key_path: "{{ edpm_libvirt_ceph_path }}/{{ cluster }}.client.{{ user }}.keyring"