From 3251eadacbd15741db652e396ac443823ebd6b03 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Mon, 13 Jul 2020 18:14:53 -0400 Subject: [PATCH 01/52] Add basic framework for baby steps runner --- src/Runner.Worker/CompositeStepsRunner.cs | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 src/Runner.Worker/CompositeStepsRunner.cs diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs new file mode 100644 index 00000000000..88ee683d633 --- /dev/null +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -0,0 +1,46 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using GitHub.DistributedTask.Expressions2; +using GitHub.DistributedTask.ObjectTemplating.Tokens; +using GitHub.DistributedTask.Pipelines; +using GitHub.DistributedTask.Pipelines.ContextData; +using GitHub.DistributedTask.Pipelines.ObjectTemplating; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common; +using GitHub.Runner.Common.Util; +using GitHub.Runner.Sdk; +using GitHub.Runner.Worker.Expressions; +using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating; +using Pipelines = GitHub.DistributedTask.Pipelines; + +namespace GitHub.Runner.Worker +{ + [ServiceLocator(Default = typeof(StepsRunner))] + public interface ICompositeStepsRunner : IRunnerService + { + Task RunAsync(IExecutionContext Context); + } + + + public sealed class CompositeStepsRunner : RunnerService, ICompositeStepsRunner + { + public async Task RunAsync(IExecutionContext actionContext) + { + ArgUtil.NotNull(actionContext, nameof(actionContext)); + ArgUtil.NotNull(actionContext.JobSteps, nameof(actionContext.CompositeSteps)); + + // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this + + // Add status logic here (start when composite action steps start) + // End when clean up is done + + // TODO: Remove Composite Action logic from StepsRunner and move it here + + // Namely, we need to move the envContext logic! + + } + +} \ No newline at end of file From 06adbaebbe53bd1af47380b066dece5dbcde5896 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 14:06:25 -0400 Subject: [PATCH 02/52] Basic logic for adding steps / invoking composite action steps --- src/Runner.Worker/CompositeStepsRunner.cs | 2 +- src/Runner.Worker/ExecutionContext.cs | 9 +++++- .../Handlers/CompositeActionHandler.cs | 30 +++++++++++++++++-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 88ee683d633..49166032ff8 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -30,7 +30,7 @@ public sealed class CompositeStepsRunner : RunnerService, ICompositeStepsRunner public async Task RunAsync(IExecutionContext actionContext) { ArgUtil.NotNull(actionContext, nameof(actionContext)); - ArgUtil.NotNull(actionContext.JobSteps, nameof(actionContext.CompositeSteps)); + ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index b31dc97e4c3..91fe0431090 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -72,6 +72,9 @@ public interface IExecutionContext : IRunnerService IExecutionContext FinalizeContext { get; set; } + // Only Composite Level ExecutionContext has CompositeSteps + List CompositeSteps { get; } + // Initialize void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token); void CancelToken(); @@ -172,6 +175,9 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext // Only job level ExecutionContext has StepsWithPostRegistered public HashSet StepsWithPostRegistered { get; private set; } + // Only Composite Level ExecutionContext has CompositeSteps + public List CompositeSteps { get; private set; } + public bool EchoOnActionCommand { get; set; } public IExecutionContext FinalizeContext { get; set; } @@ -315,7 +321,8 @@ public IStep RegisterNestedStep( } step.ExecutionContext.ExpressionValues["env"] = envContext; - Root.JobSteps.Insert(location, step); + // TODO Erase this! + // Root.JobSteps.Insert(location, step); return step; } diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index d48f922d177..963889b1575 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -23,7 +23,7 @@ public sealed class CompositeActionHandler : Handler, ICompositeActionHandler { public CompositeActionExecutionData Data { get; set; } - public Task RunAsync(ActionRunStage stage) + public async Task RunAsync(ActionRunStage stage) { // Validate args. Trace.Entering(); @@ -48,6 +48,10 @@ public Task RunAsync(ActionRunStage stage) // Add each composite action step to the front of the queue int location = 0; + // Initialize Steps Runner for Composite Steps and Environment for it + var compositeContext = HostContext.CreateService(); + var compositeStepsRunner = HostContext.CreateService(); + foreach (Pipelines.ActionStep aStep in actionSteps) { // Ex: @@ -86,9 +90,12 @@ public Task RunAsync(ActionRunStage stage) actionRunner.Condition = aStep.Condition; var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, location, Environment); - InitializeScope(step); + // Add all steps to the Composite StepsRunner + // Follows similar logic to how JobRunner invokes the StepsRunner for job steps! + compositeContext.CompositeSteps.Add(step); + location++; } @@ -104,7 +111,24 @@ public Task RunAsync(ActionRunStage stage) actionRunner2.Condition = "always()"; ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); - return Task.CompletedTask; + // Then run the Composite StepsRunner + try + { + await compositeStepsRunner.RunAsync(compositeContext); + } + catch (Exception ex) + { + // StepRunner should never throw exception out. + // End up here mean there is a bug in StepRunner + // Log the error and fail the job. + Trace.Error($"Caught exception from composite steps {nameof(CompositeStepsRunner)}: {ex}"); + compositeContext.Error(ex); + // return await CompleteCompositeActionAsync(jobServer, compositeContext, message, TaskResult.Failed); + ExecutionContext.Result = TaskResult.Failed; + } + + ExecutionContext.Result = TaskResult.Succeeded; + // return Task.CompletedTask; } private void InitializeScope(IStep step) From 9fc6f8030440beeb442583792376607acc067e6b Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 15:18:18 -0400 Subject: [PATCH 03/52] Composite Steps Runner MVP --- src/Runner.Worker/CompositeStepsRunner.cs | 220 +++++++++++++++++- .../Handlers/CompositeActionHandler.cs | 6 + 2 files changed, 225 insertions(+), 1 deletion(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 49166032ff8..60e3fb25cfd 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -32,7 +32,83 @@ public async Task RunAsync(IExecutionContext actionContext) ArgUtil.NotNull(actionContext, nameof(actionContext)); ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); + actionContext.Start(); + // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this + while (actionContext.CompositeSteps.Count > 0) + { + var step = actionContext.CompositeSteps[0]; + actionContext.CompositeSteps.RemoveAt(0); + + Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); + + step.ExecutionContext.ExpressionValues["steps"] = step.ExecutionContext.StepsContext.GetScope(step.ExecutionContext.ScopeName); + + // Populate env context for each step + Trace.Info("Initialize Env context for step"); +#if OS_WINDOWS + var envContext = new DictionaryContextData(); +#else + var envContext = new CaseSensitiveDictionaryContextData(); +#endif + + // Global env + foreach (var pair in step.ExecutionContext.EnvironmentVariables) + { + envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); + } + + // Stomps over with outside step env + if (step.ExecutionContext.ExpressionValues.TryGetValue("env", out var envContextData)) + { +#if OS_WINDOWS + var dict = envContextData as DictionaryContextData; +#else + var dict = envContextData as CaseSensitiveDictionaryContextData; +#endif + foreach (var pair in dict) + { + envContext[pair.Key] = pair.Value; + } + } + + step.ExecutionContext.ExpressionValues["env"] = envContext; + + + + if (step is IActionRunner actionStep) + { + // Set GITHUB_ACTION + step.ExecutionContext.SetGitHubContext("action", actionStep.Action.Name); + + try + { + // Evaluate and merge action's env block to env context + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + var actionEnvironment = templateEvaluator.EvaluateStepEnvironment(actionStep.Action.Environment, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, VarUtil.EnvironmentVariableKeyComparer); + foreach (var env in actionEnvironment) + { + envContext[env.Key] = new StringContextData(env.Value ?? string.Empty); + } + } + catch (Exception ex) + { + // fail the step since there is an evaluate error. + Trace.Info("Caught exception from expression for step.env"); + // evaluateStepEnvFailed = true; + step.ExecutionContext.Error(ex); + CompleteStep(step, TaskResult.Failed); + } + } + + // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) + + // await RunStepAsync(step, actionContext.CancellationToken); + await RunStepAsync(step); + + // TODO: Add compat for other types of steps. + } + // Add status logic here (start when composite action steps start) // End when clean up is done @@ -40,7 +116,149 @@ public async Task RunAsync(IExecutionContext actionContext) // TODO: Remove Composite Action logic from StepsRunner and move it here // Namely, we need to move the envContext logic! + actionContext.Complete(TaskResult.Succeeded); - } + } + private async Task RunStepAsync(IStep step) + { + // Check to see if we can expand the display name + if (step is IActionRunner actionRunner && + actionRunner.Stage == ActionRunStage.Main && + actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext)) + { + step.ExecutionContext.UpdateTimelineRecordDisplayName(actionRunner.DisplayName); + } + + // Start the step. + Trace.Info("Starting the step."); + step.ExecutionContext.Debug($"Starting: {step.DisplayName}"); + + // Set the timeout + var timeoutMinutes = 0; + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + try + { + timeoutMinutes = templateEvaluator.EvaluateStepTimeout(step.Timeout, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); + } + catch (Exception ex) + { + Trace.Info("An error occurred when attempting to determine the step timeout."); + Trace.Error(ex); + step.ExecutionContext.Error("An error occurred when attempting to determine the step timeout."); + step.ExecutionContext.Error(ex); + } + if (timeoutMinutes > 0) + { + var timeout = TimeSpan.FromMinutes(timeoutMinutes); + step.ExecutionContext.SetTimeout(timeout); + } + +#if OS_WINDOWS + try + { + if (Console.InputEncoding.CodePage != 65001) + { + using (var p = HostContext.CreateService()) + { + // Use UTF8 code page + int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), + fileName: WhichUtil.Which("chcp", true, Trace), + arguments: "65001", + environment: null, + requireExitCodeZero: false, + outputEncoding: null, + killProcessOnCancel: false, + redirectStandardIn: null, + inheritConsoleHandler: true, + cancellationToken: step.ExecutionContext.CancellationToken); + if (exitCode == 0) + { + Trace.Info("Successfully returned to code page 65001 (UTF8)"); + } + else + { + Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); + } + } + } + } + catch (Exception ex) + { + Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); + } +#endif + + try + { + await step.RunAsync(); + } + catch (OperationCanceledException ex) + { + if (step.ExecutionContext.CancellationToken.IsCancellationRequested) + { + Trace.Error($"Caught timeout exception from step: {ex.Message}"); + step.ExecutionContext.Error("The action has timed out."); + step.ExecutionContext.Result = TaskResult.Failed; + } + else + { + // Log the exception and cancel the step. + Trace.Error($"Caught cancellation exception from step: {ex}"); + step.ExecutionContext.Error(ex); + step.ExecutionContext.Result = TaskResult.Canceled; + } + } + catch (Exception ex) + { + // Log the error and fail the step. + Trace.Error($"Caught exception from step: {ex}"); + step.ExecutionContext.Error(ex); + step.ExecutionContext.Result = TaskResult.Failed; + } + + // Merge execution context result with command result + if (step.ExecutionContext.CommandResult != null) + { + step.ExecutionContext.Result = TaskResultUtil.MergeTaskResults(step.ExecutionContext.Result, step.ExecutionContext.CommandResult.Value); + } + + // Fixup the step result if ContinueOnError. + if (step.ExecutionContext.Result == TaskResult.Failed) + { + var continueOnError = false; + try + { + continueOnError = templateEvaluator.EvaluateStepContinueOnError(step.ContinueOnError, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); + } + catch (Exception ex) + { + Trace.Info("The step failed and an error occurred when attempting to determine whether to continue on error."); + Trace.Error(ex); + step.ExecutionContext.Error("The step failed and an error occurred when attempting to determine whether to continue on error."); + step.ExecutionContext.Error(ex); + } + + if (continueOnError) + { + step.ExecutionContext.Outcome = step.ExecutionContext.Result; + step.ExecutionContext.Result = TaskResult.Succeeded; + Trace.Info($"Updated step result (continue on error)"); + } + } + Trace.Info($"Step result: {step.ExecutionContext.Result}"); + + // Complete the step context. + step.ExecutionContext.Debug($"Finishing: {step.DisplayName}"); + + } + + private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) + { + var executionContext = step.ExecutionContext; + + executionContext.Complete(result, resultCode: resultCode); + } + + } } \ No newline at end of file diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 963889b1575..dee505e87d1 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -52,6 +52,12 @@ public async Task RunAsync(ActionRunStage stage) var compositeContext = HostContext.CreateService(); var compositeStepsRunner = HostContext.CreateService(); + // TODO: Add initilize composite steps, messaging objects, etc. for UI + // First try without doing this + // Check out: jobContext.InitializeJob(message, jobRequestCancellationToken); + + // TODO: Could we get rid of RegisterNestedStep in ExecutionContext? + foreach (Pipelines.ActionStep aStep in actionSteps) { // Ex: From 50d4f9de13f5d2fcf759dcff6586d4561b5af9f4 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 16:32:06 -0400 Subject: [PATCH 04/52] Fix null object reference error --- src/Runner.Worker/CompositeStepsRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 60e3fb25cfd..a613087dd58 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -18,7 +18,7 @@ namespace GitHub.Runner.Worker { - [ServiceLocator(Default = typeof(StepsRunner))] + [ServiceLocator(Default = typeof(CompositeStepsRunner))] public interface ICompositeStepsRunner : IRunnerService { Task RunAsync(IExecutionContext Context); From bb849211204ec7a695e6e751f6987f9c8b76fd78 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 17:02:58 -0400 Subject: [PATCH 05/52] intialize composiute --- src/Runner.Worker/ExecutionContext.cs | 123 ++++++++++++++++++ .../Handlers/CompositeActionHandler.cs | 3 + 2 files changed, 126 insertions(+) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 91fe0431090..578944247ac 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -80,6 +80,9 @@ public interface IExecutionContext : IRunnerService void CancelToken(); IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null); + // Initialize Composite Action + void InitializeCompositeAction(ExecutionContext executionContext); + // logging bool WriteDebug { get; } long Write(string tag, string message); @@ -635,6 +638,126 @@ public void UpdateTimelineRecordDisplayName(string displayName) _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); } + public void InitializeCompositeAction(ExecutionContext executionContext) + { + // Based off of InitializeJob() + Trace.Entering(); + ArgUtil.NotNull(executionContext, nameof(executionContext)); + + Plan = executionContext.Plan; + Features = executionContext.Features; + + // Endpoints + Endpoints = executionContext.Endpoints; + + // Variables + Variables = executionContext.Variables; + + // Environment variables shared across all actions + EnvironmentVariables = executionContext.EnvironmentVariables; + + // Job defaults shared across all actions + JobDefaults = executionContext.JobDefaults; + + // Job Outputs + JobOutputs = executionContext.JobOutputs; + + // Service container info + ServiceContainers = executionContext.ServiceContainers; + + // Steps context (StepsRunner manages adding the scoped steps context) + + // TODO: Create a seperate StepsContext? + StepsContext = new StepsContext(); + + // File table + FileTable = executionContext.FileTable; + + // Expression values + // if (message.ContextData?.Count > 0) + // { + // foreach (var pair in message.ContextData) + // { + // ExpressionValues[pair.Key] = pair.Value; + // } + // } + + // ExpressionValues["secrets"] = Variables.ToSecretsContext(); + // ExpressionValues["runner"] = new RunnerContext(); + // ExpressionValues["job"] = new JobContext(); + foreach (var key in executionContext.ExpressionValues.Keys) + { + ExpressionValues[key] = executionContext.ExpressionValues[key]; + } + // ExpressionValues = executionContext.ExpressionValues; + + // Trace.Info("Initialize GitHub context"); + // var githubAccessToken = new StringContextData(Variables.Get("system.github.token")); + // var base64EncodedToken = Convert.ToBase64String(Encoding.UTF8.GetBytes($"x-access-token:{githubAccessToken}")); + // HostContext.SecretMasker.AddValue(base64EncodedToken); + // var githubJob = Variables.Get("system.github.job"); + // var githubContext = new GitHubContext(); + // githubContext["token"] = githubAccessToken; + // if (!string.IsNullOrEmpty(githubJob)) + // { + // githubContext["job"] = new StringContextData(githubJob); + // } + // var githubDictionary = ExpressionValues["github"].AssertDictionary("github"); + // foreach (var pair in githubDictionary) + // { + // githubContext[pair.Key] = pair.Value; + // } + // ExpressionValues["github"] = githubContext; + +// Trace.Info("Initialize Env context"); +// #if OS_WINDOWS +// ExpressionValues["env"] = new DictionaryContextData(); +// #else + +// ExpressionValues["env"] = new CaseSensitiveDictionaryContextData(); +// #endif + + // Prepend Path + // PrependPath = new List(); + + // JobSteps for job ExecutionContext + // JobSteps = new List(); + + // PostJobSteps for job ExecutionContext + // PostJobSteps = new Stack(); + + // StepsWithPostRegistered for job ExecutionContext + // StepsWithPostRegistered = new HashSet(); + + CompositeSteps = new List(); + + // TODO: Figure out how to create another timeline record + + // Job timeline record. + // InitializeTimelineRecord( + // timelineId: message.Timeline.Id, + // timelineRecordId: message.JobId, + // parentTimelineRecordId: null, + // recordType: ExecutionContextType.Job, + // displayName: message.JobDisplayName, + // refName: message.JobName, + // order: null); // The job timeline record's order is set by server. + + // Logger (must be initialized before writing warnings). + // _logger = HostContext.CreateService(); + // _logger.Setup(_mainTimelineId, _record.Id); + _logger = executionContext._logger; + + // Initialize 'echo on action command success' property, default to false, unless Step_Debug is set + EchoOnActionCommand = Variables.Step_Debug ?? false; + + // Verbosity (from GitHub.Step_Debug). + WriteDebug = Variables.Step_Debug ?? false; + + // Hook up JobServerQueueThrottling event, we will log warning on server tarpit. + // _jobServerQueue.JobServerQueueThrottling += JobServerQueueThrottling_EventReceived; + } + public void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token) { // Validation diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index dee505e87d1..a5956f609e4 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -52,6 +52,9 @@ public async Task RunAsync(ActionRunStage stage) var compositeContext = HostContext.CreateService(); var compositeStepsRunner = HostContext.CreateService(); + // TODO: Create something similar to ExecutionContext::InitializeJob() for composite action steps. + compositeContext.InitializeCompositeAction(); + // TODO: Add initilize composite steps, messaging objects, etc. for UI // First try without doing this // Check out: jobContext.InitializeJob(message, jobRequestCancellationToken); From eb0e4560c9307436aa75cfb78c72815adc635296 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 17:22:24 -0400 Subject: [PATCH 06/52] Comment out code that is handled by stepsrunner --- src/Runner.Worker/CompositeStepsRunner.cs | 17 ++++++++++------- src/Runner.Worker/ExecutionContext.cs | 6 +++--- .../Handlers/CompositeActionHandler.cs | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index a613087dd58..b78632c7ca7 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -32,7 +32,8 @@ public async Task RunAsync(IExecutionContext actionContext) ArgUtil.NotNull(actionContext, nameof(actionContext)); ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); - actionContext.Start(); + // This is also not needed since the Composite aCtion is treated as one whole step in stepsRunner and is handled as such. + // actionContext.Start(); // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this while (actionContext.CompositeSteps.Count > 0) @@ -97,7 +98,7 @@ public async Task RunAsync(IExecutionContext actionContext) Trace.Info("Caught exception from expression for step.env"); // evaluateStepEnvFailed = true; step.ExecutionContext.Error(ex); - CompleteStep(step, TaskResult.Failed); + // CompleteStep(step, TaskResult.Failed); } } @@ -253,12 +254,14 @@ private async Task RunStepAsync(IStep step) } - private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) - { - var executionContext = step.ExecutionContext; + // Since the composite action is treated as 1 step, this will not be needed + // private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) + // { + // Trace.Info("In Composite CompleteStep"); + // var executionContext = step.ExecutionContext; - executionContext.Complete(result, resultCode: resultCode); - } + // executionContext.Complete(result, resultCode: resultCode); + // } } } \ No newline at end of file diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 578944247ac..b17b2c2a4d0 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -81,7 +81,7 @@ public interface IExecutionContext : IRunnerService IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null); // Initialize Composite Action - void InitializeCompositeAction(ExecutionContext executionContext); + void InitializeCompositeAction(IExecutionContext executionContext); // logging bool WriteDebug { get; } @@ -638,7 +638,7 @@ public void UpdateTimelineRecordDisplayName(string displayName) _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); } - public void InitializeCompositeAction(ExecutionContext executionContext) + public void InitializeCompositeAction(IExecutionContext executionContext) { // Based off of InitializeJob() Trace.Entering(); @@ -746,7 +746,7 @@ public void InitializeCompositeAction(ExecutionContext executionContext) // Logger (must be initialized before writing warnings). // _logger = HostContext.CreateService(); // _logger.Setup(_mainTimelineId, _record.Id); - _logger = executionContext._logger; + // _logger = executionContext._logger; // Initialize 'echo on action command success' property, default to false, unless Step_Debug is set EchoOnActionCommand = Variables.Step_Debug ?? false; diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index a5956f609e4..d70d215cf9f 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -53,7 +53,7 @@ public async Task RunAsync(ActionRunStage stage) var compositeStepsRunner = HostContext.CreateService(); // TODO: Create something similar to ExecutionContext::InitializeJob() for composite action steps. - compositeContext.InitializeCompositeAction(); + compositeContext.InitializeCompositeAction(ExecutionContext); // TODO: Add initilize composite steps, messaging objects, etc. for UI // First try without doing this From c63709d6b01d337b4d3a0c535d5c9a37f429f9b7 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Tue, 14 Jul 2020 17:47:53 -0400 Subject: [PATCH 07/52] Add composite clean up step --- src/Runner.Worker/CompositeStepsRunner.cs | 6 +++++- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index b78632c7ca7..0d7d92cf40d 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -38,6 +38,8 @@ public async Task RunAsync(IExecutionContext actionContext) // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this while (actionContext.CompositeSteps.Count > 0) { + System.Threading.Thread.Sleep(5000); + var step = actionContext.CompositeSteps[0]; actionContext.CompositeSteps.RemoveAt(0); @@ -117,7 +119,9 @@ public async Task RunAsync(IExecutionContext actionContext) // TODO: Remove Composite Action logic from StepsRunner and move it here // Namely, we need to move the envContext logic! - actionContext.Complete(TaskResult.Succeeded); + + // For now, we let the stepsrunner finish it off. + // actionContext.Complete(TaskResult.Succeeded); } diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index d70d215cf9f..7f2a6949603 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -118,7 +118,8 @@ public async Task RunAsync(ActionRunStage stage) actionRunner2.Action = cleanOutputsStep; actionRunner2.Stage = ActionRunStage.Main; actionRunner2.Condition = "always()"; - ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); + var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); + compositeContext.CompositeSteps.Add(cleanUpStep); // Then run the Composite StepsRunner try From 2067778522a77f20e6d1669b81a855f197f0413e Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 12:32:49 -0400 Subject: [PATCH 08/52] Remove previous 'workarounds' from StepsRunner. Clean Up PR --- src/Runner.Worker/CompositeStepsRunner.cs | 34 +---- src/Runner.Worker/ExecutionContext.cs | 131 +----------------- .../Handlers/CompositeActionHandler.cs | 17 +-- src/Runner.Worker/StepsRunner.cs | 15 -- 4 files changed, 14 insertions(+), 183 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 0d7d92cf40d..ff7fb98afba 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -29,15 +29,17 @@ public sealed class CompositeStepsRunner : RunnerService, ICompositeStepsRunner { public async Task RunAsync(IExecutionContext actionContext) { + // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. + ArgUtil.NotNull(actionContext, nameof(actionContext)); ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); - // This is also not needed since the Composite aCtion is treated as one whole step in stepsRunner and is handled as such. - // actionContext.Start(); + // TODO: Remove old logic from StepsRunner. - // TODO: Add CompositeSteps attribute to ExecutionContext and replace that with this + // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. while (actionContext.CompositeSteps.Count > 0) { + // This is used for testing UI appearance. System.Threading.Thread.Sleep(5000); var step = actionContext.CompositeSteps[0]; @@ -97,7 +99,7 @@ public async Task RunAsync(IExecutionContext actionContext) catch (Exception ex) { // fail the step since there is an evaluate error. - Trace.Info("Caught exception from expression for step.env"); + Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); // evaluateStepEnvFailed = true; step.ExecutionContext.Error(ex); // CompleteStep(step, TaskResult.Failed); @@ -106,23 +108,11 @@ public async Task RunAsync(IExecutionContext actionContext) // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) - // await RunStepAsync(step, actionContext.CancellationToken); await RunStepAsync(step); // TODO: Add compat for other types of steps. } - - - // Add status logic here (start when composite action steps start) - // End when clean up is done - - // TODO: Remove Composite Action logic from StepsRunner and move it here - - // Namely, we need to move the envContext logic! - - // For now, we let the stepsrunner finish it off. - // actionContext.Complete(TaskResult.Succeeded); - + // Completion Status handled by StepsRunner for the whole Composite Action Step } private async Task RunStepAsync(IStep step) @@ -257,15 +247,5 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.Debug($"Finishing: {step.DisplayName}"); } - - // Since the composite action is treated as 1 step, this will not be needed - // private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) - // { - // Trace.Info("In Composite CompleteStep"); - // var executionContext = step.ExecutionContext; - - // executionContext.Complete(result, resultCode: resultCode); - // } - } } \ No newline at end of file diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index b17b2c2a4d0..fa22823847f 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -72,17 +72,14 @@ public interface IExecutionContext : IRunnerService IExecutionContext FinalizeContext { get; set; } - // Only Composite Level ExecutionContext has CompositeSteps - List CompositeSteps { get; } + // Only Composite Level ExecutionContext has CompositeSteps (public setter for handler) + List CompositeSteps { get; set; } // 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); - // Initialize Composite Action - void InitializeCompositeAction(IExecutionContext executionContext); - // logging bool WriteDebug { get; } long Write(string tag, string message); @@ -178,8 +175,8 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext // Only job level ExecutionContext has StepsWithPostRegistered public HashSet StepsWithPostRegistered { get; private set; } - // Only Composite Level ExecutionContext has CompositeSteps - public List CompositeSteps { get; private set; } + // Only Composite Level ExecutionContext has CompositeSteps (public setter for handler) + public List CompositeSteps { get; set; } public bool EchoOnActionCommand { get; set; } @@ -638,126 +635,6 @@ public void UpdateTimelineRecordDisplayName(string displayName) _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); } - public void InitializeCompositeAction(IExecutionContext executionContext) - { - // Based off of InitializeJob() - Trace.Entering(); - ArgUtil.NotNull(executionContext, nameof(executionContext)); - - Plan = executionContext.Plan; - Features = executionContext.Features; - - // Endpoints - Endpoints = executionContext.Endpoints; - - // Variables - Variables = executionContext.Variables; - - // Environment variables shared across all actions - EnvironmentVariables = executionContext.EnvironmentVariables; - - // Job defaults shared across all actions - JobDefaults = executionContext.JobDefaults; - - // Job Outputs - JobOutputs = executionContext.JobOutputs; - - // Service container info - ServiceContainers = executionContext.ServiceContainers; - - // Steps context (StepsRunner manages adding the scoped steps context) - - // TODO: Create a seperate StepsContext? - StepsContext = new StepsContext(); - - // File table - FileTable = executionContext.FileTable; - - // Expression values - // if (message.ContextData?.Count > 0) - // { - // foreach (var pair in message.ContextData) - // { - // ExpressionValues[pair.Key] = pair.Value; - // } - // } - - // ExpressionValues["secrets"] = Variables.ToSecretsContext(); - // ExpressionValues["runner"] = new RunnerContext(); - // ExpressionValues["job"] = new JobContext(); - foreach (var key in executionContext.ExpressionValues.Keys) - { - ExpressionValues[key] = executionContext.ExpressionValues[key]; - } - // ExpressionValues = executionContext.ExpressionValues; - - // Trace.Info("Initialize GitHub context"); - // var githubAccessToken = new StringContextData(Variables.Get("system.github.token")); - // var base64EncodedToken = Convert.ToBase64String(Encoding.UTF8.GetBytes($"x-access-token:{githubAccessToken}")); - // HostContext.SecretMasker.AddValue(base64EncodedToken); - // var githubJob = Variables.Get("system.github.job"); - // var githubContext = new GitHubContext(); - // githubContext["token"] = githubAccessToken; - // if (!string.IsNullOrEmpty(githubJob)) - // { - // githubContext["job"] = new StringContextData(githubJob); - // } - // var githubDictionary = ExpressionValues["github"].AssertDictionary("github"); - // foreach (var pair in githubDictionary) - // { - // githubContext[pair.Key] = pair.Value; - // } - // ExpressionValues["github"] = githubContext; - -// Trace.Info("Initialize Env context"); -// #if OS_WINDOWS -// ExpressionValues["env"] = new DictionaryContextData(); -// #else - -// ExpressionValues["env"] = new CaseSensitiveDictionaryContextData(); -// #endif - - // Prepend Path - // PrependPath = new List(); - - // JobSteps for job ExecutionContext - // JobSteps = new List(); - - // PostJobSteps for job ExecutionContext - // PostJobSteps = new Stack(); - - // StepsWithPostRegistered for job ExecutionContext - // StepsWithPostRegistered = new HashSet(); - - CompositeSteps = new List(); - - // TODO: Figure out how to create another timeline record - - // Job timeline record. - // InitializeTimelineRecord( - // timelineId: message.Timeline.Id, - // timelineRecordId: message.JobId, - // parentTimelineRecordId: null, - // recordType: ExecutionContextType.Job, - // displayName: message.JobDisplayName, - // refName: message.JobName, - // order: null); // The job timeline record's order is set by server. - - // Logger (must be initialized before writing warnings). - // _logger = HostContext.CreateService(); - // _logger.Setup(_mainTimelineId, _record.Id); - // _logger = executionContext._logger; - - // Initialize 'echo on action command success' property, default to false, unless Step_Debug is set - EchoOnActionCommand = Variables.Step_Debug ?? false; - - // Verbosity (from GitHub.Step_Debug). - WriteDebug = Variables.Step_Debug ?? false; - - // Hook up JobServerQueueThrottling event, we will log warning on server tarpit. - // _jobServerQueue.JobServerQueueThrottling += JobServerQueueThrottling_EventReceived; - } - public void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token) { // Validation diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 7f2a6949603..ca4003a68db 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -52,14 +52,8 @@ public async Task RunAsync(ActionRunStage stage) var compositeContext = HostContext.CreateService(); var compositeStepsRunner = HostContext.CreateService(); - // TODO: Create something similar to ExecutionContext::InitializeJob() for composite action steps. - compositeContext.InitializeCompositeAction(ExecutionContext); - - // TODO: Add initilize composite steps, messaging objects, etc. for UI - // First try without doing this - // Check out: jobContext.InitializeJob(message, jobRequestCancellationToken); - - // TODO: Could we get rid of RegisterNestedStep in ExecutionContext? + // Initialize Composite Steps List of Steps + ExecutionContext.CompositeSteps = new List(); foreach (Pipelines.ActionStep aStep in actionSteps) { @@ -128,17 +122,12 @@ public async Task RunAsync(ActionRunStage stage) } catch (Exception ex) { - // StepRunner should never throw exception out. - // End up here mean there is a bug in StepRunner - // Log the error and fail the job. + // Composite StepRunner should never throw exception out. Trace.Error($"Caught exception from composite steps {nameof(CompositeStepsRunner)}: {ex}"); compositeContext.Error(ex); - // return await CompleteCompositeActionAsync(jobServer, compositeContext, message, TaskResult.Failed); ExecutionContext.Result = TaskResult.Failed; } - ExecutionContext.Result = TaskResult.Succeeded; - // return Task.CompletedTask; } private void InitializeScope(IStep step) diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 553f792482a..fd7c21f90fe 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -92,26 +92,11 @@ public async Task RunAsync(IExecutionContext jobContext) var envContext = new CaseSensitiveDictionaryContextData(); #endif - // Global env foreach (var pair in step.ExecutionContext.EnvironmentVariables) { envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); } - // Stomps over with outside step env - if (step.ExecutionContext.ExpressionValues.TryGetValue("env", out var envContextData)) - { -#if OS_WINDOWS - var dict = envContextData as DictionaryContextData; -#else - var dict = envContextData as CaseSensitiveDictionaryContextData; -#endif - foreach (var pair in dict) - { - envContext[pair.Key] = pair.Value; - } - } - step.ExecutionContext.ExpressionValues["env"] = envContext; bool evaluateStepEnvFailed = false; From 96ac86f9e8071a41223e5e515632916e36722ddf Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 12:34:34 -0400 Subject: [PATCH 09/52] Remove todo --- src/Runner.Worker/CompositeStepsRunner.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index ff7fb98afba..1ac299d6030 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -34,8 +34,6 @@ public async Task RunAsync(IExecutionContext actionContext) ArgUtil.NotNull(actionContext, nameof(actionContext)); ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); - // TODO: Remove old logic from StepsRunner. - // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. while (actionContext.CompositeSteps.Count > 0) { From f322697df71c02f32ee7eca7bcd590ac52de728b Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 12:41:04 -0400 Subject: [PATCH 10/52] Remove todo --- src/Runner.Worker/ExecutionContext.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index fa22823847f..f5b84989910 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -320,10 +320,7 @@ public IStep RegisterNestedStep( envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); } step.ExecutionContext.ExpressionValues["env"] = envContext; - - // TODO Erase this! - // Root.JobSteps.Insert(location, step); - + return step; } From 2c157b483d5ec940a35cfd43fa77aa8fa846940f Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 13:50:08 -0400 Subject: [PATCH 11/52] Fix using unitialized object yikes --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index ca4003a68db..47589e81a8a 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -49,7 +49,6 @@ public async Task RunAsync(ActionRunStage stage) int location = 0; // Initialize Steps Runner for Composite Steps and Environment for it - var compositeContext = HostContext.CreateService(); var compositeStepsRunner = HostContext.CreateService(); // Initialize Composite Steps List of Steps @@ -97,7 +96,7 @@ public async Task RunAsync(ActionRunStage stage) // Add all steps to the Composite StepsRunner // Follows similar logic to how JobRunner invokes the StepsRunner for job steps! - compositeContext.CompositeSteps.Add(step); + ExecutionContext.CompositeSteps.Add(step); location++; } @@ -113,18 +112,18 @@ public async Task RunAsync(ActionRunStage stage) actionRunner2.Stage = ActionRunStage.Main; actionRunner2.Condition = "always()"; var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); - compositeContext.CompositeSteps.Add(cleanUpStep); + ExecutionContext.CompositeSteps.Add(cleanUpStep); // Then run the Composite StepsRunner try { - await compositeStepsRunner.RunAsync(compositeContext); + await compositeStepsRunner.RunAsync(ExecutionContext); } catch (Exception ex) { // Composite StepRunner should never throw exception out. Trace.Error($"Caught exception from composite steps {nameof(CompositeStepsRunner)}: {ex}"); - compositeContext.Error(ex); + ExecutionContext.Error(ex); ExecutionContext.Result = TaskResult.Failed; } ExecutionContext.Result = TaskResult.Succeeded; From 256e455bff76880d70899995ff01f96c101b7865 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 13:51:12 -0400 Subject: [PATCH 12/52] Remove time delay --- src/Runner.Worker/CompositeStepsRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 1ac299d6030..66fb3783d3f 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -38,7 +38,7 @@ public async Task RunAsync(IExecutionContext actionContext) while (actionContext.CompositeSteps.Count > 0) { // This is used for testing UI appearance. - System.Threading.Thread.Sleep(5000); + // System.Threading.Thread.Sleep(5000); var step = actionContext.CompositeSteps[0]; actionContext.CompositeSteps.RemoveAt(0); From e42a7c30d126363d823bedc1847d66648123b0c7 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 13:55:33 -0400 Subject: [PATCH 13/52] Format handler --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 47589e81a8a..e69b15b0800 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -115,7 +115,7 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.CompositeSteps.Add(cleanUpStep); // Then run the Composite StepsRunner - try + try { await compositeStepsRunner.RunAsync(ExecutionContext); } From 49ba1c88a80b2cc209b80e1059e204604d3401e8 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 15:07:13 -0400 Subject: [PATCH 14/52] Move output handler into action handler --- src/Runner.Worker/CompositeStepsRunner.cs | 2 - .../Handlers/CompositeActionHandler.cs | 30 +++++++++++ .../Handlers/CompositeActionOutputHandler.cs | 53 ------------------- src/Runner.Worker/Handlers/HandlerFactory.cs | 12 +---- 4 files changed, 32 insertions(+), 65 deletions(-) delete mode 100644 src/Runner.Worker/Handlers/CompositeActionOutputHandler.cs diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 66fb3783d3f..77a15fa25f3 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -77,8 +77,6 @@ public async Task RunAsync(IExecutionContext actionContext) step.ExecutionContext.ExpressionValues["env"] = envContext; - - if (step is IActionRunner actionStep) { // Set GITHUB_ACTION diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index e69b15b0800..e6ff0f37313 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -35,6 +35,13 @@ public async Task RunAsync(ActionRunStage stage) var tempDirectory = HostContext.GetDirectory(WellKnownDirectory.Temp); + // If clean up outputs step, we finish setting the outputs object and then return out of this function. + if (ExecutionContext.FinalizeContext != null) + { + HandleOutput(); + return; + } + // Resolve action steps var actionSteps = Data.Steps; @@ -129,6 +136,29 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.Result = TaskResult.Succeeded; } + private void HandleOutput() + { + // Evaluate the mapped outputs value + if (Data.Outputs != null) + { + // Evaluate the outputs in the steps context to easily retrieve the values + var actionManifestManager = HostContext.GetService(); + + // Format ExpressionValues to Dictionary + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var pair in ExecutionContext.ExpressionValues) + { + evaluateContext[pair.Key] = pair.Value; + } + + // Get the evluated composite outputs' values mapped to the outputs named + DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); + + // Set the outputs for the outputs object in the whole composite action + actionManifestManager.SetAllCompositeOutputs(ExecutionContext.FinalizeContext, actionOutputs); + } + } + private void InitializeScope(IStep step) { var stepsContext = step.ExecutionContext.StepsContext; diff --git a/src/Runner.Worker/Handlers/CompositeActionOutputHandler.cs b/src/Runner.Worker/Handlers/CompositeActionOutputHandler.cs deleted file mode 100644 index ea52412003e..00000000000 --- a/src/Runner.Worker/Handlers/CompositeActionOutputHandler.cs +++ /dev/null @@ -1,53 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using GitHub.DistributedTask.ObjectTemplating.Schema; -using GitHub.DistributedTask.ObjectTemplating.Tokens; -using GitHub.DistributedTask.Pipelines.ContextData; -using GitHub.DistributedTask.WebApi; -using GitHub.Runner.Common; -using GitHub.Runner.Sdk; -using Pipelines = GitHub.DistributedTask.Pipelines; - -namespace GitHub.Runner.Worker.Handlers -{ - [ServiceLocator(Default = typeof(CompositeActionOutputHandler))] - public interface ICompositeActionOutputHandler : IHandler - { - CompositeActionExecutionData Data { get; set; } - } - - public sealed class CompositeActionOutputHandler : Handler, ICompositeActionOutputHandler - { - public CompositeActionExecutionData Data { get; set; } - - - public Task RunAsync(ActionRunStage stage) - { - // Evaluate the mapped outputs value - if (Data.Outputs != null) - { - // Evaluate the outputs in the steps context to easily retrieve the values - var actionManifestManager = HostContext.GetService(); - - // Format ExpressionValues to Dictionary - var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in ExecutionContext.ExpressionValues) - { - evaluateContext[pair.Key] = pair.Value; - } - - // Get the evluated composite outputs' values mapped to the outputs named - DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); - - // Set the outputs for the outputs object in the whole composite action - actionManifestManager.SetAllCompositeOutputs(ExecutionContext.FinalizeContext, actionOutputs); - } - - return Task.CompletedTask; - } - } -} \ No newline at end of file diff --git a/src/Runner.Worker/Handlers/HandlerFactory.cs b/src/Runner.Worker/Handlers/HandlerFactory.cs index 4591ccab21d..db4d6559c88 100644 --- a/src/Runner.Worker/Handlers/HandlerFactory.cs +++ b/src/Runner.Worker/Handlers/HandlerFactory.cs @@ -68,16 +68,8 @@ public IHandler Create( } else if (data.ExecutionType == ActionExecutionType.Composite) { - if (executionContext.FinalizeContext == null) - { - handler = HostContext.CreateService(); - (handler as ICompositeActionHandler).Data = data as CompositeActionExecutionData; - } - else - { - handler = HostContext.CreateService(); - (handler as ICompositeActionOutputHandler).Data = data as CompositeActionExecutionData; - } + handler = HostContext.CreateService(); + (handler as ICompositeActionHandler).Data = data as CompositeActionExecutionData; } else { From 45639a2ec4cf180a267d23645dbe2a07fc68fef1 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 15:25:54 -0400 Subject: [PATCH 15/52] Add try to evaluate display name --- src/Runner.Worker/CompositeStepsRunner.cs | 47 +++++++++++------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 77a15fa25f3..19831033540 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -77,30 +77,29 @@ public async Task RunAsync(IExecutionContext actionContext) step.ExecutionContext.ExpressionValues["env"] = envContext; - if (step is IActionRunner actionStep) - { - // Set GITHUB_ACTION - step.ExecutionContext.SetGitHubContext("action", actionStep.Action.Name); + var actionStep = step as IActionRunner; - try - { - // Evaluate and merge action's env block to env context - var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); - var actionEnvironment = templateEvaluator.EvaluateStepEnvironment(actionStep.Action.Environment, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, VarUtil.EnvironmentVariableKeyComparer); - foreach (var env in actionEnvironment) - { - envContext[env.Key] = new StringContextData(env.Value ?? string.Empty); - } - } - catch (Exception ex) + // Set GITHUB_ACTION + step.ExecutionContext.SetGitHubContext("action", actionStep.Action.Name); + + try + { + // Evaluate and merge action's env block to env context + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + var actionEnvironment = templateEvaluator.EvaluateStepEnvironment(actionStep.Action.Environment, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, VarUtil.EnvironmentVariableKeyComparer); + foreach (var env in actionEnvironment) { - // fail the step since there is an evaluate error. - Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); - // evaluateStepEnvFailed = true; - step.ExecutionContext.Error(ex); - // CompleteStep(step, TaskResult.Failed); + envContext[env.Key] = new StringContextData(env.Value ?? string.Empty); } } + catch (Exception ex) + { + // fail the step since there is an evaluate error. + Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); + // evaluateStepEnvFailed = true; + step.ExecutionContext.Error(ex); + // CompleteStep(step, TaskResult.Failed); + } // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) @@ -113,12 +112,10 @@ public async Task RunAsync(IExecutionContext actionContext) private async Task RunStepAsync(IStep step) { - // Check to see if we can expand the display name - if (step is IActionRunner actionRunner && - actionRunner.Stage == ActionRunStage.Main && - actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext)) + // Try to evaluate the display name + if (step is IActionRunner actionRunner && actionRunner.Stage == ActionRunStage.Main) { - step.ExecutionContext.UpdateTimelineRecordDisplayName(actionRunner.DisplayName); + actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext); } // Start the step. From 87d30303c945cadd63381beddee146230d1ab527 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 15:43:17 -0400 Subject: [PATCH 16/52] Remove while loop yikes --- src/Runner.Worker/CompositeStepsRunner.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 19831033540..e5cea1d9572 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -35,14 +35,11 @@ public async Task RunAsync(IExecutionContext actionContext) ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. - while (actionContext.CompositeSteps.Count > 0) + foreach (IStep step in actionContext.CompositeSteps) { // This is used for testing UI appearance. // System.Threading.Thread.Sleep(5000); - var step = actionContext.CompositeSteps[0]; - actionContext.CompositeSteps.RemoveAt(0); - Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); step.ExecutionContext.ExpressionValues["steps"] = step.ExecutionContext.StepsContext.GetScope(step.ExecutionContext.ScopeName); From e0561ad45fdf441a017389f95e0ebb009bfef84b Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 17:35:41 -0400 Subject: [PATCH 17/52] Abstract away the windows encoding check during step running --- src/Runner.Sdk/Util/IOUtil.cs | 38 +++++++++++++++++++++++ src/Runner.Worker/CompositeStepsRunner.cs | 35 +-------------------- src/Runner.Worker/ExecutionContext.cs | 2 +- src/Runner.Worker/StepsRunner.cs | 35 +-------------------- 4 files changed, 41 insertions(+), 69 deletions(-) diff --git a/src/Runner.Sdk/Util/IOUtil.cs b/src/Runner.Sdk/Util/IOUtil.cs index 45957dee7b3..0aae9acb334 100644 --- a/src/Runner.Sdk/Util/IOUtil.cs +++ b/src/Runner.Sdk/Util/IOUtil.cs @@ -424,6 +424,44 @@ public static void ValidateExecutePermission(string directory) throw new NotSupportedException($"Unable to validate execute permissions for directory '{directory}'. Exceeded maximum iterations."); } + public static void CheckWindowsEncoding(System.Threading.CancellationToken cancellationToken) + { +#if OS_WINDOWS + try + { + if (Console.InputEncoding.CodePage != 65001) + { + using (var p = HostContext.CreateService()) + { + // Use UTF8 code page + int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), + fileName: WhichUtil.Which("chcp", true, Trace), + arguments: "65001", + environment: null, + requireExitCodeZero: false, + outputEncoding: null, + killProcessOnCancel: false, + redirectStandardIn: null, + inheritConsoleHandler: true, + cancellationToken: step.ExecutionContext.CancellationToken); + if (exitCode == 0) + { + Trace.Info("Successfully returned to code page 65001 (UTF8)"); + } + else + { + Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); + } + } + } + } + catch (Exception ex) + { + Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); + } +#endif + } + /// /// Recursively enumerates a directory without following directory reparse points. /// diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index e5cea1d9572..6eb35c66d80 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -139,40 +139,7 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.SetTimeout(timeout); } -#if OS_WINDOWS - try - { - if (Console.InputEncoding.CodePage != 65001) - { - using (var p = HostContext.CreateService()) - { - // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), - fileName: WhichUtil.Which("chcp", true, Trace), - arguments: "65001", - environment: null, - requireExitCodeZero: false, - outputEncoding: null, - killProcessOnCancel: false, - redirectStandardIn: null, - inheritConsoleHandler: true, - cancellationToken: step.ExecutionContext.CancellationToken); - if (exitCode == 0) - { - Trace.Info("Successfully returned to code page 65001 (UTF8)"); - } - else - { - Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); - } - } - } - } - catch (Exception ex) - { - Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); - } -#endif + IOUtil.CheckWindowsEncoding(step.ExecutionContext.CancellationToken); try { diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index f5b84989910..5a7a7657fca 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -320,7 +320,7 @@ public IStep RegisterNestedStep( envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); } step.ExecutionContext.ExpressionValues["env"] = envContext; - + return step; } diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index fd7c21f90fe..6e133093867 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,40 +285,7 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } -#if OS_WINDOWS - try - { - if (Console.InputEncoding.CodePage != 65001) - { - using (var p = HostContext.CreateService()) - { - // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), - fileName: WhichUtil.Which("chcp", true, Trace), - arguments: "65001", - environment: null, - requireExitCodeZero: false, - outputEncoding: null, - killProcessOnCancel: false, - redirectStandardIn: null, - inheritConsoleHandler: true, - cancellationToken: step.ExecutionContext.CancellationToken); - if (exitCode == 0) - { - Trace.Info("Successfully returned to code page 65001 (UTF8)"); - } - else - { - Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); - } - } - } - } - catch (Exception ex) - { - Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); - } -#endif + IOUtil.CheckWindowsEncoding(step.ExecutionContext.CancellationToken); try { From 25bd4b1ad5111b4c13bb636682671cdc760b078b Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 17:42:32 -0400 Subject: [PATCH 18/52] Github context set to {ScopeName}.{ContextName} or {ContextName} if ScopeName is null --- src/Runner.Worker/CompositeStepsRunner.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 6eb35c66d80..65eee4b83c9 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -77,7 +77,14 @@ public async Task RunAsync(IExecutionContext actionContext) var actionStep = step as IActionRunner; // Set GITHUB_ACTION - step.ExecutionContext.SetGitHubContext("action", actionStep.Action.Name); + if (!String.IsNullOrEmpty(step.ExecutionContext.ScopeName)) + { + step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ScopeName); + } + else + { + step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ContextName); + } try { @@ -202,7 +209,6 @@ private async Task RunStepAsync(IStep step) // Complete the step context. step.ExecutionContext.Debug($"Finishing: {step.DisplayName}"); - } } } \ No newline at end of file From f5680aa3f4e1c0cfce2765e0e3758d01549b251a Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 17:44:54 -0400 Subject: [PATCH 19/52] Remove setting result to sucess since result defaults to sucess --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index e6ff0f37313..3c8b31ad825 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -133,7 +133,6 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.Error(ex); ExecutionContext.Result = TaskResult.Failed; } - ExecutionContext.Result = TaskResult.Succeeded; } private void HandleOutput() From 74ba0eba71a89a8a14ce9527fd6eced018fe38d0 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Wed, 15 Jul 2020 17:53:59 -0400 Subject: [PATCH 20/52] Fix windows error --- src/Runner.Sdk/Util/IOUtil.cs | 4 ++-- src/Runner.Worker/CompositeStepsRunner.cs | 11 +++++++++-- src/Runner.Worker/StepsRunner.cs | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Runner.Sdk/Util/IOUtil.cs b/src/Runner.Sdk/Util/IOUtil.cs index 0aae9acb334..d000dfbb525 100644 --- a/src/Runner.Sdk/Util/IOUtil.cs +++ b/src/Runner.Sdk/Util/IOUtil.cs @@ -424,7 +424,7 @@ public static void ValidateExecutePermission(string directory) throw new NotSupportedException($"Unable to validate execute permissions for directory '{directory}'. Exceeded maximum iterations."); } - public static void CheckWindowsEncoding(System.Threading.CancellationToken cancellationToken) + public static void CheckWindowsEncoding(string workingDir, System.Threading.CancellationToken cancellationToken) { #if OS_WINDOWS try @@ -434,7 +434,7 @@ public static void CheckWindowsEncoding(System.Threading.CancellationToken cance using (var p = HostContext.CreateService()) { // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), + int exitCode = await p.ExecuteAsync(workingDirectory: workingDir, fileName: WhichUtil.Which("chcp", true, Trace), arguments: "65001", environment: null, diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 65eee4b83c9..8b000230ab0 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -102,7 +102,7 @@ public async Task RunAsync(IExecutionContext actionContext) Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); // evaluateStepEnvFailed = true; step.ExecutionContext.Error(ex); - // CompleteStep(step, TaskResult.Failed); + CompleteStep(step, TaskResult.Failed); } // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) @@ -114,6 +114,13 @@ public async Task RunAsync(IExecutionContext actionContext) // Completion Status handled by StepsRunner for the whole Composite Action Step } + private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) + { + var executionContext = step.ExecutionContext; + + executionContext.Complete(result, resultCode: resultCode); + } + private async Task RunStepAsync(IStep step) { // Try to evaluate the display name @@ -146,7 +153,7 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.SetTimeout(timeout); } - IOUtil.CheckWindowsEncoding(step.ExecutionContext.CancellationToken); + IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), step.ExecutionContext.CancellationToken); try { diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 6e133093867..e812779cf08 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,7 +285,7 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } - IOUtil.CheckWindowsEncoding(step.ExecutionContext.CancellationToken); + IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), step.ExecutionContext.CancellationToken); try { From 8bf57f70cf3a21533334052cb7544bb15da49e25 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 10:00:10 -0400 Subject: [PATCH 21/52] Fix windows --- src/Runner.Sdk/Util/IOUtil.cs | 4 ++-- src/Runner.Worker/CompositeStepsRunner.cs | 4 ++-- src/Runner.Worker/StepsRunner.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Runner.Sdk/Util/IOUtil.cs b/src/Runner.Sdk/Util/IOUtil.cs index d000dfbb525..5eb37ff3c6c 100644 --- a/src/Runner.Sdk/Util/IOUtil.cs +++ b/src/Runner.Sdk/Util/IOUtil.cs @@ -424,7 +424,7 @@ public static void ValidateExecutePermission(string directory) throw new NotSupportedException($"Unable to validate execute permissions for directory '{directory}'. Exceeded maximum iterations."); } - public static void CheckWindowsEncoding(string workingDir, System.Threading.CancellationToken cancellationToken) + public static void CheckWindowsEncoding(string workingDir, string util, System.Threading.CancellationToken cancellationToken) { #if OS_WINDOWS try @@ -435,7 +435,7 @@ public static void CheckWindowsEncoding(string workingDir, System.Threading.Canc { // Use UTF8 code page int exitCode = await p.ExecuteAsync(workingDirectory: workingDir, - fileName: WhichUtil.Which("chcp", true, Trace), + fileName: util, arguments: "65001", environment: null, requireExitCodeZero: false, diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 8b000230ab0..285ecde4ff4 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -152,8 +152,8 @@ private async Task RunStepAsync(IStep step) var timeout = TimeSpan.FromMinutes(timeoutMinutes); step.ExecutionContext.SetTimeout(timeout); } - - IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), step.ExecutionContext.CancellationToken); + + IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), WhichUtil.Which("chcp", true, Trace), step.ExecutionContext.CancellationToken); try { diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index e812779cf08..a54c81e9693 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,7 +285,7 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } - IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), step.ExecutionContext.CancellationToken); + IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), WhichUtil.Which("chcp", true, Trace), step.ExecutionContext.CancellationToken); try { From b0d92bbc053c5bb66e2910d8b0bd3007a73ad1e5 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 10:10:46 -0400 Subject: [PATCH 22/52] revert: --- src/Runner.Sdk/Util/IOUtil.cs | 38 ----------------------- src/Runner.Worker/CompositeStepsRunner.cs | 36 +++++++++++++++++++-- src/Runner.Worker/StepsRunner.cs | 36 +++++++++++++++++++-- 3 files changed, 68 insertions(+), 42 deletions(-) diff --git a/src/Runner.Sdk/Util/IOUtil.cs b/src/Runner.Sdk/Util/IOUtil.cs index 5eb37ff3c6c..45957dee7b3 100644 --- a/src/Runner.Sdk/Util/IOUtil.cs +++ b/src/Runner.Sdk/Util/IOUtil.cs @@ -424,44 +424,6 @@ public static void ValidateExecutePermission(string directory) throw new NotSupportedException($"Unable to validate execute permissions for directory '{directory}'. Exceeded maximum iterations."); } - public static void CheckWindowsEncoding(string workingDir, string util, System.Threading.CancellationToken cancellationToken) - { -#if OS_WINDOWS - try - { - if (Console.InputEncoding.CodePage != 65001) - { - using (var p = HostContext.CreateService()) - { - // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: workingDir, - fileName: util, - arguments: "65001", - environment: null, - requireExitCodeZero: false, - outputEncoding: null, - killProcessOnCancel: false, - redirectStandardIn: null, - inheritConsoleHandler: true, - cancellationToken: step.ExecutionContext.CancellationToken); - if (exitCode == 0) - { - Trace.Info("Successfully returned to code page 65001 (UTF8)"); - } - else - { - Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); - } - } - } - } - catch (Exception ex) - { - Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); - } -#endif - } - /// /// Recursively enumerates a directory without following directory reparse points. /// diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 285ecde4ff4..2981ac13653 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -152,9 +152,41 @@ private async Task RunStepAsync(IStep step) var timeout = TimeSpan.FromMinutes(timeoutMinutes); step.ExecutionContext.SetTimeout(timeout); } - - IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), WhichUtil.Which("chcp", true, Trace), step.ExecutionContext.CancellationToken); +#if OS_WINDOWS + try + { + if (Console.InputEncoding.CodePage != 65001) + { + using (var p = HostContext.CreateService()) + { + // Use UTF8 code page + int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), + fileName: WhichUtil.Which("chcp", true, Trace), + arguments: "65001", + environment: null, + requireExitCodeZero: false, + outputEncoding: null, + killProcessOnCancel: false, + redirectStandardIn: null, + inheritConsoleHandler: true, + cancellationToken: step.ExecutionContext.CancellationToken); + if (exitCode == 0) + { + Trace.Info("Successfully returned to code page 65001 (UTF8)"); + } + else + { + Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); + } + } + } + } + catch (Exception ex) + { + Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); + } +#endif try { await step.RunAsync(); diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index a54c81e9693..45ccc90bc53 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,8 +285,40 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } - IOUtil.CheckWindowsEncoding(HostContext.GetDirectory(WellKnownDirectory.Work), WhichUtil.Which("chcp", true, Trace), step.ExecutionContext.CancellationToken); - +#if OS_WINDOWS + try + { + if (Console.InputEncoding.CodePage != 65001) + { + using (var p = HostContext.CreateService()) + { + // Use UTF8 code page + int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), + fileName: WhichUtil.Which("chcp", true, Trace), + arguments: "65001", + environment: null, + requireExitCodeZero: false, + outputEncoding: null, + killProcessOnCancel: false, + redirectStandardIn: null, + inheritConsoleHandler: true, + cancellationToken: step.ExecutionContext.CancellationToken); + if (exitCode == 0) + { + Trace.Info("Successfully returned to code page 65001 (UTF8)"); + } + else + { + Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); + } + } + } + } + catch (Exception ex) + { + Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); + } +#endif try { await step.RunAsync(); From 309e09bff802b952ca5e9d08ec06c24941b81455 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:18:13 -0400 Subject: [PATCH 23/52] Windows fix --- src/Runner.Common/Util/EncodingUtil.cs | 48 +++++++++++++++++++++++ src/Runner.Worker/CompositeStepsRunner.cs | 38 ++---------------- src/Runner.Worker/StepsRunner.cs | 36 +---------------- 3 files changed, 53 insertions(+), 69 deletions(-) create mode 100644 src/Runner.Common/Util/EncodingUtil.cs diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs new file mode 100644 index 00000000000..a869a7e1fd8 --- /dev/null +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -0,0 +1,48 @@ +using GitHub.Runner.Sdk; +using GitHub.Runner.Common; + +namespace GitHub.Runner.Common.Util +{ + using System; + + public static class EncodingUtil + { + public static void EnsureUtfEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) + { +#if OS_WINDOWS + try + { + if (Console.InputEncoding.CodePage != 65001) + { + using (var p = hostContext.CreateService()) + { + // Use UTF8 code page + int exitCode = await p.ExecuteAsync(workingDirectory: hostContext.GetDirectory(WellKnownDirectory.Work), + fileName: WhichUtil.Which("chcp", true, trace), + arguments: "65001", + environment: null, + requireExitCodeZero: false, + outputEncoding: null, + killProcessOnCancel: false, + redirectStandardIn: null, + inheritConsoleHandler: true, + cancellationToken: cancellationToken); + if (exitCode == 0) + { + trace.Info("Successfully returned to code page 65001 (UTF8)"); + } + else + { + trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); + } + } + } + } + catch (Exception ex) + { + trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); + } +#endif + } + } +} \ No newline at end of file diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs index 2981ac13653..53f6b1d64fa 100644 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ b/src/Runner.Worker/CompositeStepsRunner.cs @@ -81,7 +81,7 @@ public async Task RunAsync(IExecutionContext actionContext) { step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ScopeName); } - else + else { step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ContextName); } @@ -153,40 +153,8 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.SetTimeout(timeout); } -#if OS_WINDOWS - try - { - if (Console.InputEncoding.CodePage != 65001) - { - using (var p = HostContext.CreateService()) - { - // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), - fileName: WhichUtil.Which("chcp", true, Trace), - arguments: "65001", - environment: null, - requireExitCodeZero: false, - outputEncoding: null, - killProcessOnCancel: false, - redirectStandardIn: null, - inheritConsoleHandler: true, - cancellationToken: step.ExecutionContext.CancellationToken); - if (exitCode == 0) - { - Trace.Info("Successfully returned to code page 65001 (UTF8)"); - } - else - { - Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); - } - } - } - } - catch (Exception ex) - { - Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); - } -#endif + EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + try { await step.RunAsync(); diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 45ccc90bc53..4ee34277baa 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,40 +285,8 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } -#if OS_WINDOWS - try - { - if (Console.InputEncoding.CodePage != 65001) - { - using (var p = HostContext.CreateService()) - { - // Use UTF8 code page - int exitCode = await p.ExecuteAsync(workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work), - fileName: WhichUtil.Which("chcp", true, Trace), - arguments: "65001", - environment: null, - requireExitCodeZero: false, - outputEncoding: null, - killProcessOnCancel: false, - redirectStandardIn: null, - inheritConsoleHandler: true, - cancellationToken: step.ExecutionContext.CancellationToken); - if (exitCode == 0) - { - Trace.Info("Successfully returned to code page 65001 (UTF8)"); - } - else - { - Trace.Warning($"'chcp 65001' failed with exit code {exitCode}"); - } - } - } - } - catch (Exception ex) - { - Trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); - } -#endif + EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + try { await step.RunAsync(); From 7be28ab14dc91fdb008ad1a8121a5c817f3400c1 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:30:15 -0400 Subject: [PATCH 24/52] Fix Windows Error in Abstraction --- src/Runner.Common/Util/EncodingUtil.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index a869a7e1fd8..cab46ff959d 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -7,7 +7,7 @@ namespace GitHub.Runner.Common.Util public static class EncodingUtil { - public static void EnsureUtfEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) + public static async void EnsureUtfEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) { #if OS_WINDOWS try @@ -43,6 +43,8 @@ public static void EnsureUtfEncoding(IHostContext hostContext, Tracing trace, Sy trace.Warning($"'chcp 65001' failed with exception {ex.Message}"); } #endif + // Dummy variable to prevent compiler error CS1998: "This async method lacks 'await' operators and will run synchronously..." + await System.Threading.Tasks.Task.CompletedTask; } } } \ No newline at end of file From 7d95fa61cffef50f9a7ba46f0385426480bbad0d Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:38:20 -0400 Subject: [PATCH 25/52] Remove Composite Steps Runner => consolidate into Composite Steps Runner --- src/Runner.Worker/CompositeStepsRunner.cs | 221 ------------------ .../Handlers/CompositeActionHandler.cs | 197 +++++++++++++++- 2 files changed, 192 insertions(+), 226 deletions(-) delete mode 100644 src/Runner.Worker/CompositeStepsRunner.cs diff --git a/src/Runner.Worker/CompositeStepsRunner.cs b/src/Runner.Worker/CompositeStepsRunner.cs deleted file mode 100644 index 53f6b1d64fa..00000000000 --- a/src/Runner.Worker/CompositeStepsRunner.cs +++ /dev/null @@ -1,221 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using GitHub.DistributedTask.Expressions2; -using GitHub.DistributedTask.ObjectTemplating.Tokens; -using GitHub.DistributedTask.Pipelines; -using GitHub.DistributedTask.Pipelines.ContextData; -using GitHub.DistributedTask.Pipelines.ObjectTemplating; -using GitHub.DistributedTask.WebApi; -using GitHub.Runner.Common; -using GitHub.Runner.Common.Util; -using GitHub.Runner.Sdk; -using GitHub.Runner.Worker.Expressions; -using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating; -using Pipelines = GitHub.DistributedTask.Pipelines; - -namespace GitHub.Runner.Worker -{ - [ServiceLocator(Default = typeof(CompositeStepsRunner))] - public interface ICompositeStepsRunner : IRunnerService - { - Task RunAsync(IExecutionContext Context); - } - - - public sealed class CompositeStepsRunner : RunnerService, ICompositeStepsRunner - { - public async Task RunAsync(IExecutionContext actionContext) - { - // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. - - ArgUtil.NotNull(actionContext, nameof(actionContext)); - ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); - - // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. - foreach (IStep step in actionContext.CompositeSteps) - { - // This is used for testing UI appearance. - // System.Threading.Thread.Sleep(5000); - - Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); - - step.ExecutionContext.ExpressionValues["steps"] = step.ExecutionContext.StepsContext.GetScope(step.ExecutionContext.ScopeName); - - // Populate env context for each step - Trace.Info("Initialize Env context for step"); -#if OS_WINDOWS - var envContext = new DictionaryContextData(); -#else - var envContext = new CaseSensitiveDictionaryContextData(); -#endif - - // Global env - foreach (var pair in step.ExecutionContext.EnvironmentVariables) - { - envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); - } - - // Stomps over with outside step env - if (step.ExecutionContext.ExpressionValues.TryGetValue("env", out var envContextData)) - { -#if OS_WINDOWS - var dict = envContextData as DictionaryContextData; -#else - var dict = envContextData as CaseSensitiveDictionaryContextData; -#endif - foreach (var pair in dict) - { - envContext[pair.Key] = pair.Value; - } - } - - step.ExecutionContext.ExpressionValues["env"] = envContext; - - var actionStep = step as IActionRunner; - - // Set GITHUB_ACTION - if (!String.IsNullOrEmpty(step.ExecutionContext.ScopeName)) - { - step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ScopeName); - } - else - { - step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ContextName); - } - - try - { - // Evaluate and merge action's env block to env context - var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); - var actionEnvironment = templateEvaluator.EvaluateStepEnvironment(actionStep.Action.Environment, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, VarUtil.EnvironmentVariableKeyComparer); - foreach (var env in actionEnvironment) - { - envContext[env.Key] = new StringContextData(env.Value ?? string.Empty); - } - } - catch (Exception ex) - { - // fail the step since there is an evaluate error. - Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); - // evaluateStepEnvFailed = true; - step.ExecutionContext.Error(ex); - CompleteStep(step, TaskResult.Failed); - } - - // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) - - await RunStepAsync(step); - - // TODO: Add compat for other types of steps. - } - // Completion Status handled by StepsRunner for the whole Composite Action Step - } - - private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) - { - var executionContext = step.ExecutionContext; - - executionContext.Complete(result, resultCode: resultCode); - } - - private async Task RunStepAsync(IStep step) - { - // Try to evaluate the display name - if (step is IActionRunner actionRunner && actionRunner.Stage == ActionRunStage.Main) - { - actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext); - } - - // Start the step. - Trace.Info("Starting the step."); - step.ExecutionContext.Debug($"Starting: {step.DisplayName}"); - - // Set the timeout - var timeoutMinutes = 0; - var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); - try - { - timeoutMinutes = templateEvaluator.EvaluateStepTimeout(step.Timeout, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); - } - catch (Exception ex) - { - Trace.Info("An error occurred when attempting to determine the step timeout."); - Trace.Error(ex); - step.ExecutionContext.Error("An error occurred when attempting to determine the step timeout."); - step.ExecutionContext.Error(ex); - } - if (timeoutMinutes > 0) - { - var timeout = TimeSpan.FromMinutes(timeoutMinutes); - step.ExecutionContext.SetTimeout(timeout); - } - - EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); - - try - { - await step.RunAsync(); - } - catch (OperationCanceledException ex) - { - if (step.ExecutionContext.CancellationToken.IsCancellationRequested) - { - Trace.Error($"Caught timeout exception from step: {ex.Message}"); - step.ExecutionContext.Error("The action has timed out."); - step.ExecutionContext.Result = TaskResult.Failed; - } - else - { - // Log the exception and cancel the step. - Trace.Error($"Caught cancellation exception from step: {ex}"); - step.ExecutionContext.Error(ex); - step.ExecutionContext.Result = TaskResult.Canceled; - } - } - catch (Exception ex) - { - // Log the error and fail the step. - Trace.Error($"Caught exception from step: {ex}"); - step.ExecutionContext.Error(ex); - step.ExecutionContext.Result = TaskResult.Failed; - } - - // Merge execution context result with command result - if (step.ExecutionContext.CommandResult != null) - { - step.ExecutionContext.Result = TaskResultUtil.MergeTaskResults(step.ExecutionContext.Result, step.ExecutionContext.CommandResult.Value); - } - - // Fixup the step result if ContinueOnError. - if (step.ExecutionContext.Result == TaskResult.Failed) - { - var continueOnError = false; - try - { - continueOnError = templateEvaluator.EvaluateStepContinueOnError(step.ContinueOnError, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); - } - catch (Exception ex) - { - Trace.Info("The step failed and an error occurred when attempting to determine whether to continue on error."); - Trace.Error(ex); - step.ExecutionContext.Error("The step failed and an error occurred when attempting to determine whether to continue on error."); - step.ExecutionContext.Error(ex); - } - - if (continueOnError) - { - step.ExecutionContext.Outcome = step.ExecutionContext.Result; - step.ExecutionContext.Result = TaskResult.Succeeded; - Trace.Info($"Updated step result (continue on error)"); - } - } - Trace.Info($"Step result: {step.ExecutionContext.Result}"); - - // Complete the step context. - step.ExecutionContext.Debug($"Finishing: {step.DisplayName}"); - } - } -} \ No newline at end of file diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 3c8b31ad825..d6e0f9f1941 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -55,9 +55,6 @@ public async Task RunAsync(ActionRunStage stage) // Add each composite action step to the front of the queue int location = 0; - // Initialize Steps Runner for Composite Steps and Environment for it - var compositeStepsRunner = HostContext.CreateService(); - // Initialize Composite Steps List of Steps ExecutionContext.CompositeSteps = new List(); @@ -124,16 +121,109 @@ public async Task RunAsync(ActionRunStage stage) // Then run the Composite StepsRunner try { - await compositeStepsRunner.RunAsync(ExecutionContext); + await RunAsync(ExecutionContext); } catch (Exception ex) { // Composite StepRunner should never throw exception out. - Trace.Error($"Caught exception from composite steps {nameof(CompositeStepsRunner)}: {ex}"); + Trace.Error($"Caught exception from composite steps {nameof(CompositeActionHandler)}: {ex}"); ExecutionContext.Error(ex); ExecutionContext.Result = TaskResult.Failed; } } + public async Task RunAsync(IExecutionContext actionContext) + { + // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. + + ArgUtil.NotNull(actionContext, nameof(actionContext)); + ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); + + // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. + foreach (IStep step in actionContext.CompositeSteps) + { + // This is used for testing UI appearance. + // System.Threading.Thread.Sleep(5000); + + Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); + + step.ExecutionContext.ExpressionValues["steps"] = step.ExecutionContext.StepsContext.GetScope(step.ExecutionContext.ScopeName); + + // Populate env context for each step + Trace.Info("Initialize Env context for step"); +#if OS_WINDOWS + var envContext = new DictionaryContextData(); +#else + var envContext = new CaseSensitiveDictionaryContextData(); +#endif + + // Global env + foreach (var pair in step.ExecutionContext.EnvironmentVariables) + { + envContext[pair.Key] = new StringContextData(pair.Value ?? string.Empty); + } + + // Stomps over with outside step env + if (step.ExecutionContext.ExpressionValues.TryGetValue("env", out var envContextData)) + { +#if OS_WINDOWS + var dict = envContextData as DictionaryContextData; +#else + var dict = envContextData as CaseSensitiveDictionaryContextData; +#endif + foreach (var pair in dict) + { + envContext[pair.Key] = pair.Value; + } + } + + step.ExecutionContext.ExpressionValues["env"] = envContext; + + var actionStep = step as IActionRunner; + + // Set GITHUB_ACTION + if (!String.IsNullOrEmpty(step.ExecutionContext.ScopeName)) + { + step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ScopeName); + } + else + { + step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ContextName); + } + + try + { + // Evaluate and merge action's env block to env context + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + var actionEnvironment = templateEvaluator.EvaluateStepEnvironment(actionStep.Action.Environment, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, Common.Util.VarUtil.EnvironmentVariableKeyComparer); + foreach (var env in actionEnvironment) + { + envContext[env.Key] = new StringContextData(env.Value ?? string.Empty); + } + } + catch (Exception ex) + { + // fail the step since there is an evaluate error. + Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); + // evaluateStepEnvFailed = true; + step.ExecutionContext.Error(ex); + CompleteStep(step, TaskResult.Failed); + } + + // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) + + await RunStepAsync(step); + + // TODO: Add compat for other types of steps. + } + // Completion Status handled by StepsRunner for the whole Composite Action Step + } + + private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) + { + var executionContext = step.ExecutionContext; + + executionContext.Complete(result, resultCode: resultCode); + } private void HandleOutput() { @@ -164,5 +254,102 @@ private void InitializeScope(IStep step) var scopeName = step.ExecutionContext.ScopeName; step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } + + private async Task RunStepAsync(IStep step) + { + // Try to evaluate the display name + if (step is IActionRunner actionRunner && actionRunner.Stage == ActionRunStage.Main) + { + actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext); + } + + // Start the step. + Trace.Info("Starting the step."); + step.ExecutionContext.Debug($"Starting: {step.DisplayName}"); + + // Set the timeout + var timeoutMinutes = 0; + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + try + { + timeoutMinutes = templateEvaluator.EvaluateStepTimeout(step.Timeout, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); + } + catch (Exception ex) + { + Trace.Info("An error occurred when attempting to determine the step timeout."); + Trace.Error(ex); + step.ExecutionContext.Error("An error occurred when attempting to determine the step timeout."); + step.ExecutionContext.Error(ex); + } + if (timeoutMinutes > 0) + { + var timeout = TimeSpan.FromMinutes(timeoutMinutes); + step.ExecutionContext.SetTimeout(timeout); + } + + Common.Util.EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + + try + { + await step.RunAsync(); + } + catch (OperationCanceledException ex) + { + if (step.ExecutionContext.CancellationToken.IsCancellationRequested) + { + Trace.Error($"Caught timeout exception from step: {ex.Message}"); + step.ExecutionContext.Error("The action has timed out."); + step.ExecutionContext.Result = TaskResult.Failed; + } + else + { + // Log the exception and cancel the step. + Trace.Error($"Caught cancellation exception from step: {ex}"); + step.ExecutionContext.Error(ex); + step.ExecutionContext.Result = TaskResult.Canceled; + } + } + catch (Exception ex) + { + // Log the error and fail the step. + Trace.Error($"Caught exception from step: {ex}"); + step.ExecutionContext.Error(ex); + step.ExecutionContext.Result = TaskResult.Failed; + } + + // Merge execution context result with command result + if (step.ExecutionContext.CommandResult != null) + { + step.ExecutionContext.Result = Common.Util.TaskResultUtil.MergeTaskResults(step.ExecutionContext.Result, step.ExecutionContext.CommandResult.Value); + } + + // Fixup the step result if ContinueOnError. + if (step.ExecutionContext.Result == TaskResult.Failed) + { + var continueOnError = false; + try + { + continueOnError = templateEvaluator.EvaluateStepContinueOnError(step.ContinueOnError, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); + } + catch (Exception ex) + { + Trace.Info("The step failed and an error occurred when attempting to determine whether to continue on error."); + Trace.Error(ex); + step.ExecutionContext.Error("The step failed and an error occurred when attempting to determine whether to continue on error."); + step.ExecutionContext.Error(ex); + } + + if (continueOnError) + { + step.ExecutionContext.Outcome = step.ExecutionContext.Result; + step.ExecutionContext.Result = TaskResult.Succeeded; + Trace.Info($"Updated step result (continue on error)"); + } + } + Trace.Info($"Step result: {step.ExecutionContext.Result}"); + + // Complete the step context. + step.ExecutionContext.Debug($"Finishing: {step.DisplayName}"); + } } } From 16df48538309cf7c98da4974a90ecb32a04c567a Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:44:55 -0400 Subject: [PATCH 26/52] Remove unn. attribute in ExecutionContext --- src/Runner.Worker/ExecutionContext.cs | 6 ------ .../Handlers/CompositeActionHandler.cs | 18 +++++++++--------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 5a7a7657fca..fb26de91f2c 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -72,9 +72,6 @@ public interface IExecutionContext : IRunnerService IExecutionContext FinalizeContext { get; set; } - // Only Composite Level ExecutionContext has CompositeSteps (public setter for handler) - List CompositeSteps { get; set; } - // Initialize void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token); void CancelToken(); @@ -175,9 +172,6 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext // Only job level ExecutionContext has StepsWithPostRegistered public HashSet StepsWithPostRegistered { get; private set; } - // Only Composite Level ExecutionContext has CompositeSteps (public setter for handler) - public List CompositeSteps { get; set; } - public bool EchoOnActionCommand { get; set; } public IExecutionContext FinalizeContext { get; set; } diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index d6e0f9f1941..bbc66ace003 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -56,7 +56,7 @@ public async Task RunAsync(ActionRunStage stage) int location = 0; // Initialize Composite Steps List of Steps - ExecutionContext.CompositeSteps = new List(); + var compositeSteps = new List(); foreach (Pipelines.ActionStep aStep in actionSteps) { @@ -100,7 +100,7 @@ public async Task RunAsync(ActionRunStage stage) // Add all steps to the Composite StepsRunner // Follows similar logic to how JobRunner invokes the StepsRunner for job steps! - ExecutionContext.CompositeSteps.Add(step); + compositeSteps.Add(step); location++; } @@ -116,12 +116,12 @@ public async Task RunAsync(ActionRunStage stage) actionRunner2.Stage = ActionRunStage.Main; actionRunner2.Condition = "always()"; var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); - ExecutionContext.CompositeSteps.Add(cleanUpStep); + compositeSteps.Add(cleanUpStep); // Then run the Composite StepsRunner try { - await RunAsync(ExecutionContext); + await RunAsync(compositeSteps); } catch (Exception ex) { @@ -131,15 +131,13 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.Result = TaskResult.Failed; } } - public async Task RunAsync(IExecutionContext actionContext) + public async Task RunAsync(List compositeSteps) { // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. - - ArgUtil.NotNull(actionContext, nameof(actionContext)); - ArgUtil.NotNull(actionContext.CompositeSteps, nameof(actionContext.CompositeSteps)); + ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. - foreach (IStep step in actionContext.CompositeSteps) + foreach (IStep step in compositeSteps) { // This is used for testing UI appearance. // System.Threading.Thread.Sleep(5000); @@ -268,6 +266,8 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.Debug($"Starting: {step.DisplayName}"); // Set the timeout + // TODO: Fix for Step Level Timeout Attributes for an individual Composite Run Step + // For now, we are not going to support this for an individual composite run step var timeoutMinutes = 0; var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); try From 79e572fa3e387060a037e0d72ea0c6470dd30217 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:49:26 -0400 Subject: [PATCH 27/52] Change protection levels, plus change function name to more clear meaning --- .../Handlers/CompositeActionHandler.cs | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index bbc66ace003..953eed275e3 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -121,7 +121,7 @@ public async Task RunAsync(ActionRunStage stage) // Then run the Composite StepsRunner try { - await RunAsync(compositeSteps); + await RunStepsAsync(compositeSteps); } catch (Exception ex) { @@ -131,7 +131,45 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.Result = TaskResult.Failed; } } - public async Task RunAsync(List compositeSteps) + + private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) + { + var executionContext = step.ExecutionContext; + + executionContext.Complete(result, resultCode: resultCode); + } + + private void HandleOutput() + { + // Evaluate the mapped outputs value + if (Data.Outputs != null) + { + // Evaluate the outputs in the steps context to easily retrieve the values + var actionManifestManager = HostContext.GetService(); + + // Format ExpressionValues to Dictionary + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var pair in ExecutionContext.ExpressionValues) + { + evaluateContext[pair.Key] = pair.Value; + } + + // Get the evluated composite outputs' values mapped to the outputs named + DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); + + // Set the outputs for the outputs object in the whole composite action + actionManifestManager.SetAllCompositeOutputs(ExecutionContext.FinalizeContext, actionOutputs); + } + } + + private void InitializeScope(IStep step) + { + var stepsContext = step.ExecutionContext.StepsContext; + var scopeName = step.ExecutionContext.ScopeName; + step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); + } + + private async Task RunStepsAsync(List compositeSteps) { // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); @@ -216,43 +254,6 @@ public async Task RunAsync(List compositeSteps) // Completion Status handled by StepsRunner for the whole Composite Action Step } - private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) - { - var executionContext = step.ExecutionContext; - - executionContext.Complete(result, resultCode: resultCode); - } - - private void HandleOutput() - { - // Evaluate the mapped outputs value - if (Data.Outputs != null) - { - // Evaluate the outputs in the steps context to easily retrieve the values - var actionManifestManager = HostContext.GetService(); - - // Format ExpressionValues to Dictionary - var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in ExecutionContext.ExpressionValues) - { - evaluateContext[pair.Key] = pair.Value; - } - - // Get the evluated composite outputs' values mapped to the outputs named - DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); - - // Set the outputs for the outputs object in the whole composite action - actionManifestManager.SetAllCompositeOutputs(ExecutionContext.FinalizeContext, actionOutputs); - } - } - - private void InitializeScope(IStep step) - { - var stepsContext = step.ExecutionContext.StepsContext; - var scopeName = step.ExecutionContext.ScopeName; - step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); - } - private async Task RunStepAsync(IStep step) { // Try to evaluate the display name From ce52070bd9c666ebc44b73e3f512e8bd4f50f500 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:54:39 -0400 Subject: [PATCH 28/52] Remove location param --- src/Runner.Worker/ExecutionContext.cs | 3 +-- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 7 +------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index fb26de91f2c..6b027c4e9e2 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -107,7 +107,7 @@ public interface IExecutionContext : IRunnerService // others void ForceTaskComplete(); void RegisterPostJobStep(IStep step); - IStep RegisterNestedStep(IActionRunner step, DictionaryContextData inputsData, int location, Dictionary envData, bool cleanUp = false); + IStep RegisterNestedStep(IActionRunner step, DictionaryContextData inputsData, Dictionary envData, bool cleanUp = false); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -279,7 +279,6 @@ public void RegisterPostJobStep(IStep step) public IStep RegisterNestedStep( IActionRunner step, DictionaryContextData inputsData, - int location, Dictionary envData, bool cleanUp = false) { diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 953eed275e3..c4b891b2c3e 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -52,9 +52,6 @@ public async Task RunAsync(ActionRunStage stage) inputsData[i.Key] = new StringContextData(i.Value); } - // Add each composite action step to the front of the queue - int location = 0; - // Initialize Composite Steps List of Steps var compositeSteps = new List(); @@ -95,14 +92,12 @@ public async Task RunAsync(ActionRunStage stage) actionRunner.Stage = stage; actionRunner.Condition = aStep.Condition; - var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, location, Environment); + var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, Environment); InitializeScope(step); // Add all steps to the Composite StepsRunner // Follows similar logic to how JobRunner invokes the StepsRunner for job steps! compositeSteps.Add(step); - - location++; } // Create a step that handles all the composite action steps' outputs From dc36e2e7d6b6ab320072de6a46d41ab2d5a274c8 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 11:55:12 -0400 Subject: [PATCH 29/52] location pt.2 fix --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index c4b891b2c3e..3b5e14d95eb 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -110,7 +110,7 @@ public async Task RunAsync(ActionRunStage stage) actionRunner2.Action = cleanOutputsStep; actionRunner2.Stage = ActionRunStage.Main; actionRunner2.Condition = "always()"; - var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); + var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, Environment, true); compositeSteps.Add(cleanUpStep); // Then run the Composite StepsRunner From 3d4f7bd472ac64e5eaf9662c083a460656d7c26a Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:10:31 -0400 Subject: [PATCH 30/52] Remove outputs step --- src/Runner.Worker/ExecutionContext.cs | 15 ++------ .../Handlers/CompositeActionHandler.cs | 36 +++++++------------ 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 6b027c4e9e2..aae205cdb07 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -70,8 +70,6 @@ public interface IExecutionContext : IRunnerService bool EchoOnActionCommand { get; set; } - IExecutionContext FinalizeContext { get; set; } - // Initialize void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token); void CancelToken(); @@ -107,7 +105,7 @@ public interface IExecutionContext : IRunnerService // others void ForceTaskComplete(); void RegisterPostJobStep(IStep step); - IStep RegisterNestedStep(IActionRunner step, DictionaryContextData inputsData, Dictionary envData, bool cleanUp = false); + IStep RegisterNestedStep(IActionRunner step, DictionaryContextData inputsData, Dictionary envData); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -174,8 +172,6 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext public bool EchoOnActionCommand { get; set; } - public IExecutionContext FinalizeContext { get; set; } - public TaskResult? Result { get @@ -279,8 +275,7 @@ public void RegisterPostJobStep(IStep step) public IStep RegisterNestedStep( IActionRunner step, DictionaryContextData inputsData, - Dictionary envData, - bool cleanUp = false) + Dictionary envData) { // If the context name is empty and the scope name is empty, we would generate a unique scope name for this child in the following format: // "__" @@ -296,12 +291,6 @@ public IStep RegisterNestedStep( step.ExecutionContext.ExpressionValues["inputs"] = inputsData; - // Set Parent Attribute for Clean Up Step - if (cleanUp) - { - step.ExecutionContext.FinalizeContext = this; - } - // Add the composite action environment variables to each step. #if OS_WINDOWS var envContext = new DictionaryContextData(); diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 3b5e14d95eb..48af186f696 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -35,13 +35,6 @@ public async Task RunAsync(ActionRunStage stage) var tempDirectory = HostContext.GetDirectory(WellKnownDirectory.Temp); - // If clean up outputs step, we finish setting the outputs object and then return out of this function. - if (ExecutionContext.FinalizeContext != null) - { - HandleOutput(); - return; - } - // Resolve action steps var actionSteps = Data.Steps; @@ -100,23 +93,18 @@ public async Task RunAsync(ActionRunStage stage) compositeSteps.Add(step); } - // Create a step that handles all the composite action steps' outputs - Pipelines.ActionStep cleanOutputsStep = new Pipelines.ActionStep(); - cleanOutputsStep.ContextName = ExecutionContext.ContextName; - // Use the same reference type as our composite steps. - cleanOutputsStep.Reference = Action; + // All steps have the same initial ExecutionContext/ + // We store one of these ExecutionContexts in this variable + // so that we can process the outputs correctly. + var stepExecutionContext = compositeSteps?[0].ExecutionContext; - var actionRunner2 = HostContext.CreateService(); - actionRunner2.Action = cleanOutputsStep; - actionRunner2.Stage = ActionRunStage.Main; - actionRunner2.Condition = "always()"; - var cleanUpStep = ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, Environment, true); - compositeSteps.Add(cleanUpStep); - - // Then run the Composite StepsRunner try { + // This is where we run each step. await RunStepsAsync(compositeSteps); + + // This is where we set the outputs. + HandleOutput(stepExecutionContext); } catch (Exception ex) { @@ -134,7 +122,7 @@ private void CompleteStep(IStep step, TaskResult? result = null, string resultCo executionContext.Complete(result, resultCode: resultCode); } - private void HandleOutput() + private void HandleOutput(IExecutionContext stepExecutionContext) { // Evaluate the mapped outputs value if (Data.Outputs != null) @@ -144,16 +132,16 @@ private void HandleOutput() // Format ExpressionValues to Dictionary var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in ExecutionContext.ExpressionValues) + foreach (var pair in stepExecutionContext.ExpressionValues) { evaluateContext[pair.Key] = pair.Value; } // Get the evluated composite outputs' values mapped to the outputs named - DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); + DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(stepExecutionContext, Data.Outputs, evaluateContext); // Set the outputs for the outputs object in the whole composite action - actionManifestManager.SetAllCompositeOutputs(ExecutionContext.FinalizeContext, actionOutputs); + actionManifestManager.SetAllCompositeOutputs(ExecutionContext, actionOutputs); } } From bc613a0f92789f670798e72578c8179d53753db5 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:11:32 -0400 Subject: [PATCH 31/52] Remove temp directory --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 48af186f696..cce8938dd56 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -33,8 +33,6 @@ public async Task RunAsync(ActionRunStage stage) var githubContext = ExecutionContext.ExpressionValues["github"] as GitHubContext; ArgUtil.NotNull(githubContext, nameof(githubContext)); - var tempDirectory = HostContext.GetDirectory(WellKnownDirectory.Temp); - // Resolve action steps var actionSteps = Data.Steps; From a0af7d6555e3d40b1164d16b41126351300354c7 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:12:27 -0400 Subject: [PATCH 32/52] new line --- src/Runner.Common/Util/EncodingUtil.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index cab46ff959d..42b49bbc474 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -47,4 +47,4 @@ public static async void EnsureUtfEncoding(IHostContext hostContext, Tracing tra await System.Threading.Tasks.Task.CompletedTask; } } -} \ No newline at end of file +} From 81bdee6972415b6780a3b79f9905fa7148c424fa Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:18:43 -0400 Subject: [PATCH 33/52] Add arguitl not null --- .../Handlers/CompositeActionHandler.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index cce8938dd56..e827e4e8de9 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -29,6 +29,7 @@ public async Task RunAsync(ActionRunStage stage) Trace.Entering(); ArgUtil.NotNull(ExecutionContext, nameof(ExecutionContext)); ArgUtil.NotNull(Inputs, nameof(Inputs)); + ArgUtil.NotNull(Data.Steps, nameof(Data.Steps)); var githubContext = ExecutionContext.ExpressionValues["github"] as GitHubContext; ArgUtil.NotNull(githubContext, nameof(githubContext)); @@ -91,18 +92,15 @@ public async Task RunAsync(ActionRunStage stage) compositeSteps.Add(step); } - // All steps have the same initial ExecutionContext/ - // We store one of these ExecutionContexts in this variable - // so that we can process the outputs correctly. - var stepExecutionContext = compositeSteps?[0].ExecutionContext; - try { // This is where we run each step. await RunStepsAsync(compositeSteps); // This is where we set the outputs. - HandleOutput(stepExecutionContext); + // All steps have the same initial ExecutionContext/ + // So we can use one of them so that we can get the right "outputs" attribute. + HandleOutput(compositeSteps?[0].ExecutionContext); } catch (Exception ex) { @@ -122,6 +120,8 @@ private void CompleteStep(IStep step, TaskResult? result = null, string resultCo private void HandleOutput(IExecutionContext stepExecutionContext) { + ArgUtil.NotNull(stepExecutionContext, nameof(stepExecutionContext)); + // Evaluate the mapped outputs value if (Data.Outputs != null) { @@ -152,7 +152,6 @@ private void InitializeScope(IStep step) private async Task RunStepsAsync(List compositeSteps) { - // Another approach we can explore, is moving all this logic to the CompositeActionHandler if it's small enough. ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. From 44ed24720f34cbe9c045c0a034426563879a19be Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:22:08 -0400 Subject: [PATCH 34/52] better comment --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index e827e4e8de9..b0f0ff4f09e 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -98,7 +98,7 @@ public async Task RunAsync(ActionRunStage stage) await RunStepsAsync(compositeSteps); // This is where we set the outputs. - // All steps have the same initial ExecutionContext/ + // All steps have the same scope for their ExecutionContext/ // So we can use one of them so that we can get the right "outputs" attribute. HandleOutput(compositeSteps?[0].ExecutionContext); } From 3c2d8f1c9a3da72c977760963c161c498ce54efe Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 12:52:47 -0400 Subject: [PATCH 35/52] Change encoding name --- src/Runner.Common/Util/EncodingUtil.cs | 2 +- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- src/Runner.Worker/StepsRunner.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index 42b49bbc474..8b8a445fa56 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -7,7 +7,7 @@ namespace GitHub.Runner.Common.Util public static class EncodingUtil { - public static async void EnsureUtfEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) + public static async void SetEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) { #if OS_WINDOWS try diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index b0f0ff4f09e..a94ea8cb70e 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -268,7 +268,7 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.SetTimeout(timeout); } - Common.Util.EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + Common.Util.EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); try { diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 4ee34277baa..928d40f910b 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,7 +285,7 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } - EncodingUtil.EnsureUtfEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); try { From 868a42a4332d5091276ee7a860c7e81fe2c1b9af Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 15:34:42 -0400 Subject: [PATCH 36/52] Check count > 0 for composite steps, import System.Threading --- src/Runner.Common/Util/EncodingUtil.cs | 3 ++- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index 8b8a445fa56..3af01e16e88 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -1,3 +1,4 @@ +using System.Threading; using GitHub.Runner.Sdk; using GitHub.Runner.Common; @@ -7,7 +8,7 @@ namespace GitHub.Runner.Common.Util public static class EncodingUtil { - public static async void SetEncoding(IHostContext hostContext, Tracing trace, System.Threading.CancellationToken cancellationToken) + public static async void SetEncoding(IHostContext hostContext, Tracing trace, CancellationToken cancellationToken) { #if OS_WINDOWS try diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index a94ea8cb70e..fb07132ee1a 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -100,7 +100,11 @@ public async Task RunAsync(ActionRunStage stage) // This is where we set the outputs. // All steps have the same scope for their ExecutionContext/ // So we can use one of them so that we can get the right "outputs" attribute. - HandleOutput(compositeSteps?[0].ExecutionContext); + if (compositeSteps.Count > 0) + { + // TODO: Find better solution. + HandleOutput(compositeSteps[0].ExecutionContext); + } } catch (Exception ex) { @@ -296,6 +300,7 @@ private async Task RunStepAsync(IStep step) Trace.Error($"Caught exception from step: {ex}"); step.ExecutionContext.Error(ex); step.ExecutionContext.Result = TaskResult.Failed; + } // Merge execution context result with command result From 61a0049203cf8dbbbe6938c02a03232fa3b58454 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 15:43:37 -0400 Subject: [PATCH 37/52] Change function header encodingutil --- src/Runner.Common/Util/EncodingUtil.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index 3af01e16e88..c1c25f5b42c 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -1,4 +1,5 @@ using System.Threading; +using System.Threading.Tasks; using GitHub.Runner.Sdk; using GitHub.Runner.Common; @@ -8,7 +9,7 @@ namespace GitHub.Runner.Common.Util public static class EncodingUtil { - public static async void SetEncoding(IHostContext hostContext, Tracing trace, CancellationToken cancellationToken) + public static async Task SetEncoding(IHostContext hostContext, Tracing trace, CancellationToken cancellationToken) { #if OS_WINDOWS try @@ -45,7 +46,7 @@ public static async void SetEncoding(IHostContext hostContext, Tracing trace, Ca } #endif // Dummy variable to prevent compiler error CS1998: "This async method lacks 'await' operators and will run synchronously..." - await System.Threading.Tasks.Task.CompletedTask; + await Task.CompletedTask; } } } From df4e4c1c1b5e1a888d63c92e527c4d7d3cd2289f Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 15:46:54 -0400 Subject: [PATCH 38/52] Add TODO --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index fb07132ee1a..15c0763c1a3 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -201,6 +201,7 @@ private async Task RunStepsAsync(List compositeSteps) var actionStep = step as IActionRunner; // Set GITHUB_ACTION + // TODO: Fix this after SDK Changes. if (!String.IsNullOrEmpty(step.ExecutionContext.ScopeName)) { step.ExecutionContext.SetGitHubContext("action", step.ExecutionContext.ScopeName); From 1cafbf068bcb602f64d2f271f96f81f65c292256 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 15:51:27 -0400 Subject: [PATCH 39/52] Add await --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- src/Runner.Worker/StepsRunner.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 15c0763c1a3..9917e52903d 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -273,7 +273,7 @@ private async Task RunStepAsync(IStep step) step.ExecutionContext.SetTimeout(timeout); } - Common.Util.EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + await Common.Util.EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); try { diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 928d40f910b..39f22ee5f60 100644 --- a/src/Runner.Worker/StepsRunner.cs +++ b/src/Runner.Worker/StepsRunner.cs @@ -285,7 +285,7 @@ private async Task RunStepAsync(IStep step, CancellationToken jobCancellationTok step.ExecutionContext.SetTimeout(timeout); } - EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); + await EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); try { From 470708de1871f9f2b8da5db1be93309cbb9f827c Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 16:03:55 -0400 Subject: [PATCH 40/52] Handle Failed Step --- .../Handlers/CompositeActionHandler.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 9917e52903d..49cd71d6c71 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -95,7 +95,7 @@ public async Task RunAsync(ActionRunStage stage) try { // This is where we run each step. - await RunStepsAsync(compositeSteps); + await RunStepsAsync(compositeSteps, ExecutionContext); // This is where we set the outputs. // All steps have the same scope for their ExecutionContext/ @@ -154,7 +154,7 @@ private void InitializeScope(IStep step) step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } - private async Task RunStepsAsync(List compositeSteps) + private async Task RunStepsAsync(List compositeSteps, IExecutionContext executionContext) { ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); @@ -234,6 +234,14 @@ private async Task RunStepsAsync(List compositeSteps) await RunStepAsync(step); + // Handle Failed Step + // We will break out of loop immediately and display the result + if (step.ExecutionContext.Result == TaskResult.Failed) + { + executionContext.Result = step.ExecutionContext.Result; + break; + } + // TODO: Add compat for other types of steps. } // Completion Status handled by StepsRunner for the whole Composite Action Step @@ -301,7 +309,6 @@ private async Task RunStepAsync(IStep step) Trace.Error($"Caught exception from step: {ex}"); step.ExecutionContext.Error(ex); step.ExecutionContext.Result = TaskResult.Failed; - } // Merge execution context result with command result From 39157913ba929a60db827fc2fe66f0afa83442c8 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 16:24:21 -0400 Subject: [PATCH 41/52] Move over SetAllCompositeOutputs to the handler --- src/Runner.Worker/ActionManifestManager.cs | 30 ------------------- .../Handlers/CompositeActionHandler.cs | 30 ++++++++++++++++++- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index a7be7cb17cc..57c59e34b0d 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -30,8 +30,6 @@ public interface IActionManifestManager : IRunnerService Dictionary EvaluateContainerEnvironment(IExecutionContext executionContext, MappingToken token, IDictionary extraExpressionValues); string EvaluateDefaultInput(IExecutionContext executionContext, string inputName, TemplateToken token); - - void SetAllCompositeOutputs(IExecutionContext parentExecutionContext, DictionaryContextData actionOutputs); } public sealed class ActionManifestManager : RunnerService, IActionManifestManager @@ -170,34 +168,6 @@ public ActionDefinitionData Load(IExecutionContext executionContext, string mani return actionDefinition; } - public void SetAllCompositeOutputs( - IExecutionContext parentExecutionContext, - DictionaryContextData actionOutputs) - { - // Each pair is structured like this - // We ignore "description" for now - // { - // "the-output-name": { - // "description": "", - // "value": "the value" - // }, - // ... - // } - foreach (var pair in actionOutputs) - { - var outputsName = pair.Key; - var outputsAttributes = pair.Value as DictionaryContextData; - outputsAttributes.TryGetValue("value", out var val); - var outputsValue = val as StringContextData; - - // Set output in the whole composite scope. - if (!String.IsNullOrEmpty(outputsName) && !String.IsNullOrEmpty(outputsValue)) - { - parentExecutionContext.SetOutput(outputsName, outputsValue, out _); - } - } - } - public DictionaryContextData EvaluateCompositeOutputs( IExecutionContext executionContext, TemplateToken token, diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 49cd71d6c71..a41e883b0e5 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -143,7 +143,7 @@ private void HandleOutput(IExecutionContext stepExecutionContext) DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(stepExecutionContext, Data.Outputs, evaluateContext); // Set the outputs for the outputs object in the whole composite action - actionManifestManager.SetAllCompositeOutputs(ExecutionContext, actionOutputs); + SetAllCompositeOutputs(ExecutionContext, actionOutputs); } } @@ -154,6 +154,34 @@ private void InitializeScope(IStep step) step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } + private void SetAllCompositeOutputs( + IExecutionContext parentExecutionContext, + DictionaryContextData actionOutputs) + { + // Each pair is structured like this + // We ignore "description" for now + // { + // "the-output-name": { + // "description": "", + // "value": "the value" + // }, + // ... + // } + foreach (var pair in actionOutputs) + { + var outputsName = pair.Key; + var outputsAttributes = pair.Value as DictionaryContextData; + outputsAttributes.TryGetValue("value", out var val); + var outputsValue = val as StringContextData; + + // Set output in the whole composite scope. + if (!String.IsNullOrEmpty(outputsName) && !String.IsNullOrEmpty(outputsValue)) + { + parentExecutionContext.SetOutput(outputsName, outputsValue, out _); + } + } + } + private async Task RunStepsAsync(List compositeSteps, IExecutionContext executionContext) { ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); From a40c07b1d17836c86c4265a7da326dc8c0850318 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 16 Jul 2020 16:53:03 -0400 Subject: [PATCH 42/52] Remove timeout-minutes setting in steps-level --- .../Handlers/CompositeActionHandler.cs | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index a41e883b0e5..31803cbb38c 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -190,7 +190,7 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e foreach (IStep step in compositeSteps) { // This is used for testing UI appearance. - // System.Threading.Thread.Sleep(5000); + // System.Threading.Thread.Sleep(60000); Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); @@ -290,24 +290,8 @@ private async Task RunStepAsync(IStep step) // Set the timeout // TODO: Fix for Step Level Timeout Attributes for an individual Composite Run Step // For now, we are not going to support this for an individual composite run step - var timeoutMinutes = 0; + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); - try - { - timeoutMinutes = templateEvaluator.EvaluateStepTimeout(step.Timeout, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions); - } - catch (Exception ex) - { - Trace.Info("An error occurred when attempting to determine the step timeout."); - Trace.Error(ex); - step.ExecutionContext.Error("An error occurred when attempting to determine the step timeout."); - step.ExecutionContext.Error(ex); - } - if (timeoutMinutes > 0) - { - var timeout = TimeSpan.FromMinutes(timeoutMinutes); - step.ExecutionContext.SetTimeout(timeout); - } await Common.Util.EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); From 7d96e5d23547905d4e101a5a0b417307ae57c96c Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:07:38 -0400 Subject: [PATCH 43/52] Use only ExecutionContext --- .../Handlers/CompositeActionHandler.cs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 31803cbb38c..bc3d8d9f9b2 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -87,8 +87,6 @@ public async Task RunAsync(ActionRunStage stage) var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, Environment); InitializeScope(step); - // Add all steps to the Composite StepsRunner - // Follows similar logic to how JobRunner invokes the StepsRunner for job steps! compositeSteps.Add(step); } @@ -102,8 +100,13 @@ public async Task RunAsync(ActionRunStage stage) // So we can use one of them so that we can get the right "outputs" attribute. if (compositeSteps.Count > 0) { - // TODO: Find better solution. - HandleOutput(compositeSteps[0].ExecutionContext); + // Get the pointer of the correct "steps" object and pass it to the ExecutionContext + // This will always be the same for every step so we can pull this from thefirst step if it exists + var stepExecutionContext = compositeSteps.Count > 0 ? compositeSteps[0].ExecutionContext : null; + ExecutionContext.ExpressionValues["inputs"] = inputsData; + ExecutionContext.ExpressionValues["steps"] = stepExecutionContext.StepsContext.GetScope(stepExecutionContext.ScopeName); + + HandleOutput(); } } catch (Exception ex) @@ -122,9 +125,9 @@ private void CompleteStep(IStep step, TaskResult? result = null, string resultCo executionContext.Complete(result, resultCode: resultCode); } - private void HandleOutput(IExecutionContext stepExecutionContext) + private void HandleOutput() { - ArgUtil.NotNull(stepExecutionContext, nameof(stepExecutionContext)); + ArgUtil.NotNull(ExecutionContext, nameof(ExecutionContext)); // Evaluate the mapped outputs value if (Data.Outputs != null) @@ -134,13 +137,13 @@ private void HandleOutput(IExecutionContext stepExecutionContext) // Format ExpressionValues to Dictionary var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in stepExecutionContext.ExpressionValues) + foreach (var pair in ExecutionContext.ExpressionValues) { evaluateContext[pair.Key] = pair.Value; } // Get the evluated composite outputs' values mapped to the outputs named - DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(stepExecutionContext, Data.Outputs, evaluateContext); + DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); // Set the outputs for the outputs object in the whole composite action SetAllCompositeOutputs(ExecutionContext, actionOutputs); @@ -190,7 +193,7 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e foreach (IStep step in compositeSteps) { // This is used for testing UI appearance. - // System.Threading.Thread.Sleep(60000); + // System.Threading.Thread.Sleep(5000); Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); From 87143159545ca46a7d7adf7d7b6a782c4f4869a8 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:08:46 -0400 Subject: [PATCH 44/52] Move using to the top --- src/Runner.Common/Util/EncodingUtil.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs index c1c25f5b42c..2b6c393fb8e 100644 --- a/src/Runner.Common/Util/EncodingUtil.cs +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -1,3 +1,4 @@ +using System; using System.Threading; using System.Threading.Tasks; using GitHub.Runner.Sdk; @@ -5,8 +6,6 @@ namespace GitHub.Runner.Common.Util { - using System; - public static class EncodingUtil { public static async Task SetEncoding(IHostContext hostContext, Tracing trace, CancellationToken cancellationToken) From ca79af5fe1b2790a541a7f4ecf65b5c40bd202db Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:11:47 -0400 Subject: [PATCH 45/52] Remove redundant check --- .../Handlers/CompositeActionHandler.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index bc3d8d9f9b2..318e95fc221 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -95,19 +95,13 @@ public async Task RunAsync(ActionRunStage stage) // This is where we run each step. await RunStepsAsync(compositeSteps, ExecutionContext); - // This is where we set the outputs. - // All steps have the same scope for their ExecutionContext/ - // So we can use one of them so that we can get the right "outputs" attribute. - if (compositeSteps.Count > 0) - { - // Get the pointer of the correct "steps" object and pass it to the ExecutionContext - // This will always be the same for every step so we can pull this from thefirst step if it exists - var stepExecutionContext = compositeSteps.Count > 0 ? compositeSteps[0].ExecutionContext : null; - ExecutionContext.ExpressionValues["inputs"] = inputsData; - ExecutionContext.ExpressionValues["steps"] = stepExecutionContext.StepsContext.GetScope(stepExecutionContext.ScopeName); + // Get the pointer of the correct "steps" object and pass it to the ExecutionContext so that we can process the outputs correctly + // This will always be the same for every step so we can pull this from the first step if it exists + var stepExecutionContext = compositeSteps.Count > 0 ? compositeSteps[0].ExecutionContext : null; + ExecutionContext.ExpressionValues["inputs"] = inputsData; + ExecutionContext.ExpressionValues["steps"] = stepExecutionContext.StepsContext.GetScope(stepExecutionContext.ScopeName); - HandleOutput(); - } + HandleOutput(); } catch (Exception ex) { From b6c30911fc62cb4c08bab089f3f8e82558f8ce65 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:14:13 -0400 Subject: [PATCH 46/52] Change function name --- src/Runner.Worker/ExecutionContext.cs | 4 ++-- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index aae205cdb07..b13f6878435 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -105,7 +105,7 @@ public interface IExecutionContext : IRunnerService // others void ForceTaskComplete(); void RegisterPostJobStep(IStep step); - IStep RegisterNestedStep(IActionRunner step, DictionaryContextData inputsData, Dictionary envData); + IStep CreateCompositeStep(IActionRunner step, DictionaryContextData inputsData, Dictionary envData); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -272,7 +272,7 @@ public void RegisterPostJobStep(IStep step) /// Helper function used in CompositeActionHandler::RunAsync to /// add a child node, aka a step, to the current job to the Root.JobSteps based on the location. /// - public IStep RegisterNestedStep( + public IStep CreateCompositeStep( IActionRunner step, DictionaryContextData inputsData, Dictionary envData) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 318e95fc221..3e6155fafa3 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -84,7 +84,7 @@ public async Task RunAsync(ActionRunStage stage) actionRunner.Stage = stage; actionRunner.Condition = aStep.Condition; - var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, Environment); + var step = ExecutionContext.CreateCompositeStep(actionRunner, inputsData, Environment); InitializeScope(step); compositeSteps.Add(step); From 22c4cb96b842ee9eb77c22c25f8a3e6396a37a49 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:14:47 -0400 Subject: [PATCH 47/52] Remove testing code --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 3e6155fafa3..08d914f8d65 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -186,9 +186,6 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. foreach (IStep step in compositeSteps) { - // This is used for testing UI appearance. - // System.Threading.Thread.Sleep(5000); - Trace.Info($"Processing composite step: DisplayName='{step.DisplayName}'"); step.ExecutionContext.ExpressionValues["steps"] = step.ExecutionContext.StepsContext.GetScope(step.ExecutionContext.ScopeName); From 89002ce360580c9e134aa1afc74b85133955b763 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:28:12 -0400 Subject: [PATCH 48/52] Consolidate error code --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 08d914f8d65..e013108d797 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -112,13 +112,6 @@ public async Task RunAsync(ActionRunStage stage) } } - private void CompleteStep(IStep step, TaskResult? result = null, string resultCode = null) - { - var executionContext = step.ExecutionContext; - - executionContext.Complete(result, resultCode: resultCode); - } - private void HandleOutput() { ArgUtil.NotNull(ExecutionContext, nameof(ExecutionContext)); @@ -249,7 +242,7 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e Trace.Info("Caught exception in Composite Steps Runner from expression for step.env"); // evaluateStepEnvFailed = true; step.ExecutionContext.Error(ex); - CompleteStep(step, TaskResult.Failed); + step.ExecutionContext.Complete(TaskResult.Failed); } // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) From 0ad860ebb853e150b2cd985bf5a5ab66e1712216 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:30:53 -0400 Subject: [PATCH 49/52] Consolidate code --- .../Handlers/CompositeActionHandler.cs | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index e013108d797..f3032dac788 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -133,7 +133,28 @@ private void HandleOutput() DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); // Set the outputs for the outputs object in the whole composite action - SetAllCompositeOutputs(ExecutionContext, actionOutputs); + // Each pair is structured like this + // We ignore "description" for now + // { + // "the-output-name": { + // "description": "", + // "value": "the value" + // }, + // ... + // } + foreach (var pair in actionOutputs) + { + var outputsName = pair.Key; + var outputsAttributes = pair.Value as DictionaryContextData; + outputsAttributes.TryGetValue("value", out var val); + var outputsValue = val as StringContextData; + + // Set output in the whole composite scope. + if (!String.IsNullOrEmpty(outputsName) && !String.IsNullOrEmpty(outputsValue)) + { + ExecutionContext.SetOutput(outputsName, outputsValue, out _); + } + } } } @@ -144,34 +165,6 @@ private void InitializeScope(IStep step) step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } - private void SetAllCompositeOutputs( - IExecutionContext parentExecutionContext, - DictionaryContextData actionOutputs) - { - // Each pair is structured like this - // We ignore "description" for now - // { - // "the-output-name": { - // "description": "", - // "value": "the value" - // }, - // ... - // } - foreach (var pair in actionOutputs) - { - var outputsName = pair.Key; - var outputsAttributes = pair.Value as DictionaryContextData; - outputsAttributes.TryGetValue("value", out var val); - var outputsValue = val as StringContextData; - - // Set output in the whole composite scope. - if (!String.IsNullOrEmpty(outputsName) && !String.IsNullOrEmpty(outputsValue)) - { - parentExecutionContext.SetOutput(outputsName, outputsValue, out _); - } - } - } - private async Task RunStepsAsync(List compositeSteps, IExecutionContext executionContext) { ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); From 28db0aa70295902014df6b0ce320363b13aecbfa Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:44:33 -0400 Subject: [PATCH 50/52] Change HandleOutput => ProcessCompositeActionOutputs --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index f3032dac788..8698c2ed23d 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -101,7 +101,7 @@ public async Task RunAsync(ActionRunStage stage) ExecutionContext.ExpressionValues["inputs"] = inputsData; ExecutionContext.ExpressionValues["steps"] = stepExecutionContext.StepsContext.GetScope(stepExecutionContext.ScopeName); - HandleOutput(); + ProcessCompositeActionOutputs(); } catch (Exception ex) { @@ -112,7 +112,7 @@ public async Task RunAsync(ActionRunStage stage) } } - private void HandleOutput() + private void ProcessCompositeActionOutputs() { ArgUtil.NotNull(ExecutionContext, nameof(ExecutionContext)); From dd476b8c8864010c24da4395b876e4a692306aef Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 12:48:05 -0400 Subject: [PATCH 51/52] Remove set the timeout comment --- src/Runner.Worker/Handlers/CompositeActionHandler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 8698c2ed23d..dc7046627b2 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -267,7 +267,6 @@ private async Task RunStepAsync(IStep step) Trace.Info("Starting the step."); step.ExecutionContext.Debug($"Starting: {step.DisplayName}"); - // Set the timeout // TODO: Fix for Step Level Timeout Attributes for an individual Composite Run Step // For now, we are not going to support this for an individual composite run step From d9e510975a592dccdbc5a5b9bc6c81256666ae94 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Fri, 17 Jul 2020 15:46:08 -0400 Subject: [PATCH 52/52] Add Cancelling functionality + Remove unn. parameter --- .../Handlers/CompositeActionHandler.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index dc7046627b2..c24a14f9da6 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -93,7 +93,7 @@ public async Task RunAsync(ActionRunStage stage) try { // This is where we run each step. - await RunStepsAsync(compositeSteps, ExecutionContext); + await RunStepsAsync(compositeSteps); // Get the pointer of the correct "steps" object and pass it to the ExecutionContext so that we can process the outputs correctly // This will always be the same for every step so we can pull this from the first step if it exists @@ -165,7 +165,7 @@ private void InitializeScope(IStep step) step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } - private async Task RunStepsAsync(List compositeSteps, IExecutionContext executionContext) + private async Task RunStepsAsync(List compositeSteps) { ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); @@ -238,7 +238,13 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e step.ExecutionContext.Complete(TaskResult.Failed); } - // We don't have to worry about the cancellation token stuff because that's handled by the composite action level (in the StepsRunner) + // Handle Cancellation + // We will break out of loop immediately and display the result + if (ExecutionContext.CancellationToken.IsCancellationRequested) + { + ExecutionContext.Result = TaskResult.Canceled; + break; + } await RunStepAsync(step); @@ -246,7 +252,7 @@ private async Task RunStepsAsync(List compositeSteps, IExecutionContext e // We will break out of loop immediately and display the result if (step.ExecutionContext.Result == TaskResult.Failed) { - executionContext.Result = step.ExecutionContext.Result; + ExecutionContext.Result = step.ExecutionContext.Result; break; }