From f467e9e1255530d3bf2e33f580d041925ab01951 Mon Sep 17 00:00:00 2001 From: eeSquared Date: Wed, 27 Mar 2024 14:49:58 -0400 Subject: [PATCH] Add new SessionConflict return code (#3215) * Add new SessionConflict return code * formatting * Change return type of CreateSessionAsync to new enum * Update entry scripts to handle new exit code * Move enum --- src/Misc/layoutbin/RunnerService.js | 5 +++ src/Misc/layoutroot/run-helper.cmd.template | 5 +++ src/Misc/layoutroot/run-helper.sh.template | 3 ++ src/Runner.Common/Constants.cs | 1 + src/Runner.Listener/BrokerMessageListener.cs | 14 +++++--- src/Runner.Listener/MessageListener.cs | 25 +++++++++++---- src/Runner.Listener/Runner.cs | 7 +++- .../L0/Listener/BrokerMessageListenerL0.cs | 4 +-- src/Test/L0/Listener/MessageListenerL0.cs | 32 +++++++++---------- src/Test/L0/Listener/RunnerL0.cs | 12 +++---- 10 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/Misc/layoutbin/RunnerService.js b/src/Misc/layoutbin/RunnerService.js index ba0a8c659..1024e8a5e 100644 --- a/src/Misc/layoutbin/RunnerService.js +++ b/src/Misc/layoutbin/RunnerService.js @@ -114,6 +114,11 @@ var runService = function () { ); stopping = true; } + } else if (code === 5) { + console.log( + "Runner listener exit with Session Conflict error, stop the service, no retry needed." + ); + stopping = true; } else { var messagePrefix = "Runner listener exit with undefined return code"; unknownFailureRetryCount++; diff --git a/src/Misc/layoutroot/run-helper.cmd.template b/src/Misc/layoutroot/run-helper.cmd.template index 221e8b1c0..6b594d4f3 100644 --- a/src/Misc/layoutroot/run-helper.cmd.template +++ b/src/Misc/layoutroot/run-helper.cmd.template @@ -49,5 +49,10 @@ if %ERRORLEVEL% EQU 4 ( exit /b 1 ) +if %ERRORLEVEL% EQU 5 ( + echo "Runner listener exit with Session Conflict error, stop the service, no retry needed." + exit /b 0 +) + echo "Exiting after unknown error code: %ERRORLEVEL%" exit /b 0 \ No newline at end of file diff --git a/src/Misc/layoutroot/run-helper.sh.template b/src/Misc/layoutroot/run-helper.sh.template index 743fd8b69..9f2b3cc44 100755 --- a/src/Misc/layoutroot/run-helper.sh.template +++ b/src/Misc/layoutroot/run-helper.sh.template @@ -70,6 +70,9 @@ elif [[ $returnCode == 4 ]]; then "$DIR"/safe_sleep.sh 1 done exit 2 +elif [[ $returnCode == 5 ]]; then + echo "Runner listener exit with Session Conflict error, stop the service, no retry needed." + exit 0 else echo "Exiting with unknown error code: ${returnCode}" exit 0 diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 98d9871c8..9378104c4 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -153,6 +153,7 @@ namespace GitHub.Runner.Common public const int RetryableError = 2; public const int RunnerUpdating = 3; public const int RunOnceRunnerUpdating = 4; + public const int SessionConflict = 5; } public static class Features diff --git a/src/Runner.Listener/BrokerMessageListener.cs b/src/Runner.Listener/BrokerMessageListener.cs index 6767d0beb..8f44fe843 100644 --- a/src/Runner.Listener/BrokerMessageListener.cs +++ b/src/Runner.Listener/BrokerMessageListener.cs @@ -42,7 +42,7 @@ namespace GitHub.Runner.Listener _brokerServer = HostContext.GetService(); } - public async Task CreateSessionAsync(CancellationToken token) + public async Task CreateSessionAsync(CancellationToken token) { Trace.Entering(); @@ -99,7 +99,7 @@ namespace GitHub.Runner.Listener encounteringError = false; } - return true; + return CreateSessionResult.Success; } catch (OperationCanceledException) when (token.IsCancellationRequested) { @@ -123,7 +123,7 @@ namespace GitHub.Runner.Listener if (string.Equals(vssOAuthEx.Error, "invalid_client", StringComparison.OrdinalIgnoreCase)) { _term.WriteError("Failed to create a session. The runner registration has been deleted from the server, please re-configure. Runner registrations are automatically deleted for runners that have not connected to the service recently."); - return false; + return CreateSessionResult.Failure; } // Check whether we get 401 because the runner registration already removed by the service. @@ -134,14 +134,18 @@ namespace GitHub.Runner.Listener if (string.Equals(authError, "invalid_client", StringComparison.OrdinalIgnoreCase)) { _term.WriteError("Failed to create a session. The runner registration has been deleted from the server, please re-configure. Runner registrations are automatically deleted for runners that have not connected to the service recently."); - return false; + return CreateSessionResult.Failure; } } if (!IsSessionCreationExceptionRetriable(ex)) { _term.WriteError($"Failed to create session. {ex.Message}"); - return false; + if (ex is TaskAgentSessionConflictException) + { + return CreateSessionResult.SessionConflict; + } + return CreateSessionResult.Failure; } if (!encounteringError) //print the message only on the first error diff --git a/src/Runner.Listener/MessageListener.cs b/src/Runner.Listener/MessageListener.cs index ab91cff05..403c2bfc7 100644 --- a/src/Runner.Listener/MessageListener.cs +++ b/src/Runner.Listener/MessageListener.cs @@ -18,10 +18,17 @@ using GitHub.Services.WebApi; namespace GitHub.Runner.Listener { + public enum CreateSessionResult + { + Success, + Failure, + SessionConflict + } + [ServiceLocator(Default = typeof(MessageListener))] public interface IMessageListener : IRunnerService { - Task CreateSessionAsync(CancellationToken token); + Task CreateSessionAsync(CancellationToken token); Task DeleteSessionAsync(); Task GetNextMessageAsync(CancellationToken token); Task DeleteMessageAsync(TaskAgentMessage message); @@ -59,7 +66,7 @@ namespace GitHub.Runner.Listener _brokerServer = hostContext.GetService(); } - public async Task CreateSessionAsync(CancellationToken token) + public async Task CreateSessionAsync(CancellationToken token) { Trace.Entering(); @@ -123,7 +130,7 @@ namespace GitHub.Runner.Listener encounteringError = false; } - return true; + return CreateSessionResult.Success; } catch (OperationCanceledException) when (token.IsCancellationRequested) { @@ -147,7 +154,7 @@ namespace GitHub.Runner.Listener if (string.Equals(vssOAuthEx.Error, "invalid_client", StringComparison.OrdinalIgnoreCase)) { _term.WriteError("Failed to create a session. The runner registration has been deleted from the server, please re-configure. Runner registrations are automatically deleted for runners that have not connected to the service recently."); - return false; + return CreateSessionResult.Failure; } // Check whether we get 401 because the runner registration already removed by the service. @@ -158,14 +165,18 @@ namespace GitHub.Runner.Listener if (string.Equals(authError, "invalid_client", StringComparison.OrdinalIgnoreCase)) { _term.WriteError("Failed to create a session. The runner registration has been deleted from the server, please re-configure. Runner registrations are automatically deleted for runners that have not connected to the service recently."); - return false; + return CreateSessionResult.Failure; } } if (!IsSessionCreationExceptionRetriable(ex)) { _term.WriteError($"Failed to create session. {ex.Message}"); - return false; + if (ex is TaskAgentSessionConflictException) + { + return CreateSessionResult.SessionConflict; + } + return CreateSessionResult.Failure; } if (!encounteringError) //print the message only on the first error @@ -303,7 +314,7 @@ namespace GitHub.Runner.Listener Trace.Error(ex); // don't retry if SkipSessionRecover = true, DT service will delete agent session to stop agent from taking more jobs. - if (ex is TaskAgentSessionExpiredException && !_settings.SkipSessionRecover && await CreateSessionAsync(token)) + if (ex is TaskAgentSessionExpiredException && !_settings.SkipSessionRecover && (await CreateSessionAsync(token) == CreateSessionResult.Success)) { Trace.Info($"{nameof(TaskAgentSessionExpiredException)} received, recovered by recreate session."); } diff --git a/src/Runner.Listener/Runner.cs b/src/Runner.Listener/Runner.cs index e38cd0570..9acad8395 100644 --- a/src/Runner.Listener/Runner.cs +++ b/src/Runner.Listener/Runner.cs @@ -359,7 +359,12 @@ namespace GitHub.Runner.Listener { Trace.Info(nameof(RunAsync)); _listener = GetMesageListener(settings); - if (!await _listener.CreateSessionAsync(HostContext.RunnerShutdownToken)) + CreateSessionResult createSessionResult = await _listener.CreateSessionAsync(HostContext.RunnerShutdownToken); + if (createSessionResult == CreateSessionResult.SessionConflict) + { + return Constants.Runner.ReturnCode.SessionConflict; + } + else if (createSessionResult == CreateSessionResult.Failure) { return Constants.Runner.ReturnCode.TerminatedError; } diff --git a/src/Test/L0/Listener/BrokerMessageListenerL0.cs b/src/Test/L0/Listener/BrokerMessageListenerL0.cs index 7dface3b2..64a71515c 100644 --- a/src/Test/L0/Listener/BrokerMessageListenerL0.cs +++ b/src/Test/L0/Listener/BrokerMessageListenerL0.cs @@ -56,11 +56,11 @@ namespace GitHub.Runner.Common.Tests.Listener BrokerMessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); trace.Info("result: {0}", result); // Assert. - Assert.True(result); + Assert.Equal(CreateSessionResult.Success, result); _brokerServer .Verify(x => x.CreateSessionAsync( It.Is(y => y != null), diff --git a/src/Test/L0/Listener/MessageListenerL0.cs b/src/Test/L0/Listener/MessageListenerL0.cs index b6837ad9a..f44d49889 100644 --- a/src/Test/L0/Listener/MessageListenerL0.cs +++ b/src/Test/L0/Listener/MessageListenerL0.cs @@ -75,11 +75,11 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); trace.Info("result: {0}", result); // Assert. - Assert.True(result); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Verify(x => x.CreateAgentSessionAsync( _settings.PoolId, @@ -135,11 +135,11 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); trace.Info("result: {0}", result); // Assert. - Assert.True(result); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Verify(x => x.CreateAgentSessionAsync( @@ -185,8 +185,8 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); - Assert.True(result); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Setup(x => x.DeleteAgentSessionAsync( @@ -245,10 +245,10 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); trace.Info("result: {0}", result); - Assert.True(result); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Verify(x => x.CreateAgentSessionAsync( @@ -309,8 +309,8 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); - Assert.True(result); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); + Assert.Equal(CreateSessionResult.Success, result); var arMessages = new TaskAgentMessage[] { @@ -390,8 +390,8 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); - Assert.True(result); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); + Assert.Equal(CreateSessionResult.Success, result); var brokerMigrationMesage = new BrokerMigrationMessage(new Uri("https://actions.broker.com")); @@ -497,11 +497,11 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); trace.Info("result: {0}", result); // Assert. - Assert.True(result); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Verify(x => x.CreateAgentSessionAsync( _settings.PoolId, @@ -541,8 +541,8 @@ namespace GitHub.Runner.Common.Tests.Listener MessageListener listener = new(); listener.Initialize(tc); - bool result = await listener.CreateSessionAsync(tokenSource.Token); - Assert.True(result); + CreateSessionResult result = await listener.CreateSessionAsync(tokenSource.Token); + Assert.Equal(CreateSessionResult.Success, result); _runnerServer .Setup(x => x.GetAgentMessageAsync( diff --git a/src/Test/L0/Listener/RunnerL0.cs b/src/Test/L0/Listener/RunnerL0.cs index 9c57f2adc..10b6d8c0f 100644 --- a/src/Test/L0/Listener/RunnerL0.cs +++ b/src/Test/L0/Listener/RunnerL0.cs @@ -88,7 +88,7 @@ namespace GitHub.Runner.Common.Tests.Listener _configurationManager.Setup(x => x.IsConfigured()) .Returns(true); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(CreateSessionResult.Success)); _messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny())) .Returns(async () => { @@ -184,7 +184,7 @@ namespace GitHub.Runner.Common.Tests.Listener _configStore.Setup(x => x.IsServiceConfigured()).Returns(configureAsService); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(CreateSessionResult.Failure)); var runner = new Runner.Listener.Runner(); runner.Initialize(hc); @@ -217,7 +217,7 @@ namespace GitHub.Runner.Common.Tests.Listener .Returns(false); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(CreateSessionResult.Failure)); var runner = new Runner.Listener.Runner(); runner.Initialize(hc); @@ -263,7 +263,7 @@ namespace GitHub.Runner.Common.Tests.Listener _configurationManager.Setup(x => x.IsConfigured()) .Returns(true); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(CreateSessionResult.Success)); _messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny())) .Returns(async () => { @@ -363,7 +363,7 @@ namespace GitHub.Runner.Common.Tests.Listener _configurationManager.Setup(x => x.IsConfigured()) .Returns(true); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(CreateSessionResult.Success)); _messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny())) .Returns(async () => { @@ -458,7 +458,7 @@ namespace GitHub.Runner.Common.Tests.Listener _configurationManager.Setup(x => x.IsConfigured()) .Returns(true); _messageListener.Setup(x => x.CreateSessionAsync(It.IsAny())) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(CreateSessionResult.Success)); _messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny())) .Returns(async () => {