Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timeout support to SilentProcessRunner via special variable to terminate stuck processes #468

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion source/Calamari.Shared/Deployment/ConventionProcessor.cs
Expand Up @@ -75,7 +75,6 @@ void RunInstallConventions()
}
}
}

void RunRollbackConventions()
{
foreach (var convention in conventions.OfType<IRollbackConvention>())
Expand Down
1 change: 1 addition & 0 deletions source/Calamari.Shared/Deployment/SpecialVariables.cs
Expand Up @@ -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)
{
Expand Down
Expand Up @@ -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))
{
Expand Down
Expand Up @@ -8,12 +8,13 @@ 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, TimeSpan? timeout = null)
{
Executable = executable;
Arguments = arguments;
EnvironmentVars = environmentVars;
Isolate = isolate;
Timeout = timeout ?? TimeSpan.MaxValue;
}

public CommandLineInvocation(
Expand All @@ -23,12 +24,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,
TimeSpan? timeout = null)
: this(executable, arguments, environmentVars, isolate)
{
this.workingDirectory = workingDirectory;
UserName = userName;
Password = password;
Timeout = timeout ?? TimeSpan.MaxValue;
}

public string Executable { get; }
Expand All @@ -41,6 +44,7 @@ public CommandLineInvocation(string executable, string arguments, Dictionary<str

public Dictionary<string, string> EnvironmentVars { get; }
public bool Isolate { get; }
public TimeSpan Timeout { get; internal set; }

/// <summary>
/// The initial working-directory for the invocation.
Expand Down
Expand Up @@ -24,13 +24,15 @@ public CommandResult Execute(CommandLineInvocation invocation)
invocation.UserName,
invocation.Password,
commandOutput.WriteInfo,
commandOutput.WriteError);
commandOutput.WriteError,
invocation.Timeout);

return new CommandResult(
invocation.ToString(),
exitCode.ExitCode,
exitCode.ErrorOutput,
invocation.WorkingDirectory);
invocation.WorkingDirectory,
exitCode.TimedOut);
}
catch (Exception ex)
{
Expand All @@ -46,7 +48,8 @@ public CommandResult Execute(CommandLineInvocation invocation)
invocation.ToString(),
-1,
ex.ToString(),
invocation.WorkingDirectory);
invocation.WorkingDirectory,
false);
}
}

Expand Down
15 changes: 14 additions & 1 deletion source/Calamari.Shared/Integration/Processes/CommandResult.cs
Expand Up @@ -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)
{
Expand All @@ -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)
Expand All @@ -39,7 +51,8 @@ public CommandResult VerifySuccess()
command,
exitCode,
additionalErrors,
workingDirectory);
workingDirectory,
timedOut);
}

return this;
Expand Down
Expand Up @@ -42,9 +42,10 @@ static SilentProcessRunner()
string arguments,
string workingDirectory,
Action<string> output,
Action<string> error)
Action<string> error,
TimeSpan? timeout = null)
{
return ExecuteCommand(executable, arguments, workingDirectory, null, null, null, output, error);
return ExecuteCommand(executable, arguments, workingDirectory, null, null, null, output, error, timeout);
}

public static SilentProcessRunnerResult ExecuteCommand(
Expand All @@ -53,9 +54,10 @@ static SilentProcessRunner()
string workingDirectory,
Dictionary<string, string> environmentVars,
Action<string> output,
Action<string> error)
Action<string> error,
TimeSpan? timeout = null)
{
return ExecuteCommand(executable, arguments, workingDirectory, environmentVars, null, null, output, error);
return ExecuteCommand(executable, arguments, workingDirectory, environmentVars, null, null, output, error, timeout);
}

