Giuseppe Lavagetto has submitted this change and it was merged. Change subject: base: add the service_unit init wrapper ......................................................................
base: add the service_unit init wrapper The base::service_unit resource aims at being a init-system-agnostic way to manage a service in puppet, including its own init scripts if they are created by us. It should also take care of reloading systemd if the service file has changed. As suggested, it allows you to choose wether to notify config changes to the daemon or not. This is a (partial? good enough?) solution for T89086 Change-Id: I5d1472f7c737aa7a917509394ec997b8e25f6d2b Signed-off-by: Giuseppe Lavagetto <[email protected]> --- A modules/base/.rspec A modules/base/Rakefile A modules/base/lib/puppet/parser/functions/pick_initscript.rb A modules/base/manifests/service_unit.pp A modules/base/spec/defines/service_unit_spec.rb A modules/base/spec/fixtures/manifests/site.pp A modules/base/spec/fixtures/templates/initscripts/nginx.systemd.erb A modules/base/spec/fixtures/templates/initscripts/nginx.sysvinit.erb A modules/base/spec/fixtures/templates/initscripts/nginx.upstart.erb A modules/base/spec/functions/pick_initscript_spec.rb A modules/base/spec/spec_helper.rb 11 files changed, 314 insertions(+), 0 deletions(-) Approvals: Giuseppe Lavagetto: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/base/.rspec b/modules/base/.rspec new file mode 100644 index 0000000..f449dae --- /dev/null +++ b/modules/base/.rspec @@ -0,0 +1,2 @@ +--format doc +--color diff --git a/modules/base/Rakefile b/modules/base/Rakefile new file mode 100644 index 0000000..165b532 --- /dev/null +++ b/modules/base/Rakefile @@ -0,0 +1,47 @@ +require 'rake' +require 'fileutils' + +require 'rspec/core/rake_task' + +# Note: The nginx puppet module is a git submodule right now, so don't forget to +# checkout submodules before testing +extra_modules = [ 'wmflib' ] + +modulename = File.basename(File.expand_path(File.dirname(__FILE__))) + +symlinks = { 'spec/fixtures/modules/%s/files' % modulename => '../../../../files', + 'spec/fixtures/modules/%s/manifests' % modulename => '../../../../manifests', + 'spec/fixtures/modules/%s/templates' % modulename => '../../../../templates', +} + + +task :setup do + FileUtils.mkdir_p('spec/fixtures/modules/%s' % modulename) + extra_modules.each do |x| + FileUtils.ln_s('../../../../%s' % x, 'spec/fixtures/modules/%s' % x) + end + symlinks.each do |x| + if !File.exist?(x[0]) + FileUtils.ln_s(x[1], x[0]) + end + end +end + +task :teardown do + symlinks.each { |x| FileUtils.rm(x[0], :force => true) } + FileUtils.rmdir('spec/fixtures/modules/%s' % modulename) + extra_modules.each do |x| + FileUtils.rm('spec/fixtures/modules/%s' % x, :force => true) + end + FileUtils.rmdir('spec/fixtures/modules') +end + +RSpec::Core::RakeTask.new(:realspec) do |t| + t.fail_on_error = false + t.pattern = 'spec/*/*_spec.rb' +end + +task :spec => [ :setup, :realspec, :teardown] + +task :default => :spec do +end diff --git a/modules/base/lib/puppet/parser/functions/pick_initscript.rb b/modules/base/lib/puppet/parser/functions/pick_initscript.rb new file mode 100644 index 0000000..8a97c2c --- /dev/null +++ b/modules/base/lib/puppet/parser/functions/pick_initscript.rb @@ -0,0 +1,33 @@ +# Choose which init script to install, based on the init system. + +Puppet::Parser::Functions.newfunction(:pick_initscript, + :type => :rvalue, + :arity => 4, + :doc => <<-'HEREDOC' +Takes as an input the init system currently installed, the +available init scripts, and returns the chosen one. +HEREDOC +) do |vals| + init_system, has_systemd, has_upstart, has_sysvinit = vals + has_custom = has_systemd || has_upstart || has_sysvinit + # if we don't have custom scripts, we use the system defaults + return false unless has_custom + case init_system + when 'systemd' + return 'systemd' if has_systemd + return 'sysvinit' if has_sysvinit + raise(ArgumentError, + 'This service unit has an upstart script but nothing useful for systemd') + when 'upstart' + return 'upstart' if has_upstart + return 'sysvinit' if has_sysvinit + raise(ArgumentError, + 'This service unit has a systemd script but nothing useful for upstart') + when 'sysvinit' + return 'sysvinit' if has_sysvinit + raise(ArgumentError, + 'This service unit lacks a custom sysvinit script') + else + raise(ArgumentError, 'Unsupported init system') + end +end diff --git a/modules/base/manifests/service_unit.pp b/modules/base/manifests/service_unit.pp new file mode 100644 index 0000000..360307d --- /dev/null +++ b/modules/base/manifests/service_unit.pp @@ -0,0 +1,105 @@ +# == Base::service_unit == +# +# Allows defining services and their corresponding init scripts in a +# init-system agnostic way on Debian derivatives. +# +# We prefer convention over configuration, so this define will require +# you to respect those in order to work. +# +# === Parameters === +# +#[*ensure*] +# Is the usual metaparameter, defaults to present +# +#[*systemd*] +# Boolean - set it to true to make the resource include personalized +# init file. As this is used to You are expected to put them in a +# specific subdirectory of the current module, which is +# $module/initscripts/$name.systemd.erb for systemd (and similarly for +# other init systems) +# +#[*upstart*] +# As the preceding param, but for upstart scripts +# +#[*sysvinit*] +# As the preceding param, but for traditional sysvinit scripts +# +#[*refresh*] +# Boolean - tells puppet if a change in the config should notify the service directly +# +#[*service_params*] +# An hash of parameters that we want to apply to the service resource +# +# === Example === +# +# A init-agnostic class that runs apache, with its own init scripts +# (please, don't do it at home!) +# +# class foo { +# base::service_unit { 'apache2': +# ensure => present, +# sysvinit => true, +# service_params => { +# hasrestart => true, +# restart => '/usr/sbin/service apache2 restart' +# } +# } +# } +define base::service_unit ( + $ensure = present, + $systemd = false, + $upstart = false, + $sysvinit = false, + $refresh = true, + $service_params = {}, + ) { + + validate_ensure($ensure) + # Validates the service name, and picks the valid init script + $initscript = pick_initscript( + $::initsystem, $systemd, $upstart, $sysvinit) + # we assume init scripts are templated + if $initscript { + if $caller_module_name { + $template = "${caller_module_name}/initscripts/${name}.${initscript}.erb" + } + else { + $template = "initscripts/${name}.${initscript}.erb" + } + $path = $initscript ? { + 'systemd' => "/etc/systemd/system/${name}.service", + 'upstart' => "/etc/init/${name}.conf", + default => "/etc/init.d/${name}" + } + + file {$path: + ensure => $ensure, + content => template($template), + mode => '0544', + owner => root, + group => root, + } + + if $refresh { + File[$path] ~> Service[$name] + } else { + File[$path] -> Service[$name] + } + + if $::initsystem == 'systemd' { + exec { "systemd reload for ${name}": + refreshonly => true, + command => '/bin/systemctl daemon-reload', + subscribe => File[$path], + before => Service[$name] + } + + } + } + + $base_params = { + ensure => ensure_service($ensure), + provider => $::initsystem } + $params = merge($base_params, $service_params) + ensure_resource('service',$name, $params) +} diff --git a/modules/base/spec/defines/service_unit_spec.rb b/modules/base/spec/defines/service_unit_spec.rb new file mode 100644 index 0000000..6be090c --- /dev/null +++ b/modules/base/spec/defines/service_unit_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe 'base::service_unit' do + let(:title) { 'nginx' } + + context 'with systemd as init' do + let(:facts) { {:initsystem => 'systemd'} } + context 'with a systemd unit file' do + let(:params) { { :ensure => 'present', :systemd => true}} + + it 'should activate the service' do + should contain_service('nginx') + end + + it 'should install a service file' do + should contain_file('/etc/systemd/system/nginx.service') + end + + it 'should execute daemon-reload' do + should contain_file('/etc/systemd/system/nginx.service').that_notifies('Exec[systemd reload for nginx]').that_notifies('Service[nginx]') + should contain_exec('systemd reload for nginx').that_comes_before('Service[nginx]') + end + end + + context 'without a unit file' do + let(:params) { { :ensure => 'present', :sysvinit => true}} + it 'should install an init script' do + should contain_file('/etc/init.d/nginx') + end + + it 'should execute daemon-reload' do + should contain_file('/etc/init.d/nginx').that_notifies('Exec[systemd reload for nginx]').that_notifies('Service[nginx]') + should contain_exec('systemd reload for nginx').that_comes_before('Service[nginx]') + end + + end + end + context 'with refresh false' do + let(:facts) { {:initsystem => 'sysvinit'} } + let(:params) { { :ensure => 'present', :sysvinit => true, :refresh => false}} + + it 'should not refresh service' do + expect { + should contain_file('/etc/init.d/nginx').that_notifies('Service[nginx]') + }.to raise_error() + end + end + + context 'with refresh true' do + let(:facts) { {:initsystem => 'sysvinit'} } + let(:params) { { :ensure => 'present', :sysvinit => true, :refresh => true}} + + it 'should refresh service' do + should contain_file('/etc/init.d/nginx').that_notifies('Service[nginx]') + end + + end + context 'with upstart as init' do + let(:facts) { {:initsystem => 'upstart'} } + context 'with an upstart conf file' do + let(:params) { { :ensure => 'present', :upstart => true}} + it 'should activate the service' do + should contain_service('nginx') + end + + it 'should install a service file' do + should contain_file('/etc/init/nginx.conf') + end + + it 'should notify service' do + should contain_file('/etc/init/nginx.conf').that_notifies('Service[nginx]') + end + end + + context 'without a unit file' do + let(:params) { { :ensure => 'present', :sysvinit => true}} + it 'should install an init script' do + should contain_file('/etc/init.d/nginx') + end + + it 'should notify service' do + should contain_file('/etc/init.d/nginx').that_notifies('Service[nginx]') + end + end + + #MARK + end +end diff --git a/modules/base/spec/fixtures/manifests/site.pp b/modules/base/spec/fixtures/manifests/site.pp new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/modules/base/spec/fixtures/manifests/site.pp diff --git a/modules/base/spec/fixtures/templates/initscripts/nginx.systemd.erb b/modules/base/spec/fixtures/templates/initscripts/nginx.systemd.erb new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/modules/base/spec/fixtures/templates/initscripts/nginx.systemd.erb diff --git a/modules/base/spec/fixtures/templates/initscripts/nginx.sysvinit.erb b/modules/base/spec/fixtures/templates/initscripts/nginx.sysvinit.erb new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/modules/base/spec/fixtures/templates/initscripts/nginx.sysvinit.erb diff --git a/modules/base/spec/fixtures/templates/initscripts/nginx.upstart.erb b/modules/base/spec/fixtures/templates/initscripts/nginx.upstart.erb new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/modules/base/spec/fixtures/templates/initscripts/nginx.upstart.erb diff --git a/modules/base/spec/functions/pick_initscript_spec.rb b/modules/base/spec/functions/pick_initscript_spec.rb new file mode 100755 index 0000000..80745f2 --- /dev/null +++ b/modules/base/spec/functions/pick_initscript_spec.rb @@ -0,0 +1,29 @@ +#! /usr/bin/env ruby -S rspec +require 'spec_helper' + +describe 'pick_initscript' do + before :each do + @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foo")) + @scope = Puppet::Parser::Scope.new(@compiler) + end + + it 'Returns false if no init script provided' do + should run.with_params('systemd', false, false, false).and_return(false) + end + + it 'Returns systemd if provided, sysvinit otherwise' do + should run.with_params('systemd', true, true, true).and_return('systemd') + should run.with_params('systemd', false, true, true).and_return('sysvinit') + end + + it 'Returns upstart if provided, sysvinit otherwise' do + should run.with_params('upstart', true, true, true).and_return('upstart') + should run.with_params('upstart', true, false, true).and_return('sysvinit') + end + + it 'Fails if on systemd and only upstart is provided, and vice versa' do + should run.with_params('upstart', true, false, false).and_raise_error(ArgumentError) + should run.with_params('systemd', false, true, false).and_raise_error(ArgumentError) + end + +end diff --git a/modules/base/spec/spec_helper.rb b/modules/base/spec/spec_helper.rb new file mode 100644 index 0000000..04f9311 --- /dev/null +++ b/modules/base/spec/spec_helper.rb @@ -0,0 +1,10 @@ +require 'rspec-puppet' + +fixture_path = File.expand_path(File.join(__FILE__, '..', 'fixtures')) + + +RSpec.configure do |c| + c.module_path = File.expand_path("../..", File.dirname(__FILE__)) + c.manifest_dir = File.join(fixture_path, 'manifests') + c.template_dir = File.join(fixture_path, 'templates') +end -- To view, visit https://gerrit.wikimedia.org/r/189753 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5d1472f7c737aa7a917509394ec997b8e25f6d2b Gerrit-PatchSet: 6 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Giuseppe Lavagetto <[email protected]> Gerrit-Reviewer: BBlack <[email protected]> Gerrit-Reviewer: Faidon Liambotis <[email protected]> Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
