From 16649df8789f565f927e94b6358ddf43c3ae8a5e Mon Sep 17 00:00:00 2001
From: Sorin Sbarnea <ssbarnea@redhat.com>
Date: Sun, 5 Jan 2020 16:00:10 +0000
Subject: [PATCH] lint: bumping to latest versions

- run `pre-commit autoupdate` and fixed new issues
- adopted newer pre-commit config for ansible-lint 4.2.0+
- fixed some reported broken rules
- temporary disabled few rules, just to contain the size of of review,
  planning to drop/fix them in follow-ups.

Change-Id: I807ba4e919527be56c85ec72d0f4c7148f04e994
---
 .ansible-lint                                    |  3 +++
 .pre-commit-config.yaml                          | 16 +++++++++++-----
 roles/baremetal-prep-virthost/tasks/main.yml     |  1 +
 .../baremetal-undercloud/tasks/machine-setup.yml |  5 ++++-
 roles/build-test-packages/tasks/dlrn-build.yml   |  1 +
 roles/build-test-packages/tasks/main.yml         |  8 +++++---
 roles/install-built-repo/tasks/main.yml          |  2 +-
 roles/modify-image/tasks/libguestfs.yml          |  1 +
 roles/modify-image/tasks/manual.yml              |  3 ++-
 roles/multinodes/tasks/main.yml                  |  3 ++-
 roles/nodepool-setup/tasks/main.yml              |  5 ++++-
 .../ovb-manage-stack/tasks/ovb-create-stack.yml  |  3 ++-
 .../tasks/overcloud-prep-containers.yml          |  3 +++
 roles/overcloud-ssl/tasks/main.yml               |  1 +
 roles/standalone/tasks/main.yml                  |  1 +
 roles/validate-tempest/tasks/tempest-rpm.yml     |  1 +
 roles/validate-tempest/tasks/tempest-venv.yml    |  2 +-
 roles/virthost-full-cleanup/tasks/main.yml       |  9 ++++++---
 18 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/.ansible-lint b/.ansible-lint
index 634269011..7c4c642d7 100644
--- a/.ansible-lint
+++ b/.ansible-lint
@@ -7,7 +7,10 @@ quiet: false
 skip_list:
     # TODO(ssbarnea): Gradually remove these skips ASAP
     - 204  # Lines should be no longer than 160 chars
+    - 301  # Commands should not change things if nothing needs doing
+    - 302  # rm used in place of argument state=absent to file module
     - 303  # sed used in place of template, replace or lineinfile module
+    - 306  # Shells that use pipes should set the pipefail option
     - 504  # Do not use 'local_action', use 'delegate_to: localhost'
     - 601  # Don't compare to literal True/False
     - 602  # Don't compare to empty string
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index e23560e39..ecfbf6bb0 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,7 +1,7 @@
 ---
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v2.3.0
+    rev: v2.4.0
     hooks:
       - id: trailing-whitespace
       - id: mixed-line-ending
@@ -15,18 +15,24 @@ repos:
       - id: check-yaml
         files: .*\.(yaml|yml)$
   - repo: https://github.com/adrienverge/yamllint.git
-    rev: v1.18.0
+    rev: v1.20.0
     hooks:
       - id: yamllint
         files: \.(yaml|yml)$
         types: [file, yaml]
         entry: yamllint --strict -f parsable
   - repo: https://github.com/ansible/ansible-lint
-    rev: v4.1.1a1
+    rev: v4.2.0
     hooks:
       - id: ansible-lint
-        files: \.(yaml|yml)$
-        entry: ansible-lint --force-color -v
+        always_run: true
+        pass_filenames: false
+        # do not add file filters here as ansible-lint does not give reliable
+        # results when called with individual files.
+        # https://github.com/ansible/ansible-lint/issues/611
+        verbose: true
+        # plugins is the standard collection dir for modules
+        entry: env ANSIBLE_LIBRARY=plugins ansible-lint --force-color -p -v
   - repo: https://github.com/openstack-dev/bashate.git
     rev: 0.6.0
     hooks:
