Merge pull request #361 from caseywilliams/MODULES-8443/puppet6-foss-…

…mco-restarts

(MODULES-8443) Don't restart mco for FOSS puppet6 upgrades

files/solaris_start_puppet.sh

@@ -1,14 +1,13 @@
 #!/bin/bash
 
 puppet_pid=$1
+shift
+service_names=$*
+
 while $(kill -0 ${puppet_pid:?}); do
   sleep 5
 done
 
-function start_service() {
-  service="${1:?}"
-  /opt/puppetlabs/bin/puppet resource service "${service:?}" ensure=running enable=true
-}
-
-start_service puppet
-start_service mcollective
+for service_name in $service_names; do
+  /opt/puppetlabs/bin/puppet resource service "${service_name:?}" ensure=running enable=true
+done

manifests/init.pp

@@ -172,14 +172,16 @@
     contain '::puppet_agent::prepare'
     contain '::puppet_agent::install'
 
-    # On windows, our MSI handles the services
-    # On PE AIO nodes, PE Agent nodegroup is managing the services
+    # Service management:
+    # - Under Puppet Enterprise, the agent nodegroup is managed by PE, and we don't need to manage services here.
+    # - On Windows, services are handled by the puppet-agent MSI packages themselves.
+    # ...but outside of PE, on other platforms, we must make sure the services are restarted. We do that with the
+    # ::puppet_agent::service class. Make sure it's applied after the install process finishes if needed:
     if $::osfamily != 'windows' and (!$is_pe or versioncmp($::clientversion, '4.0.0') < 0) {
       class { '::puppet_agent::service':
         require => Class['::puppet_agent::install'],
       }
       contain '::puppet_agent::service'
     }
-
   }
 }

manifests/install.pp

@@ -40,6 +40,7 @@
     # The following are expected to be available in the solaris_install.sh.erb template:
     $adminfile = '/opt/puppetlabs/packages/solaris-noask'
     $sourcefile = "/opt/puppetlabs/packages/${_unzipped_package_name}"
+    $service_names = $puppet_agent::service_names
 
     # Puppet prior to 5.0 would not use a separate process contract when forking from the Puppet
     # service. That resulted in service-initiated upgrades failing because trying to remove or

manifests/params.pp

@@ -8,6 +8,18 @@
     fail('The puppet_agent class requires stringify_facts to be disabled')
   }
 
+  # Which services should be started after the upgrade process?
+  if ($::osfamily == 'Solaris' and $::operatingsystemmajrelease == '11') {
+    # Solaris 11 is a special case; it uses a custom script.
+    $service_names = []
+  } elsif (versioncmp("${::clientversion}", '5.99.0') < 0) { # (5.99.z indicates pre-release puppet6)
+    # Earlier versions Puppet (4 and 5) rely on mcollective:
+    $service_names = ['puppet', 'mcollective']
+  } else {
+    # Puppet 6+ only needs to manage the puppet service:
+    $service_names = ['puppet']
+  }
+
   # The `is_pe` fact currently works by echoing out the puppet version
   # and greping for "puppet enterprise". With Puppet 4 and PE 2015.2, there
   # is no longer a "PE Puppet", and so that fact will no longer work.
