Dealing with cancellation issues

OctopusDeploy · Jan 21, 2025 · 3ed6087 · 3ed6087
1 parent a377da4
commit 3ed6087
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 50 deletions.
diff --git a/source/Octopus.Tentacle.Client/ScriptExecutor.cs b/source/Octopus.Tentacle.Client/ScriptExecutor.cs
@@ -60,49 +60,33 @@ public async Task<ScriptExecutorResult> StartScript(ExecuteScriptCommand execute
             StartScriptIsBeingReAttempted startScriptIsBeingReAttempted,
             CancellationToken cancellationToken)
         {
-            var scriptServiceToUse = await DetermineScriptServiceVersionToUse(cancellationToken);
+            var scriptServiceVersionToUse = await DetermineScriptServiceVersionToUse(cancellationToken);
 
             var scriptExecutorFactory = CreateScriptExecutorFactory();
+            var scriptExecutor = scriptExecutorFactory.CreateScriptExecutor(scriptServiceVersionToUse);
 
-            var scriptExecutor = scriptExecutorFactory.CreateScriptExecutor(scriptServiceToUse);
             return await scriptExecutor.StartScript(executeScriptCommand, startScriptIsBeingReAttempted, cancellationToken);
         }
 
-        async Task<ScriptServiceVersion> DetermineScriptServiceVersionToUse(CancellationToken cancellationToken)
-        {
-            try
-            {
-                var scriptServiceVersionSelector = new ScriptServiceVersionSelector(allClients.CapabilitiesServiceV2, logger, rpcCallExecutor, clientOptions, operationMetricsBuilder);
-                return await scriptServiceVersionSelector.DetermineScriptServiceVersionToUse(cancellationToken);
-            }
-            catch (Exception ex) when (cancellationToken.IsCancellationRequested)
-            {
-                throw new OperationCanceledException("Script execution was cancelled", ex);
-            }
-        }
-
         public async Task<ScriptExecutorResult> GetStatus(CommandContext ticketForNextNextStatus, CancellationToken cancellationToken)
         {
             var scriptExecutorFactory = CreateScriptExecutorFactory();
-
             var scriptExecutor = scriptExecutorFactory.CreateScriptExecutor(ticketForNextNextStatus.ScripServiceVersionUsed);
 
             return await scriptExecutor.GetStatus(ticketForNextNextStatus, cancellationToken);
         }
 
-        public async Task<ScriptExecutorResult> CancelScript(CommandContext ticketForNextNextStatus, CancellationToken cancellationToken)
+        public async Task<ScriptExecutorResult> CancelScript(CommandContext ticketForNextNextStatus)
         {
             var scriptExecutorFactory = CreateScriptExecutorFactory();
-
             var scriptExecutor = scriptExecutorFactory.CreateScriptExecutor(ticketForNextNextStatus.ScripServiceVersionUsed);
 
-            return await scriptExecutor.CancelScript(ticketForNextNextStatus, cancellationToken);
+            return await scriptExecutor.CancelScript(ticketForNextNextStatus);
         }
 
         public async Task<ScriptStatus?> CompleteScript(CommandContext ticketForNextNextStatus, CancellationToken cancellationToken)
         {
             var scriptExecutorFactory = CreateScriptExecutorFactory();
-
             var scriptExecutor = scriptExecutorFactory.CreateScriptExecutor(ticketForNextNextStatus.ScripServiceVersionUsed);
 
             return await scriptExecutor.CompleteScript(ticketForNextNextStatus, cancellationToken);
@@ -118,5 +102,18 @@ ScriptExecutorFactory CreateScriptExecutorFactory()
                 logger);
         }
 
+        async Task<ScriptServiceVersion> DetermineScriptServiceVersionToUse(CancellationToken cancellationToken)
+        {
+            try
+            {
+                var scriptServiceVersionSelector = new ScriptServiceVersionSelector(allClients.CapabilitiesServiceV2, logger, rpcCallExecutor, clientOptions, operationMetricsBuilder);
+                return await scriptServiceVersionSelector.DetermineScriptServiceVersionToUse(cancellationToken);
+            }
+            catch (Exception ex) when (cancellationToken.IsCancellationRequested)
+            {
+                throw new OperationCanceledException("Script execution was cancelled", ex);
+            }
+        }
+
     }
 }
diff --git a/source/Octopus.Tentacle.Client/Scripts/IScriptExecutor.cs b/source/Octopus.Tentacle.Client/Scripts/IScriptExecutor.cs
@@ -15,7 +15,7 @@ Task<ScriptExecutorResult> StartScript(ExecuteScriptCommand command,
 
         Task<ScriptExecutorResult> GetStatus(CommandContext commandContext, CancellationToken scriptExecutionCancellationToken);
 
-        Task<ScriptExecutorResult> CancelScript(CommandContext commandContext, CancellationToken scriptExecutionCancellationToken);
+        Task<ScriptExecutorResult> CancelScript(CommandContext commandContext);
 
         Task<ScriptStatus?> CompleteScript(CommandContext commandContext, CancellationToken scriptExecutionCancellationToken);
     }

diff --git a/source/Octopus.Tentacle.Client/Scripts/KubernetesScriptServiceV1Executor.cs b/source/Octopus.Tentacle.Client/Scripts/KubernetesScriptServiceV1Executor.cs
@@ -68,19 +68,12 @@ StartKubernetesScriptCommandV1 Map(ExecuteScriptCommand command)
                 kubernetesScriptCommand.Files.ToArray(),
                 kubernetesScriptCommand.IsRawScript);
         }
-        ScriptExecutorResult Map(KubernetesScriptStatusResponseV1 r)
-        {
-            return new (MapToScriptStatus(r), MapToContextForNextCommand(r));
-        }
-
-        ScriptStatus MapToScriptStatus(KubernetesScriptStatusResponseV1 scriptStatusResponse)
-        {
-            return new ScriptStatus(scriptStatusResponse.State, scriptStatusResponse.ExitCode, scriptStatusResponse.Logs);
-        }
 
-        CommandContext MapToContextForNextCommand(KubernetesScriptStatusResponseV1 scriptStatusResponse)
+        static ScriptExecutorResult Map(KubernetesScriptStatusResponseV1 scriptStatusResponse)
         {
-            return new CommandContext(scriptStatusResponse.ScriptTicket, scriptStatusResponse.NextLogSequence, ScriptServiceVersion.KubernetesScriptServiceVersion1);
+            return new(
+                new ScriptStatus(scriptStatusResponse.State, scriptStatusResponse.ExitCode, scriptStatusResponse.Logs),
+                new CommandContext(scriptStatusResponse.ScriptTicket, scriptStatusResponse.NextLogSequence, ScriptServiceVersion.KubernetesScriptServiceVersion1));
         }
 
         public async Task<ScriptExecutorResult> StartScript(ExecuteScriptCommand executeScriptCommand,
@@ -117,7 +110,7 @@ void OnErrorAction(Exception ex)
                     clientOperationMetricsBuilder,
                     scriptExecutionCancellationToken).ConfigureAwait(false);
 
