Merge "Fix key-order[task] linter warnings"

.ansible-lint

@@ -4,7 +4,6 @@ skip_list:
   - fqcn[action-core] # Use FQCN for builtin actions
   - ignore-errors #  Use failed_when and specify error conditions instead of using ignore_errors
   - jinja[invalid] # Unrecognized type
-  - key-order[task] # You can improve the task key order
   - meta-no-info # meta/main.yml should contain relevant info
   - meta-no-tags # Tags must contain lowercase letters and digits only
   - name[casing] #All names should start with an uppercase letter

playbooks/roles/bifrost-create-vm-nodes/tasks/create_vm.yml

@@ -42,43 +42,44 @@
   register: existing_vms
 
 # NOTE(pas-ha) wrapping in block/rescue to have diagnostic output, requires Ansible>=2
-- block:
+- when: vm_name not in existing_vms.list_vms
-  # NOTE(pas-ha) Ansible still lacks modules to operate on libvirt volumes
+  block:
-  # NOTE(pas-ha) adding extra 1G for disk size to accomodate for partition table / configdrive
+    # NOTE(pas-ha) Ansible still lacks modules to operate on libvirt volumes
-  - name: create volume for vm
+    # NOTE(pas-ha) adding extra 1G for disk size to accomodate for partition table / configdrive
-    command: >
+    - name: create volume for vm
-      virsh --connect {{ test_vm_libvirt_uri }}
+      command: >
-      vol-create-as {{ test_vm_storage_pool }} {{ vm_name }}.qcow2
+        virsh --connect {{ test_vm_libvirt_uri }}
-      {{ test_vm_disk_gib | int + 1 }}G
+        vol-create-as {{ test_vm_storage_pool }} {{ vm_name }}.qcow2
-      --format qcow2 {{ prealloc|default("") }}
+        {{ test_vm_disk_gib | int + 1 }}G
-    when: (vm_name + '.qcow2') not in ansible_libvirt_pools[test_vm_storage_pool].volumes
+        --format qcow2 {{ prealloc|default("") }}
+      when: (vm_name + '.qcow2') not in ansible_libvirt_pools[test_vm_storage_pool].volumes
 
-  - name: set path to the volume created
+    - name: set path to the volume created
-    set_fact:
+      set_fact:
-      vm_volume_path: "{{ ansible_libvirt_pools[test_vm_storage_pool].path }}/{{ vm_name }}.qcow2"
+        vm_volume_path: "{{ ansible_libvirt_pools[test_vm_storage_pool].path }}/{{ vm_name }}.qcow2"
 
-  - name: pre-touch the vm volume
+    - name: pre-touch the vm volume
-    file:
+      file:
-      state: touch
+        state: touch
-      path: "{{ vm_volume_path }}"
+        path: "{{ vm_volume_path }}"
-    when: test_vm_libvirt_uri == 'qemu:///system'
+      when: test_vm_libvirt_uri == 'qemu:///system'
 
-  # NOTE(TheJulia): CentOS default installs with an XFS root, and chattr
+    # NOTE(TheJulia): CentOS default installs with an XFS root, and chattr
-  # fails to set +C on XFS.  This could be more elegant, however the use
+    # fails to set +C on XFS.  This could be more elegant, however the use
-  # case is for CI testing.
+    # case is for CI testing.
-  - name: set copy-on-write for volume on non-CentOS systems
+    - name: set copy-on-write for volume on non-CentOS systems
-    command: chattr +C {{ vm_volume_path }}
+      command: chattr +C {{ vm_volume_path }}
-    ignore_errors: yes
+      ignore_errors: yes
-    when:
+      when:
-      - ansible_distribution != 'CentOS'
+        - ansible_distribution != 'CentOS'
-      - test_vm_libvirt_uri == 'qemu:///system'
+        - test_vm_libvirt_uri == 'qemu:///system'
 
-  - name: create_vm
+    - name: create_vm
-    virt:
+      virt:
-      command: define
+        command: define
-      name: "{{ vm_name }}"
+        name: "{{ vm_name }}"
-      uri: "{{ test_vm_libvirt_uri }}"
+        uri: "{{ test_vm_libvirt_uri }}"
-      xml: "{{ lookup('template', 'testvm.xml.j2') }}"
+        xml: "{{ lookup('template', 'testvm.xml.j2') }}"
 
   rescue:
     - name: "Execute `dmesg` to collect debugging output should VM creation fail."
