Fix ansible-lint for ceph_migrate role

This commit fixes most of the ansible-lint issues to match the production profile. Signed-off-by: Francesco Pantano <[email protected]>
openstack-k8s-operators · May 15, 2024 · 4fafe4f · 4fafe4f
1 parent 86bac17
commit 4fafe4f
Show file tree

Hide file tree

Showing 18 changed files with 180 additions and 158 deletions.
diff --git a/tests/roles/ceph_migrate/defaults/main.yml b/tests/roles/ceph_migrate/defaults/main.yml
@@ -9,7 +9,7 @@ ceph_daemons_layout:
 ceph_container_ns: quay.io/ceph
 ceph_container_image: ceph
 ceph_container_tag: v18
-ceph_container: "{{ ceph_container_ns }}/{{ceph_container_image}}:{{ceph_container_tag}}"
+ceph_container: "{{ ceph_container_ns }}/{{ ceph_container_image }}:{{ ceph_container_tag }}"
 # ceph_config_tmp_client_home is used to get a tmp ceph_cli that can be run
 # from a mon (usually controller-0) to connect to a Ceph cluster after the
 # node is drained and removed

diff --git a/tests/roles/ceph_migrate/meta/main.yml b/tests/roles/ceph_migrate/meta/main.yml
@@ -1,7 +1,7 @@
 galaxy_info:
-  author: your name
-  description: your role description
-  company: your company (optional)
+  author: OpenStack
+  description: Externalize a Ceph cluster and migrate daemons
+  company: Red Hat
 
   # If the issue tracker for your role is not on github, uncomment the
   # next line and provide a value
@@ -14,9 +14,9 @@ galaxy_info:
   # - GPL-3.0-only
   # - Apache-2.0
   # - CC-BY-4.0
-  license: license (GPL-2.0-or-later, MIT, etc)
+  license: Apache-2.0
 
-  min_ansible_version: 2.1
+  min_ansible_version: '2.9'
 
   # If this a Container Enabled role, provide the minimum Ansible Container version.
   # min_ansible_container_version:

diff --git a/tests/roles/ceph_migrate/tasks/ceph_cli.yaml b/tests/roles/ceph_migrate/tasks/ceph_cli.yaml
@@ -19,16 +19,15 @@
     ceph_cli: >-
       {{ ceph_container_cli }} run --rm {{ ceph_container_options }}
       --volume {{ ceph_config_home }}:/etc/ceph:z
-      {% if admin_daemon|default(false) %}
+      {% if admin_daemon | default(false) %}
       --volume /var/run/ceph/{{ ceph_fsid }}:/var/run/ceph:z
       {% endif %}
       --entrypoint {{ ceph_command | default('ceph') }}
       {{ ceph_container }}
-      {% if ceph_command|default('ceph') == 'ceph' or ceph_command|default('ceph') == 'rados' or ceph_command|default('ceph') == 'rbd' -%}
-        {% if not admin_daemon|default(false) -%}
-        --fsid {{ ceph_fsid }} -c /etc/ceph/{{ ceph_cluster }}.conf -k /etc/ceph/{{ ceph_cluster }}.client.{{ select_keyring| default('admin') }}.keyring
-        {%- endif %}
-        {% if external_cluster|default(false) -%}
-        -n client.{{ select_keyring }}
-        {%- endif %}
+      {% if ceph_command | default('ceph') == 'ceph' or ceph_command | default('ceph') == 'rados' or ceph_command | default('ceph') == 'rbd' -%}
+        {% if not admin_daemon | default(false) -%}
+        --fsid {{ ceph_fsid }} -c /etc/ceph/{{ ceph_cluster }}.conf -k /etc/ceph/{{ ceph_cluster }}.client.{{ select_keyring | default('admin') }}.keyring
       {%- endif %}
+        {% if external_cluster | default(false) -%}
+        -n client.{{ select_keyring }}
+      {%- endif %}{%- endif %}
diff --git a/tests/roles/ceph_migrate/tasks/ceph_client.yaml b/tests/roles/ceph_migrate/tasks/ceph_client.yaml
@@ -17,11 +17,12 @@
   delegate_to: "{{ cur_mon.split('.')[0] }}.ctlplane"
   block:
     - name: TMP_CLIENT - Patch os-net-config config and setup a tmp client IP
