More poolmon improvements

[fraenki]

This fixes two issues:

1. It was not possible to replace values in $dovecot::poolmon_config, because deep merging was enabled.

This was especially problematic for Poolmon's check_port configuration, because the default values may not work everywhere.

I've disabling deep merging for the whole $dovecot::poolmon_config key. As a side effect it is now required to specify ALL options if one wants to customize any item in $dovecot::poolmon_config.

2. Add more safekeeping to the poolmon template.

The following error would occur if either check_ssl or check_port where not an array (or not set at all):

Evaluation Error: Error while evaluating a Method call, 'join' parameter 'arg' expects an Array value, got Undef (file: /etc/puppetlabs/code/environments/production/modules/dovecot/templates/poolmon.config.epp, line: 36, column: 605)

[oxc]

@fraenki, did you consider adding a knockout prefix instead of disabling deep merging?

[fraenki]

did you consider adding a knockout prefix instead of disabling deep merging?

Maybe I'm missing something, but a knockout prefix could be used to remove values, but I want to replace values.

For example, the default for check_port (in dovecot::poolmon_config) is [ 110, 143] and I want to replace it with [143].

[fraenki]

Maybe I'm missing something, but a knockout prefix could be used to remove values, but I want to replace values.

Thinking about this a little further, it seems to be possible to remove all defaults and add new values at the same time:

dovecot::poolmon_config:
  check_port:
    - --
    - 999
    - 998

Looks good, I guess:

Notice: /Stage[main]/Dovecot::Poolmon/File[/etc/sysconfig/poolmon]/content: 
--- /etc/sysconfig/poolmon	2019-01-06 22:55:47.361115248 +0100
+++ /tmp/puppet-file20190106-14071-1a2tgb5	2019-01-06 22:57:29.600321625 +0100
@@ -2,4 +2,4 @@
 
-OPTIONS=--logfile="syslog" --lockfile="/var/run/poolmon.pid" --socket="/var/run/dovecot/director-admin" --interval="30" --timeout="5" --port=110 --port=143  
+OPTIONS=--logfile="syslog" --lockfile="/var/run/poolmon.pid" --socket="/var/run/dovecot/director-admin" --interval="30" --timeout="5" --port=999 --port=998  

[oxc]

@fraenki Which of the solutions would you prefer? I'm swaying...

Is there even a decent use-case for merging config values?

[fraenki]

Which of the solutions would you prefer? I'm swaying...

While I think the knockout_prefix, especially the syntax in the example, is somewhat odd. I'd prefer it nonetheless, because it's the lesser evil (see below).

Is there even a decent use-case for merging config values?

Yes. Actually If we decide to disable deep merging, then one needs to specify ALL required values for the dovecot::poolmon_config hash, even if only one should be replaced/changed.

[oxc]

Yes. Actually If we decide to disable deep merging, then one needs to specify ALL values, even if only one should be replaced/changed.

As an alternative, we could switch to hash merging for the config hash. Not sure if that's not even more confusing, though.

[fraenki]

The root of all evil is the fact that I've put ALL poolmon config in a single hash dovecot::poolmon_config. Maybe it's time to split this into individual dovecot::poolmon_xxx parameters instead?

[oxc]

Yes, but with hash merging that would mean that you could replace every single entry, right? Sub-hashes/arrays would not be merged recursively.
That's what you wanted initially, we just would have to make it quite clear in the README that this config uses a different merge strategy.

[fraenki]

I've settled with the hash merge behavior and added two new sections to the README. Feel free to correct wording/grammar :)

[oxc]

Released as v2.0.0 due to this change being potentially breaking.