From d779b75a937dd846323481e72eb604f5829e4c0e Mon Sep 17 00:00:00 2001 From: Frank Wall Date: Sun, 19 Jan 2020 22:12:34 +0100 Subject: [PATCH] add finishing touches to multi-instance support --- .fixtures.yml | 3 + README.md | 31 +++++++++ manifests/init.pp | 6 -- manifests/instance.pp | 9 ++- manifests/ulimit.pp | 9 ++- .../redis_multi_instances_one_host_spec.rb | 63 ++++++++++++------- spec/classes/redis_spec.rb | 58 +++++++++++++++-- spec/spec_helper_acceptance.rb | 1 + templates/service_templates/redis.service.erb | 12 +++- 9 files changed, 150 insertions(+), 42 deletions(-) diff --git a/.fixtures.yml b/.fixtures.yml index 52ee5db1..526b960b 100644 --- a/.fixtures.yml +++ b/.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" diff --git a/README.md b/README.md index bd9441c3..77fecab4 100644 --- a/README.md +++ b/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. diff --git a/manifests/init.pp b/manifests/init.pp index 8d455386..0a0f5257 100644 --- a/manifests/init.pp +++ b/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', - } - } diff --git a/manifests/instance.pp b/manifests/instance.pp index 6b694247..01ad5231 100644 --- a/manifests/instance.pp +++ b/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: diff --git a/manifests/ulimit.pp b/manifests/ulimit.pp index dc48da94..f7fa88fa 100644 --- a/manifests/ulimit.pp +++ b/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'] { diff --git a/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb b/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb index f727621b..9d01b461 100644 --- a/spec/acceptance/suites/default/redis_multi_instances_one_host_spec.rb +++ b/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 diff --git a/spec/classes/redis_spec.rb b/spec/classes/redis_spec.rb index eb7e0f77..0bec5324 100644 --- a/spec/classes/redis_spec.rb +++ b/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 diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index fcc7d0a9..8ef29a3e 100644 --- a/spec/spec_helper_acceptance.rb +++ b/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 diff --git a/templates/service_templates/redis.service.erb b/templates/service_templates/redis.service.erb index 1567e21c..63b71c42 100644 --- a/templates/service_templates/redis.service.erb +++ b/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