openstack/bifrost
Code Issues Proposed changes

Fix key-order[task] linter warnings

Browse Source 
It looks a little jarring at first so see keys ordered this way,
but once you get used to it, it reads so much better!

Also make indentation consistent, clean up epel install
and remove traces of Suse.

Change-Id: Id6279d681faf0c9a1893c00953b0b59d9319e08b
This commit is contained in:
Erik Berg 2023-02-21 18:29:45 +01:00
parent bc53f52a12
commit db24a0c721
No known key found for this signature in database
GPG Key ID: 1182D19B0E5ED030
13 changed files with 202 additions and 211 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.ansible-lint
View File

@ -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

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

@ -42,43 +42,44 @@
register: existing_vms
# NOTE(pas-ha) wrapping in block/rescue to have diagnostic output, requires Ansible>=2
- block:
# NOTE(pas-ha) Ansible still lacks modules to operate on libvirt volumes
# NOTE(pas-ha) adding extra 1G for disk size to accomodate for partition table / configdrive
- name: create volume for vm
command: >
virsh --connect {{ test_vm_libvirt_uri }}
vol-create-as {{ test_vm_storage_pool }} {{ vm_name }}.qcow2
{{ test_vm_disk_gib | int + 1 }}G
--format qcow2 {{ prealloc|default("") }}
when: (vm_name + '.qcow2') not in ansible_libvirt_pools[test_vm_storage_pool].volumes
- when: vm_name not in existing_vms.list_vms
block:
# NOTE(pas-ha) Ansible still lacks modules to operate on libvirt volumes
# NOTE(pas-ha) adding extra 1G for disk size to accomodate for partition table / configdrive
- name: create volume for vm
command: >
virsh --connect {{ test_vm_libvirt_uri }}
vol-create-as {{ test_vm_storage_pool }} {{ vm_name }}.qcow2
{{ test_vm_disk_gib | int + 1 }}G
--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
set_fact:
vm_volume_path: "{{ ansible_libvirt_pools[test_vm_storage_pool].path }}/{{ vm_name }}.qcow2"
- name: set path to the volume created
set_fact:
vm_volume_path: "{{ ansible_libvirt_pools[test_vm_storage_pool].path }}/{{ vm_name }}.qcow2"
- name: pre-touch the vm volume
file:
state: touch
path: "{{ vm_volume_path }}"
when: test_vm_libvirt_uri == 'qemu:///system'
- name: pre-touch the vm volume
file:
state: touch
path: "{{ vm_volume_path }}"
when: test_vm_libvirt_uri == 'qemu:///system'
# 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
# case is for CI testing.
- name: set copy-on-write for volume on non-CentOS systems
command: chattr +C {{ vm_volume_path }}
ignore_errors: yes
when:
- ansible_distribution != 'CentOS'
- test_vm_libvirt_uri == 'qemu:///system'
# 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
# case is for CI testing.
- name: set copy-on-write for volume on non-CentOS systems
command: chattr +C {{ vm_volume_path }}
ignore_errors: yes
when:
- ansible_distribution != 'CentOS'
- test_vm_libvirt_uri == 'qemu:///system'
- name: create_vm
virt:
command: define
name: "{{ vm_name }}"
uri: "{{ test_vm_libvirt_uri }}"
xml: "{{ lookup('template', 'testvm.xml.j2') }}"
- name: create_vm
virt:
command: define
name: "{{ vm_name }}"
uri: "{{ test_vm_libvirt_uri }}"
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

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

@ -309,17 +309,18 @@
path: /tmp/sushy-emulator
register: redfish_emulator_old_state_dir
- 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 }}"
become: true
- become: true
ignore_errors: true
when: redfish_emulator_old_state_dir.stat.exists
and redfish_emulator_state_dir != '/tmp/sushy-emulator'
when:
- 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:

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

@ -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) }}"

56
playbooks/roles/bifrost-download-packages/tasks/main.yml
View File

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

4
playbooks/roles/bifrost-ironic-install/ssh_public_key_path.yaml
View File

@ -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:

15
playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
View File

@ -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:

3
playbooks/roles/bifrost-ironic-install/tasks/create_tftpboot.yml
View File

@ -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:

109
playbooks/roles/bifrost-ironic-install/tasks/install.yml
View File

