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.

Sign up for GitHub

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

Jump to bottom

Add new SessionConflict return code #3215

Merged
merged 5 commits into from
Mar 27, 2024
Merged

Add new SessionConflict return code #3215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7df3a1
Add new SessionConflict return code
eeSquared Mar 21, 2024
877490b
formatting
eeSquared Mar 21, 2024
2eae3e0
Change return type of CreateSessionAsync to new enum
eeSquared Mar 26, 2024
cff9972
Update entry scripts to handle new exit code
eeSquared Mar 26, 2024
f5e7b23
Move enum
eeSquared Mar 27, 2024
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
5 changes: 5 additions & 0 deletions src/Misc/layoutbin/RunnerService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down
5 changes: 5 additions & 0 deletions src/Misc/layoutroot/run-helper.cmd.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to get a review from Ting regarding these scripts. I looked over the changes and makes sense to me.

Odd that we return exit code 0 here but that looks to be consistent with other non-retriable errors.

)

echo "Exiting after unknown error code: %ERRORLEVEL%"
exit /b 0
3 changes: 3 additions & 0 deletions src/Misc/layoutroot/run-helper.sh.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/Runner.Common/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public static class ReturnCode
public const int RetryableError = 2;
public const int RunnerUpdating = 3;
public const int RunOnceRunnerUpdating = 4;
public const int SessionConflict = 5;
TingluoHuang marked this conversation as resolved.
Show resolved Hide resolved
}

public static class Features
Expand Down
14 changes: 9 additions & 5 deletions src/Runner.Listener/BrokerMessageListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public override void Initialize(IHostContext hostContext)
_brokerServer = HostContext.GetService<IBrokerServer>();
}

public async Task<Boolean> CreateSessionAsync(CancellationToken token)
public async Task<CreateSessionResult> CreateSessionAsync(CancellationToken token)
{
Trace.Entering();

Expand Down Expand Up @@ -99,7 +99,7 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
encounteringError = false;
}

return true;
return CreateSessionResult.Success;
}
catch (OperationCanceledException) when (token.IsCancellationRequested)
{
Expand All @@ -123,7 +123,7 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
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.
Expand All @@ -134,14 +134,18 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
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
Expand Down
25 changes: 18 additions & 7 deletions src/Runner.Listener/MessageListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@

namespace GitHub.Runner.Listener
{
public enum CreateSessionResult
{
Success,
Failure,
SessionConflict
}

[ServiceLocator(Default = typeof(MessageListener))]
public interface IMessageListener : IRunnerService
{
Task<Boolean> CreateSessionAsync(CancellationToken token);
Task<CreateSessionResult> CreateSessionAsync(CancellationToken token);
Task DeleteSessionAsync();
Task<TaskAgentMessage> GetNextMessageAsync(CancellationToken token);
Task DeleteMessageAsync(TaskAgentMessage message);
Expand Down Expand Up @@ -59,7 +66,7 @@ public override void Initialize(IHostContext hostContext)
_brokerServer = hostContext.GetService<IBrokerServer>();
}

public async Task<Boolean> CreateSessionAsync(CancellationToken token)
public async Task<CreateSessionResult> CreateSessionAsync(CancellationToken token)
{
Trace.Entering();

Expand Down Expand Up @@ -123,7 +130,7 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
encounteringError = false;
}

return true;
return CreateSessionResult.Success;
}
catch (OperationCanceledException) when (token.IsCancellationRequested)
{
Expand All @@ -147,7 +154,7 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
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.
Expand All @@ -158,14 +165,18 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
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
Expand Down Expand Up @@ -303,7 +314,7 @@ public async Task<TaskAgentMessage> GetNextMessageAsync(CancellationToken token)
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.");
}
Expand Down
7 changes: 6 additions & 1 deletion src/Runner.Listener/Runner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,12 @@ private async Task<int> RunAsync(RunnerSettings settings, bool runOnce = false)
{
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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Test/L0/Listener/BrokerMessageListenerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ public async void CreatesSession()
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<TaskAgentSession>(y => y != null),
Expand Down
32 changes: 16 additions & 16 deletions src/Test/L0/Listener/MessageListenerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ public async void CreatesSession()
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,
Expand Down Expand Up @@ -135,11 +135,11 @@ public async void CreatesSessionWithBrokerMigration()
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(
Expand Down Expand Up @@ -185,8 +185,8 @@ public async void DeleteSession()
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(
Expand Down Expand Up @@ -245,10 +245,10 @@ public async void DeleteSessionWithBrokerMigration()
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(
Expand Down Expand Up @@ -309,8 +309,8 @@ public async void GetNextMessage()
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[]
{
Expand Down Expand Up @@ -390,8 +390,8 @@ public async void GetNextMessageWithBrokerMigration()
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"));

Expand Down Expand Up @@ -497,11 +497,11 @@ public async void CreateSessionWithOriginalCredential()
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,
Expand Down Expand Up @@ -541,8 +541,8 @@ public async void SkipDeleteSession_WhenGetNextMessageGetTaskAgentAccessTokenExp
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(
Expand Down
12 changes: 6 additions & 6 deletions src/Test/L0/Listener/RunnerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public async void TestRunAsync()
_configurationManager.Setup(x => x.IsConfigured())
.Returns(true);
_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult<bool>(true));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Success));
_messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny<CancellationToken>()))
.Returns(async () =>
{
Expand Down Expand Up @@ -184,7 +184,7 @@ public async void TestExecuteCommandForRunAsService(string[] args, bool configur
_configStore.Setup(x => x.IsServiceConfigured()).Returns(configureAsService);

_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult(false));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Failure));

var runner = new Runner.Listener.Runner();
runner.Initialize(hc);
Expand Down Expand Up @@ -217,7 +217,7 @@ public async void TestMachineProvisionerCLI()
.Returns(false);

_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult(false));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Failure));

var runner = new Runner.Listener.Runner();
runner.Initialize(hc);
Expand Down Expand Up @@ -263,7 +263,7 @@ public async void TestRunOnce()
_configurationManager.Setup(x => x.IsConfigured())
.Returns(true);
_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult<bool>(true));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Success));
_messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny<CancellationToken>()))
.Returns(async () =>
{
Expand Down Expand Up @@ -363,7 +363,7 @@ public async void TestRunOnceOnlyTakeOneJobMessage()
_configurationManager.Setup(x => x.IsConfigured())
.Returns(true);
_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult<bool>(true));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Success));
_messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny<CancellationToken>()))
.Returns(async () =>
{
Expand Down Expand Up @@ -458,7 +458,7 @@ public async void TestRunOnceHandleUpdateMessage()
_configurationManager.Setup(x => x.IsConfigured())
.Returns(true);
_messageListener.Setup(x => x.CreateSessionAsync(It.IsAny<CancellationToken>()))
.Returns(Task.FromResult<bool>(true));
.Returns(Task.FromResult<CreateSessionResult>(CreateSessionResult.Success));
_messageListener.Setup(x => x.GetNextMessageAsync(It.IsAny<CancellationToken>()))
.Returns(async () =>
{
Expand Down
Loading