Cleanup how python modules are configured

Convert templates for `collectd::module::python` to `epp`

Example

```puppet
collectd::plugin::python::module{'example':
  config => [{'Verbose' => true,
              'Values'  => ['abc',def'],
              'Name     => 'My Name',
              'Limit    => 3.4
             },
            ],
  }
}
```

This will now render as

```apache
Import "example"
<Module "example">
  Verbose true
  Values "abc" "def"
  Name "My Name"
  Limit 3.4
</Module>
```

Specifying a hash value in the `config` paramater will now result in a type
failure. Previously it would add nonsense to the configuration file.
e.g

```puppet
collectd::plugin::python::module{'example':
  config => [{'Values' => {'abc' => 'def'}}],
}
```
will now give a type error.

To specify a list of quoted strings use an array. Previously
the following could be used.

```puppet
collectd::plugin::python::module{'example':
  config => [{'Values' => '"abc" "def"',
              'Name'   => '"My Name"',
            }],
}
```

to acheive the same result use

```puppet
collectd::plugin::python::module{'example':
  config => [{'Values' => ['abc','def'],
              'Name'   => 'My Name',
            }],
}
```

to render

```apacheconf
Import "example"
<Module "example">
  Values "abc" "def"
  Name "My Name"
</Module>

It is now possible to specify multiple instances of single python
module by using `collectd::plugin::python::module` twice. e.g.

```puppet
collectd::plugin::python::module{'exampleA':
  module => 'example,
  config => [{'Values' => 'abc'],}
}
collectd::plugin::python::module{'exampleB':
  module => 'example,
  config => [{'Values' => 'def'],}
}
```

Fixes #819

README.md

@@ -1460,11 +1460,29 @@ Or define single module:
 collectd::plugin::python::module {'zk-collectd':
   script_source => 'puppet:///modules/myorg/zk-collectd.py',
   config        => [
-    {'Hosts' => "localhost:2181"}
+    {'Hosts'   => "localhost:2181",
+     'Verbose' => true,
+     'Values'  => ["abc","def"],
+     'Name'    => 'My Name',
+     'Limit'   => 4.5,
+    }
   ]
 }
 ```
 
+The resulting configuration would be 
+
+```apache
+Import "zk-collectd"
+<Module "zk-collectd">
+  Hosts "localhost:2181"
+  Verbose true
+  Values "abc" "def"
+  Limit 4.5
+</Module>
+ 
+```
+
 Each plugin might use different `modulepath`, however make sure that all paths
 are included in `collectd::plugin::python` variable `modulepaths`. If no
 `modulepath` is specified, OS default will be used.