@ -18,41 +18,40 @@
update_cache: yes
when: ansible_os_family == 'Debian'
- block:
- name: "Enable powertools repository for EL8"
command: dnf config-manager --set-enabled powertools
when: ansible_distribution_version|int == 8
- name: "Enable EPEL repositories for known EL distributions"
when:
- enable_epel | bool
- 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"
command: dnf config-manager --set-enabled crb
when: ansible_distribution_version|int == 9
- name: "Enable crb repository for EL9"
command: dnf config-manager --set-enabled crb
when: ansible_distribution_version|int == 9
- name: "Enable EPEL repository for EL7/8"
ansible.builtin.dnf:
name: epel-release
state: latest
when:
- enable_epel | bool
- ansible_distribution in ["AlmaLinux", "CentOS", "Rocky"]
- name: "Enable EPEL repository for EL7/8/9"
ansible.builtin.dnf:
name: epel-release
state: latest
- name: "Enable EPEL repositories for EL9"
ansible.builtin.dnf:
name:
- epel-next-release
state: latest
when:
- enable_epel | bool
- ansible_distribution == 'CentOS'
- ansible_distribution_version|int == 9
- name: "Enable EPEL-next repository for Centos 9-Stream"
ansible.builtin.dnf:
name: epel-next-release
state: latest
when:
- 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"
file:
path: /usr/lib/ironic
state: directory
owner: root
group: root
mode: "0755"
- name: "Create a binary cache directory"
file:
path: /usr/lib/ironic
state: directory
owner: root
group: root
mode: "0755"
- name: "Download grub2 and shim binaries"
import_role:
name: bifrost-download-packages
vars:
download_packages:
- "{{ grub_efi_package }}"
- "{{ shim_efi_package }}"
download_dest: "{{ efi_unpack_dir.path }}"
- name: "Download grub2 and shim binaries"
import_role:
name: bifrost-download-packages
vars:
download_packages:
- "{{ grub_efi_package }}"
- "{{ shim_efi_package }}"
download_dest: "{{ efi_unpack_dir.path }}"
- name: "Copy grub2 and shim into cache"
copy:
src: "{{ efi_unpack_dir.path }}/{{ item.src }}"
remote_src: true
dest: "/usr/lib/ironic/{{ item.dest }}"
loop:
- src: "{{ grub_efi_package }}/{{ grub_efi_binary }}"
dest: grub-efi
- src: "{{ shim_efi_package }}/{{ shim_efi_binary }}"
dest: shim-efi
when: ansible_os_family != 'Suse'
- name: "Copy grub2 and shim into cache"
copy:
src: "{{ efi_unpack_dir.path }}/{{ item.src }}"
remote_src: true
dest: "/usr/lib/ironic/{{ item.dest }}"
loop:
- src: "{{ grub_efi_package }}/{{ grub_efi_binary }}"
dest: grub-efi
- src: "{{ shim_efi_package }}/{{ shim_efi_binary }}"
dest: shim-efi
always:
- name: "Delete the temporary directory"
file:

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

@ -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') }}"

10
playbooks/roles/bifrost-keystone-install/tasks/bootstrap.yml
View File

@ -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"

104
playbooks/roles/bifrost-pip-install/tasks/main.yml
View File

@ -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"
fail:
msg: Source installation of requires sourcedir to be provided
when:
- source_install | bool
- not sourcedir
- name: "Set extra_args if upper_constraints_file is defined"
set_fact:
constraints_extra_args: "{{ extra_args | default('') }} -c {{ upper_constraints_file }}"
when:
- upper_constraints_file | length > 0
# 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"
set_fact:
constraints_extra_args: "{{ extra_args | default('') }} -c {{ upper_constraints_file }}"
when:
- upper_constraints_file | length > 0
# NOTE(dtantsur): constraining does not work correctly correctly with
# source installation if the package itself is in constraints.
- not source_install | bool
- name: "Install {{ package }} package from pip using virtualenv"
pip:
name: "{{ package }}"
state: "{{ state | default(omit) }}"
version: "{{ version | default(omit) }}"
virtualenv: "{{ bifrost_venv_dir }}"
virtualenv_command: "python3 -m venv"
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"
pip:
name: "{{ package }}"
state: "{{ state | default(omit) }}"
version: "{{ version | default(omit) }}"
virtualenv: "{{ bifrost_venv_dir }}"
virtualenv_command: "python3 -m venv"
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 requirements from {{ sourcedir }} using pip"
pip:
extra_args: "{{ extra_args }} {% if upper_constraints_file %}-c {{ upper_constraints_file }}{% endif %}"
requirements: "{{ sourcedir }}/requirements.txt"
register: pip_package_install_done
until: pip_package_install_done is succeeded
retries: 5
delay: 10
when: source_install | bool
environment: "{{ bifrost_venv_env | combine({'SETUPTOOLS_USE_DISTUTILS': 'stdlib'}) }}"
- name: "Install requirements from {{ sourcedir }} using pip"
pip:
extra_args: "{{ extra_args }} {% if upper_constraints_file %}-c {{ upper_constraints_file }}{% endif %}"
requirements: "{{ sourcedir }}/requirements.txt"
register: pip_package_install_done
until: pip_package_install_done is succeeded
retries: 5
delay: 10
when: source_install | bool
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
# 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'}) }}"

4
playbooks/test-bifrost.yaml
View File

@ -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.1-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