public static SilentProcessRunnerResult ExecuteCommand(
Expand All @@ -66,7 +68,8 @@ static SilentProcessRunner()
string userName,
SecureString password,
Action<string> output,
Action<string> error)
Action<string> error,
TimeSpan? timeout = null)
{
try
{
Expand Down Expand Up @@ -149,11 +152,21 @@ static SilentProcessRunner()
process.BeginOutputReadLine();
process.BeginErrorReadLine();

process.WaitForExit();
// TimeSpan.TotalMilliseconds can have a value > int.MaxValue, so we just assume wait for ever if this happens.
var timeoutMilliseconds = timeout == null || timeout.Value.TotalMilliseconds > int.MaxValue ? -1 : (int)(timeout.Value.TotalMilliseconds);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work:

Suggested change
var timeoutMilliseconds = timeout == null || timeout.Value.TotalMilliseconds > int.MaxValue ? -1 : (int)(timeout.Value.TotalMilliseconds);
var timeoutMilliseconds = (int) (timeout ?? Timeout.InfiniteTimeSpan).TotalMilliseconds;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timeout.InfiniteTimeSpan is -1

var processExited = process.WaitForExit(timeoutMilliseconds);

if (!processExited)
{
Log.Error($"Process with ID {process.Id} exceeded the max allowed runtime of {timeout} and will be killed.");
process.Kill();
process.WaitForExit();
}

outputWaitHandle.WaitOne();
errorWaitHandle.WaitOne();

return new SilentProcessRunnerResult(process.ExitCode, errorData.ToString());
return new SilentProcessRunnerResult(process.ExitCode, errorData.ToString(), !processExited);
}
}
}
Expand Down
Expand Up @@ -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;
}
}
}
18 changes: 17 additions & 1 deletion source/Calamari.Shared/Integration/Scripting/ScriptEngine.cs
@@ -1,6 +1,6 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Calamari.Deployment;
using Calamari.Integration.FileSystem;
using Calamari.Integration.Processes;
Expand Down Expand Up @@ -31,6 +31,22 @@ 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.Timeout = timeout.HasValue && timeout > 0 ? TimeSpan.FromMilliseconds(timeout.Value) : TimeSpan.MaxValue;

if (execution.CommandLineInvocation.Timeout == TimeSpan.Zero)
{
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}. It will be ignored.");
}
else if (execution.CommandLineInvocation.Timeout > TimeSpan.Zero && execution.CommandLineInvocation.Timeout != TimeSpan.MaxValue)
{
Log.Verbose($"The script for this action will be executed with a timeout of {execution.CommandLineInvocation.Timeout} milliseconds. To remove this timeout, set the Action.Script.Timeout special variable to -1 or delete the variable.");
}
}

try
{
if (execution.CommandLineInvocation.Isolate)
Expand Down
1 change: 1 addition & 0 deletions source/Calamari.Tests/Calamari.Tests.csproj
Expand Up @@ -33,6 +33,7 @@
<PackageReference Include="Assent" Version="1.3.1" />
<PackageReference Include="Polly" Version="5.4.0" />
<PackageReference Include="System.ComponentModel.TypeConverter" Version="4.3.0" />
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFrameworkIdentifier)' == '.NETCoreApp' ">
<PackageReference Include="Markdown" Version="2.1.0" />
Expand Down
@@ -1,7 +1,11 @@
using Calamari.Deployment;
using Calamari.Integration.Processes;
using Calamari.Tests.Helpers;
using FluentAssertions;
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;

namespace Calamari.Tests.Fixtures.Integration.Process
{
Expand All @@ -18,5 +22,59 @@ public void ScriptShouldFailIfExecutableDoesNotExist()
result.HasErrors.Should().BeTrue();
output.Errors.Should().Contain(CommandLineRunner.ConstructWin32ExceptionMessage(executable));
}

[Test]
[Category(TestCategory.CompatibleOS.OnlyWindows)]
public void ScriptShouldFailWhenTimeoutIsSpecifiedAfterSomeTimeInWindowsSystems()
{
// For the sake of those running it with test adapters
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
{
return;
}

// 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> { };
var timeout = TimeSpan.FromMilliseconds(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, timeout: timeout));
result.HasErrors.Should().BeFalse();
result.TimedOut.Should().BeTrue();
output.Errors.Should().BeEmpty();
}

[Test]
[Category(TestCategory.CompatibleOS.OnlyNixOrMac)]
public void ScriptShouldFailWhenTimeoutIsSpecifiedAfterSomeTimeInNixSystems()
{
// For the sake of those running it with test adapters
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return;
}

// *Nix function. See `man sleep`
const string executable = "sleep";
var output = new CaptureCommandOutput();
var subject = new CommandLineRunner(output);

// Set a timeout of 500 milliseconds
var environmentVars = new Dictionary<string, string> { };
var timeout = TimeSpan.FromMilliseconds(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", environmentVars: environmentVars, timeout: timeout));
result.HasErrors.Should().BeFalse();
result.TimedOut.Should().BeTrue();
output.Errors.Should().BeEmpty();
}
}
}