diff --git a/roles/baremetal-prep-virthost/tasks/main.yml b/roles/baremetal-prep-virthost/tasks/main.yml
index e6c6ea3b6..7a337f953 100644
--- a/roles/baremetal-prep-virthost/tasks/main.yml
+++ b/roles/baremetal-prep-virthost/tasks/main.yml
@@ -12,3 +12,4 @@
   become: true
   shell: >
     "{{ working_dir }}"/add-provisioning-interface.sh
+  changed_when: true
diff --git a/roles/baremetal-undercloud/tasks/machine-setup.yml b/roles/baremetal-undercloud/tasks/machine-setup.yml
index 7c43a9c2d..ce27f8475 100644
--- a/roles/baremetal-undercloud/tasks/machine-setup.yml
+++ b/roles/baremetal-undercloud/tasks/machine-setup.yml
@@ -70,8 +70,11 @@
 
 - name: get undercloud ip
   delegate_to: "{{ virthost }}"
-  shell: ip route get 1 | awk '{print $NF;exit}'
+  shell: |
+    set -euo pipefail
+    ip route get 1 | awk '{print $NF;exit}'
   register: registered_undercloud_ip
+  changed_when: false
 
 - name: Re-add the virthost to the inventory
   add_host:
diff --git a/roles/build-test-packages/tasks/dlrn-build.yml b/roles/build-test-packages/tasks/dlrn-build.yml
index 9dbd290e0..c60eff375 100644
--- a/roles/build-test-packages/tasks/dlrn-build.yml
+++ b/roles/build-test-packages/tasks/dlrn-build.yml
@@ -33,6 +33,7 @@
 - name: Map project name to DLRN project name
   register: project_name_mapped
   shell: >
+    set -euo pipefail;
     source {{ build_repo_dir }}/dlrn-venv/bin/activate;
     export PROJECT_NAME=$(echo {{ artg_change.project }} | sed "s|openstack/||");
     rdopkg findpkg -s $PROJECT_NAME -l rdoinfo | grep ^name | awk '{print $2}'
diff --git a/roles/build-test-packages/tasks/main.yml b/roles/build-test-packages/tasks/main.yml
index b7a8c2045..fa10f0775 100644
--- a/roles/build-test-packages/tasks/main.yml
+++ b/roles/build-test-packages/tasks/main.yml
@@ -41,7 +41,7 @@
         {% endif %}
 
 - name: Check if virtualenv is in the system
-  shell: "{{ python_cmd }} -m virtualenv --version"
+  command: "{{ python_cmd }} -m virtualenv --version"
   args:
     warn: false
   register: virtualenv_exist
@@ -92,6 +92,7 @@
     dest: '{{ build_repo_dir }}/DLRN/projects.ini'
 
 - name: Install and update pip
+  # noqa 403
   pip:
     name: pip
     virtualenv: "{{ build_repo_dir }}/dlrn-venv"
@@ -106,6 +107,7 @@
   when: not dlrn_pre_installed|bool
 
 - name: Pip install DLRN
+  # noqa 403
   pip:
     name: dlrn
     virtualenv: "{{ build_repo_dir }}/dlrn-venv"
@@ -254,12 +256,12 @@
         find {{ build_repo_dir }}/DLRN/data/repos -type f -name '*.rpm' -print0 | xargs -0 cp -t {{ build_repo_dir }}/gating_repo;
 
     - name: Run createrepo on generated rpms
-      shell: 'createrepo gating_repo'
+      command: createrepo gating_repo
       args:
         chdir: '{{ build_repo_dir }}'
 
     - name: Compress the repo
-      shell: 'tar czf {{ artg_compressed_gating_repo }} gating_repo'
+      command: 'tar czf {{ artg_compressed_gating_repo }} gating_repo'
       args:
         chdir: '{{ build_repo_dir }}'
     - name: Trigger repo injection for quickstart
diff --git a/roles/install-built-repo/tasks/main.yml b/roles/install-built-repo/tasks/main.yml
index 2099257f6..817332259 100644
--- a/roles/install-built-repo/tasks/main.yml
+++ b/roles/install-built-repo/tasks/main.yml
@@ -21,7 +21,7 @@
       copy: "src={{ local_working_dir }}/gating_repo.tar.gz dest=/tmp/gating_repo.tar.gz"
 
 - name: Copy compressed repo to /tmp
