import_tasks instead of include; bringing role up to ansible-prometheus standards; minor changes (#48)

[minor] modification to introduce some of good ideas seen in #47 and ansible-prometheus
- add ansible_managed info to service file
- use `import_tasks` instead of `include`
- specify `become: true` on role level not on playbook level
- split `main.yml` into smaller files: `install.yml` and `configure.yml`
- use one task for installing dependecies
- use vars files for distro-specific values
- use `node_exporter_system_group` and `node_exporter_system_user` which shouldn't be user-defined (defined in `vars` not in `defaults`), but simplify role management
- add tags
- add 2 more preflight checks

Author: paulfantom

README.md

@@ -34,7 +34,6 @@ All variables which can be overridden are stored in [defaults/main.yml](defaults
 Use it in a playbook as follows:
 ```yaml
 - hosts: all
-  become: yes
   roles:
     - cloudalchemy.node-exporter
 ```

handlers/main.yml

@@ -1,5 +1,5 @@
 ---
-- name: restart node exporter
+- name: restart node_exporter
   become: true
   systemd:
     daemon_reload: true

meta/main.yml

@@ -5,26 +5,25 @@ galaxy_info:
   license: MIT
   min_ansible_version: 2.4
   platforms:
-  - name: Ubuntu
-    versions:
-    - bionic
-    - xenial
-  - name: Debian
-    versions:
-    - jessie
-    - stretch
-  - name: EL
-    versions:
-    - 7
-  - name: Fedora
-    versions:
-    - 27
+    - name: Ubuntu
+      versions:
+        - bionic
+        - xenial
+    - name: Debian
+      versions:
+        - jessie
+        - stretch
+    - name: EL
+      versions:
+        - 7
+    - name: Fedora
+      versions:
+        - 27
   galaxy_tags:
-  - monitoring
-  - prometheus
-  - exporter
-  - metrics
-  - system
-
+    - monitoring
+    - prometheus
+    - exporter
+    - metrics
+    - system
 
 dependencies: []

tasks/configure.yml

@@ -0,0 +1,27 @@
+---
+- name: Create texfile collector dir
+  file:
+    path: "{{ node_exporter_textfile_dir }}"
+    state: directory
+    owner: "{{ node_exporter_system_user }}"
+    group: "{{ node_exporter_system_group }}"
+    recurse: true
+    mode: 0755
+  when: node_exporter_textfile_dir != ""
+
+- name: Node exporter can read anything (omit file permissions)
+  capabilities:
+    path: '/usr/local/bin/node_exporter'
+    capability: cap_dac_read_search+ep
+    state: present
+  when: not ansible_check_mode
+
+- name: Allow Node Exporter port in SELinux on RedHat OS family
+  seport:
+    ports: "{{ node_exporter_web_listen_address.split(':')[1] }}"
+    proto: tcp
+    setype: http_port_t
+    state: present
+  when:
+    - ansible_version.full is version_compare('2.4', '>=')
+    - ansible_selinux.status == "enabled"

tasks/install.yml

@@ -0,0 +1,63 @@
+---
+- name: Install dependencies
+  package:
+    name: "{{ item }}"
+    state: present
+  with_items: "{{ node_exporter_dependencies }}"
+
+- name: Create the node_exporter group
+  group:
+    name: "{{ node_exporter_system_group }}"
+    state: present
+    system: true
+
+- name: Create the node_exporter user
+  user:
+    name: "{{ node_exporter_system_user }}"
+    groups: "{{ node_exporter_system_group }}"
+    append: true
+    shell: /usr/sbin/nologin
+    system: true
+    createhome: false
+    home: /
+
+- name: Download node_exporter binary to local folder
+  become: false
+  get_url:
+    url: "https://github.com/prometheus/node_exporter/releases/download/v{{ node_exporter_version }}/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
+    dest: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
+    checksum: "sha256:{{ node_exporter_checksum }}"
+  register: _download_binary
+  until: _download_binary is succeeded
+  retries: 5
+  delay: 2
+  delegate_to: localhost
+  check_mode: false
+
+- name: Unpack node_exporter binary
+  become: false
+  unarchive:
+    src: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
+    dest: "/tmp"
+    creates: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}/node_exporter"
+  delegate_to: localhost
+  check_mode: false
+
+- name: Propagate node_exporter binaries
+  copy:
+    src: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}/node_exporter"
+    dest: "/usr/local/bin/node_exporter"
+    mode: 0750
+    owner: "{{ node_exporter_system_user }}"
+    group: "{{ node_exporter_system_group }}"
+  notify: restart node_exporter
+  when: not ansible_check_mode
+
+- name: Copy the Node Exporter systemd service file
+  template:
+    src: node_exporter.service.j2
+    dest: /etc/systemd/system/node_exporter.service
+    owner: root
+    group: root
+    mode: 0644
+  notify: restart node_exporter

tasks/main.yml

@@ -1,110 +1,30 @@
 ---