@@ -32,13 +44,6 @@
 
   case $::osfamily {
     'RedHat', 'Debian', 'Suse', 'Solaris', 'Darwin', 'AIX': {
-      if !($::osfamily == 'Solaris' and $::operatingsystemmajrelease == '11') {
-        $service_names = ['puppet', 'mcollective']
-      }
-      else {
-        $service_names = []
-      }
-
       $local_puppet_dir = '/opt/puppetlabs'
       $local_packages_dir = "${local_puppet_dir}/packages"
 
@@ -59,8 +64,6 @@
       $group = 0
     }
     'windows' : {
-      $service_names = ['puppet', 'mcollective']
-
       $local_puppet_dir = windows_native_path("${::puppet_agent_appdata}/Puppetlabs")
       $local_packages_dir = windows_native_path("${local_puppet_dir}/packages")
 

manifests/service.pp

@@ -1,7 +1,7 @@
 # == Class puppet_agent::service
 #
 # This class is meant to be called from puppet_agent.
-# It ensures services is running.
+# It ensures that managed services are running.
 #
 class puppet_agent::service {
   assert_private()
@@ -12,21 +12,22 @@
       ((versioncmp("${::clientversion}", '4.0.0') < 0) or $puppet_agent::aio_upgrade_required) {
     # Only use script if we just performed an upgrade.
     $_logfile = "${::env_temp_variable}/solaris_start_puppet.log"
+    # We'll need to pass the names of the services to start to the script
+    $_service_names_arg = join($::puppet_agent::service_names, ' ')
     notice ("Puppet service start log file at ${_logfile}")
     file { "${::env_temp_variable}/solaris_start_puppet.sh":
       ensure => file,
       source => 'puppet:///modules/puppet_agent/solaris_start_puppet.sh',
       mode   => '0755',
     }
     -> exec { 'solaris_start_puppet.sh':
-      command => "${::env_temp_variable}/solaris_start_puppet.sh ${::puppet_agent_pid} 2>&1 > ${_logfile} &",
+      command => "${::env_temp_variable}/solaris_start_puppet.sh ${::puppet_agent_pid} ${_service_names_arg} 2>&1 > ${_logfile} &",
       path    => '/usr/bin:/bin:/usr/sbin',
     }
     file { ['/var/opt/lib', '/var/opt/lib/pe-puppet', '/var/opt/lib/pe-puppet/state']:
       ensure => directory,
     }
   } else {
-
     $::puppet_agent::service_names.each |$service| {
       service { $service:
         ensure     => running,

manifests/windows/install.pp

@@ -13,6 +13,8 @@
   ) {
   assert_private()
 
+  $service_names         = $::puppet_agent::service_names
+
   if $::puppet_agent::is_pe {
     $_agent_version = $puppet_agent::params::master_agent_version
     $_pe_server_version = pe_build_version()

spec/classes/puppet_agent_spec.rb

@@ -173,13 +173,16 @@ def global_facts(facts, os)
               end
             end
 
-            # Windows platform does not use Service resources
+            # Windows platform does not use Service resources; their services
+            # are managed by the MSI installer.
             unless facts[:osfamily] == 'windows'
               if params[:service_names].nil? &&
                   !(facts[:osfamily] == 'Solaris' && facts[:operatingsystemmajrelease] == '11') &&
                   os !~ /sles/
                 it { is_expected.to contain_service('puppet') }
-                it { is_expected.to contain_service('mcollective') }
+                if Puppet.version < "5.99.0" # mcollective was removed in puppet 6 (5.99 indicates a pre-release version)
+                  it { is_expected.to contain_service('mcollective') }
+                end
               else
                 it { is_expected.to_not contain_service('puppet') }
                 it { is_expected.to_not contain_service('mcollective') }

templates/install_puppet.bat.erb

@@ -39,9 +39,10 @@ REM sets its startup type, which prevents installs from proceeding.
 REM This may fail on agents without pxp-agent, but since this is not
 REM run interactively and the next command sets ERRORLEVEL, it's OK.
 net stop pxp-agent
-REM Same for the Marionette Collective Service and Puppet Agent service
-net stop mcollective
-net stop puppet
+REM Same for the <% @service_names.join(', ') %> services
+<% @service_names.each do |service_name| %>
+net stop <%= service_name %>
+<% end %>
 
 REM Log the current user token privileges for debugging
 whoami /all

templates/solaris_install.sh.erb

@@ -12,5 +12,7 @@ pkgadd -a <%= @adminfile %> -d <%= @sourcefile %> -G <%= @install_options.join('
 # Ensure services are running. We do this on Solaris 10 b/c the installer cannot restart
 # services on its own since that only happens when the service manifests change, which is
 # highly unlikely in most agent upgrade scenarios.
-start_service puppet
-start_service mcollective
+
+<% @service_names.each do |service_name| %>
+start_service <%= service_name %>
+<% end %>