From 16a52af548343e87e2bc7753ae3dd3051380d260 Mon Sep 17 00:00:00 2001 From: Callum Banbery Date: Wed, 24 Apr 2019 14:53:45 +0100 Subject: [PATCH 1/2] Fix manage_service_file variable The manage_service_file is configured incorrectly and doesn't allow for overriding the value with hieradata. Updaing the init.pp class params allows the file to be passed in. --- manifests/init.pp | 1 + 1 file changed, 1 insertion(+) diff --git a/manifests/init.pp b/manifests/init.pp index b2ebf72a..762d6b93 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -180,6 +180,7 @@ Stdlib::Filemode $log_dir_mode = $::redis::params::log_dir_mode, Stdlib::Absolutepath $log_file = $::redis::params::log_file, $log_level = $::redis::params::log_level, + Boolean $manage_service_file = $::redis::params::manage_service_file, Boolean $manage_package = $::redis::params::manage_package, Boolean $manage_repo = $::redis::params::manage_repo, $masterauth = $::redis::params::masterauth, From eed9525e78cd1d0178dc1644bfceebe752bebab0 Mon Sep 17 00:00:00 2001 From: Callum Banbery Date: Wed, 24 Apr 2019 15:24:16 +0100 Subject: [PATCH 2/2] Fix manage_service_file variable Fixes #308 Fixes #310 The manage_service_file is configured incorrectly and doesn't allow for overriding the value with hieradata. Updaing the init.pp class params allows the file to be passed in. The filename for the systemd default service was incorrectly set as the variable is undefined. --- manifests/instance.pp | 1 + spec/classes/redis_spec.rb | 34 +++++++++++++++++++++++++++++++++- spec/defines/instance_spec.rb | 26 ++++++++++++++++++++++++++ spec/spec_helper.rb | 22 ++++++++++++++++------ 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/manifests/instance.pp b/manifests/instance.pp index d9c5a82e..31455039 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -217,6 +217,7 @@ ) { if $title == 'default' { + $redis_server_name = $::redis::service_name $redis_file_name_orig = $config_file_orig $redis_file_name = $config_file } else { diff --git a/spec/classes/redis_spec.rb b/spec/classes/redis_spec.rb index 0850d536..e6c318fe 100644 --- a/spec/classes/redis_spec.rb +++ b/spec/classes/redis_spec.rb @@ -1,10 +1,18 @@ require 'spec_helper' describe 'redis', type: :class do + let(:service_file) { redis_service_file(service_provider: facts[:service_provider]) } + on_supported_os.each do |os, facts| context "on #{os}" do let(:facts) do - facts.merge(redis_server_version: '3.2.3') + merged_facts = facts.merge(redis_server_version: '3.2.3') + + if facts[:operatingsystem].casecmp('archlinux') == 0 + merged_facts = merged_facts.merge(service_provider: 'systemd') + end + + merged_facts end let(:package_name) { manifest_vars[:package_name] } @@ -1191,6 +1199,30 @@ ) } end + + describe 'with parameter manage_service_file' do + let(:params) do + { + manage_service_file: true + } + end + + it { + is_expected.to contain_file(service_file) + } + end + + describe 'with parameter manage_service_file' do + let(:params) do + { + manage_service_file: false + } + end + + it { + is_expected.not_to contain_file(service_file) + } + end end end end diff --git a/spec/defines/instance_spec.rb b/spec/defines/instance_spec.rb index 43354a9d..59d77c6e 100644 --- a/spec/defines/instance_spec.rb +++ b/spec/defines/instance_spec.rb @@ -9,6 +9,8 @@ let :title do 'app2' end + let(:service_name) { redis_service_name(service_name: title) } + let(:service_file) { redis_service_file(service_name: service_name, service_provider: facts[:service_provider]) } describe 'os-dependent items' do context 'on Ubuntu systems' do @@ -26,6 +28,12 @@ it { is_expected.to contain_service('redis-server-app2').with_enable('true') } it { is_expected.to contain_file('/etc/init.d/redis-server-app2').with_content(%r{DAEMON_ARGS=/etc/redis/redis-server-app2\.conf}) } it { is_expected.to contain_file('/etc/init.d/redis-server-app2').with_content(%r{PIDFILE=/var/run/redis/redis-server-app2\.pid}) } + + context 'with default title' do + let(:title) { 'default' } + + it { is_expected.to contain_file(service_file).with_content(%r{DAEMON_ARGS=/etc/redis/redis.conf}) } + end end context '16.04' do let(:facts) do @@ -40,6 +48,12 @@ it { is_expected.to contain_service('redis-server-app2').with_ensure('running') } it { is_expected.to contain_service('redis-server-app2').with_enable('true') } it { is_expected.to contain_file('/etc/systemd/system/redis-server-app2.service').with_content(%r{ExecStart=/usr/bin/redis-server /etc/redis/redis-server-app2\.conf}) } + + context 'with default title' do + let(:title) { 'default' } + + it { is_expected.to contain_file(service_file).with_content(%r{ExecStart=/usr/bin/redis-server /etc/redis/redis.conf}) } + end end end context 'on CentOS systems' do @@ -57,6 +71,12 @@ it { is_expected.to contain_service('redis-server-app2').with_enable('true') } it { is_expected.to contain_file('/etc/init.d/redis-server-app2').with_content(%r{REDIS_CONFIG="/etc/redis-server-app2\.conf"}) } it { is_expected.to contain_file('/etc/init.d/redis-server-app2').with_content(%r{pidfile="/var/run/redis/redis-server-app2\.pid"}) } + + context 'with default title' do + let(:title) { 'default' } + + it { is_expected.to contain_file(service_file).with_content(%r{REDIS_CONFIG="/etc/redis.conf"}) } + end end context '7' do let(:facts) do @@ -71,6 +91,12 @@ it { is_expected.to contain_service('redis-server-app2').with_ensure('running') } it { is_expected.to contain_service('redis-server-app2').with_enable('true') } it { is_expected.to contain_file('/etc/systemd/system/redis-server-app2.service').with_content(%r{ExecStart=/usr/bin/redis-server /etc/redis-server-app2\.conf}) } + + context 'with default title' do + let(:title) { 'default' } + + it { is_expected.to contain_file(service_file).with_content(%r{ExecStart=/usr/bin/redis-server /etc/redis.conf}) } + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8a211b2e..85a7113f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -76,12 +76,22 @@ def manifest_vars vars end -def centos_facts - { - operatingsystem: 'CentOS', - osfamily: 'RedHat', - puppetversion: '4.5.2' - } +def redis_service_name(service_name: 'default') + case service_name.to_s + when 'default' + manifest_vars[:service_name] + else + "#{manifest_vars[:service_name]}-#{service_name}" + end +end + +def redis_service_file(service_name: redis_service_name, service_provider: nil) + case service_provider.to_s + when 'systemd' + "/etc/systemd/system/#{service_name}.service" + else + "/etc/init.d/#{service_name}" + end end def debian_facts