Fix ansible lint (#574)

* simplify github workflows

* role_name and namespace vars are needed for ansible-lint

* bump minimum ansible-version to 2.9

* fix linting

* make tests work in containers using systemd

* remove now obsolete pytest files - we use ansible asserts now

.ansible-lint

@@ -1,26 +1,3 @@
 ---
-skip_list:
-  # TODO: Fix ansible-lint violations for these errors and remove from skip_list
-  - "fqcn-builtins"
-  - "fqcn[action-core]"
-  - "fqcn[action]"
-  - "ignore-errors"
-  - "jinja[spacing]"
-  - "key-order[task]"
-  - "no-changed-when"
-  - "no-free-form"
-  - "no-handler"
-  - "risky-file-permissions"
-  - "schema[meta]"
-  - "var-naming"
-  - "var-spacing"
-
-  # Pipefail is a bit more difficult, because the shell shipped in the Docker image used
-  # by the Molecule tests doesn't recognize the pipefail option. Likewise, we also need to
-  # make sure that the playbook works for hosts that don't have a modern bash or a default
-  # shell such as /bin/sh.
-  - "risky-shell-pipe"
-
-  # These errors should continue to be ignored
-  - "no-jinja-when"
-  - "role-name"
+exclude_paths:
+  - .github/

.github/workflows/ansible-lint.yml

@@ -1,46 +1,24 @@
 ---
-name: Ansible-lint
+name: Ansible Lint
 
 on:
   push:
-    branches:
-      - master
+    branches: [master]
   pull_request:
+    # The branches below must be a subset of the branches above
+    branches: [master]
 
 jobs:
-  build:
+  ansible-lint:
     runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v3
-        with:
-          path: "${{ github.repository }}"
-
-      # This is unfortunately necessary because this role doesn't follow Ansible's rules
-      # for role naming. If we should manage to rename the role, this task could be
-      # eliminated.
-      - name: Configure role symlink
-        run: |
-          sudo mkdir -p /usr/share/ansible/roles
-          sudo ln -s $PWD/${{ github.repository }} /usr/share/ansible/roles/ansible-consul
-
-      # Similar to the above, we can't use Ansible's ansible-lint-action workflow because
-      # it makes too many assumptions for us. We need to install it ourselves and run it
-      # manually. If we could use that action, the next two steps could be removed.
-      - name: Install dependencies
-        run: |
-          sudo apt update
-          sudo apt install -y python3-pip
-          sudo pip3 install -r ${{ github.repository }}/requirements.txt
 
-      - name: Run ansible-lint
-        run: |
-          ansible-lint --offline -c ${{ github.repository }}/.ansible-lint ${{ github.repository }}
+    steps:
+      - uses: actions/checkout@v4
 
-      - name: Run yamllint
+      - name: Temporary fix for roles
         run: |
-          yamllint -c ${{ github.repository }}/.yamllint ${{ github.repository }}
+          mkdir -p /home/runner/.ansible
+          ln -s /home/runner/work/ansible-consul/ /home/runner/.ansible/roles
 
-      - name: Run flake8
-        run: |
-          flake8 ${{ github.repository }}
+      - name: Lint Ansible Playbook
+        uses: ansible/ansible-lint@v6

.github/workflows/molecule.yml

@@ -33,12 +33,10 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v3
-        with:
-          path: "${{ github.repository }}"
 
       - name: Molecule
         uses: gofrolist/[email protected]
         with:
           molecule_options: --base-config molecule/_shared/base.yml
           molecule_args: --scenario-name ${{ matrix.scenario }}
-          molecule_working_dir: "ansible-community/ansible-consul"
+          molecule_working_dir: ansible-community/ansible-consul

.yamllint

@@ -10,5 +10,7 @@ rules:
     level: error
   line-length: disable
   truthy:
-    allowed-values: ['true', 'false']
+    allowed-values: ["true", "false"]
     check-keys: false
+  comments:
+    min-spaces-from-content: 1 # same as ansible-lint

handlers/main.yml

@@ -2,38 +2,45 @@
 # File: main.yml - Handlers for Consul
 
 - name: Restart consul
-  import_tasks: restart_consul.yml
+  ansible.builtin.import_tasks:
+    file: restart_consul.yml
 
 - name: Start consul
-  import_tasks: start_consul.yml
+  ansible.builtin.import_tasks:
+    file: start_consul.yml
 
 - name: Reload consul configuration
-  import_tasks: reload_consul_conf.yml
+  ansible.builtin.import_tasks:
+    file: reload_consul_conf.yml
 
 - name: Restart dnsmasq
-  service:
+  ansible.builtin.service:
     name: dnsmasq
     enabled: true
     state: restarted
     # Needed to force SysV service manager on Docker for Molecule tests
     use: "{{ ansible_service_mgr }}"
   become: true
-  listen: 'restart dnsmasq'
+  listen: restart dnsmasq
 
 - name: Restart rsyslog
-  import_tasks: restart_rsyslog.yml
+  ansible.builtin.import_tasks:
+    file: restart_rsyslog.yml
 
 - name: Restart syslog-ng
-  import_tasks: restart_syslogng.yml
+  ansible.builtin.import_tasks:
+    file: restart_syslogng.yml
 
 - name: Restart syslog-ng
-  import_tasks: restart_syslogng.yml
+  ansible.builtin.import_tasks:
+    file: restart_syslogng.yml
 
 - name: Start snapshot
-  import_tasks: start_snapshot.yml
+  ansible.builtin.import_tasks:
+    file: start_snapshot.yml
 
 - name: Systemctl daemon-reload
-  systemd:
+  ansible.builtin.systemd_service:
     daemon_reload: true
   become: true
-  listen: 'systemctl daemon-reload'
+  listen: systemctl daemon-reload

handlers/reload_consul_conf.yml

@@ -2,7 +2,7 @@
 # Use SIGHUP to reload most configurations as per https://www.consul.io/docs/agent/options.html
 # Cannot use `consul reload` because it requires the HTTP API to be bound to a non-loopback interface
 
-- name: Reload consul configuration on unix
-  command: "pkill --pidfile '{{ consul_run_path }}/consul.pid' --signal SIGHUP"
+- name: Reload consul configuration on unix # noqa no-changed-when
+  ansible.builtin.command: pkill --pidfile "{{ consul_run_path }}/consul.pid" --signal SIGHUP
   when: ansible_os_family != "Windows"
-  listen: 'reload consul configuration'
+  listen: reload consul configuration

handlers/restart_consul.yml

@@ -1,29 +1,29 @@
 ---
 - name: Daemon reload systemd in case the binaries upgraded
-  systemd:
+  ansible.builtin.systemd_service:
     daemon_reload: true
   become: true
   when: ansible_service_mgr == "systemd"
-  listen: 'reload systemd daemon'
+  listen: reload systemd daemon
 
 - name: Restart consul on Unix
-  service:
+  ansible.builtin.service:
     name: consul
     state: restarted
     # Needed to force SysV service manager on Docker for Molecule tests
     use: "{{ ansible_service_mgr }}"
   when:
     - ansible_os_family != "Darwin"
     - ansible_os_family != "Windows"
-  listen: 'restart consul'
+  listen: restart consul
 
 - name: Restart consul on Mac
-  include_tasks: "{{ role_path }}/handlers/restart_consul_mac.yml"
+  ansible.builtin.include_tasks: "{{ role_path }}/handlers/restart_consul_mac.yml"
   when: ansible_os_family == "Darwin"
-  listen: 'restart consul'
+  listen: restart consul
 
 - name: Restart consul on Windows
-  win_service:
+  ansible.windows.win_service:
     name: consul
     state: restarted
   # Some tasks with `become: true` end up calling this task. Unfortunately, the `become`
@@ -35,4 +35,4 @@
   retries: 3
   delay: 1
   until: windows_service_started is succeeded
-  listen: 'restart consul'
+  listen: restart consul

handlers/restart_consul_mac.yml

@@ -1,6 +1,8 @@
 ---
 - name: (Mac) Stop consul service
-  import_tasks: "stop_consul_mac.yml"
+  ansible.builtin.import_tasks:
+    file: stop_consul_mac.yml
 
 - name: (Mac) Start consul service
-  import_tasks: "start_consul_mac.yml"
+  ansible.builtin.import_tasks:
+    file: start_consul_mac.yml

handlers/restart_rsyslog.yml

@@ -1,11 +1,11 @@
 ---
 - name: Restart rsyslog
-  service:
+  ansible.builtin.service:
     name: rsyslog
     state: restarted
     # Needed to force SysV service manager on Docker for Molecule tests
     use: "{{ ansible_service_mgr }}"
   when:
     - ansible_os_family != "Darwin"
     - ansible_os_family != "Windows"
-  listen: 'restart rsyslog'
+  listen: restart rsyslog

handlers/restart_syslogng.yml

@@ -1,8 +1,8 @@
 ---
 - name: Restart syslog-ng
-  service:
+  ansible.builtin.service:
     name: syslog-ng
     state: restarted
     # Needed to force SysV service manager on Docker for Molecule tests
     use: "{{ ansible_service_mgr }}"
-  listen: 'restart syslog-ng'
+  listen: restart syslog-ng

handlers/start_consul.yml

@@ -1,23 +1,23 @@
 ---
 - name: Start consul on Unix
-  service:
+  ansible.builtin.service:
     name: consul
     state: started
     # Needed to force SysV service manager on Docker for Molecule tests
     use: "{{ ansible_service_mgr }}"
   when:
     - ansible_os_family != "Darwin"
     - ansible_os_family != "Windows"
-  listen: 'start consul'
+  listen: start consul
 
 - name: Start consul on Mac
-  include_tasks: "{{ role_path }}/handlers/start_consul_mac.yml"
+  ansible.builtin.include_tasks: "{{ role_path }}/handlers/start_consul_mac.yml"
   when: ansible_os_family == "Darwin"
-  listen: 'start consul'
+  listen: start consul
 
 - name: Start consul on Windows
-  win_service:
+  ansible.windows.win_service:
     name: consul
     state: started
   when: ansible_os_family == "Windows"
-  listen: 'start consul'
+  listen: start consul

handlers/start_consul_mac.yml

@@ -2,28 +2,29 @@
 - name: See if consul service exists
   become: true
   become_user: "{{ consul_user }}"
-  command: "launchctl list {{ consul_launchctl_ident }}"
+  ansible.builtin.command: launchctl list {{ consul_launchctl_ident }}
   changed_when: false
   ignore_errors: true
   register: consul_service_list
 
 - name: Get UID for consul user
-  command: "id -u {{ consul_user }}"
+  ansible.builtin.command: id -u {{ consul_user }}
   changed_when: false
   register: uid
   when: consul_service_list is failed
 
-- name: Load consul service
+- name: Load consul service # noqa no-changed-when
   become: true
   become_user: "{{ consul_user }}"
-  command: "launchctl bootstrap gui/{{ uid.stdout }} {{ consul_launchctl_plist }}"
+  ansible.builtin.command: launchctl bootstrap gui/{{ uid.stdout }} {{ consul_launchctl_plist }}
   when: consul_service_list is failed
 
 - name: Assert that consul service is running
   # Normally we'd want to use `launchctl list` for this, but it has a nasty habit of
   # randomly not returning the PID in its output, which makes it hard for us to see if the
   # service is running.
-  command: "pgrep consul"
+  ansible.builtin.command:
+    cmd: pgrep consul
   changed_when: false
   register: consul_mac_service_pgrep
   retries: 20

handlers/start_snapshot.yml

@@ -1,6 +1,6 @@
 ---
 - name: Start consul snapshot on Unix
-  service:
+  ansible.builtin.service:
     name: consul_snapshot
     state: started
     enabled: true
@@ -9,4 +9,4 @@
   when:
     - ansible_os_family != "Darwin"
     - ansible_os_family != "Windows"
-  listen: 'start snapshot'
+  listen: start snapshot

handlers/stop_consul_mac.yml

@@ -2,21 +2,21 @@
 - name: See if consul service exists
   become: true
   become_user: "{{ consul_user }}"
-  command: "launchctl list {{ consul_launchctl_ident }}"
+  ansible.builtin.command: launchctl list {{ consul_launchctl_ident }}
   changed_when: false
   ignore_errors: true
   register: consul_service_list
 
 - name: Get UID for consul user
-  command: "id -u {{ consul_user }}"
+  ansible.builtin.command: id -u {{ consul_user }}
   changed_when: false
   register: uid
   when: consul_service_list is succeeded
 
 - name: Unload consul service
   become: true
   become_user: "{{ consul_user }}"
-  command: "launchctl bootout gui/{{ uid.stdout }} {{ consul_launchctl_plist }}"
+  ansible.builtin.command: launchctl bootout gui/{{ uid.stdout }} {{ consul_launchctl_plist }}
   changed_when: false
   register: unload_consul_service
   # Code 113 is returned by launchctl for the error "Could not find service in domain for