Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Sps/token migration tweak, ActionResult casing. #462

Merged
merged 1 commit into from
May 11, 2020
Merged

Sps/token migration tweak, ActionResult casing. #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 42 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 @@ -164,9 +164,30 @@ public async Task<Boolean> CreateSessionAsync(CancellationToken token)
}
}

if (ex is TaskAgentSessionConflictException)
Copy link
Member Author

Choose a reason for hiding this comment

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

try migration even on session conflict since there are a small number of runners (might be orchestrated by some auto-provision/scaling tools) hitting session conflict all the time.

{
try
{
var newCred = await GetNewOAuthAuthorizationSetting(token, true);
if (newCred != null)
{
await _runnerServer.ConnectAsync(new Uri(_settings.ServerUrl), newCred);
Trace.Info("Updated connection to use migrated credential for next CreateSession call.");
_useMigratedCredentials = true;
_authorizationUrlMigrationBackgroundTask = null;
_needToCheckAuthorizationUrlUpdate = false;
}
}
catch (Exception e)
{
Trace.Error("Fail to refresh connection with new authorization url.");
Trace.Error(e);
}
}

if (!IsSessionCreationExceptionRetriable(ex))
{
if (_useMigratedCredentials)
if (_useMigratedCredentials && !(ex is TaskAgentSessionConflictException))
{
// migrated credentials might cause lose permission during permission check,
// we will force to use original credential and try again
Expand Down Expand Up @@ -516,14 +537,11 @@ ex is AccessDeniedException ||
}
}

