Merge pull request #60 from B1ue-W01f/fix/environ_correction

Fix/environ correction

.yamllint

@@ -12,6 +12,7 @@ extends: 'default'
 # 5. Any YAML files under directory `.kitchen/`, introduced during local testing
 # 6. `kitchen.vagrant.yml`, which contains Embedded Ruby (ERB) template syntax
 ignore: |
+  .bundle/
   .cache/
   .git/
   node_modules/

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/config/environ.sls

@@ -4,15 +4,19 @@
 {%- set tplroot = tpldir.split('/')[0] %}
 {%- from tplroot ~ "/map.jinja" import prometheus as p with context %}
 {%- from tplroot ~ "/libtofs.jinja" import files_switch with context %}
-{%- from tplroot ~ "/files/macros.jinja" import concat_environ %}
+{%- from tplroot ~ "/files/macros.jinja" import concat_args %}
 {%- set sls_archive_install = tplroot ~ '.archive.install' %}
 {%- set sls_package_install = tplroot ~ '.package.install' %}
+{%- set sls_service_running = tplroot ~ '.service.running' %}
 
 include:
   - {{ sls_archive_install if p.pkg.use_upstream_archive else sls_package_install }}
+  - {{ sls_service_running }}
 
     {%- for name in p.wanted.component %}
-        {%- if 'environ' in p.pkg.component[name] and p.pkg.component[name]['environ'] %}
+        {%- if 'environ' in p.pkg.component[name] and 'args' in p.pkg.component[name]['environ'] %}
+            {%- set args = p.pkg.component[name]['environ']['args'] %}
+            {%- set arg_name = p.pkg.component[name]['environ']['environ_arg_name'] %}
             {%- if 'environ_file' in p.pkg.component[name] and p.pkg.component[name]['environ_file'] %}
 
 prometheus-config-install-{{ name }}-environ_file:
@@ -29,8 +33,9 @@ prometheus-config-install-{{ name }}-environ_file:
     - user: {{ p.identity.rootuser }}
     - group: {{ p.identity.rootgroup }}
                 {%- endif %}
-    - contents: |
+    - context:
-       command_args="{{ concat_environ(environ) }}"
+        args: {{ concat_args(args) }}
+        arg_name: {{ arg_name }}
     - watch_in:
       - service: prometheus-service-running-{{ name }}
     - require:
@@ -43,7 +48,7 @@ prometheus-config-environ-{{ name }}-all:
     - name: {{ name }}_environ
     # service prometheus restart tends to hang on FreeBSD
     # https://github.com/saltstack/salt/issues/44848#issuecomment-487016414
-    - value: "{{ concat_environ(p.pkg.component[name]['environ']) }} >/dev/null 2>&1"
+    - value: "{{ concat_args(p.pkg.component[name]['environ']) }} >/dev/null 2>&1"
     - watch_in:
       - service: prometheus-service-running-{{ name }}
 

prometheus/config/init.sls

@@ -4,3 +4,4 @@
 include:
   - .file
   - .environ
+  - .storage

prometheus/config/storage.sls

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+# vim: ft=sls
+
+{%- set tplroot = tpldir.split('/')[0] %}
+{%- from tplroot ~ "/map.jinja" import prometheus as p with context %}
+{%- from tplroot ~ "/libtofs.jinja" import files_switch with context %}
+{%- from tplroot ~ "/files/macros.jinja" import concat_args %}
+{%- set sls_archive_install = tplroot ~ '.archive.install' %}
+{%- set sls_package_install = tplroot ~ '.package.install' %}
+
+include:
+  - {{ sls_archive_install if p.pkg.use_upstream_archive else sls_package_install }}
+
+    {%- for name in p.wanted.component %}
+        {%- if 'environ' in p.pkg.component[name] and 'args' in p.pkg.component[name]['environ'] %}
+            {%- set args = p.pkg.component[name]['environ']['args'] %}
+            {%- if 'storage.tsdb.path' in args.keys() %}
+
+prometheus-service-args-{{ name }}-data-dir:
+  file.directory:
+    - name: {{ args['storage.tsdb.path'] }}
+    - owner: {{ name }}
+    - group: {{ name }}
+    - makedirs: True
+    - watch_in:
+      - service: prometheus-service-running-{{ name }}
+    - require:
+      - user: prometheus-config-users-install-{{ name }}-user-present
+      - group: prometheus-config-users-install-{{ name }}-group-present
+
+            {%- endif %}
+        {% endif %}
+    {% endfor %}

prometheus/defaults.yaml

@@ -44,8 +44,11 @@ prometheus:
         version: v0.21.0
         config_file: /etc/prometheus/alertmanager.yml
         config: {}