-  shell: cp -f {{ ib_repo_file_path }} /tmp/gating_repo.tar.gz
+  command: cp -f {{ ib_repo_file_path }} /tmp/gating_repo.tar.gz
   when: ib_repo_host is not defined
 
 - include: install_built_repo.yml
diff --git a/roles/modify-image/tasks/libguestfs.yml b/roles/modify-image/tasks/libguestfs.yml
index 51e0248c9..3f9a65799 100644
--- a/roles/modify-image/tasks/libguestfs.yml
+++ b/roles/modify-image/tasks/libguestfs.yml
@@ -1,5 +1,6 @@
 ---
 - name: ensure libguestfs is installed
+  # noqa 403
   yum: name=libguestfs-tools-c state=latest
   become: true
 
diff --git a/roles/modify-image/tasks/manual.yml b/roles/modify-image/tasks/manual.yml
index 97e5b2c80..30cb9e70a 100644
--- a/roles/modify-image/tasks/manual.yml
+++ b/roles/modify-image/tasks/manual.yml
@@ -1,7 +1,8 @@
 ---
 - name: Set abs path for image
-  shell: echo "{{ image_to_modify }}"
+  command: echo "{{ image_to_modify }}"
   register: image_to_modify_abs_path
+  changed_when: false
 
 - name: Create a temp dir for extracting images
   command: mktemp -d
diff --git a/roles/multinodes/tasks/main.yml b/roles/multinodes/tasks/main.yml
index 78b13d1ba..5a57f960f 100644
--- a/roles/multinodes/tasks/main.yml
+++ b/roles/multinodes/tasks/main.yml
@@ -120,7 +120,8 @@
     - delete
 
 - name: Get output from nodes
-  shell: openstack stack output show {{ stack_name }} subnode_ip_pairs -c output_value -f value
+  command: >
+    openstack stack output show {{ stack_name }} subnode_ip_pairs -c output_value -f value
   register: subnode_ips
 
 - name: Add hosts
diff --git a/roles/nodepool-setup/tasks/main.yml b/roles/nodepool-setup/tasks/main.yml
index f82dbaa4d..1da01cd44 100644
--- a/roles/nodepool-setup/tasks/main.yml
+++ b/roles/nodepool-setup/tasks/main.yml
@@ -66,6 +66,7 @@
   when: ansible_distribution|lower != 'fedora'
 
 - name: Install packages
+  # noqa 403
   package:
     name: "{{ packages_list }}"
     state: latest
@@ -74,9 +75,10 @@
 - include: clone-ci-repos.yml
 
 - name: Set primary public key on all hosts
-  shell: cat /etc/nodepool/id_rsa.pub
+  command: cat /etc/nodepool/id_rsa.pub
   register: primary_key
   when: inventory_hostname == "subnode-0"
+  changed_when: false
 
 - name: Add primary key
   lineinfile:
@@ -141,6 +143,7 @@
   block:
 
     - name: Update packages
+      # noqa 403
       package:
         name: '*'
         state: latest
diff --git a/roles/ovb-manage-stack/tasks/ovb-create-stack.yml b/roles/ovb-manage-stack/tasks/ovb-create-stack.yml
index 10ead8d1d..afa075f9f 100644
--- a/roles/ovb-manage-stack/tasks/ovb-create-stack.yml
+++ b/roles/ovb-manage-stack/tasks/ovb-create-stack.yml
@@ -96,8 +96,9 @@
 
 - block:
     - name: Get full stack status info in case of failure
-      shell: openstack stack show "{{ stack_name }}"
+      command: openstack stack show "{{ stack_name }}"
       register: failed_stack
+      changed_when: false
 
     - name: Show stack status in case of failure
       debug: var="failed_stack.stdout"
diff --git a/roles/overcloud-prep-containers/tasks/overcloud-prep-containers.yml b/roles/overcloud-prep-containers/tasks/overcloud-prep-containers.yml
index 5826347ab..3a245550c 100644
--- a/roles/overcloud-prep-containers/tasks/overcloud-prep-containers.yml
+++ b/roles/overcloud-prep-containers/tasks/overcloud-prep-containers.yml
@@ -23,6 +23,7 @@
 
             - name: Install python setuptools (easy_install)
               when: easy_install_exists.rc != 0
+              # noqa 403
               package:
                 state: latest
                 name:
@@ -34,6 +35,7 @@
               command: "{{ python_cmd }} -m easy_install pip"
 
     - name: Update pip
+      # noqa 403
       pip:
         name: pip
         state: latest
@@ -41,6 +43,7 @@
       become: true
 
     - name: Pip install container-check
+      # noqa 403
       pip:
         name: container-check
         state: latest
diff --git a/roles/overcloud-ssl/tasks/main.yml b/roles/overcloud-ssl/tasks/main.yml
index fdbe3fea9..f688b2c5f 100644
--- a/roles/overcloud-ssl/tasks/main.yml
+++ b/roles/overcloud-ssl/tasks/main.yml
@@ -4,6 +4,7 @@
   block:
     - name: Ensure rpm requirements for ssl and heat template are installed
       become: true
+      # noqa 403
       package:
         name:
           - openssl
diff --git a/roles/standalone/tasks/main.yml b/roles/standalone/tasks/main.yml
index 0445a4078..85030feda 100644
--- a/roles/standalone/tasks/main.yml
+++ b/roles/standalone/tasks/main.yml
@@ -183,6 +183,7 @@
     quickstart_venv: "{{ lookup('env','OPT_WORKDIR') }}"
   block:
     - name: Install ansible-lint
+      # noqa 403
       pip:
         name: ansible-lint
         state: latest
diff --git a/roles/validate-tempest/tasks/tempest-rpm.yml b/roles/validate-tempest/tasks/tempest-rpm.yml
index 20484e524..f0bc35070 100644
--- a/roles/validate-tempest/tasks/tempest-rpm.yml
+++ b/roles/validate-tempest/tasks/tempest-rpm.yml
@@ -18,6 +18,7 @@
   when: release == 'newton'
 
 - name: Install openstack services tempest plugins
+  # noqa 403
   package:
     state: latest
     name: "{{ item | regex_replace( '^python[0-9]*-(.*)$', python_package_prefix + '-\\1' ) }}"
diff --git a/roles/validate-tempest/tasks/tempest-venv.yml b/roles/validate-tempest/tasks/tempest-venv.yml
index bf333c7f3..17ae9ea57 100644
--- a/roles/validate-tempest/tasks/tempest-venv.yml
+++ b/roles/validate-tempest/tasks/tempest-venv.yml
@@ -21,7 +21,7 @@
   when: release != 'newton'
 
 - name: Check if virtualenv is in the system
-  shell: "{{ python_cmd }} -m virtualenv --version"
+  command: "{{ python_cmd }} -m virtualenv --version"
   register: virtualenv_exist
   failed_when: false
   changed_when: false
diff --git a/roles/virthost-full-cleanup/tasks/main.yml b/roles/virthost-full-cleanup/tasks/main.yml
index f24439319..c1e3ab0f9 100644
--- a/roles/virthost-full-cleanup/tasks/main.yml
+++ b/roles/virthost-full-cleanup/tasks/main.yml
@@ -35,7 +35,8 @@
       shell: find /var/cache/tripleo-quickstart/images/*  -mtime +3 -exec rm -rf {} \;
 
     - name: remove tripleo-quickstart /var/oooqvms
-      shell: rm -rf /var/oooqvms/
+      command: rm -rf /var/oooqvms/
+      changed_when: true
 
     - name: remove ansible-role-tripleo-image-build artifacts
       shell: >
@@ -56,10 +57,11 @@
 
     - name: destroy local libvirt storage, networks and config
       become: true
-      shell: "rm -rf {{ working_dir }}/.config/libvirt"
+      command: "rm -rf {{ working_dir }}/.config/libvirt"
 
     - name: get user_id for stack user
       shell: >
+        set -euo pipefail;
         cat /etc/passwd | grep stack | awk -F ':' '{ print $3 }'
       register: user_id
 
@@ -103,10 +105,11 @@
       user: name={{ non_root_user }} remove=yes force=yes state=absent
 
     - name: remove stack home dir
-      shell: rm -Rf {{ item }}
+      command: rm -Rf {{ item }}
       with_items:
         - /home/{{ non_root_user }}
         - /var/spool/mail/{{ non_root_user }}
+      changed_when: true
 
     - name: remove files automation files from /root
       file: name={{ item }} state=absent