private async Task<VssCredentials> GetNewOAuthAuthorizationSetting(CancellationToken token)
private async Task<VssCredentials> GetNewOAuthAuthorizationSetting(CancellationToken token, bool adhoc = false)
{
Trace.Info("Start checking oauth authorization url update.");
while (true)
{
var backoff = BackoffTimerHelper.GetRandomBackoff(TimeSpan.FromMinutes(30), TimeSpan.FromMinutes(45));
Copy link
Member Author

Choose a reason for hiding this comment

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

move the sleep to the end of the iteration, since most runner has finished migration. there are a small number of runners connected to the server once a while only for a few minutes, so the delay is too long for them.

await HostContext.Delay(backoff, token);

try
{
var migratedAuthorizationUrl = await _runnerServer.GetRunnerAuthUrlAsync(_settings.PoolId, _settings.AgentId);
Expand All @@ -538,7 +556,14 @@ private async Task<VssCredentials> GetNewOAuthAuthorizationSetting(CancellationT
{
// We don't need to update credentials.
Trace.Info("No needs to update authorization url");
await Task.Delay(TimeSpan.FromMilliseconds(-1), token);
if (adhoc)
{
return null;
}
else
{
await Task.Delay(TimeSpan.FromMilliseconds(-1), token);
}
}

var keyManager = HostContext.GetService<IRSAKeyManager>();
Expand Down Expand Up @@ -572,7 +597,7 @@ private async Task<VssCredentials> GetNewOAuthAuthorizationSetting(CancellationT
Trace.Verbose("No authorization url updates");
}
}
catch (Exception ex)
catch (Exception ex) when (!token.IsCancellationRequested)
Copy link
Member Author

Choose a reason for hiding this comment

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

don't report migration error when user ctrl-c the runner.

{
Trace.Error("Fail to get/test new authorization url.");
Trace.Error(ex);
Expand All @@ -588,6 +613,16 @@ private async Task<VssCredentials> GetNewOAuthAuthorizationSetting(CancellationT
Trace.Error(e);
}
}

if (adhoc)
{
return null;
}
else
{
var backoff = BackoffTimerHelper.GetRandomBackoff(TimeSpan.FromMinutes(30), TimeSpan.FromMinutes(45));
await HostContext.Delay(backoff, token);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Runner.Worker/ExecutionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,8 @@ public TaskResult Complete(TaskResult? result = null, string currentOperation =

if (!string.IsNullOrEmpty(ContextName))
{
StepsContext.SetOutcome(ScopeName, ContextName, (Outcome ?? Result ?? TaskResult.Succeeded).ToActionResult().ToString());
StepsContext.SetConclusion(ScopeName, ContextName, (Result ?? TaskResult.Succeeded).ToActionResult().ToString());
StepsContext.SetOutcome(ScopeName, ContextName, (Outcome ?? Result ?? TaskResult.Succeeded).ToActionResult());
StepsContext.SetConclusion(ScopeName, ContextName, (Result ?? TaskResult.Succeeded).ToActionResult());
}

return Result.Value;
Expand Down
2 changes: 1 addition & 1 deletion src/Runner.Worker/JobContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public ActionResult? Status
}
set
{
this["status"] = new StringContextData(value.ToString());
this["status"] = new StringContextData(value.ToString().ToLowerInvariant());
Copy link
Member Author

Choose a reason for hiding this comment

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

change to lower case to match GitHub check result

}
}

Expand Down
8 changes: 4 additions & 4 deletions src/Runner.Worker/StepsContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ public void SetOutput(
public void SetConclusion(
string scopeName,
string stepName,
string conclusion)
ActionResult conclusion)
{
var step = GetStep(scopeName, stepName);
step["conclusion"] = new StringContextData(conclusion);
step["conclusion"] = new StringContextData(conclusion.ToString().ToLowerInvariant());
Copy link
Member Author

Choose a reason for hiding this comment

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

change to lower case to match GitHub check result

}

public void SetOutcome(
string scopeName,
string stepName,
string outcome)
ActionResult outcome)
{
var step = GetStep(scopeName, stepName);
step["outcome"] = new StringContextData(outcome);
step["outcome"] = new StringContextData(outcome.ToString().ToLowerInvariant());
Copy link
Member Author

Choose a reason for hiding this comment

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

change to lower case to match GitHub check result

}

private DictionaryContextData GetStep(string scopeName, string stepName)
Expand Down
57 changes: 56 additions & 1 deletion src/Test/L0/Worker/ExecutionContextL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using GitHub.DistributedTask.WebApi;
using GitHub.DistributedTask.Pipelines.ContextData;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Worker;
using Moq;
using System;
Expand Down Expand Up @@ -323,6 +324,60 @@ public void RegisterPostJobAction_NotRegisterPostTwice()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void ActionResult_Lowercase()
{
using (TestHostContext hc = CreateTestContext())
{
TaskOrchestrationPlanReference plan = new TaskOrchestrationPlanReference();
TimelineReference timeline = new TimelineReference();
Guid jobId = Guid.NewGuid();
string jobName = "some job name";
var jobRequest = new Pipelines.AgentJobRequestMessage(plan, timeline, jobId, jobName, jobName, null, null, null, new Dictionary<string, VariableValue>(), new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null);
jobRequest.Resources.Repositories.Add(new Pipelines.RepositoryResource()
{
Alias = Pipelines.PipelineConstants.SelfAlias,
Id = "github",
Version = "sha1"
});
jobRequest.ContextData["github"] = new Pipelines.ContextData.DictionaryContextData();
jobRequest.Variables["ACTIONS_STEP_DEBUG"] = "true";

// Arrange: Setup the paging logger.
var pagingLogger1 = new Mock<IPagingLogger>();
var jobServerQueue = new Mock<IJobServerQueue>();
hc.EnqueueInstance(pagingLogger1.Object);
hc.SetSingleton(jobServerQueue.Object);

var jobContext = new Runner.Worker.ExecutionContext();
jobContext.Initialize(hc);

// Act.
jobContext.InitializeJob(jobRequest, CancellationToken.None);

jobContext.StepsContext.SetConclusion(null, "step1", ActionResult.Success);
var conclusion1 = (jobContext.StepsContext.GetScope(null)["step1"] as DictionaryContextData)["conclusion"].ToString();
Assert.Equal(conclusion1, conclusion1.ToLowerInvariant());

jobContext.StepsContext.SetOutcome(null, "step2", ActionResult.Cancelled);
var outcome1 = (jobContext.StepsContext.GetScope(null)["step2"] as DictionaryContextData)["outcome"].ToString();
Assert.Equal(outcome1, outcome1.ToLowerInvariant());

jobContext.StepsContext.SetConclusion(null, "step3", ActionResult.Failure);
var conclusion2 = (jobContext.StepsContext.GetScope(null)["step3"] as DictionaryContextData)["conclusion"].ToString();
Assert.Equal(conclusion2, conclusion2.ToLowerInvariant());

jobContext.StepsContext.SetOutcome(null, "step4", ActionResult.Skipped);
var outcome2 = (jobContext.StepsContext.GetScope(null)["step4"] as DictionaryContextData)["outcome"].ToString();
Assert.Equal(outcome2, outcome2.ToLowerInvariant());

jobContext.JobContext.Status = ActionResult.Success;
Assert.Equal(jobContext.JobContext["status"].ToString(), jobContext.JobContext["status"].ToString().ToLowerInvariant());
}
}

private TestHostContext CreateTestContext([CallerMemberName] String testName = "")
{
var hc = new TestHostContext(this, testName);
Expand Down
28 changes: 14 additions & 14 deletions src/Test/L0/Worker/StepsRunnerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,12 +536,12 @@ public async Task StepContextOutcome()
step2.Verify(x => x.RunAsync(), Times.Once);
step3.Verify(x => x.RunAsync(), Times.Once);

Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Failed.ToActionResult().ToString(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Failed.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["conclusion"].AssertString(""));

}
}
Expand Down Expand Up @@ -572,12 +572,12 @@ public async Task StepContextConclusion()
step2.Verify(x => x.RunAsync(), Times.Once);
step3.Verify(x => x.RunAsync(), Times.Once);

Assert.Equal(TaskResult.Skipped.ToActionResult().ToString(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Skipped.ToActionResult().ToString(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Failed.ToActionResult().ToString(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Skipped.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Skipped.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step1"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Failed.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step2"].AssertDictionary("")["conclusion"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["outcome"].AssertString(""));
Assert.Equal(TaskResult.Succeeded.ToActionResult().ToString().ToLowerInvariant(), _stepContext.GetScope(null)["step3"].AssertDictionary("")["conclusion"].AssertString(""));
}
}

Expand Down Expand Up @@ -615,8 +615,8 @@ private Mock<IActionRunner> CreateStep(TestHostContext hc, TaskResult result, st
stepContext.Object.Result = r;
}

_stepContext.SetOutcome("", stepContext.Object.ContextName, (stepContext.Object.Outcome ?? stepContext.Object.Result ?? TaskResult.Succeeded).ToActionResult().ToString());
_stepContext.SetConclusion("", stepContext.Object.ContextName, (stepContext.Object.Result ?? TaskResult.Succeeded).ToActionResult().ToString());
_stepContext.SetOutcome("", stepContext.Object.ContextName, (stepContext.Object.Outcome ?? stepContext.Object.Result ?? TaskResult.Succeeded).ToActionResult());
_stepContext.SetConclusion("", stepContext.Object.ContextName, (stepContext.Object.Result ?? TaskResult.Succeeded).ToActionResult());
});
var trace = hc.GetTrace();
stepContext.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>())).Callback((string tag, string message) => { trace.Info($"[{tag}]{message}"); });
Expand Down