-      lineinfile:
+      ansible.builtin.lineinfile:
         dest: "{{ os_net_conf_path }}"
         insertafter: "{{ ceph_storage_net_prefix }}"
         line: "    - ip_netmask: {{ ceph_client_ip }}/24"
-        backup: yes
+        mode: '0666'
+        backup: true
 
     - name: TMP_CLIENT - Refresh os-net-config
       ansible.builtin.command:
@@ -37,6 +38,7 @@
       ansible.builtin.file:
         path: "{{ ceph_config_tmp_client_home }}"
         state: directory
+        mode: '0755'
     - name: Check file in the src directory
       ansible.builtin.find:
         paths: /etc/ceph
@@ -45,10 +47,11 @@
     - name: Backup ceph client data
       when: dir_ceph_files.files | length > 0
       become: true
-      copy:
+      ansible.builtin.copy:
         remote_src: true
         src: "/etc/ceph/{{ item }}"
         dest: "{{ ceph_config_tmp_client_home }}"
+        mode: '0666'
       loop:
         - ceph.conf
         - ceph.client.admin.keyring
diff --git a/tests/roles/ceph_migrate/tasks/ceph_containers.yaml b/tests/roles/ceph_migrate/tasks/ceph_containers.yaml
@@ -15,13 +15,13 @@
 # under the License.
 
 - name: Get ceph_cli
-  include_tasks: ceph_cli.yaml
+  ansible.builtin.include_tasks: ceph_cli.yaml
   vars:
     ceph_fsid: "{{ mon_dump.fsid }}"
     ceph_cluster: ceph
 
 - name: set container image base in ceph configuration
-  command: |
+  ansible.builtin.command: |
     {{ ceph_cli }} config set mgr mgr/cephadm/container_image_base {{ ceph_container }}
   changed_when: false
   become: true
@@ -31,25 +31,25 @@
   become: true
   block:
     - name: set alertmanager container image in ceph configuration
-      command: |
+      ansible.builtin.command: |
         {{ ceph_cli }} config set mgr mgr/cephadm/container_image_alertmanager \
         {{ ceph_alertmanager_container_image }}
       changed_when: false
 
     - name: set grafana container image in ceph configuration
-      command: |
+      ansible.builtin.command: |
         {{ ceph_cli }} config set mgr mgr/cephadm/container_image_grafana \
         {{ ceph_grafana_container_image }}
       changed_when: false
 
     - name: set node-exporter container image in ceph configuration
-      command: |
+      ansible.builtin.command: |
         {{ ceph_cli }} config set mgr mgr/cephadm/container_image_node_exporter \
         {{ ceph_node_exporter_container_image }}
       changed_when: false
 
     - name: set prometheus container image in ceph configuration
-      command: |
+      ansible.builtin.command: |
         {{ ceph_cli }} config set mgr mgr/cephadm/container_image_prometheus \
         {{ ceph_prometheus_container_image }}
       changed_when: false

diff --git a/tests/roles/ceph_migrate/tasks/ceph_load.yaml b/tests/roles/ceph_migrate/tasks/ceph_load.yaml
@@ -15,6 +15,7 @@
         remote_src: true
         src: "{{ ceph_config_tmp_client_home }}/{{ item }}"
         dest: "/etc/ceph"
+        mode: '0644'
       loop:
         - ceph.conf
         - ceph.client.admin.keyring
@@ -75,15 +76,16 @@
       become: true
       ansible.builtin.command: |
         cephadm shell -- ceph orch host ls -f json
-      register: hosts
-    - ansible.builtin.set_fact:
-        hosts: "{{ hosts.stdout | from_json }}"
+      register: nds
+    - name: Load nodes
+      ansible.builtin.set_fact:
+        nds: "{{ nds.stdout | from_json }}"
     - name: Load hostmap List
       ansible.builtin.set_fact:
         hostmap: |-
           {% set hm = {} %}
-          {% for h in hosts %}
-          {%     set _ = hm.__setitem__(h.hostname,h.labels) %}
+          {% for h in nds %}
+          {%     set _ = hm.__setitem__(h.hostname, h.labels) %}
           {% endfor %}
           {{ hm }}
     - name: Print Host Map
@@ -100,7 +102,8 @@
       ansible.builtin.command:
         cephadm shell -- ceph mon dump -f json
       register: mons
-    - ansible.builtin.set_fact:
+    - name: Get Monmap
+      ansible.builtin.set_fact:
         mon_dump: "{{ mons.stdout | from_json }}"
     - name: Print monmap
       when: debug | default(false)

diff --git a/tests/roles/ceph_migrate/tasks/ceph_validate.yaml b/tests/roles/ceph_migrate/tasks/ceph_validate.yaml
@@ -1,86 +1,101 @@
-- name: Fail if input is not provided
-  fail:
+- name: ansible.builtin.fail if input is not provided
+  ansible.builtin.fail:
     msg: "Input not ready"
   when: ceph is undefined or ceph | length == 0
 
 - name: Validate SRC Ceph Cluster HEALTH
+  tags:
+    - ceph_health
   block:
-    - ansible.builtin.debug:
+    - name: Get cluster health
+      ansible.builtin.debug:
         msg: "{{ ceph.health.status }}"
       when: debug | default(false)
 
-    - name: Fail if health is HEALTH_WARN || HEALTH_ERR
-      fail:
+    - name: ansible.builtin.fail if health is HEALTH_WARN || HEALTH_ERR
+      ansible.builtin.fail:
         msg: Ceph is in {{ ceph.health.status }} state.
       when:
         - ceph.health.status == 'HEALTH_WARN' or
           ceph.health.status == 'HEALTH_ERR'
 
 - name: Validate SRC Ceph Cluster PGMAP
+  when: ceph.pgmap is defined
+  vars:
+    pgstate: "{{ ceph | community.general.json_query('pgmap.pgs_by_state[0].state_name') }}"
   block:
-    - ansible.builtin.debug:
-        msg: "{{ ceph.pgmap.pgs_by_state[0].state_name }}"
+    - name: PgMap
       when: debug | default(false)
+      ansible.builtin.debug:
+        msg: "PGs State is: {{ pgstate }}"
 
-    - name: Fail if PGs are not in active+clean state
-      fail:
-        msg: "{{ PGs State is: ceph.pgmap.pgs_by_state[0].state_name }}"
-      when: ceph.pgmap.pgs_by_state[0].state_name != 'active+clean'
+    - name: ansible.builtin.fail if PGs are not in active+clean state
+      when: pgstate != 'active+clean'
+      ansible.builtin.fail:
+        msg: "PGs State is: {{ pgstate }}"
 
 - name: Validate SRC Ceph Cluster OSDMAP
+  when: ceph.osdmap is defined
+  vars:
+    num_up_osds: "{{ ceph | community.general.json_query('osdmap.num_up_osds') | default(0) }}"
+    num_osds: "{{ ceph | community.general.json_query('osdmap.num_osds') | default(1) }}"
+    pct: "{{ (num_up_osds | float | default(0.0) / num_osds | float | default(1.0)) * 100 }}"
   block:
-    - ansible.builtin.debug:
-        msg: "{{ ceph.osdmap.num_up_osds | default(0) / ceph.osdmap.num_osds | default(0) }}"
-      when: debug | default(false)
+    - name: OSDMap
+      ansible.builtin.debug:
+        msg: "{{ pct }}"
+      when: debug | default(true)
 
-    - name: Fail if there is an unacceptable OSDs number
-      fail:
-        msg: "{{ ceph.osdmap.num_up_osds|default(0) }} / {{ ceph.osdmap.num_osds|default(0) }} OSDs available"
-      when: osds | float < 1.0
-      vars:
-        osds: "{{ ceph.osdmap.num_osds / ceph.osdmap.num_up_osds }}"
+    - name: ansible.builtin.fail if there is an unacceptable OSDs number
+      ansible.builtin.fail:
+        msg: "{{ pct }} % OSDs available"
+      when: pct | float < 100
 
 - name: Validate the Ceph Cluster release
   block:
-    - ansible.builtin.debug:
+    - name: MonMap
+      ansible.builtin.debug:
         msg: "{{ ceph.monmap.min_mon_release_name }}"
       when: debug | default(false)
 
-    - name: Fail if Ceph <= Quincy
-      fail:
-        msg: "{{ Ceph version is <= Reef }}"
+    - name: ansible.builtin.fail if Ceph <= Quincy
+      ansible.builtin.fail:
+        msg: "Ceph version is <= Reef"
       when: ceph.monmap.min_mon_release_name != "quincy"
 
 - name: Mons are in quorum
   block:
-    - ansible.builtin.debug:
+    - name: Mons in quorum
+      ansible.builtin.debug:
         msg: "{{ ceph.monmap.num_mons }}"
       when: debug | default(false)
 
