From b041b313915483dae044ca25f0e740ed51756724 Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Fri, 17 Jul 2020 16:31:48 -0400 Subject: [PATCH] Composite Run Steps Refactoring (#591) * Add basic framework for baby steps runner * Basic logic for adding steps / invoking composite action steps * Composite Steps Runner MVP * Fix null object reference error * intialize composiute * Comment out code that is handled by stepsrunner * Add composite clean up step * Remove previous 'workarounds' from StepsRunner. Clean Up PR * Remove todo * Remove todo * Fix using unitialized object yikes * Remove time delay * Format handler * Move output handler into action handler * Add try to evaluate display name * Remove while loop yikes * Abstract away the windows encoding check during step running * Github context set to {ScopeName}.{ContextName} or {ContextName} if ScopeName is null * Remove setting result to sucess since result defaults to sucess * Fix windows error * Fix windows * revert: * Windows fix * Fix Windows Error in Abstraction * Remove Composite Steps Runner => consolidate into Composite Steps Runner * Remove unn. attribute in ExecutionContext * Change protection levels, plus change function name to more clear meaning * Remove location param * location pt.2 fix * Remove outputs step * Remove temp directory * new line * Add arguitl not null * better comment * Change encoding name * Check count > 0 for composite steps, import System.Threading * Change function header encodingutil * Add TODO * Add await * Handle Failed Step * Move over SetAllCompositeOutputs to the handler * Remove timeout-minutes setting in steps-level * Use only ExecutionContext * Move using to the top * Remove redundant check * Change function name * Remove testing code * Consolidate error code * Consolidate code * Change HandleOutput => ProcessCompositeActionOutputs * Remove set the timeout comment * Add Cancelling functionality + Remove unn. parameter --- src/Runner.Common/Util/EncodingUtil.cs | 51 ++++ src/Runner.Worker/ActionManifestManager.cs | 30 -- src/Runner.Worker/ExecutionContext.cs | 20 +- .../Handlers/CompositeActionHandler.cs | 267 ++++++++++++++++-- .../Handlers/CompositeActionOutputHandler.cs | 53 ---- src/Runner.Worker/Handlers/HandlerFactory.cs | 12 +- src/Runner.Worker/StepsRunner.cs | 50 +--- 7 files changed, 305 insertions(+), 178 deletions(-) create mode 100644 src/Runner.Common/Util/EncodingUtil.cs delete mode 100644 src/Runner.Worker/Handlers/CompositeActionOutputHandler.cs diff --git a/src/Runner.Common/Util/EncodingUtil.cs b/src/Runner.Common/Util/EncodingUtil.cs new file mode 100644 index 00000000000..2b6c393fb8e --- /dev/null +++ b/src/Runner.Common/Util/EncodingUtil.cs @@ -0,0 +1,51 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using GitHub.Runner.Sdk; +using GitHub.Runner.Common; + +namespace GitHub.Runner.Common.Util +{ + public static class EncodingUtil + { + public static async Task SetEncoding(IHostContext hostContext, Tracing trace, 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 + // Dummy variable to prevent compiler error CS1998: "This async method lacks 'await' operators and will run synchronously..." + await Task.CompletedTask; + } + } +} 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/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index b31dc97e4c3..b13f6878435 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, int location, Dictionary envData, bool cleanUp = false); + IStep CreateCompositeStep(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 @@ -276,12 +272,10 @@ 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, - int location, - 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: // "__" @@ -297,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(); @@ -315,8 +303,6 @@ public IStep RegisterNestedStep( } step.ExecutionContext.ExpressionValues["env"] = envContext; - 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..c24a14f9da6 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -23,18 +23,17 @@ 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(); 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)); - var tempDirectory = HostContext.GetDirectory(WellKnownDirectory.Temp); - // Resolve action steps var actionSteps = Data.Steps; @@ -45,8 +44,8 @@ public 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(); foreach (Pipelines.ActionStep aStep in actionSteps) { @@ -85,26 +84,78 @@ public Task RunAsync(ActionRunStage stage) actionRunner.Stage = stage; actionRunner.Condition = aStep.Condition; - var step = ExecutionContext.RegisterNestedStep(actionRunner, inputsData, location, Environment); - + var step = ExecutionContext.CreateCompositeStep(actionRunner, inputsData, Environment); InitializeScope(step); - location++; + compositeSteps.Add(step); + } + + try + { + // This is where we run each step. + 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 + var stepExecutionContext = compositeSteps.Count > 0 ? compositeSteps[0].ExecutionContext : null; + ExecutionContext.ExpressionValues["inputs"] = inputsData; + ExecutionContext.ExpressionValues["steps"] = stepExecutionContext.StepsContext.GetScope(stepExecutionContext.ScopeName); + + ProcessCompositeActionOutputs(); } + catch (Exception ex) + { + // Composite StepRunner should never throw exception out. + Trace.Error($"Caught exception from composite steps {nameof(CompositeActionHandler)}: {ex}"); + ExecutionContext.Error(ex); + ExecutionContext.Result = TaskResult.Failed; + } + } + + private void ProcessCompositeActionOutputs() + { + ArgUtil.NotNull(ExecutionContext, nameof(ExecutionContext)); + + // 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(); - // 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; + // Format ExpressionValues to Dictionary + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var pair in ExecutionContext.ExpressionValues) + { + evaluateContext[pair.Key] = pair.Value; + } - var actionRunner2 = HostContext.CreateService(); - actionRunner2.Action = cleanOutputsStep; - actionRunner2.Stage = ActionRunStage.Main; - actionRunner2.Condition = "always()"; - ExecutionContext.RegisterNestedStep(actionRunner2, inputsData, location, Environment, true); + // Get the evluated composite outputs' values mapped to the outputs named + DictionaryContextData actionOutputs = actionManifestManager.EvaluateCompositeOutputs(ExecutionContext, Data.Outputs, evaluateContext); - return Task.CompletedTask; + // Set the outputs for the outputs object in the whole composite action + // 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 _); + } + } + } } private void InitializeScope(IStep step) @@ -113,5 +164,183 @@ private void InitializeScope(IStep step) var scopeName = step.ExecutionContext.ScopeName; step.ExecutionContext.ExpressionValues["steps"] = stepsContext.GetScope(scopeName); } + + private async Task RunStepsAsync(List compositeSteps) + { + ArgUtil.NotNull(compositeSteps, nameof(compositeSteps)); + + // The parent StepsRunner of the whole Composite Action Step handles the cancellation stuff already. + foreach (IStep step in compositeSteps) + { + 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 + // TODO: Fix this after SDK Changes. + 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); + step.ExecutionContext.Complete(TaskResult.Failed); + } + + // 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); + + // 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 + } + + 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}"); + + // 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 templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(); + + await Common.Util.EncodingUtil.SetEncoding(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}"); + } } } 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 { diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs index 553f792482a..39f22ee5f60 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; @@ -300,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 + await EncodingUtil.SetEncoding(HostContext, Trace, step.ExecutionContext.CancellationToken); try {