Add loop var policy to ansible-lint

This adds a custom ansible-lint rule at .rules/ZuulJobsNamespaceLoopVar.py
that enforces the loop var policy described at:
https://zuul-ci.org/docs/zuul-jobs/policy.html#ansible-loops-in-roles

It also updates existing roles to follow the policy.

Change-Id: I92b2ff56a1c2702542fc07b316f1809087a4c92f

.ansible-lint

@@ -6,5 +6,7 @@ parseable: true
 skip_list:
   - '204'
   - '301'
+rulesdir:
+  - ./.rules/
 use_default_rules: true
 verbosity: 1

.rules/ZuulJobsNamespaceLoopVar.py

@@ -0,0 +1,29 @@
+from ansiblelint import AnsibleLintRule
+
+
+class ZuulJobsNamespaceLoopVar(AnsibleLintRule):
+
+    id = 'ZUULJOBS0001'
+    shortdesc = 'Loop vars should have zj_ prefix'
+    description = """
+Check for tasks that does not follow
+the policy of namespacing loop variables with zj_ prefix.
+See: \
+https://zuul-ci.org/docs/zuul-jobs/policy.html\
+#ansible-loops-in-roles
+"""
+
+    tags = {'zuul-jobs-namespace-loop-var'}
+
+    def matchtask(self, file, task):
+        if file.get('type') != 'tasks':
+            return False
+        if 'loop' in set(task.keys()):
+            if 'loop_control' not in set(task.keys()):
+                return True
+            elif 'loop_var' not in task.get('loop_control'):
+                return True
+            elif not task.get('loop_control')\
+                    .get('loop_var').startswith('zj_'):
+                return True
+        return False

playbooks/helm/run.yaml

@@ -3,6 +3,8 @@
     - include_role:
         name: helm-template
       vars:
-        helm_release_name: "{{ item.key }}"
+        helm_release_name: "{{ zj_item.key }}"
-        helm_chart: "{{ item.value }}"
+        helm_chart: "{{ zj_item.value }}"
       loop: "{{ helm_charts | dict2items }}"
+      loop_control:
+        loop_var: 'zj_item'

roles/build-container-image/tasks/build.yaml

@@ -18,11 +18,11 @@
 
 - name: Copy sibling source directories
   command:
-    cmd: 'cp --parents -r {{ sibling }} {{ ansible_user_dir }}/{{ zuul_work_dir }}/{{ item.context }}/.zuul-siblings'
+    cmd: 'cp --parents -r {{ zj_sibling }} {{ ansible_user_dir }}/{{ zuul_work_dir }}/{{ item.context }}/.zuul-siblings'
     chdir: '~/src'
   loop: '{{ item.siblings }}'
   loop_control:
-    loop_var: sibling
+    loop_var: zj_sibling
   when: item.siblings is defined
 
 - name: Build a container image

roles/build-container-image/tasks/push.yaml

@@ -1,12 +1,12 @@
 - name: Tag image for buildset registry
   command: >-
-    {{ container_command }} tag {{ image.repository }}:{{ image_tag }} {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ image_tag }}
+    {{ container_command }} tag {{ image.repository }}:{{ zj_image_tag }} {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ zj_image_tag }}
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag
 - name: Push tag to buildset registry
   command: >-
-    {{ container_command }} push {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ image_tag }}
+    {{ container_command }} push {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ zj_image_tag }}
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag

roles/build-docker-image/tasks/build.yaml

@@ -20,11 +20,11 @@
 # Ansible 2.8 only.  take the simple approach.
 - name: Copy sibling source directories
   command:
-    cmd: 'cp --parents -r {{ sibling }} {{ ansible_user_dir }}/{{ zuul_work_dir }}/{{ item.context }}/.zuul-siblings'
+    cmd: 'cp --parents -r {{ zj_sibling }} {{ ansible_user_dir }}/{{ zuul_work_dir }}/{{ item.context }}/.zuul-siblings'
     chdir: '~/src'
   loop: '{{ item.siblings }}'
   loop_control:
-    loop_var: sibling
+    loop_var: zj_sibling
   when: item.siblings is defined
 
 - name: Build a docker image

roles/build-docker-image/tasks/push.yaml

@@ -1,12 +1,12 @@
 - name: Tag image for buildset registry
   command: >-
