From b51efc140f18235ebd8c107ce876e0bc4a709b40 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 24 Aug 2020 13:53:33 +0200 Subject: [PATCH] ansible-lint: fix errors 303, 305, 306 303: Using command rather than module 305: Use shell only when shell functionality is required 306: Shells that use pipes should set the pipefail option Change-Id: Iac23df907354d0b8c82f51b61e3680f44f3f858c --- .ansible-lint | 3 --- .../tasks/main.yml | 25 +++++++++++-------- .../tasks/create_bootable_image.yml | 17 ++++++++----- .../tasks/inspector_bootstrap.yml | 2 +- .../bifrost-ironic-install/tasks/start.yml | 3 ++- .../bifrost-keystone-install/tasks/start.yml | 3 ++- playbooks/test-bifrost-create-vm.yaml | 2 +- playbooks/test-bifrost.yaml | 2 +- 8 files changed, 33 insertions(+), 24 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index d427d52b2..97d41a14d 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -3,9 +3,6 @@ skip_list: - '206' # Variables should have spaces before and after: {{ var_name }} - '208' # File permissions not mentioned - '301' # Commands should not change things if nothing needs doing - - '303' # Using command rather than module - - '305' # Use shell only when shell functionality is required - - '306' # Shells that use pipes should set the pipefail option - '502' # All tasks should be named - '504' # Do not use 'local_action', use 'delegate_to: localhost' - '601' # Don't compare to literal True/False diff --git a/playbooks/roles/bifrost-configdrives-dynamic/tasks/main.yml b/playbooks/roles/bifrost-configdrives-dynamic/tasks/main.yml index 9bbff2bc1..36a2521e4 100644 --- a/playbooks/roles/bifrost-configdrives-dynamic/tasks/main.yml +++ b/playbooks/roles/bifrost-configdrives-dynamic/tasks/main.yml @@ -22,19 +22,20 @@ when: ssh_public_key is undefined - name: "Make temporary folder to build configdrive" - command: mktemp -d + tempfile: + state: directory register: variable_configdrive_location - name: "Make metadata folders - /openstack/" file: state: directory - name: "{{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/{{ item }}/" + name: "{{ variable_configdrive_location.path }}/{{ uuid }}/openstack/{{ item }}/" loop: "{{ metadata_versions }}" - name: "Place template in each openstack/ folder" template: src: openstack_meta_data.json.j2 - dest: "{{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/{{ item }}/meta_data.json" + dest: "{{ variable_configdrive_location.path }}/{{ uuid }}/openstack/{{ item }}/meta_data.json" loop: "{{ metadata_versions }}" - name: "Generate network_data" @@ -53,7 +54,7 @@ - name: "Place network data template in each openstack/ folder" template: src: network_data.json.j2 - dest: "{{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/{{ item }}/network_data.json" + dest: "{{ variable_configdrive_location.path }}/{{ uuid }}/openstack/{{ item }}/network_data.json" loop: "{{ metadata_versions }}" when: - item in metadata_versions_supporting_network_data @@ -66,7 +67,7 @@ - name: "Place deprecated network info file location in each openstack/ folder" template: src: network_data.json.j2 - dest: "{{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/{{ item }}/network_info.json" + dest: "{{ variable_configdrive_location.path }}/{{ uuid }}/openstack/{{ item }}/network_info.json" loop: "{{ metadata_versions }}" when: - item in metadata_versions_supporting_network_data @@ -75,10 +76,10 @@ - name: "Make metadata folder - /openstack/content" file: state: directory - name: "{{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/content/" + name: "{{ variable_configdrive_location.path }}/{{ uuid }}/openstack/content/" - name: "Write network Debian style interface template" - template: src=interfaces.j2 dest={{ variable_configdrive_location.stdout }}/{{ uuid }}/openstack/content/0000 + template: src=interfaces.j2 dest={{ variable_configdrive_location.path }}/{{ uuid }}/openstack/content/0000 when: write_interfaces_file | bool == true - name: "Check if mkisofs is available" @@ -102,11 +103,15 @@ - name: "Make config drive files" become: yes - command: "{{iso_gen_utility}} -R -V config-2 -o {{http_boot_folder}}/configdrive-{{ uuid }}.iso {{ variable_configdrive_location.stdout }}/{{ uuid }}" + command: "{{iso_gen_utility}} -R -V config-2 -o {{http_boot_folder}}/configdrive-{{ uuid }}.iso {{ variable_configdrive_location.path }}/{{ uuid }}" - name: "Make config drive files base64 encoded and gzip compressed" become: yes - shell: gzip -c {{http_boot_folder}}/configdrive-{{ uuid }}.iso | base64 > {{http_boot_folder}}/configdrive-{{ uuid }}.iso.gz + shell: | + set -o pipefail + gzip -c {{http_boot_folder}}/configdrive-{{ uuid }}.iso | base64 > {{http_boot_folder}}/configdrive-{{ uuid }}.iso.gz + args: + executable: /bin/bash - name: "Cleanup configdrive .iso files" become: yes @@ -119,4 +124,4 @@ file: state: absent force: yes - name: "{{ variable_configdrive_location.stdout }}" + name: "{{ variable_configdrive_location.path }}" diff --git a/playbooks/roles/bifrost-create-bootable-image/tasks/create_bootable_image.yml b/playbooks/roles/bifrost-create-bootable-image/tasks/create_bootable_image.yml index a4d8b1b19..2774ced55 100644 --- a/playbooks/roles/bifrost-create-bootable-image/tasks/create_bootable_image.yml +++ b/playbooks/roles/bifrost-create-bootable-image/tasks/create_bootable_image.yml @@ -18,7 +18,7 @@ - name: "Copy image however with 32k of empty space at the beginning of the file." command: dd if="{{deploy_image}}.raw" of="{{deploy_image}}.bootimg" seek=64 - name: "Create partition table lining up with the copied file's contents." - shell: echo '32;' | sfdisk "{{deploy_image}}.bootimg" -uB -f + shell: echo '32;' | sfdisk "{{deploy_image}}.bootimg" -uB -f # noqa 306 - name: "Allocate one of two loopbacks" command: losetup -f register: stored_value_loopback_alpha @@ -31,18 +31,23 @@ command: losetup -f register: stored_value_loopback_beta - name: "Bind second loopback to the first partition" - shell: losetup "{{stored_value_loopback_beta.stdout}}" /dev/mapper/$(echo "{{stored_value_loopback_alpha.stdout}}"|cut -f3 -d '/')p1 + shell: | + set -o pipefail + losetup "{{stored_value_loopback_beta.stdout}}" /dev/mapper/$(echo "{{stored_value_loopback_alpha.stdout}}" | cut -f3 -d '/')p1 + executable: /bin/bash # TODO parameterize folder name/location - name: "Ensure we have a location to mount the disk to" file: path=/mnt/bootimg state=directory +# NOTE(dtantsur): skipping ansible-lint 303 because the mount module does not +# support temporary mounts: https://github.com/ansible-collections/ansible.posix/issues/84 - name: "Mount volume on /mnt/bootimg" - command: mount "{{stored_value_loopback_beta.stdout}}" /mnt/bootimg + command: mount "{{stored_value_loopback_beta.stdout}}" /mnt/bootimg # noqa 303 - name: "Bind /sys into /mnt/bootimg/sys" - command: mount -t sysfs sysfs /mnt/bootimg/sys + command: mount -t sysfs sysfs /mnt/bootimg/sys # noqa 303 - name: "Bind /proc into /mnt/bootimg/proc" - command: mount -t proc proc /mnt/bootimg/proc + command: mount -t proc proc /mnt/bootimg/proc # noqa 303 - name: "Bind /dev into /mnt/bootimg/dev" - command: mount --bind /dev /mnt/bootimg/dev + command: mount --bind /dev /mnt/bootimg/dev # noqa 303 - name: "Disable Grub Prober" shell: echo "GRUB_DISABLE_OS_PROBER=true" >>/etc/default/grub - name: "Disable Grub Prober" diff --git a/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml b/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml index 101e79243..c44f73125 100644 --- a/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml +++ b/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml @@ -115,7 +115,7 @@ - "{{ inspector_ramdisk_logs_local_path | default('') }}" when: item != "" - name: "Upgrade inspector DB Schema" - shell: ironic-inspector-dbsync --config-file /etc/ironic-inspector/inspector.conf upgrade + command: ironic-inspector-dbsync --config-file /etc/ironic-inspector/inspector.conf upgrade become: true environment: "{{ bifrost_venv_env if enable_venv else {} }}" - name: "Inspector - Get ironic-inspector install location" diff --git a/playbooks/roles/bifrost-ironic-install/tasks/start.yml b/playbooks/roles/bifrost-ironic-install/tasks/start.yml index 5c3cf7cc1..882eca65b 100644 --- a/playbooks/roles/bifrost-ironic-install/tasks/start.yml +++ b/playbooks/roles/bifrost-ironic-install/tasks/start.yml @@ -14,7 +14,8 @@ # limitations under the License. --- - name: "Reload systemd configuration" - command: systemctl daemon-reload + systemd: + daemon_reload: yes - name: "Ensure rsyslog is running with current config" service: name=rsyslog state=restarted diff --git a/playbooks/roles/bifrost-keystone-install/tasks/start.yml b/playbooks/roles/bifrost-keystone-install/tasks/start.yml index c32fcf6b2..76de57189 100644 --- a/playbooks/roles/bifrost-keystone-install/tasks/start.yml +++ b/playbooks/roles/bifrost-keystone-install/tasks/start.yml @@ -12,7 +12,8 @@ # limitations under the License. --- - name: "Reload systemd configuration" - command: systemctl daemon-reload + systemd: + daemon_reload: yes - name: "Ensure services are running with current config" service: name={{ item }} state=restarted enabled=yes diff --git a/playbooks/test-bifrost-create-vm.yaml b/playbooks/test-bifrost-create-vm.yaml index bcfcc243c..0714a1c5b 100644 --- a/playbooks/test-bifrost-create-vm.yaml +++ b/playbooks/test-bifrost-create-vm.yaml @@ -31,7 +31,7 @@ command: ps aux when: ci_testing | default(false)| bool - name: "Collect list of listening network sockets if running in OpenStack CI" - shell: netstat -apn|grep LISTEN + shell: netstat -apn|grep LISTEN # noqa 306 when: ci_testing | default(false)| bool roles: - role: bifrost-create-vm-nodes diff --git a/playbooks/test-bifrost.yaml b/playbooks/test-bifrost.yaml index 2f8066026..145cbbb47 100644 --- a/playbooks/test-bifrost.yaml +++ b/playbooks/test-bifrost.yaml @@ -38,7 +38,7 @@ command: ps aux when: ci_testing | default(false)| bool - name: "Collect list of listening network sockets if running in a CI system" - shell: netstat -apn|grep LISTEN + shell: netstat -apn|grep LISTEN # noqa 306 when: ci_testing | default(false)| bool - name: "Use a cached cirros image" set_fact: