Add timeout support to SilentProcessRunner by supporting a special va…

…riable "Octopus.Action.Script.Timeout"

source/Calamari.Shared/Deployment/ConventionProcessor.cs

@@ -75,7 +75,6 @@ void RunInstallConventions()
                 }
             }
         }
-
         void RunRollbackConventions()
         {
             foreach (var convention in conventions.OfType<IRollbackConvention>())

source/Calamari.Shared/Deployment/SpecialVariables.cs

@@ -386,6 +386,7 @@ public static class Script
                 public static readonly string ScriptParameters = "Octopus.Action.Script.ScriptParameters";
                 public static readonly string ScriptSource = "Octopus.Action.Script.ScriptSource";
                 public static readonly string ExitCode = "Octopus.Action.Script.ExitCode";
+                public static readonly string Timeout = "Octopus.Action.Script.Timeout";
 
                 public static string ScriptBodyBySyntax(ScriptSyntax syntax)
                 {

source/Calamari.Shared/Integration/Processes/CommandLineException.cs

@@ -6,29 +6,36 @@ namespace Calamari.Integration.Processes
     public class CommandLineException : Exception
     {
         public CommandLineException(
-            string commandLine, 
-            int exitCode, 
-            string additionalInformation, 
-            string workingDirectory = null)
-            : base(FormatMessage(commandLine, exitCode, additionalInformation, workingDirectory))
+            string commandLine,
+            int exitCode,
+            string additionalInformation,
+            string workingDirectory = null,
+            bool timedOut = false)
+            : base(FormatMessage(commandLine, exitCode, additionalInformation, workingDirectory, timedOut))
         {
         }
 
         private static string FormatMessage(
             string commandLine, 
             int exitCode, 
             string additionalInformation,
-            string workingDirectory)
+            string workingDirectory,
+            bool timedOut)
         {
             var sb = new StringBuilder("The following command: ");
             sb.AppendLine(commandLine);
 
-            if (!String.IsNullOrEmpty(workingDirectory))
+            if (!string.IsNullOrEmpty(workingDirectory))
             {
                 sb.Append("With the working directory of: ")
                     .AppendLine(workingDirectory);
             }
-
+
+            if (timedOut)
+            {
+                sb.Append("Timed out before execution completed. Check the Octopus.Action.Script.Timeout variable.").AppendLine();
+            }
+
             sb.Append("Failed with exit code: ").Append(exitCode).AppendLine();
             if (!string.IsNullOrWhiteSpace(additionalInformation))
             {

source/Calamari.Shared/Integration/Processes/CommandLineInvocation.cs

@@ -1,19 +1,21 @@
 using System;
 using System.Collections.Generic;
 using System.Security;
+using System.Threading;
 
 namespace Calamari.Integration.Processes
 {
     public class CommandLineInvocation
     {
         readonly string workingDirectory;
 
-        public CommandLineInvocation(string executable, string arguments, Dictionary<string, string> environmentVars = null, bool isolate = false)
+        public CommandLineInvocation(string executable, string arguments, Dictionary<string, string> environmentVars = null, bool isolate = false, int timeoutMilliseconds = Timeout.Infinite)
         {
             Executable = executable;
             Arguments = arguments;
             EnvironmentVars = environmentVars;
             Isolate = isolate;
+            TimeoutMilliseconds = timeoutMilliseconds;
         }
 
         public CommandLineInvocation(
@@ -23,12 +25,14 @@ public CommandLineInvocation(string executable, string arguments, Dictionary<str
             Dictionary<string, string> environmentVars = null, 
             string userName = null, 
             SecureString password = null,
-            bool isolate = false)
+            bool isolate = false,
+            int timeoutMilliseconds = Timeout.Infinite)
             : this(executable, arguments, environmentVars, isolate)
         {
             this.workingDirectory = workingDirectory;
             UserName = userName;
             Password = password;
+            TimeoutMilliseconds = timeoutMilliseconds;
         }
 
         public string Executable { get; }
@@ -41,6 +45,7 @@ public CommandLineInvocation(string executable, string arguments, Dictionary<str
 
         public Dictionary<string, string> EnvironmentVars { get; }
         public bool Isolate { get; }
+        public int TimeoutMilliseconds { get; internal set; }
 
         /// <summary>
         /// The initial working-directory for the invocation.

source/Calamari.Shared/Integration/Processes/CommandLineRunner.cs

@@ -14,6 +14,18 @@ public CommandLineRunner(ICommandOutput commandOutput)
 
         public CommandResult Execute(CommandLineInvocation invocation)
         {
+            var timedOut = false;
+
+            if (invocation.TimeoutMilliseconds > -1)
+            {
+                if (invocation.TimeoutMilliseconds == 0)
+                {
+                    var link = "https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=netframework-4.8#System_Diagnostics_Process_WaitForExit_System_Int32_";
+                    Log.Warn($"The timeout for this script was set to 0. Perhaps this was not intended. Setting the timeout to 0 will succeed only if the script exits immediately. See {link}");
+                }
+                Log.Verbose($"The script for this action will be executed with a timeout of {invocation.TimeoutMilliseconds} milliseconds. To remove this timeout, set the Action.Script.Timeout special variable to -1 or delete the variable.");
+            }
+
             try
             {
                 var exitCode = SilentProcessRunner.ExecuteCommand(
@@ -24,7 +36,10 @@ public CommandResult Execute(CommandLineInvocation invocation)
                     invocation.UserName,
                     invocation.Password,
                     commandOutput.WriteInfo,
-                    commandOutput.WriteError);
+                    commandOutput.WriteError,
+                    invocation.TimeoutMilliseconds);
+
+                timedOut = exitCode.TimedOut;
 
                 return new CommandResult(
                     invocation.ToString(),
@@ -46,7 +61,8 @@ public CommandResult Execute(CommandLineInvocation invocation)
                     invocation.ToString(), 
                     -1, 
                     ex.ToString(),
-                    invocation.WorkingDirectory);
+                    invocation.WorkingDirectory,
+                    timedOut);
             }
         }
 

source/Calamari.Shared/Integration/Processes/CommandResult.cs

@@ -6,6 +6,7 @@ public class CommandResult
         private readonly int exitCode;
         private readonly string additionalErrors;
         private readonly string workingDirectory;
+        private readonly bool timedOut;
 
         public CommandResult(string command, int exitCode) : this(command, exitCode, null)
         {
@@ -25,12 +26,23 @@ public CommandResult(string command, int exitCode, string additionalErrors, stri
             this.workingDirectory = workingDirectory;
         }
 
+        public CommandResult(string command, int exitCode, string additionalErrors, string workingDirectory, bool timedOut)
+        {
+            this.command = command;
+            this.exitCode = exitCode;
+            this.additionalErrors = additionalErrors;
+            this.workingDirectory = workingDirectory;
+            this.timedOut = timedOut;
+        }
+
         public int ExitCode => exitCode;
 
         public string Errors => additionalErrors;
 
         public bool HasErrors => !string.IsNullOrWhiteSpace(additionalErrors);
 
+        public bool TimedOut => timedOut;
+
         public CommandResult VerifySuccess()
         {
             if (exitCode != 0)
@@ -39,7 +51,8 @@ public CommandResult VerifySuccess()
                     command, 
                     exitCode, 
                     additionalErrors, 
-                    workingDirectory);
+                    workingDirectory,
+                    timedOut);
             }
 
             return this;

source/Calamari.Shared/Integration/Processes/SilentProcessRunner.cs

@@ -42,9 +42,10 @@ static SilentProcessRunner()
             string arguments, 
             string workingDirectory, 
             Action<string> output, 
-            Action<string> error)
+            Action<string> error,
+            int timeoutMilliseconds = Timeout.Infinite)
         {
-            return ExecuteCommand(executable, arguments, workingDirectory, null, null, null, output, error);
+            return ExecuteCommand(executable, arguments, workingDirectory, null, null, null, output, error, timeoutMilliseconds);
         }
 
         public static SilentProcessRunnerResult ExecuteCommand(
@@ -53,9 +54,10 @@ static SilentProcessRunner()
             string workingDirectory, 
             Dictionary<string, string> environmentVars, 
             Action<string> output, 
-            Action<string> error)
+            Action<string> error,
+            int timeoutMilliseconds = Timeout.Infinite)
         {
-            return ExecuteCommand(executable, arguments, workingDirectory, environmentVars, null, null, output, error);
+            return ExecuteCommand(executable, arguments, workingDirectory, environmentVars, null, null, output, error, timeoutMilliseconds);
         }
 
         public static SilentProcessRunnerResult ExecuteCommand(
@@ -66,7 +68,8 @@ static SilentProcessRunner()
             string userName,             
             SecureString password, 
             Action<string> output, 
-            Action<string> error)
+            Action<string> error,
+            int timeoutMilliseconds = Timeout.Infinite)
         {
             try
             {
@@ -149,11 +152,27 @@ static SilentProcessRunner()
                         process.BeginOutputReadLine();
                         process.BeginErrorReadLine();
 
-                        process.WaitForExit();
-                        outputWaitHandle.WaitOne();
-                        errorWaitHandle.WaitOne();
+                        // Some processes can have race conditions. Between the call to Start and WaitForExit part of the process may have exited already.
+                        // This can happen when calling CommitChanges in dotnet's ServerManager as shown here: https://stackoverflow.com/questions/7446632/servermanager-commitchanges-makes-changes-with-a-slight-delay
+                        // Commit changes is called by appcmd from IIS, which is called by Set-ItemProperty from the WebAdministration module in Powershell
+                        // https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=netframework-4.8#System_Diagnostics_Process_WaitForExit_System_Int32_
+                        // Add a timeout passed down from some default configuration or context up the call stack, -1 will never timeout.
+                        var timedOut = !process.WaitForExit(timeoutMilliseconds);
 
-                        return new SilentProcessRunnerResult(process.ExitCode, errorData.ToString());
+                        if (!timedOut)
+                        {
+                            // Only wait when the process has not timed out, otherwise we will be stuck here as well.
+                            outputWaitHandle.WaitOne();
+                            errorWaitHandle.WaitOne();
+                        } else
+                        {
+                            Log.Error($"Process with ID {process.Id} exceeded the max allowed runtime of {timeoutMilliseconds} milliseconds and will be killed.");
+                            process.CancelOutputRead();
+                            process.CancelErrorRead();
+                            process.Kill();
+                        }
+
+                        return new SilentProcessRunnerResult(process.ExitCode, errorData.ToString(), timedOut);
                     }
                 }
             }

source/Calamari.Shared/Integration/Processes/SilentProcessRunnerResult.cs

@@ -5,11 +5,13 @@ public class SilentProcessRunnerResult
         public int ExitCode { get; }
 
         public string ErrorOutput { get; }
+        public bool TimedOut { get; set; }
 
-        public SilentProcessRunnerResult(int exitCode, string errorOutput)
+        public SilentProcessRunnerResult(int exitCode, string errorOutput, bool timedOut)
         {
             ExitCode = exitCode;
             ErrorOutput = errorOutput;
+            TimedOut = timedOut;
         }
     }
 }

source/Calamari.Shared/Integration/Scripting/ScriptEngine.cs

@@ -1,6 +1,6 @@
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
+using System.Threading;
 using Calamari.Deployment;
 using Calamari.Integration.FileSystem;
 using Calamari.Integration.Processes;
@@ -31,6 +31,21 @@ public abstract class ScriptEngine : IScriptEngine
                         execution.CommandLineInvocation.Arguments);
                 }
 
+                if (variables.IsSet(SpecialVariables.Action.Script.Timeout))
+                {
+                    var timeout = variables.GetInt32(SpecialVariables.Action.Script.Timeout);
+                    execution.CommandLineInvocation.TimeoutMilliseconds = timeout ?? Timeout.Infinite;
+
+                    if (execution.CommandLineInvocation.TimeoutMilliseconds > 0 && execution.CommandLineInvocation.TimeoutMilliseconds != Timeout.Infinite)
+                    {
+                        Log.Verbose($"Timeout was set to {execution.CommandLineInvocation.TimeoutMilliseconds}");
+                    }
+                }
+                else
+                {
+                    Log.Verbose("Timeout was not set for this script. Octopus will wait for the script to complete indefinitely.");
+                }
+
                 try
                 {
                     if (execution.CommandLineInvocation.Isolate)

source/Calamari.Tests/Fixtures/Integration/Process/CommandLineRunnerFixture.cs

@@ -1,7 +1,9 @@
+using Calamari.Deployment;
 using Calamari.Integration.Processes;
 using Calamari.Tests.Helpers;
 using FluentAssertions;
 using NUnit.Framework;
+using System.Collections.Generic;
 
 namespace Calamari.Tests.Fixtures.Integration.Process
 {
@@ -18,5 +20,24 @@ public void ScriptShouldFailIfExecutableDoesNotExist()
             result.HasErrors.Should().BeTrue();
             output.Errors.Should().Contain(CommandLineRunner.ConstructWin32ExceptionMessage(executable));
         }
+
+        [Test]
+        public void ScriptShouldFailWhenTimeoutIsSpecifiedAfterSomeTime()
+        {
+            // Windows function. See TIMEOUT /?
+            const string executable = "TIMEOUT";
+            var output = new CaptureCommandOutput();
+            var subject = new CommandLineRunner(output);
+
+            // Set a timeout of 500 milliseconds
+            var environmentVars = new Dictionary<string, string> { { SpecialVariables.Action.Script.Timeout, "500" } };
+
+            // Run a function with a 100 second timeout, which should exit before it completes due to the configured timeout.
+            // This simulates stuck deployments on a larger scale (say stuck for 24 hours).
+            var result = subject.Execute(new CommandLineInvocation(executable: executable, arguments: "100 /NOBREAK", environmentVars: environmentVars));
+            result.HasErrors.Should().BeFalse();
+            result.TimedOut.Should().BeTrue();
+            output.Errors.Should().BeEmpty();
+        }
     }
 }