From a75c49cf39b32d545263440a567cddecb0a1793d Mon Sep 17 00:00:00 2001
From: "Sean P. McDonald" <sean.mcdonald@puppetlabs.com>
Date: Thu, 28 Feb 2019 10:44:52 -0800
Subject: [PATCH] (MODULES-8554) Add error repoting for background upgrades

This commit adds a new type/provider to check for an error log from a previous
installation and fail if the error log exists. This will allow upgrades to
leave behind error logs on failure to indicate the next puppet run should not
perform the upgrade since the previous one failed.

The provider will delete the existing error log before failing so that after
the failed puppet run the user can attempt the upgrade again.

This should provide error reporting for upgrades that require background
processes (i.e. windows upgrades) by failing the next puppet run after the
upgrade failure.
---
 files/install_puppet.ps1                      |  4 ++-
 .../provider/upgrade_error/upgrade_error.rb   | 25 +++++++++++++++++++
 lib/puppet/type/upgrade_error.rb              | 25 +++++++++++++++++++
 manifests/windows/install.pp                  |  7 +++++-
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 lib/puppet/provider/upgrade_error/upgrade_error.rb
 create mode 100644 lib/puppet/type/upgrade_error.rb

diff --git a/files/install_puppet.ps1 b/files/install_puppet.ps1
index 9e88c4c7d..393da4692 100644
--- a/files/install_puppet.ps1
+++ b/files/install_puppet.ps1
@@ -256,8 +256,9 @@ $service_names=@(
   "mcollective"
 )
 try {
+  $state_dir = (puppet.bat config print statedir)
   Write-Log "Installation PID:$PID"
-  $install_pid_lock = Join-Path -Path (Split-Path -Parent (puppet.bat config print agent_catalog_run_lockfile)) -ChildPath 'puppet_agent_upgrade.pid'
+  $install_pid_lock = Join-Path -Path $state_dir -ChildPath 'puppet_agent_upgrade.pid'
   Lock-Installation $install_pid_lock
   if ($PuppetPID) {
     # Wait for the puppet run to finish
@@ -342,6 +343,7 @@ try {
   if ($UseLockedFilesWorkaround) {
     Reset-PuppetresDLL $temp_puppetres
   }
+  "$_" | Out-File -FilePath (Join-Path -Path $state_dir -ChildPath 'puppet_agent_upgrade_failure.log')
 } finally {
   Reset-PuppetServices $services_before
   Unlock-Installation $install_pid_lock
diff --git a/lib/puppet/provider/upgrade_error/upgrade_error.rb b/lib/puppet/provider/upgrade_error/upgrade_error.rb
new file mode 100644
index 000000000..58f940b8d
--- /dev/null
+++ b/lib/puppet/provider/upgrade_error/upgrade_error.rb
@@ -0,0 +1,25 @@
+Puppet::Type.type(:upgrade_error).provide :upgrade_error do
+  def ensure_notexist
+    logfile = File.join(Puppet["statedir"].to_s, @resource[:name])
+    Puppet.debug "Checking for Error logfile #{logfile}"
+    not File.exists?(logfile)
+  end
+
+  def delete_file_and_read_content(filename)
+    logfile = File.join(Puppet["statedir"].to_s, filename)
+    Puppet.debug "Reading Error Log #{logfile}"
+    if Puppet.features.microsoft_windows?
+      # The file laid down by the installation script on windows will be UTF-16LE.
+      # In this scenario we need to open the file in binmode and read each line
+      # individually, then encode the result back to UTF-8 so we can sub out both
+      # the UTF-16 header \uFEFF and the \r\n line carriages.
+      content = File.open(logfile,"rb:UTF-16LE"){ |file| file.readlines }[0].encode!('UTF-8').gsub("\uFEFF",'').gsub("\r",'')
+    else
+      content = File.read(logfile)
+    end
+    Puppet.debug "Deleting Error Log"
+    File.delete(logfile)
+    return content
+  end
+
+end
diff --git a/lib/puppet/type/upgrade_error.rb b/lib/puppet/type/upgrade_error.rb
new file mode 100644
index 000000000..6eadb6c1a
--- /dev/null
+++ b/lib/puppet/type/upgrade_error.rb
@@ -0,0 +1,25 @@
+Puppet::Type.newtype(:upgrade_error) do
+  @doc = <<-DOC
+Fails when a previous backgroun installation failed. The type
+will check for the existance of an installation failure log
+and raise an error with the contents of the log if it exists
+DOC
+
+  newproperty(:ensure_notexist) do
+    desc "whether or not the error log exists"
+    def insync?(not_exist)
+      if not_exist
+        true
+      else
+        raise Puppet::Error.new("Failed previous installation with: #{provider.delete_file_and_read_content(@resource[:name])}")
+      end
+    end
+
+    defaultto { true }
+  end
+
+  newparam(:name) do
+    desc "The name of the failure log to check for in puppet's $statedir. If this log exists the resource will fail."
+    isnamevar
+  end
+end
diff --git a/manifests/windows/install.pp b/manifests/windows/install.pp
index 6f03e1bd5..7207f3e35 100644
--- a/manifests/windows/install.pp
+++ b/manifests/windows/install.pp
@@ -39,11 +39,12 @@
   debug ("Installing puppet from ${_msi_location}")
 
   $_installps1 = windows_native_path("${::env_temp_variable}/install_puppet.ps1")
+  upgrade_error { "puppet_agent_upgrade_failure.log": }
   file { "${_installps1}":
     ensure  => file,
     content => file('puppet_agent/install_puppet.ps1')
   }
-  -> exec { 'install_puppet.ps1':
+  exec { 'install_puppet.ps1':
     # The powershell execution uses -Command and not -File because -File will interpolate the quotes
     # in a context like cmd.exe: https://docs.microsoft.com/en-us/powershell/scripting/components/console/powershell.exe-command-line-help?view=powershell-6#-file--
     # Because of this it's much cleaner to use -Command and use single quotes for each powershell param
@@ -62,6 +63,10 @@
                           -InstallArgs '${_install_options}' \
                           ${_move_dll_workaround}",
     path    => $::path,
+    require => [
+      Upgrade_error['puppet_agent_upgrade_failure.log'],
+      File["${_installps1}"]
+    ]
   }
 
   # PUP-5480/PE-15037 Cache dir loses inheritable SYSTEM perms