-        environ_file: /etc/default/prometheus-alertmanager.sh
+        environ_file: /etc/default/prometheus-alertmanager
-        environ: {}
+        environ:
+          environ_arg_name: ARGS
+        # 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
@@ -65,6 +68,9 @@ prometheus:
         version: v0.18.0
         config_file: /etc/prometheus/blackbox_exporter.yml
         config: {}
+        environ_file: /etc/default/prometheus-blackbox-exporter
+        environ:
+          environ_arg_name: ARGS
         archive:
           source_hash: a87f9530e31e2b20d03a6d941f6f051c57ee724f24b38f2615b8580ca63966dc
         commands:
@@ -131,10 +137,13 @@ prometheus:
             - tcp/9207
       node_exporter:
         version: v1.0.1
-        environ_file: /etc/default/prometheus-node-exporter.sh
+        environ_file: /etc/default/prometheus-node-exporter
-        environ: {}
+        environ:
+          environ_arg_name: ARGS
         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
@@ -152,8 +161,11 @@ prometheus:
         version: v2.22.1
         config_file: /etc/prometheus/prometheus.yml
         config: {}
-        environ_file: /etc/default/prometheus.sh
+        environ_file: /etc/default/prometheus
-        environ: {}
+        environ:
+          environ_arg_name: ARGS
+        # 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
@@ -170,8 +182,9 @@ prometheus:
             - tcp/9090
       pushgateway:
         version: v1.3.0
-        environ_file: /etc/default/prometheus-pushgateway.sh
+        environ_file: /etc/default/prometheus-pushgateway
-        environ: {}
+        environ:
+          environ_arg_name: ARGS
         config_file: /etc/prometheus/pushgateway.yml
         config: {}
         service: {}
@@ -195,7 +208,6 @@ prometheus:
             - tcp/4505
             - tcp/4506
             - tcp/9102
-
     clientlibs:
       # https://prometheus.io/docs/instrumenting/clientlibs
       # no bash & perl client tarballs are available

prometheus/files/default/environ.sh.jinja

@@ -2,5 +2,8 @@
 # File managed by Salt at <{{ source }}>.
 # Your changes may be overwritten.
 ########################################################################
+# Set the command-line arguments to pass to the server.%}"
+{{ arg_name }}="{{ args }}"
+
+
 
-{{ config|yaml(False) }}

prometheus/osfamilymap.yaml

@@ -15,7 +15,22 @@
     {%- set macos_rootgroup = salt['cmd.run']("stat -f '%Sg' /dev/console") %}
 {%- endif %}
 
-Arch: {}
+Arch:
+  pkg:
+    component:
+      alertmanager:
+        name: alertmanager
+      blackbox_exporter:
+        name: prometheus-blackbox-exporter
+      prometheus:
+        environ_file: /etc/conf.d/prometheus
+        environ:
+          environ_arg_name: PROMETHEUS_ARGS
+      node_exporter:
+        name: prometheus-node-exporter
+        environ_file: /etc/conf.d/prometheus-node-exporter
+        environ:
+          environ_arg_name: NODE_EXPORTER_ARGS
 
 Alpine: {}
 
@@ -29,8 +44,6 @@ Debian:
     use_upstream_package: false
     use_upstream_archive: false
     component:
-      prometheus:
-        name: prometheus
       alertmanager:
         name: prometheus-alertmanager
         service:
@@ -47,11 +60,20 @@ Debian:
       bird_exporter:
         name: prometheus-bird-exporter
         args_file: /etc/default/prometheus-bird-exporter
+      prometheus:
+        name: prometheus
       blackbox_exporter:
         name: prometheus-blackbox-exporter
+        config_file: /etc/prometheus/blackbox.yml
+        environ_file: /etc/default/prometheus-blackbox-exporter
+        environ:
+          environ_arg_name: ARGS
         service:
           name: prometheus-blackbox-exporter
-        config_file: /etc/prometheus/blackbox.yml
+      consul_exporter:
+        name: prometheus-consul-exporter
+      graphite_exporter:
+        name: prometheus-graphite-exporter
       hacluster_exporter:
         name: prometheus-hacluster-exporter
         args_file: /etc/default/prometheus-hacluster-exporter
@@ -70,6 +92,8 @@ Debian:
       mailexporter:
         name: prometheus-mailexporter
         args_file: /etc/default/prometheus-mailexporter
+      memcached_exporter:
+        name: prometheus-memcached-exporter
       mongodb_exporter:
         name: prometheus-mongodb-exporter
         args_file: /etc/default/prometheus-mongodb-exporter