-                return new (MapToScriptStatus(scriptStatusResponse), MapToContextForNextCommand(scriptStatusResponse));
+                return Map(scriptStatusResponse);
             }
             catch (Exception ex) when (scriptExecutionCancellationToken.IsCancellationRequested)
             {
@@ -131,9 +124,8 @@ void OnErrorAction(Exception ex)
 
                 if (!startScriptCallIsConnecting || startScriptCallIsBeingRetried)
                 {
-                    var scriptStatus = new ScriptStatus(ProcessState.Pending, null, new List<ProcessOutput>());
-                    var contextForNextCommand = new CommandContext(command.ScriptTicket, 0, ScriptServiceVersion.KubernetesScriptServiceVersion1);
-                    return new (scriptStatus, contextForNextCommand);
+                    // We want to observe and wait till we finish when this happens. Therefore, we want to return a result that will make the caller start that process.
+                    return ScriptExecutorResult.CreateWaitForStartedScriptResult(command.ScriptTicket, ScriptServiceVersion.KubernetesScriptServiceVersion1);
                 }
 
                 // If the StartScript call was not in-flight or being retries then we know the script has not started executing on Tentacle
@@ -162,7 +154,7 @@ async Task<KubernetesScriptStatusResponseV1> GetStatusAction(CancellationToken c
             return Map(kubernetesScriptStatusResponseV1);
         }
 
-        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse, CancellationToken scriptExecutionCancellationToken)
+        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse)
         {
             async Task<KubernetesScriptStatusResponseV1> CancelScriptAction(CancellationToken ct)
             {

diff --git a/source/Octopus.Tentacle.Client/Scripts/ObservingScriptOrchestrator.cs b/source/Octopus.Tentacle.Client/Scripts/ObservingScriptOrchestrator.cs
@@ -31,16 +31,28 @@ public async Task<ScriptExecutionResult> ExecuteScript(ExecuteScriptCommand comm
                 StartScriptIsBeingReAttempted.FirstAttempt, // This is not re-entrant so this should be true.
                 scriptExecutionCancellationToken).ConfigureAwait(false);
 
-            var scriptStatus = await ObserveUntilCompleteThenFinish(startScriptResult, scriptExecutionCancellationToken).ConfigureAwait(false);
-
-            if (scriptExecutionCancellationToken.IsCancellationRequested)
+            try
             {
-                // Throw an error so the caller knows that execution of the script was cancelled
-                throw new OperationCanceledException("Script execution was cancelled");
+                var scriptStatus = await ObserveUntilCompleteThenFinish(startScriptResult, scriptExecutionCancellationToken).ConfigureAwait(false);
+
+                if (scriptExecutionCancellationToken.IsCancellationRequested)
+                {
+                    // Throw an error so the caller knows that execution of the script was cancelled
+                    throw new OperationCanceledException("Script execution was cancelled");
+                }
+
+                return new ScriptExecutionResult(scriptStatus.State, scriptStatus.ExitCode!.Value);
             }
+            catch (Exception)
+            {
+                if (scriptExecutionCancellationToken.IsCancellationRequested)
+                {
+                    // Throw an error so the caller knows that execution of the script was cancelled
+                    throw new OperationCanceledException("Script execution was cancelled");
+                }
 
-
-            return new ScriptExecutionResult(scriptStatus.State, scriptStatus.ExitCode!.Value);
+                throw;
+            }
         }
 
         async Task<ScriptStatus> ObserveUntilCompleteThenFinish(
@@ -76,7 +88,7 @@ async Task<ScriptExecutorResult> ObserveUntilComplete(
             {
                 if (scriptExecutionCancellationToken.IsCancellationRequested)
                 {
-                    lastResult = await scriptExecutor.CancelScript(lastResult.ContextForNextCommand, scriptExecutionCancellationToken).ConfigureAwait(false);
+                    lastResult = await scriptExecutor.CancelScript(lastResult.ContextForNextCommand).ConfigureAwait(false);
                 }
                 else
                 {

diff --git a/source/Octopus.Tentacle.Client/Scripts/ScriptExecutorResult.cs b/source/Octopus.Tentacle.Client/Scripts/ScriptExecutorResult.cs
@@ -1,5 +1,6 @@
 using Octopus.Tentacle.Client.EventDriven;
 using Octopus.Tentacle.Contracts;
+using System.Collections.Generic;
 
 namespace Octopus.Tentacle.Client.Scripts
 {
@@ -13,5 +14,16 @@ public ScriptExecutorResult(ScriptStatus scriptStatus, CommandContext contextFor
             ScriptStatus = scriptStatus;
             ContextForNextCommand = contextForNextCommand;
         }
+
+        /// <summary>
+        /// Create a result object for when we have most likely started a script, but cancellation has started, and we want to wait for
+        /// this script to finish.
+        /// </summary>
+        internal static ScriptExecutorResult CreateWaitForStartedScriptResult(ScriptTicket scriptTicket, ScriptServiceVersion scripServiceVersionUsed)
+        {
+            var scriptStatus = new ScriptStatus(ProcessState.Pending, null, new List<ProcessOutput>());
+            var contextForNextCommand = new CommandContext(scriptTicket, 0, scripServiceVersionUsed);
+            return new(scriptStatus, contextForNextCommand);
+        }
     }
 }
diff --git a/source/Octopus.Tentacle.Client/Scripts/ScriptServiceV1Executor.cs b/source/Octopus.Tentacle.Client/Scripts/ScriptServiceV1Executor.cs
@@ -109,7 +109,7 @@ public async Task<ScriptExecutorResult> GetStatus(CommandContext commandContext,
             return Map(scriptStatusResponseV1);
         }
 
-        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse, CancellationToken scriptExecutionCancellationToken)
+        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse)
         {
             var response = await rpcCallExecutor.ExecuteWithNoRetries(
                 RpcCall.Create<IScriptService>(nameof(IScriptService.CancelScript)),

diff --git a/source/Octopus.Tentacle.Client/Scripts/ScriptServiceV2Executor.cs b/source/Octopus.Tentacle.Client/Scripts/ScriptServiceV2Executor.cs
@@ -123,9 +123,8 @@ void OnErrorAction(Exception ex)
 
                 if (!startScriptCallIsConnecting || startScriptCallIsBeingRetried)
                 {
-                    var scriptStatus = new ScriptStatus(ProcessState.Pending, null, new List<ProcessOutput>());
-                    var contextForNextCommand = new CommandContext(command.ScriptTicket, 0, ScriptServiceVersion.ScriptServiceVersion2);
-                    return new (scriptStatus, contextForNextCommand);
+                    // We want to observe and wait till we finish when this happens. Therefore, we want to return a result that will make the caller start that process.
+                    return ScriptExecutorResult.CreateWaitForStartedScriptResult(command.ScriptTicket, ScriptServiceVersion.ScriptServiceVersion2);
                 }
 
                 // If the StartScript call was not in-flight or being retries then we know the script has not started executing on Tentacle
@@ -134,6 +133,8 @@ void OnErrorAction(Exception ex)
             }
         }
 
+
+
         public async Task<ScriptExecutorResult> GetStatus(CommandContext commandContext, CancellationToken scriptExecutionCancellationToken)
         {
             async Task<ScriptStatusResponseV2> GetStatusAction(CancellationToken ct)
@@ -154,7 +155,7 @@ async Task<ScriptStatusResponseV2> GetStatusAction(CancellationToken ct)
             return Map(scriptStatusResponseV2);
         }
 
-        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse, CancellationToken scriptExecutionCancellationToken)
+        public async Task<ScriptExecutorResult> CancelScript(CommandContext lastStatusResponse)
         {
             async Task<ScriptStatusResponseV2> CancelScriptAction(CancellationToken ct)
             {