@@ -92,7 +93,6 @@
         msg: >-
           VM creation step failed, please review dmesg
           output for additional details
-  when: vm_name not in existing_vms.list_vms
 
 # TODO(pas-ha) replace 'command: vbmc ...' tasks
 # with a custom Ansible module using vbmc Python API

playbooks/roles/bifrost-create-vm-nodes/tasks/prepare_libvirt.yml

@@ -309,17 +309,18 @@
     path: /tmp/sushy-emulator
   register: redfish_emulator_old_state_dir
 
-- block:
+- become: true
-  - name: stop Redfish Emulator before migrating its state directory
-    systemd:
-      name: redfish-emulator
-      state: stopped
-  - name: migrate existing Redfish Emulator state directory
-    shell: mv /tmp/sushy-emulator/*.sqlite "{{ redfish_emulator_state_dir }}"
-  become: true
   ignore_errors: true
-  when: redfish_emulator_old_state_dir.stat.exists
+  when:
-        and redfish_emulator_state_dir != '/tmp/sushy-emulator'
+    - redfish_emulator_old_state_dir.stat.exists
+    - redfish_emulator_state_dir != '/tmp/sushy-emulator'
+  block:
+    - name: stop Redfish Emulator before migrating its state directory
+      systemd:
+        name: redfish-emulator
+        state: stopped
+    - name: migrate existing Redfish Emulator state directory
+      shell: mv /tmp/sushy-emulator/*.sqlite "{{ redfish_emulator_state_dir }}"
 
 - name: ensure Redfish Emulator systemd service is started and enabled
   systemd:

playbooks/roles/bifrost-deploy-nodes-dynamic/tasks/main.yml

@@ -71,11 +71,13 @@
   when: inventory_dhcp | bool or inventory_dns | bool
 
 - name: "Create instance info"
+  when: instance_info is not defined or instance_info == {}
   block:
-
     - name: "Figure out image checksum"
+      when:
+        - deploy_image_checksum is not defined
+        - not deploy_image_source.startswith('file://')
       block:
-
         - name: "Collect the checksum of the deployment image."
           stat:
             path: "{{ deploy_image_path }}"
@@ -93,10 +95,6 @@
           set_fact:
             deploy_image_checksum: "{{ test_deploy_image.stat.checksum }}"
 
-      when:
-        - deploy_image_checksum is not defined
-        - not deploy_image_source.startswith('file://')
-
     - name: "Set generated instance_info"
       set_fact:
         instance_info:
@@ -106,8 +104,6 @@
           image_type: "{{ deploy_image_type | default(omit) }}"
           root_gb: "{{ deploy_root_gb if deploy_image_type | default('') == 'partition' else omit }}"
 
-  when: instance_info is not defined or instance_info == {}
-
 - name: "Deploy to hardware"
   openstack.cloud.baremetal_node_action:
     cloud: "{{ cloud_name | default(omit) }}"

playbooks/roles/bifrost-download-packages/tasks/main.yml

@@ -20,35 +20,35 @@
   set_fact:
     download_packages_string: "{{ download_packages | join(' ') }}"
 
-- block:
+- when: ansible_os_family == 'Debian'
-  - name: "Download {{ download_packages_string }}"
+  block:
-    command: apt-get download {{ download_packages_string }}  # noqa: command-instead-of-module
+    - name: "Download {{ download_packages_string }}"
-    args:
+      command: apt-get download {{ download_packages_string }}  # noqa: command-instead-of-module
-      chdir: "{{ download_dest }}"
+      args:
+        chdir: "{{ download_dest }}"
 
-  - name: "Unpack {{ download_packages_string }}"
+    - name: "Unpack {{ download_packages_string }}"
-    shell: >
+      shell: >
-      dpkg-deb -R {{ download_dest }}/{{ item }}_*.deb {{ download_dest }}/{{ item }}
+        dpkg-deb -R {{ download_dest }}/{{ item }}_*.deb {{ download_dest }}/{{ item }}
-    loop: "{{ download_packages }}"
+      loop: "{{ download_packages }}"
-  when: ansible_os_family == 'Debian'
 
-- block:
+- when: ansible_os_family == 'RedHat'
-  - name: "Download {{ download_packages_string }}"
+  block:
-    command: >
+    - name: "Download {{ download_packages_string }}"
-      dnf download --downloaddir {{ download_dest }} {{ download_packages_string }}
+      command: >
+        dnf download --downloaddir {{ download_dest }} {{ download_packages_string }}
 
-  - name: "Create a subdirectory for the package {{ item }}"
+    - name: "Create a subdirectory for the package {{ item }}"
-    file:
+      file:
-      path: "{{ download_dest }}/{{ item }}"
+        path: "{{ download_dest }}/{{ item }}"
-      state: directory
+        state: directory
-    loop: "{{ download_packages }}"
+      loop: "{{ download_packages }}"
 
-  - name: "Unpack {{ download_packages_string }}"
+    - name: "Unpack {{ download_packages_string }}"
-    shell: |
+      shell: |
-      set -eo pipefail
+        set -eo pipefail
-      rpm2cpio {{ download_dest }}/{{ item }}-*.rpm | cpio -idm
+        rpm2cpio {{ download_dest }}/{{ item }}-*.rpm | cpio -idm
-    args:
+      args:
-      chdir: "{{ download_dest }}/{{ item }}"
+        chdir: "{{ download_dest }}/{{ item }}"
-      executable: /bin/bash
+        executable: /bin/bash
-    loop: "{{ download_packages }}"
+      loop: "{{ download_packages }}"
-  when: ansible_os_family == 'RedHat'

playbooks/roles/bifrost-ironic-install/ssh_public_key_path.yaml

@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ---
-- block:
+- delegate_to: localhost
+  block:
     - name: "Find a suitable SSH public key"
       set_fact:
         ssh_public_key_path: "{{ item }}"
@@ -37,7 +38,6 @@
       fail:
         msg: "ssh_public_key_path {{ ssh_public_key_path }} was not found"
       when: not test_ssh_public_key_path.stat.exists
-  delegate_to: localhost
 
 - name: "Read SSH public key in ssh_public_key"
   set_fact:

playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml

@@ -401,7 +401,8 @@
     - not create_ipa_image | bool
     - download_ipa | bool
 
-- block:
+- when: use_cirros | bool
+  block:
     - name: "Download cirros to use for deployment if requested"
       get_url:
         url: "{{ cirros_deploy_image_upstream_url }}"
@@ -418,9 +419,9 @@
         owner: ironic
         group: ironic
         mode: "0644"
-  when: use_cirros | bool
 
-- block:
+- when: use_cirros | bool
+  block:
     - name: "Create a checksum file for cirros"
       shell: md5sum {{ deploy_image_filename }} > {{ deploy_image_filename }}.CHECKSUMS
       args:
@@ -431,7 +432,6 @@
         owner: ironic
         group: ironic
         mode: "0644"
-  when: use_cirros | bool
 
 - name: "Bootstrap Nginx"
   import_role:
@@ -522,7 +522,10 @@
     - 6385
   when: use_firewalld | bool
 
-- block:
+- when:
+    - ansible_os_family == 'RedHat'
+    - ansible_selinux.status == 'enabled'
+  block:
     - name: "Allow nginx, ironic, inspector and IPA ports on SELinux"
       seport:
         ports: "{{ file_url_port }},{{ file_url_port_tls }},6385,5050,9999"
@@ -562,8 +565,6 @@
         - "{{ http_boot_folder }}"
         - "{{ tftp_boot_folder }}"
         - "{{ ironic_tftp_master_path }}"
-  when: (ansible_os_family == 'RedHat' or ansible_os_family == 'Suse') and
-         ansible_selinux.status == 'enabled'
 
 - name: "Configure remote logging"
   template:

playbooks/roles/bifrost-ironic-install/tasks/create_tftpboot.yml

@@ -142,6 +142,7 @@
     - "{{ http_boot_folder }}"
 
 - name: "Set up iPXE for EFI booting"
+  when: "'ipxe' in enabled_boot_interfaces.split(',')"
   block:
     - name: "Check if the iPXE EFI image is present"
       stat:
@@ -177,8 +178,6 @@
         mode: "0644"
         remote_src: true
 
-  when: "'ipxe' in enabled_boot_interfaces.split(',')"
-
 # Similar logic to below can be utilized to retrieve files
 - name: "Determine if folder exists, else create and populate folder."
   stat:

playbooks/roles/bifrost-ironic-install/tasks/install.yml

@@ -18,41 +18,40 @@
     update_cache: yes
   when: ansible_os_family == 'Debian'
 
-- block:
+- name: "Enable EPEL repositories for known EL distributions"
-  - name: "Enable powertools repository for EL8"
+  when:
-    command: dnf config-manager --set-enabled powertools
+    - enable_epel | bool
-    when: ansible_distribution_version|int == 8
+    - ansible_distribution in ["AlmaLinux", "CentOS", "Rocky"]
+  block:
+    - name: "Enable powertools repository for EL8"
+      command: dnf config-manager --set-enabled powertools
+      when: ansible_distribution_version|int == 8
 
-  - name: "Enable crb repository for EL9"
+    - name: "Enable crb repository for EL9"
-    command: dnf config-manager --set-enabled crb
+      command: dnf config-manager --set-enabled crb
-    when: ansible_distribution_version|int == 9
+      when: ansible_distribution_version|int == 9
 
-  - name: "Enable EPEL repository for EL7/8"
+    - name: "Enable EPEL repository for EL7/8/9"
-    ansible.builtin.dnf:
+      ansible.builtin.dnf:
-      name: epel-release
+        name: epel-release
-      state: latest
+        state: latest
-    when:
-      - enable_epel | bool
-      - ansible_distribution in ["AlmaLinux", "CentOS", "Rocky"]
 
-  - name: "Enable EPEL repositories for EL9"
+    - name: "Enable EPEL-next repository for Centos 9-Stream"
-    ansible.builtin.dnf:
+      ansible.builtin.dnf:
-      name:
+        name: epel-next-release
-        - epel-next-release
+        state: latest
-      state: latest
+      when:
-    when:
+        - ansible_distribution == 'CentOS'
-      - enable_epel | bool
+        - ansible_distribution_version|int == 9
-      - ansible_distribution == 'CentOS'
-      - ansible_distribution_version|int == 9
 
-- name: "Enable EPEL repository for RedHat 9"
+- name: "Enable EPEL repository for RHEL 9"
+  when:
+    - enable_epel | bool
+    - ansible_distribution == "RedHat"
+    - ansible_distribution_version|int >= 9
   command: dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-9.noarch.rpm
   args:
     creates: /etc/yum.repos.d/epel.repo
-  when:
-  - enable_epel | bool
-  - ansible_distribution == "RedHat"
-  - ansible_distribution_version|int >= 9
 
 - name: "Install system packages"
   package:
@@ -108,7 +107,7 @@
     name: "{{ dib_host_required_packages | select | list }}"
     state: present
   when:
-   - install_dib | bool
+    - install_dib | bool
 
 - name: "sushy - Install"
   include_role:
@@ -179,35 +178,33 @@
   register: efi_unpack_dir
 
 - block:
-  - name: "Create a binary cache directory"
+    - name: "Create a binary cache directory"
-    file:
+      file:
-      path: /usr/lib/ironic
+        path: /usr/lib/ironic
-      state: directory
+        state: directory
-      owner: root
+        owner: root
-      group: root
+        group: root
-      mode: "0755"
+        mode: "0755"
 
-  - name: "Download grub2 and shim binaries"
+    - name: "Download grub2 and shim binaries"
-    import_role:
+      import_role:
-      name: bifrost-download-packages
+        name: bifrost-download-packages
-    vars:
+      vars:
-      download_packages:
+        download_packages:
-        - "{{ grub_efi_package }}"
+          - "{{ grub_efi_package }}"
-        - "{{ shim_efi_package }}"
+          - "{{ shim_efi_package }}"
-      download_dest: "{{ efi_unpack_dir.path }}"
+        download_dest: "{{ efi_unpack_dir.path }}"
 
-  - name: "Copy grub2 and shim into cache"
+    - name: "Copy grub2 and shim into cache"
-    copy:
+      copy:
-      src: "{{ efi_unpack_dir.path }}/{{ item.src }}"
+        src: "{{ efi_unpack_dir.path }}/{{ item.src }}"
-      remote_src: true
+        remote_src: true
-      dest: "/usr/lib/ironic/{{ item.dest }}"
+        dest: "/usr/lib/ironic/{{ item.dest }}"
-    loop:
+      loop:
-      - src: "{{ grub_efi_package }}/{{ grub_efi_binary }}"
+        - src: "{{ grub_efi_package }}/{{ grub_efi_binary }}"
-        dest: grub-efi
+          dest: grub-efi
-      - src: "{{ shim_efi_package }}/{{ shim_efi_binary }}"
+        - src: "{{ shim_efi_package }}/{{ shim_efi_binary }}"
-        dest: shim-efi
+          dest: shim-efi
-
-  when: ansible_os_family != 'Suse'
   always:
     - name: "Delete the temporary directory"
       file:

playbooks/roles/bifrost-keystone-client-config/tasks/validate.yml

@@ -15,7 +15,9 @@
   set_fact:
     testing_cloud_name: "{{ (clouds | dict2items)[0].key }}"
 
-- block:
+- become: yes
+  become_user: "{{ user | default('root') }}"
+  block:
     - name: "List bare metal nodes using CLI and clouds.yaml"
       command: baremetal --debug node list
       environment: "{{ bifrost_venv_env | combine({'OS_CLOUD': testing_cloud_name}) }}"
@@ -55,5 +57,3 @@
       when:
         - enable_keystone | bool
         - "'bifrost-admin' in clouds"
-  become: yes
-  become_user: "{{ user | default('root') }}"

playbooks/roles/bifrost-keystone-install/tasks/bootstrap.yml

@@ -242,7 +242,11 @@
     group: "{{ nginx_user }}" # TODO(TheJulia): Split webserver user/group.
     mode: "0755"
 
-- block:
+- when:
+    - ansible_os_family == 'RedHat'
+    - ansible_selinux.status == 'enabled'
+    - ansible_selinux.mode == "enforcing"
+  block:
     - name: "Explicitly allow keystone port (TCP) on selinux"
       seport:
         ports: "5000"
@@ -266,7 +270,3 @@
 
     - name: Enable keystone policy module
       command: semodule -e keystone_policy
-  when:
-    - ansible_os_family == 'RedHat'
-    - ansible_selinux.status == 'enabled'
-    - ansible_selinux.mode == "enforcing"

playbooks/roles/bifrost-pip-install/tasks/main.yml

@@ -14,60 +14,58 @@
 # limitations under the License.
 ---
 
-- block:
+- when: not skip_install | bool
+  block:
+    - name: "Check that sourcedir is provided with source_install"
+      fail:
+        msg: Source installation of requires sourcedir to be provided
+      when:
+        - source_install | bool
+        - not sourcedir
 
-  - name: "Check that sourcedir is provided with source_install"
+    - name: "Set extra_args if upper_constraints_file is defined"
-    fail:
+      set_fact:
-      msg: Source installation of requires sourcedir to be provided
+        constraints_extra_args: "{{ extra_args | default('') }} -c {{ upper_constraints_file }}"
-    when:
+      when:
-      - source_install | bool
+        - upper_constraints_file | length > 0
-      - not sourcedir
+        # NOTE(dtantsur): constraining does not work correctly correctly with
+        # source installation if the package itself is in constraints.
+        - not source_install | bool
 
-  - name: "Set extra_args if upper_constraints_file is defined"
+    - name: "Install {{ package }} package from pip using virtualenv"
-    set_fact:
+      pip:
-      constraints_extra_args: "{{ extra_args | default('') }} -c {{ upper_constraints_file }}"
+        name: "{{ package }}"
-    when:
+        state: "{{ state | default(omit) }}"
-      - upper_constraints_file | length > 0
+        version: "{{ version | default(omit) }}"
-      # NOTE(dtantsur): constraining does not work correctly correctly with
+        virtualenv: "{{ bifrost_venv_dir }}"
-      # source installation if the package itself is in constraints.
+        virtualenv_command: "python3 -m venv"
-      - not source_install | bool
+        extra_args: "{{ constraints_extra_args | default(extra_args) | default(omit) }}"
+      register: pip_package_install_done
+      until: pip_package_install_done is succeeded
+      retries: "{{ pip_install_retries }}"
+      delay: "{{ pip_install_delay }}"
+      when: not source_install | bool
+      environment:
+        # NOTE(dtantsur): https://github.com/pypa/setuptools/issues/2353
+        SETUPTOOLS_USE_DISTUTILS: stdlib
 
-  - name: "Install {{ package }} package from pip using virtualenv"
+    - name: "Install requirements from {{ sourcedir }} using pip"
-    pip:
+      pip:
-      name: "{{ package }}"
+        extra_args: "{{ extra_args }} {% if upper_constraints_file %}-c {{ upper_constraints_file }}{% endif %}"
-      state: "{{ state | default(omit) }}"
+        requirements: "{{ sourcedir }}/requirements.txt"
-      version: "{{ version | default(omit) }}"
+      register: pip_package_install_done
-      virtualenv: "{{ bifrost_venv_dir }}"
+      until: pip_package_install_done is succeeded
-      virtualenv_command: "python3 -m venv"
+      retries: 5
-      extra_args: "{{ constraints_extra_args | default(extra_args) | default(omit) }}"
+      delay: 10
-    register: pip_package_install_done
+      when: source_install | bool
-    until: pip_package_install_done is succeeded
+      environment: "{{ bifrost_venv_env | combine({'SETUPTOOLS_USE_DISTUTILS': 'stdlib'}) }}"
-    retries: "{{ pip_install_retries }}"
-    delay: "{{ pip_install_delay }}"
-    when: not source_install | bool
-    environment:
-      # NOTE(dtantsur): https://github.com/pypa/setuptools/issues/2353
-      SETUPTOOLS_USE_DISTUTILS: stdlib
 
-  - name: "Install requirements from {{ sourcedir }} using pip"
+    # NOTE(dtantsur): do not use constraints here, it does not work when the
-    pip:
+    # package itself is constrained.
-      extra_args: "{{ extra_args }} {% if upper_constraints_file %}-c {{ upper_constraints_file }}{% endif %}"
+    - name: "Install from {{ sourcedir }} using pip"
-      requirements: "{{ sourcedir }}/requirements.txt"
+      pip:
-    register: pip_package_install_done
+        name: "{{ sourcedir }}"
-    until: pip_package_install_done is succeeded
+        editable: "{{ developer_mode | bool }}"
-    retries: 5
+        extra_args: "{{ extra_args }}"
-    delay: 10
+      when: source_install | bool
-    when: source_install | bool
+      environment: "{{ bifrost_venv_env | combine({'SETUPTOOLS_USE_DISTUTILS': 'stdlib'}) }}"
-    environment: "{{ bifrost_venv_env | combine({'SETUPTOOLS_USE_DISTUTILS': 'stdlib'}) }}"
-
-  # NOTE(dtantsur): do not use constraints here, it does not work when the
-  # package itself is constrained.
-  - name: "Install from {{ sourcedir }} using pip"
-    pip:
-      name: "{{ sourcedir }}"
-      editable: "{{ developer_mode | bool }}"
-      extra_args: "{{ extra_args }}"
-    when: source_install | bool
-    environment: "{{ bifrost_venv_env | combine({'SETUPTOOLS_USE_DISTUTILS': 'stdlib'}) }}"
-
-  when: not skip_install | bool

playbooks/test-bifrost.yaml

@@ -15,7 +15,8 @@
     - name: "Set testing to true"
       set_fact:
          testing: true
-    - block:
+    - when: lookup('env', 'ZUUL_BRANCH') | length > 0
+      block:
       - name: "Set variables if running in the CI"
         set_fact:
            cirros_deploy_image_upstream_url: file:///opt/cache/files/cirros-0.5.3-x86_64-disk.img
@@ -29,7 +30,6 @@
         set_fact:
            openstacksdk_source_install: true
         when: sdk_path.stat.exists
-      when: lookup('env', 'ZUUL_BRANCH') | length > 0
 
 - import_playbook: install.yaml