Finishing touches for multi-instance support

.fixtures.yml

@@ -11,3 +11,6 @@ fixtures:
     yumrepo_core:
       repo: "https://github.com/puppetlabs/puppetlabs-yumrepo_core"
       puppet_version: ">= 6.0.0"
+    systemd:
+      repo: 'https://github.com/camptocamp/puppet-systemd.git'
+      puppet_version: "< 6.1.0"

README.md

@@ -64,6 +64,33 @@ class { '::redis':
 }
 ```
 
+### Multiple instances
+
+
+```puppet
+$listening_ports = [6379,6380,6381,6382]
+
+class { '::redis':
+  default_install => false,
+  service_enable  => false,
+  service_ensure  => 'stopped',
+}
+
+$listening_ports.each |$port| {
+  $port_string = sprintf('%d',$port)
+  redis::instance { $port_string:
+    service_enable => true,
+    service_ensure => 'running',
+    port           => $port,
+    bind           => $facts['networking']['ip'],
+    dbfilename     => "${port}-dump.rdb",
+    appendfilename => "${port}-appendonly.aof",
+    appendfsync    => 'always',
+    require        => Class['Redis'],
+  }
+}
+```
+
 ### Manage repositories
 
 Disabled by default but if you really want the module to manage the required
@@ -104,6 +131,10 @@ class { '::redis::sentinel':
 }
 ```
 
+### Soft dependency
+
+This module requires `camptocamp/systemd` on Puppet versions older than 6.1.0.
+
 ## `redis::get()` function
 
 This function is used to get data from redis.

manifests/init.pp

@@ -353,10 +353,4 @@
     ~> Class['redis::service']
   }
 
-  exec { 'systemd-reload-redis':
-    command     => 'systemctl daemon-reload',
-    refreshonly => true,
-    path        => '/bin:/usr/bin:/usr/local/bin',
-  }
-
 }

manifests/instance.pp