-    - name: Fail if Mons are not in quorum
-      fail:
+    - name: ansible.builtin.fail if Mons are not in quorum
+      ansible.builtin.fail:
         msg: "Mons: {{ ceph.monmap.num_mons }}"
       when: ceph.monmap.num_mons != decomm_nodes | length
 
 - name: Mgr is active
   block:
-    - ansible.builtin.debug:
+    - name: is Ceph Mgr available
+      ansible.builtin.debug:
         msg: "{{ ceph.mgrmap.available }}"
       when: debug | default(false)
 
-    - name: Fail if Mgr is not available
-      fail:
+    - name: ansible.builtin.fail if Mgr is not available
+      ansible.builtin.fail:
         msg: "{{ ceph.mgrmap.available }}"
       when: not ceph.mgrmap.available | bool | default(false)
 
 - name: No event in progress
   block:
-    - ansible.builtin.debug:
+    - name: in progress events
+      ansible.builtin.debug:
         msg: "{{ ceph.progress_events | length }}"
       when: debug | default(false)
 
-    - name: Fail if there are in progress events
-      fail:
+    - name: ansible.builtin.fail if there are in progress events
+      ansible.builtin.fail:
         msg: "{{ ceph.progress_events }}"
       when: ceph.progress_events | length > 0
 

diff --git a/tests/roles/ceph_migrate/tasks/drain.yaml b/tests/roles/ceph_migrate/tasks/drain.yaml
@@ -1,6 +1,6 @@
 # Refresh the client to point to a diff config-home
 - name: Get ceph_cli
-  include_tasks: ceph_cli.yaml
+  ansible.builtin.include_tasks: ceph_cli.yaml
   vars:
     ceph_config_home: "{{ ceph_config_tmp_client_home }}"
     ceph_fsid: "{{ mon_dump.fsid }}"
@@ -12,7 +12,7 @@
   vars:
     nodes:
       - "{{ host }}"
-    action: "rm"
+    act: "rm"
     labels:
       - "mon"
       - "mgr"
@@ -23,11 +23,11 @@
   become: true
   ansible.builtin.command:
     "{{ ceph_cli }} orch daemon rm mon.{{ host.split('.')[0] }} --force"
-  ignore_errors: true
+  # ignore_errors: true
 
 # Sleep before moving to the next mon
 - name: Pause
-  pause:
+  ansible.builtin.pause:
     seconds: "{{ ceph_timeout }}"
 
 - name: DRAIN - Drain the host

diff --git a/tests/roles/ceph_migrate/tasks/firewall.yaml b/tests/roles/ceph_migrate/tasks/firewall.yaml
@@ -7,7 +7,7 @@
   become: true
   block:
     - name: Ensure firewall is enabled/started
-      systemd:
+      ansible.builtin.systemd:
         name: iptables
         state: started
         enabled: true
@@ -42,13 +42,13 @@
   become: true
   block:
     - name: Save firewall rules ipv4
-      command: /usr/libexec/iptables/iptables.init save
+      ansible.builtin.command: /usr/libexec/iptables/iptables.init save
 
     - name: Save firewall rules ipv6
-      command: /usr/libexec/iptables/ip6tables.init save
+      ansible.builtin.command: /usr/libexec/iptables/ip6tables.init save
 
     - name: Reload iptables service
-      systemd:
+      ansible.builtin.systemd:
         daemon_reload: true
         enabled: true
         name: "{{ item }}"

diff --git a/tests/roles/ceph_migrate/tasks/labels.yaml b/tests/roles/ceph_migrate/tasks/labels.yaml
@@ -17,16 +17,16 @@
 - name: Print INPUT
   when: debug | default(true)
   block:
-    - name: Set/Unset labels - {{ action }}
+    - name: Set/Unset labels - {{ act }}
       ansible.builtin.debug:
         msg: "{{ labels }}"
 
     - name: Print nodes
       ansible.builtin.debug:
         msg: "{{ nodes }}"
 
-- name: Set/Unset labels - {{ action }}
+- name: Set/Unset labels - {{ act }}
   become: true
   ansible.builtin.command:
-    "{{ ceph_cli }} orch host label {{ action }} {{ item.0 }} {{ item.1 }}"
-  loop: "{{ nodes |default([]) | product(labels|default([])) | list }}"
+    "{{ ceph_cli }} orch host label {{ act }} {{ item.0 }} {{ item.1 }}"
+  loop: "{{ nodes | default([]) | product(labels | default([])) | list }}"