-    docker tag {{ image.repository }}:{{ image_tag }} {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ image_tag }}
+    docker tag {{ image.repository }}:{{ zj_image_tag }} {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ zj_image_tag }}
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag
 - name: Push tag to buildset registry
   command: >-
-    docker push {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ image_tag }}
+    docker push {{ buildset_registry_alias }}:{{ buildset_registry.port }}/{{ image.repository }}:{{ zj_image_tag }}
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag

roles/collect-kubernetes-logs/tasks/main.yaml

@@ -11,8 +11,8 @@
 - name: Save pod descriptions
   loop: "{{ podlist.stdout_lines | default([]) }}"
   loop_control:
-    loop_var: pod_name
+    loop_var: zj_pod_name
-  shell: "kubectl describe po {{ pod_name }} &> {{ ansible_user_dir }}/zuul-output/logs/pods/{{ pod_name }}.txt"
+  shell: "kubectl describe po {{ zj_pod_name }} &> {{ ansible_user_dir }}/zuul-output/logs/pods/{{ zj_pod_name }}.txt"
   args:
     executable: /bin/bash
   failed_when: false

roles/download-artifact/tasks/main.yaml

@@ -7,9 +7,9 @@
     build: "{{ build.json[0] }}"
 - name: Download archive by type
   uri:
-    url: "{{ artifact.url }}"
+    url: "{{ zj_artifact.url }}"
     dest: "{{ download_artifact_directory }}"
   loop: "{{ build.artifacts }}"
   loop_control:
-    loop_var: artifact
+    loop_var: zj_artifact
-  when: "'metadata' in artifact and 'type' in artifact.metadata and (artifact.metadata.type == download_artifact_type or ((download_artifact_type | type_debug) == 'list' and artifact.metadata.type in download_artifact_type))"
+  when: "'metadata' in zj_artifact and 'type' in zj_artifact.metadata and (zj_artifact.metadata.type == download_artifact_type or ((download_artifact_type | type_debug) == 'list' and zj_artifact.metadata.type in download_artifact_type))"

roles/emit-job-header/tasks/main.yaml

@@ -27,12 +27,14 @@
       debug:
         msg: |
           # Node Information
-          Hostname: {{ hostvars[item]['ansible_hostname']|default('unknown') }}
+          Hostname: {{ hostvars[zj_item]['ansible_hostname']|default('unknown') }}
-          Distro: {{ hostvars[item]['ansible_distribution'] | default('unknown') }} {{ hostvars[item]['ansible_distribution_version'] | default('unknown') }}
+          Distro: {{ hostvars[zj_item]['ansible_distribution'] | default('unknown') }} {{ hostvars[zj_item]['ansible_distribution_version'] | default('unknown') }}
-          Provider: {{ hostvars[item]['nodepool']['provider'] }}
+          Provider: {{ hostvars[zj_item]['nodepool']['provider'] }}
-          Label: {{ hostvars[item]['nodepool']['label'] }}
+          Label: {{ hostvars[zj_item]['nodepool']['label'] }}
-          {% if hostvars[item]['nodepool']['interface_ip'] is defined %}
+          {% if hostvars[zj_item]['nodepool']['interface_ip'] is defined %}
-          Interface IP: {{ hostvars[item]['nodepool']['interface_ip'] }}
+          Interface IP: {{ hostvars[zj_item]['nodepool']['interface_ip'] }}
           {% endif %}
       loop: "{{ query('inventory_hostnames', 'all,!localhost') }}"
+      loop_control:
+        loop_var: zj_item
       ignore_errors: yes

roles/ensure-chart-testing/tasks/main.yaml

@@ -22,8 +22,10 @@
 - name: Install configuration files
   become: true
   get_url:
-    url: "https://raw.githubusercontent.com/helm/chart-testing/v{{ chart_testing_version }}/etc/{{ item }}"
+    url: "https://raw.githubusercontent.com/helm/chart-testing/v{{ chart_testing_version }}/etc/{{ zj_item }}"
-    dest: "/etc/ct/{{ item }}"
+    dest: "/etc/ct/{{ zj_item }}"
   loop:
   - chart_schema.yaml
-  - lintconf.yaml
+  - lintconf.yaml
+  loop_control:
+    loop_var: zj_item

roles/ensure-kubernetes/tasks/minikube.yaml