@@ -85,7 +109,7 @@ Debian:
         args_file: /etc/default/prometheus-nginx-exporter
       nginx_vts_exporter:
         name: prometheus-nginx-vts-exporter
-        args_file: /etc/default/prometheus-nginx-vts-exporter
+        args_file: /etc/default/prometheus-nginx-vts-export
       node_exporter:
         name: prometheus-node-exporter
         service:
@@ -113,7 +137,11 @@ Debian:
         args_file: /etc/default/prometheus-process-exporter
       pushgateway:
         name: prometheus-pushgateway
+        service:
+          name: prometheus-pushgateway
         args_file: /etc/default/prometheus-pushgateway
+      statsd_exporter:
+        name: prometheus-statsd-exporter
       snmp_exporter:
         name: prometheus-snmp-exporter
         args_file: /etc/default/prometheus-snmp-exporter
@@ -122,7 +150,7 @@ Debian:
         args_file: /etc/default/prometheus-sql-exporter
       squid_exporter:
         name: prometheus-squid-exporter
-        args_file: /etc/default/prometheus-squid-exporter
+        args_file: /etc/default/prometheus-squid
       tplink_plug_exporter:
         name: prometheus-tplink-plug-exporter
         args_file: /etc/default/prometheus-tplink-plug-exporter
@@ -148,6 +176,35 @@ RedHat:
     component:
       prometheus:
         name: prometheus2
+        environ_file: /etc/default/prometheus
+        environ:
+          environ_arg_name: PROMETHEUS_OPTS
+          args:
+            config.file: /etc/prometheus/prometheus.yml
+            storage.tsdb.path: /var/lib/prometheus/data
+            web.console.libraries: /usr/share/prometheus/console_libraries
+            web.console.templates: /usr/share/prometheus/consoles
+      alertmanager:
+        environ_file: /etc/default/alertmanager
+        environ:
+          environ_arg_name: ALERTMANAGER_OPTS
+          args: 
+            config.file: /etc/prometheus/alertmanager.yml
+            storage.path: /var/lib/prometheus/alertmanager
+      node_exporter:
+        environ_file: /etc/default/node_exporter
+        environ:
+          environ_arg_name: NODE_EXPORTER_OPTS
+      pushgateway:
+        environ_file: /etc/default/pushgateway
+        environ:
+          environ_arg_name: PUSHGATEWAY_OPTS
+      blackbox_exporter:
+        environ_file: /etc/default/blackbox_exporter
+        environ:
+          environ_arg_name: BLACKBOX_EXPORTER_OPTS
+          args:
+            config.file: /etc/prometheus/blackbox.yml
     repo:
       # https://akai-tsuki.hatenablog.com/entry/2017/12/03/000000
       # yamllint disable rule:line-length
@@ -385,3 +442,4 @@ MacOS:
       statsd_exporter:
         archive:
           source_hash: 15132494523c2b6a89e09b2da63452c8fe587fb82fcc3fd21cc75a4aa2766644
+

test/integration/repo/controls/packages_spec.rb

@@ -11,7 +11,16 @@ control 'prometheus packages' do
         alertmanager
         node_exporter
       ]
-    when 'debian'
+    when 'linux'
+      case platform[:name]
+      when 'arch'
+        %w[
+          prometheus
+          alertmanager
+          prometheus-node-exporter
+        ]
+      end
+    else
       %w[
         prometheus
         prometheus-alertmanager

test/integration/repo/controls/service_spec.rb

@@ -21,15 +21,37 @@ control 'prometheus services' do
       ]
     end
 
+  node_exporter =
+    case platform[:family]
+    when 'redhat'
+      'node_exporter'
+    else
+      'prometheus-node-exporter'
+    end
+
   services.each do |service|
     describe service(service) do
       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/#{node_exporter}") do
+    its('content') { should include '--web.listen-address=:9110' }
+    its('content') { should include '--log.level=debug' }
+  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:
@@ -18,7 +19,7 @@ prometheus:
       - php-fpm_exporter
       - postgres_exporter
       - mysqld_exporter
-      # - memcached_exporter  # not in upstream repo, only archive
+      - memcached_exporter  # not in upstream repo, only archive
 
   exporters:
     node_exporter:
@@ -85,6 +86,10 @@ prometheus:
                 - to: 'team-X+alerts@example.org'
 
       node_exporter:
+        environ:
+          args:
+            collector.systemd: true
+            web.listen-address: ":9110"
         service:
           args:
             collector.systemd: null
@@ -113,6 +118,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://raw.githubusercontent.com/prometheus/prometheus/release-2.10/config/testdata/conf.good.yml

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:
+            log.level: debug
+            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://raw.githubusercontent.com/prometheus/prometheus/release-2.10/config/testdata/conf.good.yml