@@ -1530,7 +1548,7 @@ class { '::collectd::plugin::rabbitmq':
     'Scheme'   => 'https',
     'Port'     => '15671',
     'Host'     => $facts['fqdn'],
-    'Realm'    => '"RabbitMQ Management"',
+    'Realm'    => 'RabbitMQ Management',
   },
 }
 ```

manifests/plugin/python.pp

@@ -79,7 +79,7 @@
 
   concat::fragment { 'collectd_plugin_python_conf_header':
     order   => '00',
-    content => template('collectd/plugin/python/header.conf.erb'),
+    content => epp('collectd/plugin/python/header.conf'),
     target  => $python_conf,
   }
 

manifests/plugin/python/module.pp

@@ -1,11 +1,11 @@
 # Single module definition
 define collectd::plugin::python::module (
-  Array $config                              = [],
-  $ensure                                    = 'present',
-  $module                                    = $title,
-  $module_import                             = undef,
+  Array[Hash[String,Variant[String,Boolean,Numeric,Array[Variant[Boolean,String,Numeric]]]]] $config = [],
+  Enum['present','absent'] $ensure = 'present',
+  String $module = $title,
+  Optional[String] $module_import = undef,
   Optional[Stdlib::Absolutepath] $modulepath = undef,
-  $script_source                             = undef,
+  Optional[String] $script_source = undef,
 ) {
 
   include collectd
@@ -34,9 +34,29 @@
     }
   }
 
-  concat::fragment{ "collectd_plugin_python_conf_${module}":
-    order   => '50', # somewhere between header and footer
+  # Exactly one per module.
+  ensure_resource('concat::fragment',"collectd_plugin_python_conf_${module}_header",
+    {
+      'order'   => "50_${module}_00",
+      'target'  => $collectd::plugin::python::python_conf,
+      'content' => epp('collectd/plugin/python/module.conf_header',
+        {
+          'module_import' => $_module_import,
+        },
+      ),
+    }
+  )
+
+  # Possibly many per instance of a module configuration.
+  concat::fragment{ "collectd_plugin_python_conf_${title}_config":
+    order   => "50_${module}_50",
     target  => $collectd::plugin::python::python_conf,
-    content => template('collectd/plugin/python/module.conf.erb'),
+    content => epp('collectd/plugin/python/module.conf_config',
+      {
+        'title'  => $title,
+        'config' => $config,
+        'module' => $_module_import,
+      },
+    ),
   }
 }

spec/acceptance/class_plugin_python_spec.rb

@@ -68,4 +68,64 @@
       its(:stdout) { is_expected.to match %r{google.de} }
     end
   end
+
+  context 'two instances using same python module' do
+    it 'works idempotently with no errors' do
+      pp = <<-EOS
+      if $facts['os']['family'] == 'Debian' {
+        # for collectdctl command
+        package{['collectd-utils','python-dbus']:
+	  ensure => present,
+        }
+      }
+      package{['git','python-pip']:
+        ensure => present,
+	before => Package['collectd-systemd'],
+      }
+      package{'collectd-systemd':
+	ensure   => 'present',
+        provider => 'pip',
+	source   => 'git+https://github.com/mbachry/collectd-systemd.git',
+	before   => Service['collectd'],
+      }
+      class{'collectd':
+        typesdb => ['/usr/share/collectd/types.db'],
+      }
+      class{'collectd::plugin::python':
+	logtraces   => true,
+	interactive => false,
+        modules     => {
+           'instanceA' => {
+	     module => 'collectd_systemd',
+	     config => [{'Service' => 'collectd'}],
+	   },
+           'instanceB' => {
+	     module => 'collectd_systemd',
+	     config => [{'Service' => 'sshd'}],
+	   },
+	},
+      }
+      class{'collectd::plugin::unixsock':
+        socketfile => '/var/run/collectd-sock',
+      }
+      EOS
+
+      # Run it twice and test for idempotency
+      apply_manifest(pp, catch_failures: true)
+      apply_manifest(pp, catch_changes: true)
+      shell('sleep 10')
+    end
+    describe service('collectd') do
+      it { is_expected.to be_running }
+    end
+    # Check metric is really there.
+    describe command('collectdctl -s /var/run/collectd-sock listval') do
+      its(:exit_status) { is_expected.to eq 0 }
+      its(:stdout) { is_expected.to match %r{systemd-sshd} }
+    end
+    describe command('collectdctl -s /var/run/collectd-sock listval') do
+      its(:exit_status) { is_expected.to eq 0 }
+      its(:stdout) { is_expected.to match %r{systemd-collectd} }
+    end
+  end
 end

spec/classes/collectd_plugin_cuda_spec.rb

@@ -10,7 +10,7 @@
       context 'package ensure' do
         context ':ensure => present' do
           it 'import collectd_cuda.collectd_plugin in python-config' do
-            is_expected.to contain_concat_fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin').with_content(%r{Import "collectd_cuda.collectd_plugin"})
+            is_expected.to contain_concat_fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin_header').with_content(%r{Import "collectd_cuda.collectd_plugin"})
           end
         end
       end
@@ -21,7 +21,7 @@
         end
 
         it 'Will remove python-config' do
-          is_expected.not_to contain_concat__fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin').with(ensure: 'present')
+          is_expected.not_to contain_concat__fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin_header').with(ensure: 'present')
         end
       end
 

spec/classes/collectd_plugin_iscdhcp_spec.rb

@@ -10,7 +10,7 @@
       context 'package ensure' do
         context ':ensure => present' do
           it 'import collectd_cuda.collectd_plugin in python-config' do
-            is_expected.to contain_concat_fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin').with_content(%r{Import "collectd_cuda.collectd_plugin"})
+            is_expected.to contain_concat_fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin_header').with_content(%r{Import "collectd_cuda.collectd_plugin"})
           end
         end
       end
@@ -21,7 +21,7 @@
         end
 
         it 'Will remove python-config' do
-          is_expected.not_to contain_concat__fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin').with(ensure: 'present')
+          is_expected.not_to contain_concat__fragment('collectd_plugin_python_conf_collectd_cuda.collectd_plugin_header').with(ensure: 'present')
         end
       end
 

spec/classes/collectd_plugin_python_spec.rb

@@ -93,28 +93,28 @@
           end
 
           it 'imports elasticsearch module' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch_header').with(
               content: %r{Import "elasticsearch"},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
           end
 
           it 'includes elasticsearch module configuration' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch_config').with(
               content: %r{<Module "elasticsearch">},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
           end
 
           it 'includes elasticsearch Cluster name' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch_config').with(
               content: %r{Cluster "ES-clust"},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
           end
 
           it 'includes second elasticsearch Cluster name' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_elasticsearch_config').with(
               content: %r{Cluster "Another-ES-clust"},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
@@ -129,31 +129,90 @@
 
           # test foo module
           it 'imports foo module' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo_header').with(
               content: %r{Import "foo"},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
           end
 
           it 'includes foo module configuration' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo_config').with(
               content: %r{<Module "foo">},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo').with(content: %r{Verbose true})
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo').with(content: %r{Bar "bar"})
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo_config').with(content: %r{Verbose true})
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_foo_config').with(content: %r{Bar ""bar""})
           end
 
           it 'includes alpha module configuration' do
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha_config').with(
               content: %r{<Module "alpha">},
               target: "#{options[:plugin_conf_dir]}/python-config.conf"
             )
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(content: %r{Beta 4.2})
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(content: %r{Delta "a"})
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(content: %r{Delta 4})
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(content: %r{Delta 5})
-            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha').with(content: %r{Gamma "b"})
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha_config').with(content: %r{Beta 4.2})
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha_config').with(content: %r{Delta "a" 4 5})
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_alpha_config').with(content: %r{Gamma "b"})
+          end
+        end
+
+        context 'two modules with same python module should not clash' do
+          let :params do
+            {
+              modules: {
+                'One' => {
+                  'module' => 'funky',
+                  'config' => [{ 'Verbose' => true }, { 'Junk' => 'help' }]
+                },
+                'Two' => {
+                  'module' => 'funky',
+                  'config' => [{ 'Junk' => 'morehelp' }]
+                }
+              }
+            }
+          end
+
+          it 'import funky module' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_funky_header').with(
+              content: %r{Import "funky"},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+          it 'open instance One of module' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_One_config').with(
+              content: %r{<Module "funky">},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+          it 'open instance Two of module' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_Two_config').with(
+              content: %r{<Module "funky">},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+
+          it 'configure  instance One of module' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_One_config').with(
+              content: %r{Verbose true},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+          it 'configure  instance Two of module' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_Two_config').with(
+              content: %r{Junk "morehelp"},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+          it 'close funky module instance One' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_One_config').with(
+              content: %r{</Module>},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
+          end
+          it 'close funky module instance Two' do
+            is_expected.to contain_concat__fragment('collectd_plugin_python_conf_Two_config').with(
+              content: %r{</Module>},
+              target: "#{options[:plugin_conf_dir]}/python-config.conf"
+            )
           end
         end