@@ -76,9 +76,9 @@
   become: yes
   loop: "{{ kube_config['users'] }}"
   loop_control:
-    loop_var: item
+    loop_var: zj_item
   file:
-    path: "{{ item['user']['client-key'] }}"
+    path: "{{ zj_item['user']['client-key'] }}"
     owner: "{{ ansible_user }}"
 
 - name: Get cluster info

roles/ensure-pip/tasks/source.yaml

@@ -9,10 +9,12 @@
     url: '{{ ensure_pip_from_upstream_url }}'
     dest: '{{ _install_dir.path }}/get-pip.py'
 
-- name: 'Run get-pip.py for {{ item }}'
+- name: 'Run get-pip.py for {{ zj_item }}'
-  command: '{{ item }} {{ _install_dir.path }}/get-pip.py'
+  command: '{{ zj_item }} {{ _install_dir.path }}/get-pip.py'
   become: yes
   loop: '{{ ensure_pip_from_upstream_interpreters }}'
+  loop_control:
+    loop_var: zj_item
 
 - name: Remove temporary install dir
   file:

roles/fetch-javascript-content-tarball/tasks/main.yaml

@@ -46,11 +46,13 @@
 
 - name: Return artifacts to Zuul
   loop: "{{ result.files }}"
+  loop_control:
+    loop_var: zj_item
   zuul_return:
     data:
       zuul:
         artifacts:
           - name: Javascript content archive
-            url: "artifacts/{{ item.path | basename }}"
+            url: "artifacts/{{ zj_item.path | basename }}"
             metadata:
               type: javascript_content

roles/fetch-output-openshift/tasks/rsync.yaml

@@ -4,8 +4,8 @@
     oc --context "{{ item.1.context }}"
        --namespace "{{ item.1.namespace }}"
         rsync -q --progress=false
-          {{ item.1.pod }}:{{ output.src }}/
+          {{ item.1.pod }}:{{ zj_output.src }}/
-          {{ output.dst }}/
+          {{ zj_output.dst }}/
   no_log: "{{ not zuul_log_verbose }}"
   delegate_to: localhost
   loop:
@@ -16,4 +16,4 @@
     - src: "{{ zuul_output_dir }}/docs"
       dst: "{{ zuul.executor.work_root }}/docs"
   loop_control:
-    loop_var: output
+    loop_var: zj_output

roles/fetch-subunit-output/tasks/main.yaml

@@ -26,8 +26,10 @@
     - name: Generate subunit file
       shell:
         cmd: "{{ testr_command.stdout_lines[0] }} last --subunit >>{{ temp_subunit_file.path }}"
-        chdir: "{{ item }}"
+        chdir: "{{ zj_item }}"
       loop: "{{ all_subunit_dirs }}"
+      loop_control:
+        loop_var: zj_item
 
     - name: Copy the combined subunit file to the zuul work directory
       copy:

roles/htmlify-logs/tasks/main.yaml

@@ -9,7 +9,9 @@
 
 - name: HTMLify text files
   htmlify:
-    input: "{{ item.path }}"
+    input: "{{ zj_item.path }}"
-    output: "{{ item.path | regex_replace('\\.txt', '.html') }}"
+    output: "{{ zj_item.path | regex_replace('\\.txt', '.html') }}"
   loop: "{{ htmlify_files.files }}"
+  loop_control:
+    loop_var: zj_item
   no_log: true

roles/merge-output-to-logs/tasks/main.yaml

@@ -2,15 +2,17 @@
   when: zuul.change is defined
   delegate_to: localhost
   shell: |
-    if [ -n "$(find {{ zuul.executor.work_root }}/{{ item }} -mindepth 1)" ] ; then
+    if [ -n "$(find {{ zuul.executor.work_root }}/{{ zj_item }} -mindepth 1)" ] ; then
       # Only create target directory if it is needed.
       # Do not fail if it is already there.
-      mkdir -p {{ zuul.executor.log_root }}/{{ item }}
+      mkdir -p {{ zuul.executor.log_root }}/{{ zj_item }}
       # Leave the original directory behind so that other roles
       # operating on the interface directories can simply no-op.