@@ -286,7 +286,6 @@
   Variant[Stdlib::Absolutepath, Enum['']] $unixsocket            = "/var/run/redis/redis-server-${name}.sock",
   Stdlib::Absolutepath $workdir                                  = "${redis::workdir}/redis-server-${name}",
 ) {
-
   if $title == 'default' {
     $redis_file_name_orig = $config_file_orig
     $redis_file_name      = $config_file
@@ -330,7 +329,13 @@
         mode    => '0644',
         content => template('redis/service_templates/redis.service.erb'),
       }
-      ~> Exec['systemd-reload-redis']
+
+      # Only necessary for Puppet < 6.1.0,
+      # See https://github.com/puppetlabs/puppet/commit/f8d5c60ddb130c6429ff12736bfdb4ae669a9fd4
+      if versioncmp($facts['puppetversion'],'6.1.0') < 0 {
+        include systemd::systemctl::daemon_reload
+        File["/etc/systemd/system/${service_name}.service"] ~> Class['systemd::systemctl::daemon_reload']
+      }
 
       if $title != 'default' {
         service { $service_name:

manifests/ulimit.pp

@@ -49,9 +49,12 @@
         "defnode nofile Service/LimitNOFILE \"\"",
         "set \$nofile/value \"${redis::ulimit}\"",
       ],
-      notify  => [
-        Exec['systemd-reload-redis'],
-      ],
+    }
+    # Only necessary for Puppet < 6.1.0,
+    # See https://github.com/puppetlabs/puppet/commit/f8d5c60ddb130c6429ff12736bfdb4ae669a9fd4
+    if versioncmp($facts['puppetversion'],'6.1.0') < 0 {
+      include systemd::systemctl::daemon_reload
+      Augeas['Systemd redis ulimit'] ~> Class['systemd::systemctl::daemon_reload']
     }
   } else {
     case $facts['os']['family'] {

spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb

@@ -1,28 +1,40 @@
 require 'spec_helper_acceptance'
 
-describe 'redis::instance' do
+describe 'redis::instance example' do
+  instances = [6379, 6380, 6381, 6382]
   case fact('osfamily')
   when 'Debian'
     config_path = '/etc/redis'
     redis_name = 'redis-server'
   else
-    redis_name = 'redis'
     config_path = '/etc'
+    redis_name = 'redis'
   end
 
   it 'runs successfully' do
     pp = <<-EOS
-    class { 'redis':
+    $listening_ports = #{instances}
+
+    class { '::redis':
       default_install => false,
+      service_enable  => false,
+      service_ensure  => 'stopped',
     }
 
-    redis::instance {'redis1':
-      port => 7777,
+    $listening_ports.each |$port| {
+      $port_string = sprintf('%d',$port)
+      redis::instance { $port_string:
+        service_enable => true,
+        service_ensure => 'running',
+        port           => $port,
+        bind           => $facts['networking']['ip'],
+        dbfilename     => "${port}-dump.rdb",
+        appendfilename => "${port}-appendonly.aof",
+        appendfsync    => 'always',
+        require        => Class['Redis'],
+      }
     }
 
-    redis::instance {'redis2':
-      port => 8888,
-    }
     EOS
 
     # Apply twice to ensure no errors the second time.
@@ -34,29 +46,32 @@ class { 'redis':
     it { is_expected.to be_installed }
   end
 
-  describe service('redis-server-redis1') do
-    it { is_expected.to be_running }
+  describe service(redis_name) do
+    it { is_expected.not_to be_enabled }
+    it { is_expected.not_to be_running }
   end
 
-  describe service('redis-server-redis2') do
-    it { is_expected.to be_running }
-  end
+  instances.each do |instance|
+    describe file("/etc/systemd/system/redis-server-#{instance}.service"), if: (fact('service_provider') == 'systemd') do
+      its(:content) { is_expected.to match %r{redis-server-#{instance}.conf} }
+    end
 
-  describe file("#{config_path}/redis-server-redis1.conf") do
-    its(:content) { is_expected.to match %r{port 7777} }
-  end
+    describe service("redis-server-#{instance}") do
+      it { is_expected.to be_enabled }
+    end
 
-  describe file("#{config_path}/redis-server-redis2.conf") do
-    its(:content) { is_expected.to match %r{port 8888} }
-  end
+    describe service("redis-server-#{instance}") do
+      it { is_expected.to be_running }
+    end
 
-  context 'redis should respond to ping command' do
-    describe command('redis-cli -h 127.0.0.1 -p 7777 ping') do
-      its(:stdout) { is_expected.to match %r{PONG} }
+    describe file("#{config_path}/redis-server-#{instance}.conf") do
+      its(:content) { is_expected.to match %r{port #{instance}} }
     end
 
-    describe command('redis-cli -h 127.0.0.1 -p 8888 ping') do
-      its(:stdout) { is_expected.to match %r{PONG} }
+    context "redis instance #{instance} should respond to ping command" do
+      describe command("redis-cli -h #{fact('networking.ip')} -p #{instance} ping") do
+        its(:stdout) { is_expected.to match %r{PONG} }
+      end
     end
   end
 end

spec/classes/redis_spec.rb

@@ -4,12 +4,21 @@
   let(:service_file) { redis_service_file(service_provider: facts['service_provider']) }
   let(:package_name) { manifest_vars[:package_name] }
   let(:service_name) { manifest_vars[:service_name] }
+  let(:config_file) { manifest_vars[:config_file] }
   let(:config_file_orig) { manifest_vars[:config_file_orig] }
 
   on_supported_os.each do |os, facts|
     context "on #{os}" do
       let(:facts) { facts }
 
+      if facts[:operatingsystem] == 'Ubuntu' && facts[:operatingsystemmajrelease] == '16.04'
+        let(:systemd) { '' }
+        let(:servicetype) { 'forking' }
+      else
+        let(:systemd) { ' --supervised systemd' }
+        let(:servicetype) { 'notify' }
+      end
+
       describe 'without parameters' do
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to create_class('redis') }
@@ -108,11 +117,20 @@ class { 'redis::globals':
               with_owner('root').
               with_group('root').
               with_mode('0444')
-            is_expected.to contain_augeas('Systemd redis ulimit').
-              with_incl("/etc/systemd/system/#{service_name}.service.d/limit.conf").
-              with_lens('Systemd.lns').
-              with_changes(['defnode nofile Service/LimitNOFILE ""', 'set $nofile/value "7777"']).
-              that_notifies('Exec[systemd-reload-redis]')
+            # Only necessary for Puppet < 6.1.0,
+            # See https://github.com/puppetlabs/puppet/commit/f8d5c60ddb130c6429ff12736bfdb4ae669a9fd4
+            if Puppet.version < '6.1'
+              is_expected.to contain_augeas('Systemd redis ulimit').
+                with_incl("/etc/systemd/system/#{service_name}.service.d/limit.conf").
+                with_lens('Systemd.lns').
+                with_changes(['defnode nofile Service/LimitNOFILE ""', 'set $nofile/value "7777"']).
+                that_notifies('Class[systemd::systemctl::daemon_reload]')
+            else
+              is_expected.to contain_augeas('Systemd redis ulimit').
+                with_incl("/etc/systemd/system/#{service_name}.service.d/limit.conf").
+                with_lens('Systemd.lns').
+                with_changes(['defnode nofile Service/LimitNOFILE ""', 'set $nofile/value "7777"'])
+            end
           else
             is_expected.not_to contain_file('/etc/systemd/system/redis-server.service.d/limit.conf')
             is_expected.not_to contain_augeas('Systemd redis ulimit')
@@ -1329,6 +1347,36 @@ class { 'redis::globals':
         end
 
         it { is_expected.to contain_file(service_file) }
+
+        it do
+          content = <<-END.gsub(%r{^\s+\|}, '')
+            |[Unit]
+            |Description=Redis Advanced key-value store for instance default
+            |After=network.target
+            |After=network-online.target
+            |Wants=network-online.target
+            |
+            |[Service]
+            |RuntimeDirectory=redis
+            |RuntimeDirectoryMode=2755
+            |Type=#{servicetype}
+            |ExecStart=/usr/bin/redis-server #{config_file}#{systemd}
+            |ExecStop=/usr/bin/redis-server -p 6379 shutdown
+            |Restart=always
+            |User=redis
+            |Group=redis
+            |LimitNOFILE=65536
+            |
+            |[Install]
+            |WantedBy=multi-user.target
+          END
+
+          if facts['service_provider'] == 'systemd'
+            is_expected.to contain_file(service_file).with_content(content)
+          else
+            is_expected.to contain_file(service_file).with_content(%r{Required-Start:})
+          end
+        end
       end
 
       describe 'with parameter manage_service_file' do

spec/spec_helper_acceptance.rb

@@ -5,6 +5,7 @@
 run_puppet_install_helper unless ENV['BEAKER_provision'] == 'no'
 install_module_on(hosts)
 install_module_dependencies_on(hosts)
+install_module_from_forge('camptocamp/systemd', '>= 2.0.0 < 3.0.0')
 
 RSpec.configure do |c|
   # Readable test descriptions

templates/service_templates/redis.service.erb

@@ -1,17 +1,25 @@
 [Unit]
-Description=Advanced key-value store for <%= @title %>
+Description=Redis Advanced key-value store for instance <%= @title %>
 After=network.target
+After=network-online.target
+Wants=network-online.target
 
 [Service]
 RuntimeDirectory=redis
 RuntimeDirectoryMode=2755
+<%# Redis on Xenial is too old for systemd integration -%>
+<% if @facts['os']['name'] == 'Ubuntu' and @facts['os']['release']['major'] == '16.04' -%>
 Type=forking
 ExecStart=/usr/bin/redis-server <%= @redis_file_name %>
+<% else -%>
+Type=notify
+ExecStart=/usr/bin/redis-server <%= @redis_file_name %> --supervised systemd
+<% end -%>
 ExecStop=/usr/bin/redis-server -p <%= @port %> shutdown
-TimeoutStopSec=0
 Restart=always
 User=<%= @service_user %>
 Group=<%= @service_user %>
+LimitNOFILE=<%= @ulimit %>
 
 [Install]
 WantedBy=multi-user.target