Giuseppe Lavagetto has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/371076 )

Change subject: base::service_unit: deprecate autolookup of templates
......................................................................

base::service_unit: deprecate autolookup of templates

With the newer puppet parser, scope lookup in templates is pinned to the
local resources. Instead of going around adding ugly scope lookups to
all templates, allow moving their rendering to the calling classes,
which is also more correct.

Bug: T171704
Change-Id: I2b30c38a6f6d0be0a9798853c1049c49c8d29d14
---
M modules/base/manifests/service_unit.pp
M modules/base/spec/defines/service_unit_spec.rb
2 files changed, 32 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/puppet 
refs/changes/76/371076/1

diff --git a/modules/base/manifests/service_unit.pp 
b/modules/base/manifests/service_unit.pp
index 602dc77..e2619de 100644
--- a/modules/base/manifests/service_unit.pp
+++ b/modules/base/manifests/service_unit.pp
@@ -16,22 +16,19 @@
 #  ensure => running and conversely 'absent' will ensure => stopped.
 #
 # [*template_name*]
-#  String, default $name.  Init file template pathnames are formed
+#  String, default $name. Now deprecated. Init file template pathnames are 
formed
 #  using the pattern "$module/initscripts/$template_name.$initsystem.erb"
 #
 # [*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/$template_name.systemd.erb for systemd  (and
-#  similarly for other init systems)
+#  String or false. If it is a string, the content will be used as the content
+#  of the custom systemd service file. If you set it to true (now deprecated),
+#  the resource will fetch the templates from a default location. Same thing 
applies
+#  to all the cases below.
 #
 # [*systemd_override*]
-#  Boolean - if enabled, make the resource use a systemd unit provided
+#  String or false. If enabled, make the resource use a systemd unit provided
 #  by a Debian package, while applying an override file with site-
 #  specific changes.
-#  You are expected to place the override file in specific subdirectory of
-#  the current module: $module/initscripts/$template_name.systemd_override.erb
 #
 # [*upstart*]
 #  As the preceding param, but for upstart scripts
@@ -85,6 +82,17 @@
 ) {
 
     validate_ensure($ensure)
+    # Issue deprecation warnings
+    if $template_name != $name {
+        warning('Setting base::service_unit::template_name is deprecated. Pass 
the content directly to the define. See 
https://wikitech.wikimedia.org/wiki/User:Giuseppe_Lavagetto/PuppetFutureParser#Variable_scope_is_now_respected_in_templates')
+    }
+
+    $custom_inits = {
+        'systemd'          => $systemd,
+        'systemd_override' => $systemd_override,
+        'upstart'          => $upstart,
+        'sysvinit'         => $sysvinit
+    }
 
     # Validates the service name, and picks the valid init script
     $initscript = pick_initscript(
@@ -92,10 +100,18 @@
 
     # we assume init scripts are templated
     if $initscript {
-        if $caller_module_name {
-            $template = 
"${caller_module_name}/initscripts/${template_name}.${initscript}.erb"
-        } else {
-            $template = "initscripts/${template_name}.${initscript}.erb"
+        $custom_init =$custom_inits[$initscript]
+        if $custom_init == true {
+            warning('Please pass the content of the template to 
base::service_unit directly - See 
https://wikitech.wikimedia.org/wiki/User:Giuseppe_Lavagetto/PuppetFutureParser#Variable_scope_is_now_respected_in_templates')
+            if $caller_module_name {
+                $template = 
"${caller_module_name}/initscripts/${template_name}.${initscript}.erb"
+            } else {
+                $template = "initscripts/${template_name}.${initscript}.erb"
+            }
+            $content = template($template)
+        }
+        else {
+            $content = $custom_init
         }
 
         $path = $initscript ? {
@@ -124,7 +140,7 @@
 
         file { $path:
             ensure  => $ensure,
-            content => template($template),
+            content => $content,
             mode    => $i_mode,
             owner   => 'root',
             group   => 'root',
diff --git a/modules/base/spec/defines/service_unit_spec.rb 
b/modules/base/spec/defines/service_unit_spec.rb
index 5d89853..3171996 100644
--- a/modules/base/spec/defines/service_unit_spec.rb
+++ b/modules/base/spec/defines/service_unit_spec.rb
@@ -6,7 +6,7 @@
   context 'with systemd as init' do
     let(:facts) { {:initsystem => 'systemd'} }
     context 'with a systemd unit file' do
-      let(:params) { { :ensure => 'present', :systemd => true}}
+      let(:params) { { :ensure => 'present', :systemd => 'test'}}
 
       it 'should activate the service' do
         should contain_service('nginx')
@@ -14,6 +14,7 @@
 
       it 'should install a service file' do
         should contain_file('/lib/systemd/system/nginx.service')
+                 .with_content('test')
       end
 
       it 'should execute daemon-reload' do

-- 
To view, visit https://gerrit.wikimedia.org/r/371076
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b30c38a6f6d0be0a9798853c1049c49c8d29d14
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to