Add missing parameters in postgresql plugin config

[jengsala]

This PR will allow to added missing parameters in postgresql plugin config

Reference : https://collectd.org/wiki/index.php/Plugin:PostgreSQL

#
# DO NOT EDIT
#
# This file is managed by salt via salt://collectd/files/postgresql.conf
# Modify the config that generates this file instead
#

LoadPlugin postgresql

<Plugin "postgresql">
  <Query rt36_tickets>
    Statement "mystatement"
    Param "hostname"  
    <Result>
      Type "counter"
      InstancePrefix "rt36_tickets"
      InstancesFrom "type"
      ValuesFrom "count"
    </Result>
  </Query>
  <Query magic>
    Statement "mystatement"  
    <Result>
      Type "counter"
      InstancePrefix "rt36_tickets"
      InstancesFrom "type"
    </Result>
  </Query>
  <Database foo>
    Host "hostname"
    Port "5432"
    User "username"
    Password "secret"
    SSLMode "prefer"
    KRBSrvName "kerberos_service_name"
    Service "service_name"
    Interval "3600"
    Query rt36_tickets
    Query magic
  </Database>

[myii]

Thanks for your PR, @dsala-it. I've got a review for you, with suggested changes inline.

[daks]

Some comments about code. Tests are also mostly failing and needs to be fixed.

[myii]

@dsala-it Not a complete review, because I'm short on time at the moment. But a couple of issues to be addressed.

[daks]

It's OK for me, except for platform ubuntu which has a failing converge https://gitlab.com/saltstack-formulas/collectd-formula/-/jobs/2484292490

[myii]

@dsala-it We're on the final stretch now, just tidying up. Rather than reviewing multiple blocks, I've prepared a commit that contains the rest of the requested changes:

• myii@b4e229c

This is the CI run for that:

• https://gitlab.com/myii/collectd-formula/-/pipelines/544479549

You can grab that commit from my fork and use it here; if that is difficult, I should be able to force-push it myself. Note, that I've updated the commit message as well, to something more appropriate for the changelog.

I've also mentioned one last change inline (below), that I didn't get around to adding to the commit.

---

For clarity, this is the overall diff, including the bit below:

diff --git a/collectd/map.jinja b/collectd/map.jinja
index 11d930c..9eb9fae 100644
--- a/collectd/map.jinja
+++ b/collectd/map.jinja
@@ -9,7 +9,7 @@
         'moduledirconfig': '/usr/share/collectd/modules',
         'user': 'root',
         'group': 'root',
-        'required_dependencies': {'pkgs': ['postgresql-client']},
+        'plugin_postgresql': {'pkg': 'postgresql-client'},
     },
     'RedHat': {
         'config': '/etc/collectd.conf',
@@ -20,7 +20,7 @@
         'moduledirconfig': '/usr/share/collectd/modules',
         'user': 'root',
         'group': 'root',
-        'required_dependencies': {'pkgs': ['collectd-postgresql']},
+        'plugin_postgresql': {'pkg': 'collectd-postgresql'},
     },
     'FreeBSD': {
         'config': '/usr/local/etc/collectd.conf',
@@ -31,7 +31,7 @@
         'moduledirconfig': '/usr/local/share/collectd/modules',
         'user': 'root',
         'group': 'wheel',
-        'required_dependencies': {'pkgs': ['postgresql-client']},
+        'plugin_postgresql': {'pkg': 'postgresql-client'},
     },
     'Suse': {
         'config': '/etc/collectd.conf',
@@ -42,7 +42,7 @@
         'moduledirconfig': '/usr/share/collectd/modules',
         'user': 'root',
         'group': 'root',
-        'required_dependencies': {'pkgs': ['collectd-plugin-postgresql']},
+        'plugin_postgresql': {'pkg': 'collectd-plugin-postgresql'},
     },
 }, merge=salt['pillar.get']('collectd:lookup')) %}
 
diff --git a/collectd/postgresql.sls b/collectd/postgresql.sls
index 5a9ec3c..bdfec0e 100644
--- a/collectd/postgresql.sls
+++ b/collectd/postgresql.sls
@@ -1,16 +1,12 @@
 {%- from "collectd/map.jinja" import collectd_settings with context %}
-{%- set postgresql_settings = collectd_settings.get('plugins:postgresql') %}
-
-install_required_dependencies:
-  pkg.installed:
-    - pkgs:
-{% for _pkg in collectd_settings.required_dependencies.pkgs %}
-      - {{ _pkg }}
-{% endfor %}
 
 include:
   - collectd
 
+collectd-postgresql-pkg-installed:
+  pkg.installed:
+    - name: {{ collectd_settings.plugin_postgresql.pkg }}
+
 {{ collectd_settings.plugindirconfig }}/postgresql.conf:
   file.managed:
     - source: salt://collectd/files/postgresql.conf
@@ -21,5 +17,4 @@ include:
     - watch_in:
       - service: collectd-service
     - require:
-      - pkg: install_required_dependencies
-
+      - pkg: collectd-postgresql-pkg-installed
diff --git a/kitchen.yml b/kitchen.yml
index e2d932a..dd10a3a 100644
--- a/kitchen.yml
+++ b/kitchen.yml
@@ -296,9 +296,9 @@ suites:
       state_top:
         base:
           '*':
-            - collectd.postgresql
             - collectd._mapdata
             - collectd
+            - collectd.postgresql
       pillars:
         top.sls:
           base:

---

Partially reviewed using myii/ssf-formula#446.

[jengsala]

I redid a little cleaning and a rebase/squash commits. @myii I let you see for one last time and merge if everything is okay

[jengsala]

About the PR https://gitlab.com/saltstack-formulas/collectd-formula/-/pipelines/545480720, The converge does not pass in Ubuntu-22* jobs/2491768749.

The problem is that the collectd-core package used here in map.jinja#L5 is not yet available.

See https://pkgs.org/search/?q=collectd

I don't see any other way to solve this problem. Should we ignore it or not?

Otherwise, on the last commit I fixed the linter errors. I let you approve if everything is okay for you ...

[myii]

Merged, thanks @dsala-it. I adjusted the last commit (31fa9fe) in order to fix all the linter issues that we discussed on Slack:

• https://gitlab.com/myii/collectd-formula/-/jobs/2492752465 -- before
• https://gitlab.com/myii/collectd-formula/-/jobs/2498876164 -- after

[saltstack-formulas-travis]

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[myii]

@daks Thanks for the reviews!