-- include: preflight.yml
-
-- name: Create the Node Exporter group
-  group:
-    name: "node-exp"
-    state: present
-    system: true
-
-- name: Create the Node Exporter user
-  user:
-    name: "node-exp"
-    groups: "node-exp"
-    append: true
-    shell: /usr/sbin/nologin
-    system: true
-    createhome: false
-    home: /
-
-- name: Download node_exporter binary to local folder
-  become: false
-  get_url:
-    url: "https://github.com/prometheus/node_exporter/releases/download/v{{ node_exporter_version }}/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
-    dest: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
-    checksum: "sha256:{{ node_exporter_checksum }}"
-  register: _download_binary
-  until: _download_binary is succeeded
-  retries: 5
-  delay: 2
-  delegate_to: localhost
-  check_mode: false
-
-- name: Unpack node_exporter binary
-  become: false
-  unarchive:
-    src: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}.tar.gz"
-    dest: "/tmp"
-    creates: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}/node_exporter"
-  delegate_to: localhost
-  check_mode: false
-
-- name: Propagate Node Exporter binaries
-  copy:
-    src: "/tmp/node_exporter-{{ node_exporter_version }}.linux-{{ go_arch_map[ansible_architecture] | default(ansible_architecture) }}/node_exporter"
-    dest: "/usr/local/bin/node_exporter"
-    mode: 0750
-    owner: "node-exp"
-    group: "node-exp"
-  notify:
-    - restart node exporter
-  when: not ansible_check_mode
-
-- name: Create texfile collector dir
-  file:
-    path: "{{ node_exporter_textfile_dir }}"
-    state: directory
-    owner: "node-exp"
-    group: "node-exp"
-    recurse: true
-    mode: 0755
-  when: node_exporter_textfile_dir != ""
-
-- name: Install libcap on Debian systems
-  package:
-    name: "libcap2-bin"
-    state: present
-  when: ansible_os_family | lower == "debian"
-
-- name: Node exporter can read anything (omit file permissions)
-  capabilities:
-    path: '/usr/local/bin/node_exporter'
-    capability: cap_dac_read_search+ep
-    state: present
-  when:
-    not ansible_check_mode
-
-- name: Copy the Node Exporter systemd service file
-  template:
-    src: node_exporter.service.j2
-    dest: /etc/systemd/system/node_exporter.service
-    owner: root
-    group: root
-    mode: 0644
-  notify:
-    - restart node exporter
-
-- name: Install dependencies on RedHat OS family
-  package:
-    name: "{{ item }}"
-    state: present
-  with_items:
-    - libselinux-python
-    - policycoreutils-python
-  when:
-    - ansible_os_family == "RedHat"
-
-- name: Allow Node Exporter port in SELinux on RedHat OS family
-  seport:
-    ports: "{{ node_exporter_web_listen_address.split(':')[1] }}"
-    proto: tcp
-    setype: http_port_t
-    state: present
-  when:
-    - ansible_version.full is version_compare('2.4', '>=')
-    - ansible_selinux.status == "enabled"
+- name: Gather variables for each operating system
+  include_vars: "{{ item }}"
+  with_first_found:
+    - "{{ ansible_distribution | lower }}.yml"
+    - "{{ ansible_os_family | lower }}.yml"
+  tags:
+    - always
+
+- import_tasks: preflight.yml
+  tags:
+    - always
+
+- import_tasks: install.yml
+  become: true
+  tags:
+    - install
+
+- import_tasks: configure.yml
+  become: true
+  tags:
+    - configure
 
 - name: Ensure Node Exporter is enabled on boot
+  become: true
   systemd:
     name: node_exporter
     enabled: true
+  tags:
+    - run

tasks/preflight.yml

@@ -1,5 +1,15 @@
 ---
-- name: check collectors
+- name: Naive assertion of proper listen address
+  assert:
+    that:
+      - "':' in node_exporter_web_listen_address"
+
+- name: Fail on unsupported init systems
+  fail:
+    msg: "This module only works with systemd"
+  when: ansible_service_mgr != 'systemd'
+
+- name: Check collectors
   fail:
     msg: "Collector cannot be both disabled and enabled"
   when: item in node_exporter_enabled_collectors

templates/node_exporter.service.j2

@@ -1,27 +1,29 @@
+# {{ ansible_managed }}
+
 [Unit]
 Description=Prometheus Node Exporter
 After=network.target
 
 [Service]
 Type=simple
-User=node-exp
-Group=node-exp
+User={{ node_exporter_system_user }}
+Group={{ node_exporter_system_group }}
 Nice=-5
 ExecStart=/usr/local/bin/node_exporter \
-    --web.listen-address {{ node_exporter_web_listen_address }} \
-    {% for c in node_exporter_enabled_collectors -%}
-    {%   if not c is mapping -%}
-    --collector.{{ c }} \
-    {%   else -%}
-    {%     set name,opt = (c.items() | list)[0] -%}
-    {%     for k,v in opt.items() -%}
+{% for collector in node_exporter_enabled_collectors -%}
+{%   if not collector is mapping %}
+    --collector.{{ collector }} \
+{%   else -%}
+{%     set name, options = (collector.items()|list)[0] -%}
+{%     for k,v in options|dictsort %}
     --collector.{{ name }}.{{ k }}={{ v }} \
-    {%     endfor -%}
-    {%   endif -%}
-    {% endfor -%}
-    {% for c in node_exporter_disabled_collectors -%}
-    --no-collector.{{ c }} \
-    {% endfor %}
+{%     endfor -%}
+{%   endif -%}
+{% endfor -%}
+{% for collector in node_exporter_disabled_collectors %}
+    --no-collector.{{ collector }} \
+{% endfor %}
+    --web.listen-address={{ node_exporter_web_listen_address }}
 
 SyslogIdentifier=node_exporter
 Restart=always

vars/debian.yml

@@ -0,0 +1,3 @@
+---
+node_exporter_dependencies:
+  - libcap2-bin

vars/main.yml

@@ -5,3 +5,6 @@ go_arch_map:
   aarch64: 'arm64'
   armv7l: 'armv7'
   armv6l: 'armv6'
+
+node_exporter_system_group: "node-exp"
+node_exporter_system_user: "{{ node_exporter_system_group }}"