From 829297c37aeb13a45bb12102e58159892c4d703e Mon Sep 17 00:00:00 2001 From: Paul Belanger Date: Mon, 17 Jul 2017 16:50:06 -0400 Subject: [PATCH] Ensure we load roles for linting Did didn't have ansible-lint setup properly, as a results our roles weren't actually linted properly. Fix variable linting issues and ignore ANSIBLE0012. Change-Id: I07aa940245e700c9f08df0f1920720f0ed9d3de0 Signed-off-by: Paul Belanger --- playbooks/tox/linters.yaml | 12 +++++++++--- roles/bindep/tasks/find.yaml | 9 ++++++--- roles/bindep/tasks/install.yaml | 3 ++- roles/bindep/tasks/main.yaml | 8 +++++--- roles/bindep/tasks/packages.yaml | 7 ++++--- roles/revoke-sudo/tasks/main.yaml | 2 +- roles/test-setup/tasks/main.yaml | 2 +- roles/validate-host/tasks/main.yaml | 3 ++- tests/ansible.cfg | 2 ++ tox.ini | 9 +++++++-- 10 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 tests/ansible.cfg diff --git a/playbooks/tox/linters.yaml b/playbooks/tox/linters.yaml index 6effdd0a2..2ff4661fb 100644 --- a/playbooks/tox/linters.yaml +++ b/playbooks/tox/linters.yaml @@ -1,14 +1,20 @@ - hosts: all pre_tasks: - - shell: tox -l + - name: Register tox environment list + command: tox -l args: chdir: "src/{{ zuul.project.canonical_name }}" register: envlist - - set_fact: + + - name: Define tox_envlist fact for pep8 + set_fact: tox_envlist: 'pep8' when: envlist.stdout.find('pep8') != -1 - - set_fact: + + - name: Define tox_envlist fact for linters + set_fact: tox_envlist: 'linters' when: envlist.stdout.find('linters') != -1 + roles: - tox diff --git a/roles/bindep/tasks/find.yaml b/roles/bindep/tasks/find.yaml index d9190c469..50bcbe471 100644 --- a/roles/bindep/tasks/find.yaml +++ b/roles/bindep/tasks/find.yaml @@ -3,7 +3,8 @@ path: "{{ bindep_dir }}/bindep.txt" register: bindep_file_stat -- set_fact: +- name: Define bindep_file fact + set_fact: bindep_file: "{{ bindep_file_stat.stat.path }}" when: bindep_file_stat.stat.exists @@ -13,7 +14,8 @@ register: bindep_other_file_stat when: not bindep_file_stat.stat.exists -- set_fact: +- name: Define bindep_file fact + set_fact: bindep_file: "{{ bindep_other_file_stat.stat.path }}" when: not bindep_other_file_stat|skipped and bindep_other_file_stat.stat.exists @@ -25,6 +27,7 @@ and not bindep_other_file_stat|skipped and not bindep_other_file_stat.stat.exists -- set_fact: +- name: Define bindep_file fact + set_fact: bindep_file: "{{ bindep_fallback_file_stat.stat.path }}" when: not bindep_fallback_file_stat|skipped and bindep_fallback_file_stat.stat.exists diff --git a/roles/bindep/tasks/install.yaml b/roles/bindep/tasks/install.yaml index d98fc7274..195092904 100644 --- a/roles/bindep/tasks/install.yaml +++ b/roles/bindep/tasks/install.yaml @@ -11,5 +11,6 @@ name: bindep virtualenv: "{{ bindep_temp_dir }}/venv" -- set_fact: +- name: Define bindep_found_command + set_fact: bindep_found_command: "{{ bindep_temp_dir }}/venv/bin/bindep" diff --git a/roles/bindep/tasks/main.yaml b/roles/bindep/tasks/main.yaml index 0e998a018..36db2720c 100644 --- a/roles/bindep/tasks/main.yaml +++ b/roles/bindep/tasks/main.yaml @@ -8,12 +8,13 @@ - name: Check for system bindep args: executable: /bin/bash - shell: type -p bindep + command: type -p bindep ignore_errors: yes register: bindep_command_type when: bindep_command is not defined or not bindep_command_stat.stat.exists -- set_fact: +- name: Define bindep_command fact + set_fact: bindep_command: "{{ bindep_command_type.stdout }}" when: bindep_command_type|succeeded and not bindep_command_type|skipped @@ -23,7 +24,8 @@ - include: find.yaml when: bindep_file is not defined -- set_fact: +- name: Define bindep_command fact + set_fact: bindep_run: "{{ bindep_command }} -b -f {{ bindep_file }} {{ bindep_profile }}" - include: packages.yaml diff --git a/roles/bindep/tasks/packages.yaml b/roles/bindep/tasks/packages.yaml index 4ce2d70f6..5967a1f79 100644 --- a/roles/bindep/tasks/packages.yaml +++ b/roles/bindep/tasks/packages.yaml @@ -1,5 +1,5 @@ - name: Get list of packages to install from bindep - shell: "{{ bindep_run }}" + command: "{{ bindep_run }}" register: bindep_output failed_when: false @@ -10,11 +10,12 @@ become: yes - name: Check that packages are installed - shell: "{{ bindep_run }}" + command: "{{ bindep_run }}" register: bindep_final_check # Ignore errors then fail later so that we can give a better error message failed_when: false -- fail: +- name: Fail if we cannot install all packages + fail: msg: "bindep failed to install from {{ bindep_file}} - {{ bindep_final_check.stdout }}" when: bindep_final_check|failed diff --git a/roles/revoke-sudo/tasks/main.yaml b/roles/revoke-sudo/tasks/main.yaml index 1c1818706..2f7be669f 100644 --- a/roles/revoke-sudo/tasks/main.yaml +++ b/roles/revoke-sudo/tasks/main.yaml @@ -5,4 +5,4 @@ state: absent - name: Prove that general sudo access is actually revoked. - shell: ! sudo -n true + command: ! sudo -n true diff --git a/roles/test-setup/tasks/main.yaml b/roles/test-setup/tasks/main.yaml index 0fa552008..8d67789f0 100644 --- a/roles/test-setup/tasks/main.yaml +++ b/roles/test-setup/tasks/main.yaml @@ -4,7 +4,7 @@ register: p - name: Run tools/test-setup.sh - shell: tools/test-setup.sh + command: tools/test-setup.sh args: chdir: "{{ zuul_work_dir }}" when: diff --git a/roles/validate-host/tasks/main.yaml b/roles/validate-host/tasks/main.yaml index d1b3fceb6..a390e363a 100644 --- a/roles/validate-host/tasks/main.yaml +++ b/roles/validate-host/tasks/main.yaml @@ -1,4 +1,5 @@ -- set_fact: +- name: Define zuul_info_dir fact + set_fact: zuul_info_dir: "{{ zuul.executor.log_root }}/zuul-info" - name: Ensure Zuul Ansible directory exists diff --git a/tests/ansible.cfg b/tests/ansible.cfg new file mode 100644 index 000000000..51d344dc2 --- /dev/null +++ b/tests/ansible.cfg @@ -0,0 +1,2 @@ +[defaults] +roles_path = ../roles diff --git a/tox.ini b/tox.ini index 723d6a340..f4086d9c5 100644 --- a/tox.ini +++ b/tox.ini @@ -21,11 +21,16 @@ commands = bindep test commands = python setup.py build_sphinx [testenv:pep8] +setenv = + ANSIBLE_CONFIG = {toxinidir}/tests/ansible.cfg whitelist_externals = bash commands = flake8 {posargs} - bash -c "cd roles; find . -type f -regex '.*.y[a]?ml' -print0 | xargs -t -n1 -0 \ - ansible-lint" + # Ansible Lint Check + # NOTE(pabelanger): Ignore the following checks: + # ANSIBlE0012: Commands should not change things if nothing needs doing + bash -c "cd playbooks; find . -type f -regex '.*.y[a]?ml' -print0 | xargs -t -n1 -0 \ + ansible-lint -x ANSIBLE0012" [testenv:venv] commands = {posargs}