-      mv -f {{ zuul.executor.work_root }}/{{ item }}/* {{ zuul.executor.log_root }}/{{ item }}
+      mv -f {{ zuul.executor.work_root }}/{{ zj_item }}/* {{ zuul.executor.log_root }}/{{ zj_item }}
     fi
   loop:
     - artifacts
     - docs
+  loop_control:
+    loop_var: zj_item
   run_once: true

roles/promote-docker-image/tasks/main.yaml

@@ -23,10 +23,10 @@
 - name: Promote image
   loop: "{{ docker_images }}"
   loop_control:
-    loop_var: image
+    loop_var: zj_image
   include_tasks: promote-retag.yaml
 - name: Delete obsolete tags
   loop: "{{ docker_images }}"
   loop_control:
-    loop_var: image
+    loop_var: zj_image
   include_tasks: promote-cleanup.yaml

roles/promote-docker-image/tasks/promote-cleanup.yaml

@@ -10,10 +10,10 @@
   no_log: true
   loop: "{{ tags.json.results }}"
   loop_control:
-    loop_var: docker_tag
+    loop_var: zj_docker_tag
-  when: docker_tag.last_updated < cutoff.stdout and docker_tag.name.startswith('change_')
+  when: zj_docker_tag.last_updated < cutoff.stdout and zj_docker_tag.name.startswith('change_')
   uri:
-    url: "https://hub.docker.com/v2/repositories/{{ image.repository }}/tags/{{ docker_tag.name }}/"
+    url: "https://hub.docker.com/v2/repositories/{{ image.repository }}/tags/{{ zj_docker_tag.name }}/"
     method: DELETE
     status_code: [200,204]
     headers:

roles/push-to-intermediate-registry/tasks/push-image.yaml

@@ -5,26 +5,26 @@
 - name: Push tag to intermediate registry
   command: >-
     skopeo --insecure-policy copy
-    docker://127.0.0.1:{{ socat_port }}/{{ image.repository | regex_replace('^docker\.io/(.*)', '\1') }}:{{ image_tag }}
+    docker://127.0.0.1:{{ socat_port }}/{{ image.repository | regex_replace('^docker\.io/(.*)', '\1') }}:{{ zj_image_tag }}
-    docker://{{ intermediate_registry.host | ipwrap }}:{{ intermediate_registry.port }}/{{ image.repository }}:{{ zuul.build }}_{{ image_tag }}
+    docker://{{ intermediate_registry.host | ipwrap }}:{{ intermediate_registry.port }}/{{ image.repository }}:{{ zuul.build }}_{{ zj_image_tag }}
   retries: 3
   register: result
   until: result is success
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag
 
 - name: Return artifact to Zuul
   zuul_return:
     data:
       zuul:
         artifacts:
-          - name: "{{ image.repository }}:{{ image_tag }}"
+          - name: "{{ image.repository }}:{{ zj_image_tag }}"
-            url: "docker://{{ intermediate_registry.host | ipwrap }}:{{ intermediate_registry.port }}/{{ image.repository }}:{{ zuul.build }}_{{ image_tag }}"
+            url: "docker://{{ intermediate_registry.host | ipwrap }}:{{ intermediate_registry.port }}/{{ image.repository }}:{{ zuul.build }}_{{ zj_image_tag }}"
             metadata:
               type: container_image
               repository: "{{ image.repository }}"
-              tag: "{{ image_tag }}"
+              tag: "{{ zj_image_tag }}"
   loop: "{{ image.tags | default(['latest']) }}"
   loop_control:
-    loop_var: image_tag
+    loop_var: zj_image_tag

roles/wait-for-pods/tasks/main.yaml

@@ -10,12 +10,14 @@
       register: _statefulsets
 
     - name: Ensure the number of ready replicas matches the replicas
-      shell: kubectl get {{ item }} -ogo-template='{{ '{{' }}eq .status.replicas .status.readyReplicas{{ '}}' }}'
+      shell: kubectl get {{ zj_item }} -ogo-template='{{ '{{' }}eq .status.replicas .status.readyReplicas{{ '}}' }}'
       register: _is_ready
       until: _is_ready.stdout == 'true'
       retries: 60
       delay: 5
       loop: "{{ _statefulsets.stdout_lines }}"
+      loop_control:
+        loop_var: zj_item
 
 - name: Wait for all pods to become ready
-  command: kubectl wait --for=condition=Ready --timeout=120s pod --all
+  command: kubectl wait --for=condition=Ready --timeout=120s pod --all