From 3e46f55ac9740a641a4e4e1837f90d6d48ff1f9a Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Wed, 4 Aug 2021 13:32:44 -0400 Subject: [PATCH] Prep for m280.1 release (#1245) * Finish job when worker crashed with IOException. (#1239) * fix continue on error (#1238) * Correctly set post step step context (#1243) * Release notes for 2.280.1 runner (#1244) Co-authored-by: Tingluo Huang --- releaseNote.md | 9 +-- src/Runner.Listener/JobDispatcher.cs | 73 ++++++++----------- src/Runner.Worker/ExecutionContext.cs | 24 ++++-- .../Handlers/CompositeActionHandler.cs | 36 ++++++--- src/runnerversion | 2 +- 5 files changed, 79 insertions(+), 65 deletions(-) diff --git a/releaseNote.md b/releaseNote.md index eaf80582a..d5d043a1f 100644 --- a/releaseNote.md +++ b/releaseNote.md @@ -1,17 +1,14 @@ ## Features -- Adds support for composite actions if the server supports it (#1222) -- Adds `generateIdTokenUri` to env variables for actions (#1234) - ## Bugs -- Prefer higher `libicu` versions in `installDependencies.sh` (#1228) +- Fixed a bug where composite actions did not respect `continue-on-error` (#1238) +- Fixed a bug where composite actions post steps did not have the correct step context (#1243) ## Misc -- Send step telemetry to server on JobCompletion (#1229) -- Print out the resolved SHA for each downloaded action (#1233) +- Correctly finish Job when worker crashes with IO Exceptions (#1239) ## Windows x64 We recommend configuring the runner in a root folder of the Windows drive (e.g. "C:\actions-runner"). This will help avoid issues related to service identity folder permissions and long file path restrictions on Windows. diff --git a/src/Runner.Listener/JobDispatcher.cs b/src/Runner.Listener/JobDispatcher.cs index e8ab41459..97422ee47 100644 --- a/src/Runner.Listener/JobDispatcher.cs +++ b/src/Runner.Listener/JobDispatcher.cs @@ -507,7 +507,20 @@ namespace GitHub.Runner.Listener { detailInfo = string.Join(Environment.NewLine, workerOutput); Trace.Info($"Return code {returnCode} indicate worker encounter an unhandled exception or app crash, attach worker stdout/stderr to JobRequest result."); - await LogWorkerProcessUnhandledException(message, detailInfo); + + var jobServer = HostContext.GetService(); + VssCredentials jobServerCredential = VssUtil.GetVssCredential(systemConnection); + VssConnection jobConnection = VssUtil.CreateConnection(systemConnection.Url, jobServerCredential); + await jobServer.ConnectAsync(jobConnection); + + await LogWorkerProcessUnhandledException(jobServer, message, detailInfo); + + // Go ahead to finish the job with result 'Failed' if the STDERR from worker is System.IO.IOException, since it typically means we are running out of disk space. + if (detailInfo.Contains(typeof(System.IO.IOException).ToString(), StringComparison.OrdinalIgnoreCase)) + { + Trace.Info($"Finish job with result 'Failed' due to IOException."); + await ForceFailJob(jobServer, message); + } } TaskResult result = TaskResultUtil.TranslateFromReturnCode(returnCode); @@ -915,53 +928,16 @@ namespace GitHub.Runner.Listener } // log an error issue to job level timeline record - private async Task LogWorkerProcessUnhandledException(Pipelines.AgentJobRequestMessage message, string errorMessage) + private async Task LogWorkerProcessUnhandledException(IJobServer jobServer, Pipelines.AgentJobRequestMessage message, string errorMessage) { try { - var systemConnection = message.Resources.Endpoints.SingleOrDefault(x => string.Equals(x.Name, WellKnownServiceEndpointNames.SystemVssConnection)); - ArgUtil.NotNull(systemConnection, nameof(systemConnection)); - - var jobServer = HostContext.GetService(); - VssCredentials jobServerCredential = VssUtil.GetVssCredential(systemConnection); - VssConnection jobConnection = VssUtil.CreateConnection(systemConnection.Url, jobServerCredential); - - /* Below is the legacy 'OnPremises' code that is currently unused by the runner - ToDo: re-implement code as appropriate once GHES support is added. - // Make sure SystemConnection Url match Config Url base for OnPremises server - if (!message.Variables.ContainsKey(Constants.Variables.System.ServerType) || - string.Equals(message.Variables[Constants.Variables.System.ServerType]?.Value, "OnPremises", StringComparison.OrdinalIgnoreCase)) - { - try - { - Uri result = null; - Uri configUri = new Uri(_runnerSetting.ServerUrl); - if (Uri.TryCreate(new Uri(configUri.GetComponents(UriComponents.SchemeAndServer, UriFormat.Unescaped)), jobServerUrl.PathAndQuery, out result)) - { - //replace the schema and host portion of messageUri with the host from the - //server URI (which was set at config time) - jobServerUrl = result; - } - } - catch (InvalidOperationException ex) - { - //cannot parse the Uri - not a fatal error - Trace.Error(ex); - } - catch (UriFormatException ex) - { - //cannot parse the Uri - not a fatal error - Trace.Error(ex); - } - } */ - - await jobServer.ConnectAsync(jobConnection); - var timeline = await jobServer.GetTimelineAsync(message.Plan.ScopeIdentifier, message.Plan.PlanType, message.Plan.PlanId, message.Timeline.Id, CancellationToken.None); - ArgUtil.NotNull(timeline, nameof(timeline)); + TimelineRecord jobRecord = timeline.Records.FirstOrDefault(x => x.Id == message.JobId && x.RecordType == "Job"); ArgUtil.NotNull(jobRecord, nameof(jobRecord)); + var unhandledExceptionIssue = new Issue() { Type = IssueType.Error, Message = errorMessage }; unhandledExceptionIssue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.WorkerCrash; jobRecord.ErrorCount++; @@ -975,6 +951,21 @@ namespace GitHub.Runner.Listener } } + // raise job completed event to fail the job. + private async Task ForceFailJob(IJobServer jobServer, Pipelines.AgentJobRequestMessage message) + { + try + { + var jobCompletedEvent = new JobCompletedEvent(message.RequestId, message.JobId, TaskResult.Failed); + await jobServer.RaisePlanEventAsync(message.Plan.ScopeIdentifier, message.Plan.PlanType, message.Plan.PlanId, jobCompletedEvent, CancellationToken.None); + } + catch (Exception ex) + { + Trace.Error("Fail to raise JobCompletedEvent back to service."); + Trace.Error(ex); + } + } + private class WorkerDispatcher : IDisposable { public long RequestId { get; } diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 61ee9d2c7..6df943f5d 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -38,6 +38,7 @@ namespace GitHub.Runner.Worker Guid Id { get; } Guid EmbeddedId { get; } string ScopeName { get; } + string SiblingScopeName { get; } string ContextName { get; } Task ForceCompleted { get; } TaskResult? Result { get; set; } @@ -74,8 +75,8 @@ namespace GitHub.Runner.Worker // Initialize void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token); void CancelToken(); - IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid)); - IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary intraActionState = null); + IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null); + IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary intraActionState = null, string siblingScopeName = null); // logging long Write(string tag, string message); @@ -140,6 +141,7 @@ namespace GitHub.Runner.Worker public Guid Id => _record.Id; public Guid EmbeddedId { get; private set; } public string ScopeName { get; private set; } + public string SiblingScopeName { get; private set; } public string ContextName { get; private set; } public Task ForceCompleted => _forceCompleted.Task; public CancellationToken CancellationToken => _cancellationTokenSource.Token; @@ -258,6 +260,7 @@ namespace GitHub.Runner.Worker public void RegisterPostJobStep(IStep step) { + string siblingScopeName = null; if (this.IsEmbedded) { if (step is IActionRunner actionRunner && !Root.EmbeddedStepsWithPostRegistered.Add(actionRunner.Action.Id)) @@ -271,12 +274,16 @@ namespace GitHub.Runner.Worker Trace.Info($"'post' of '{actionRunner.DisplayName}' already push to post step stack."); return; } + if (step is IActionRunner runner) + { + siblingScopeName = runner.Action.ContextName; + } - step.ExecutionContext = Root.CreatePostChild(step.DisplayName, IntraActionState); + step.ExecutionContext = Root.CreatePostChild(step.DisplayName, IntraActionState, siblingScopeName); Root.PostJobSteps.Push(step); } - public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid)) + public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null) { Trace.Entering(); @@ -286,6 +293,7 @@ namespace GitHub.Runner.Worker child.ScopeName = scopeName; child.ContextName = contextName; child.EmbeddedId = embeddedId; + child.SiblingScopeName = siblingScopeName; if (intraActionState == null) { child.IntraActionState = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -333,9 +341,9 @@ namespace GitHub.Runner.Worker /// An embedded execution context shares the same record ID, record name, logger, /// and a linked cancellation token. /// - public IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary intraActionState = null) + public IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary intraActionState = null, string siblingScopeName = null) { - return Root.CreateChild(_record.Id, _record.Name, _record.Id.ToString("N"), scopeName, contextName, logger: _logger, isEmbedded: true, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token), intraActionState: intraActionState, embeddedId: embeddedId); + return Root.CreateChild(_record.Id, _record.Name, _record.Id.ToString("N"), scopeName, contextName, logger: _logger, isEmbedded: true, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token), intraActionState: intraActionState, embeddedId: embeddedId, siblingScopeName: siblingScopeName); } public void Start(string currentOperation = null) @@ -914,7 +922,7 @@ namespace GitHub.Runner.Worker } } - private IExecutionContext CreatePostChild(string displayName, Dictionary intraActionState) + private IExecutionContext CreatePostChild(string displayName, Dictionary intraActionState, string siblingScopeName = null) { if (!_expandedForPostJob) { @@ -924,7 +932,7 @@ namespace GitHub.Runner.Worker } var newGuid = Guid.NewGuid(); - return CreateChild(newGuid, displayName, newGuid.ToString("N"), null, null, intraActionState, _childTimelineRecordOrder - Root.PostJobSteps.Count); + return CreateChild(newGuid, displayName, newGuid.ToString("N"), null, null, intraActionState, _childTimelineRecordOrder - Root.PostJobSteps.Count, siblingScopeName: siblingScopeName); } } diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 00e4ed295..89cc43d4e 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -120,7 +120,7 @@ namespace GitHub.Runner.Worker.Handlers // only relevant for local composite actions that need to JIT download/setup containers if (LocalActionContainerSetupSteps != null && LocalActionContainerSetupSteps.Count > 0) { - foreach(var step in LocalActionContainerSetupSteps) + foreach (var step in LocalActionContainerSetupSteps) { ArgUtil.NotNull(step, step.DisplayName); var stepId = $"__{Guid.NewGuid()}"; @@ -128,17 +128,31 @@ namespace GitHub.Runner.Worker.Handlers embeddedSteps.Add(step); } } - foreach (Pipelines.ActionStep stepData in steps) { + // Compute child sibling scope names for post steps + // We need to use the main's scope to keep step context correct, makes inputs flow correctly + string siblingScopeName = null; + if (!String.IsNullOrEmpty(ExecutionContext.SiblingScopeName) && stage == ActionRunStage.Post) + { + siblingScopeName = $"{ExecutionContext.SiblingScopeName}.{stepData.ContextName}"; + } + var step = HostContext.CreateService(); step.Action = stepData; step.Stage = stage; step.Condition = stepData.Condition; ExecutionContext.Root.EmbeddedIntraActionState.TryGetValue(step.Action.Id, out var intraActionState); - step.ExecutionContext = ExecutionContext.CreateEmbeddedChild(childScopeName, stepData.ContextName, step.Action.Id, intraActionState: intraActionState); + step.ExecutionContext = ExecutionContext.CreateEmbeddedChild(childScopeName, stepData.ContextName, step.Action.Id, intraActionState: intraActionState, siblingScopeName: siblingScopeName); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; - step.ExecutionContext.ExpressionValues["steps"] = ExecutionContext.Global.StepsContext.GetScope(childScopeName); + if (!String.IsNullOrEmpty(ExecutionContext.SiblingScopeName)) + { + step.ExecutionContext.ExpressionValues["steps"] = ExecutionContext.Global.StepsContext.GetScope(ExecutionContext.SiblingScopeName); + } + else + { + step.ExecutionContext.ExpressionValues["steps"] = ExecutionContext.Global.StepsContext.GetScope(childScopeName); + } // Shallow copy github context var gitHubContext = step.ExecutionContext.ExpressionValues["github"] as GitHubContext; @@ -153,7 +167,7 @@ namespace GitHub.Runner.Worker.Handlers } // Run embedded steps - await RunStepsAsync(embeddedSteps); + await RunStepsAsync(embeddedSteps, stage); // Set outputs ExecutionContext.ExpressionValues["inputs"] = inputsData; @@ -212,7 +226,7 @@ namespace GitHub.Runner.Worker.Handlers } } - private async Task RunStepsAsync(List embeddedSteps) + private async Task RunStepsAsync(List embeddedSteps, ActionRunStage stage) { ArgUtil.NotNull(embeddedSteps, nameof(embeddedSteps)); @@ -388,9 +402,13 @@ namespace GitHub.Runner.Worker.Handlers if (step.ExecutionContext.Result == TaskResult.Failed || step.ExecutionContext.Result == TaskResult.Canceled) { Trace.Info($"Update job result with current composite step result '{step.ExecutionContext.Result}'."); - ExecutionContext.Result = step.ExecutionContext.Result; - ExecutionContext.Root.Result = TaskResultUtil.MergeTaskResults(ExecutionContext.Root.Result, step.ExecutionContext.Result.Value); - ExecutionContext.Root.JobContext.Status = ExecutionContext.Root.Result?.ToActionResult(); + ExecutionContext.Result = TaskResultUtil.MergeTaskResults(ExecutionContext.Result, step.ExecutionContext.Result.Value); + + // We should run cleanup even if one of the cleanup step fails + if (stage != ActionRunStage.Post) + { + break; + } } } } diff --git a/src/runnerversion b/src/runnerversion index 26f0f8b9a..d11ab399e 100644 --- a/src/runnerversion +++ b/src/runnerversion @@ -1 +1 @@ -2.280.0 +2.280.1