(MODULES-10653) Failed to upgrade agent using puppet task

Before this commit, when trying to run the
`puppet_agent::install_powershell` task, with a specific version as
parameter, it was failing with below error message:
Error: Timed out waiting for status response from <node_name>

On the node, both `puppet agent` and `pxp-agent` services got stopped
and the event log viewer showed:
Application or service 'task_wrapper' could not be shut down.

Due to Windows agents becoming unresponsive when running this task and
as per MODULES-10633 discussions, the `puppet_agent::install_poweshell`
task should not be used for upgrading Puppet Agents while either of
`puppet agent` or `pxp-agent` services are still running. This Puppet
Agent module task will now fail immediately and output a message if any
of the two affected services are still running.

The task will stop and output a message if the given version is already
installed on the node, thus avoiding unnecessary msi
download/installation. This behaviour is aligned with the existing
implementation for no version given and Puppet Agent already installed
on the node.

Author: luchihoratiu

task_spec/spec/acceptance/init_spec.rb

@@ -40,29 +40,31 @@ def target_platform
                        else
                          '5.5.3'
                        end
-    # test the agent isn't already installed and that the version task works
+
+    # Test the agent isn't already installed and that the version task works
     results = run_task('puppet_agent::version', 'target', {})
     results.each do |res|
       expect(res).to include('status' => 'success')
       expect(res['result']['version']).to eq(nil)
     end
 
-    # Try to install an older version
+    # Try to install an older puppet5 version
     results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5',
                                                             'version' => puppet_5_version,
                                                             'stop_service' => true })
     results.each do |res|
       expect(res).to include('status' => 'success')
     end
 
-    # It installed a version older than latest
+    # It installed a version older than latest puppet5
     results = run_task('puppet_agent::version', 'target', {})
     results.each do |res|
       expect(res).to include('status' => 'success')
       expect(res['result']['version']).to eq(puppet_5_version)
       expect(res['result']['source']).to be
     end
 
+    # Check that puppet agent service has been stopped due to 'stop_service' parameter set to true
     service = if target_platform =~ /win/
                 run_command('c:/"program files"/"puppet labs"/puppet/bin/puppet resource service puppet', 'target')
               else
@@ -71,21 +73,23 @@ def target_platform
     output = service[0]['result']['stdout']
     expect(output).to include("ensure => 'stopped'")
 
-    # Run with no argument upgrades
+    # Try to upgrade with no specific version given in parameter
+    # Expect nothing to happen and receive a message regarding this
     results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5' })
     results.each do |res|
       expect(res).to include('status' => 'success')
+      expect(res['result']['_output']).to match(%r{Version parameter not defined and agent detected. Nothing to do.})
     end
 
-    # Verify that it did nothing
+    # Verify that the version didn't change
     results = run_task('puppet_agent::version', 'target', {})
     results.each do |res|
       expect(res).to include('status' => 'success')
       expect(res['result']['version']).to eq(puppet_5_version)
       expect(res['result']['source']).to be
     end
 
-    # Run with latest upgrades
+    # Upgrade to latest puppet5 version
     results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5', 'version' => 'latest' })
     results.each do |res|
       expect(res).to include('status' => 'success')
@@ -100,19 +104,44 @@ def target_platform
       expect(res['result']['source']).to be
     end
 
-    # Upgrade from puppet5 to puppet6
+    # Puppet Agent can't be upgraded on Windows nodes while 'puppet agent' service or 'pxp-agent' service are running
+    if target_platform =~ /win/
+      # Try to upgrade from puppet5 to puppet6 but fail due to puppet agent service already running
+      results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => 'latest' })
+      results.each do |res|
+        expect(res).to include('status' => 'failure')
+        expect(res['result']['_error']['msg']).to match(%r{Puppet Agent upgrade cannot be done while Puppet services are still running.})
+      end
+
+      # Manually stop the puppet agent service
+      service = run_command('c:/"program files"/"puppet labs"/puppet/bin/puppet resource service puppet ensure=stopped', 'target')
+      output = service[0]['result']['stdout']
+      expect(output).to include("ensure => 'stopped'")       
+    end
+
+    # Succesfully upgrade from puppet5 to puppet6
     results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => 'latest' })
     results.each do |res|
       expect(res).to include('status' => 'success')
     end
 
     # Verify that it upgraded
+    installed_version = nil
     results = run_task('puppet_agent::version', 'target', {})
     results.each do |res|
       expect(res).to include('status' => 'success')
-      expect(res['result']['version']).not_to match(%r{^5\.\d+\.\d+})
-      expect(res['result']['version']).to match(%r{^6\.\d+\.\d+})
+      installed_version = res['result']['version']
+      expect(installed_version).not_to match(%r{^5\.\d+\.\d+})
+      expect(installed_version).to match(%r{^6\.\d+\.\d+})
       expect(res['result']['source']).to be
     end
+
+    # Try installing the same version again
+    # Expect nothing to happen and receive a message regarding this
+    results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => installed_version })
+    results.each do |res|
+      expect(res).to include('status' => 'success')
+      expect(res['result']['_output']).to match(%r{Puppet Agent #{installed_version} detected. Nothing to do.})
+    end
   end
 end

tasks/install_powershell.ps1

@@ -34,7 +34,45 @@ function Test-PuppetInstalled {
   }
 }
 
+function Test-PuppetInstalledVersion {
+  $rootPath = 'HKLM:\SOFTWARE\Puppet Labs\Puppet'
+
+  $reg = Get-ItemProperty -Path $rootPath -ErrorAction SilentlyContinue
+  if ($null -ne $reg) {
+    if ($null -ne $reg.RememberedInstallDir64) {
+      $loc = $reg.RememberedInstallDir64+'VERSION'
+    } elseif ($null -ne $reg.RememberedInstallDir) {
+      $loc = $reg.RememberedInstallDir+'VERSION'
+    }
+  }
+
+  if ($null -ne $loc) {
+    $installedVersion = Get-Content -Path $loc -ErrorAction SilentlyContinue
+    if ($installedVersion -eq $version) {
+      RETURN $true
+    }
+  }
+
+  RETURN $false
+}
+
+function Test-RunningServices {
+  $puppetAgentService = Get-Service -DisplayName 'Puppet Agent' -ErrorAction SilentlyContinue
+  $pxpAgentService = Get-Service -DisplayName 'Puppet PXP Agent' -ErrorAction SilentlyContinue
+
+  if ($puppetAgentService.Status -eq 'Running' -or $pxpAgentService.Status -eq 'Running') {
+    RETURN $true
+  }
+
+  RETURN $false
+}
+
 if ($version) {
+    if (Test-PuppetInstalledVersion) {
+      Write-Output "Puppet Agent ${version} detected. Nothing to do."
+      Exit
+    }
+
     if ($version -eq "latest") {
       $msi_name = "puppet-agent-${arch}-latest.msi"
     } else {
@@ -43,13 +81,17 @@ if ($version) {
 }
 else {
     if (Test-PuppetInstalled) {
-      Write-Output "Puppet Agent detected and no version specified. Nothing to do."
+      Write-Output "Version parameter not defined and agent detected. Nothing to do."
       Exit
     }
 
     $msi_name = "puppet-agent-${arch}-latest.msi"
 }
 
+if (Test-RunningServices) {
+  Write-Error "Puppet Agent upgrade cannot be done while Puppet services are still running."
+}
+
 # Change windows_source only if the collection is a nightly build, and the source was not explicitly specified.
 if (($collection -like '*nightly*') -And -Not ($PSBoundParameters.ContainsKey('windows_source'))) {
   $windows_source = 'https://nightlies.puppet.com/downloads'