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

Fix flakey WhenCompleteScriptIsNotCalled_ThenWorkspaceShouldGetDeletedWhenScriptFinishesRunning test #746

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void CanInstallWindowsService()
{
// Don't delete the user account - we don't delete the user profile, resulting in test failures when the profile names get too long
// Security: the user account is not a member of the local admin group, and we reset the password on every execution of the test
// user?.Delete();
// user?.TryDelete();
DeleteExistingService(serviceName);
}
}
Expand Down
15 changes: 15 additions & 0 deletions source/Octopus.Tentacle.Tests.Integration/Util/Wait.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@ namespace Octopus.Tentacle.Tests.Integration.Util
{
public static class Wait
{
public static async Task For(Func<bool> toBeTrue, TimeSpan timeout, Action onTimeoutOrCancellation, CancellationToken cancellationToken)
{
using var timeoutCancellationTokenSource = new CancellationTokenSource(timeout);
using var linkedCancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellationTokenSource.Token);

try
{
await For(toBeTrue, linkedCancellationTokenSource.Token);
}
catch (OperationCanceledException) when (linkedCancellationTokenSource.IsCancellationRequested)
{
onTimeoutOrCancellation();
}
}

public static async Task For(Func<bool> toBeTrue, CancellationToken cancellationToken)
{
while (true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
using FluentAssertions;
using NUnit.Framework;
using Octopus.Tentacle.CommonTestUtils.Builders;
using Octopus.Tentacle.Contracts;
using Octopus.Tentacle.Contracts.ClientServices;
using Octopus.Tentacle.Scripts;
using Octopus.Tentacle.Tests.Integration.Support;
using Octopus.Tentacle.Tests.Integration.Support.ExtensionMethods;
using Octopus.Tentacle.Tests.Integration.Util;
using Octopus.Tentacle.Tests.Integration.Util.Builders;
using Octopus.Tentacle.Tests.Integration.Util.Builders.Decorators;
Expand Down Expand Up @@ -63,6 +61,8 @@ public async Task WhenCompleteScriptIsNotCalled_ThenWorkspaceShouldGetDeletedWhe

var startScriptCommand = new LatestStartScriptCommandBuilder().WithScriptBody(b => b.Print("Hello")).Build();

string workspaceDirectory = null;

await using var clientAndTentacle = await tentacleConfigurationTestCase.CreateBuilder()
.WithTentacle(b =>
{
Expand All @@ -72,17 +72,66 @@ public async Task WhenCompleteScriptIsNotCalled_ThenWorkspaceShouldGetDeletedWhe
.HookServiceMethod(
tentacleConfigurationTestCase,
nameof(IAsyncClientScriptServiceV2.CompleteScriptAsync),
(_,_) => throw new NotImplementedException("Force failure to simulate tentacle client crashing, and ensure we do not complete the script"))
(_, _) =>
{
// Ensure the expected workspace directory exists
if (!Directory.Exists(workspaceDirectory))
{
throw new ArgumentException($"Workspace {workspaceDirectory} should exist");
}

throw new NotImplementedException("Force failure to simulate tentacle client crashing, and ensure we do not complete the script");
})
.Build())
.Build(CancellationToken);

workspaceDirectory = GetWorkspaceDirectoryPath(clientAndTentacle.RunningTentacle.HomeDirectory, startScriptCommand.ScriptTicket.TaskId);

// Ensure the workspace directory doesn't exist until the script starts to execute
if (Directory.Exists(workspaceDirectory))
{
throw new ArgumentException($"Workspace {workspaceDirectory} should not exist until the script has started running");
}

await AssertionExtensions
.Should(() => clientAndTentacle.TentacleClient.ExecuteScript(startScriptCommand, CancellationToken, null, new InMemoryLog()))
.ThrowAsync<NotImplementedException>();

var workspaceDirectory = GetWorkspaceDirectoryPath(clientAndTentacle.RunningTentacle.HomeDirectory, startScriptCommand.ScriptTicket.TaskId);
await Wait.For(
() =>
{
if (Directory.Exists(workspaceDirectory))
{
Logger.Information($"Workspace directory {workspaceDirectory} still exists. Will check again soon...");

return false;
}

await Wait.For(() => !Directory.Exists(workspaceDirectory), CancellationToken);
return true;
},
TimeSpan.FromSeconds(30),
onTimeoutOrCancellation: () =>
{
if (Directory.Exists(workspaceDirectory))
{
Logger.Information("Current folders in the directory.");
var directories = Directory.GetDirectories(workspaceDirectory);
foreach (var directory in directories)
{
Logger.Information(directory);
}

Logger.Information("Current files in the directory.");
var files = Directory.GetFiles(workspaceDirectory);
foreach (var file in files)
{
Logger.Information(file);
}

throw new Exception($"Workspace directory {workspaceDirectory} still exists. This should have been deleted.");
}
},
CancellationToken);
}

[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ private ScriptStateStore SetupScriptStateStore(ScriptTicket ticket)
private async Task CleanupWorkspace(ScriptTicket ticket, CancellationToken cancellationToken)
{
var workspace = workspaceFactory.GetWorkspace(ticket);
await workspace.Delete(cancellationToken);
await workspace.TryDelete(cancellationToken);
}

(StartScriptCommandV2 StartScriptCommand, FileInfo FileScriptWillCreate) GetStartScriptCommandForScriptThatCreatesAFile(ScriptTicket scriptTicket, int? scriptDelayInSeconds = null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ private ScriptStateStore SetupScriptStateStore(ScriptTicket ticket)
private async Task CleanupWorkspace(ScriptTicket ticket, CancellationToken cancellationToken)
{
var workspace = workspaceFactory.GetWorkspace(ticket);
await workspace.Delete(cancellationToken);
await workspace.TryDelete(cancellationToken);
}

(StartScriptCommandV3Alpha StartScriptCommand, FileInfo FileScriptWillCreate) GetStartScriptCommandForScriptThatCreatesAFile(ScriptTicket scriptTicket, int? scriptDelayInSeconds = null)
Expand Down
7 changes: 6 additions & 1 deletion source/Octopus.Tentacle/Maintenance/WorkspaceCleaner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,15 @@ public async Task Clean(CancellationToken cancellationToken)

await workspace.Delete(cancellationToken);
deletedCount++;

log.Verbose($"Deleted workspace: {workspace.WorkingDirectory}");
}
catch (Exception e)
{
log.Warn(e, $"Could not delete workspace {workspace.WorkingDirectory}.");
// If we can't delete the workspace consistently then we could end up spamming the logs with errors
// By default Tentacle only logs Info+ so by logging Verbose we limit the amount of spam to only Tentacles that
// have been configured for DEBUG logging
log.Verbose(e, $"Could not delete workspace {workspace.WorkingDirectory}.");
}
}

Expand Down
1 change: 1 addition & 0 deletions source/Octopus.Tentacle/Scripts/IScriptWorkspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public interface IScriptWorkspace
string? ScriptMutexName { get; set; }
void BootstrapScript(string scriptBody);
string ResolvePath(string fileName);
Task TryDelete(CancellationToken cancellationToken);
Task Delete(CancellationToken cancellationToken);
IScriptLog CreateLog();
string LogFilePath { get; }
Expand Down
7 changes: 6 additions & 1 deletion source/Octopus.Tentacle/Scripts/ScriptWorkspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,16 @@ public string ResolvePath(string fileName)
return path;
}

public async Task Delete(CancellationToken cancellationToken)
public async Task TryDelete(CancellationToken cancellationToken)
{
await FileSystem.DeleteDirectory(WorkingDirectory, cancellationToken, DeletionOptions.TryThreeTimesIgnoreFailure);
}

public async Task Delete(CancellationToken cancellationToken)
{
await FileSystem.DeleteDirectory(WorkingDirectory, cancellationToken, DeletionOptions.TryThreeTimes);
}

public IScriptLog CreateLog()
{
return new ScriptLog(ResolvePath(LogFileName), FileSystem, SensitiveValueMasker);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public async Task<ScriptStatusResponse> CompleteScriptAsync(CompleteScriptComman
cancellationTokens.TryRemove(command.Ticket.TaskId, out _);
var response = GetResponse(command.Ticket, script, command.LastLogSequence);
var workspace = workspaceFactory.GetWorkspace(command.Ticket);
await workspace.Delete(cancellationToken);
await workspace.TryDelete(cancellationToken);
return response;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public async Task CompleteScriptAsync(CompleteScriptCommandV2 command, Cancellat
}

var workspace = workspaceFactory.GetWorkspace(command.Ticket);
await workspace.Delete(cancellationToken);
await workspace.TryDelete(cancellationToken);
}

RunningScript LaunchShell(ScriptTicket ticket, string serverTaskId, IScriptWorkspace workspace, IScriptStateStore stateStore, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public async Task CompleteScriptAsync(CompleteScriptCommandV3Alpha command, Canc
}

var workspace = workspaceFactory.GetWorkspace(command.ScriptTicket);
await workspace.Delete(cancellationToken);
await workspace.TryDelete(cancellationToken);
}

async Task<ScriptStatusResponseV3Alpha> GetResponse(ScriptTicket ticket, long lastLogSequence, IRunningScript? runningScript)
Expand Down
2 changes: 1 addition & 1 deletion source/Octopus.Tentacle/Startup/WatchdogCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public WatchdogCommand(
log.Info("Creating watchdog task");
});
Options.Add("delete",
"Delete the watchdog task for the given instances",
"TryDelete the watchdog task for the given instances",
v =>
{
deleteTask = true;
Expand Down