From 65522633690ca61ea3460d119d9d316379cd32a5 Mon Sep 17 00:00:00 2001 From: Ethan Chiu Date: Thu, 18 Jun 2020 15:57:23 -0400 Subject: [PATCH] clean up --- src/Runner.Worker/ActionManifestManager.cs | 21 ++---------- src/Runner.Worker/ExecutionContext.cs | 21 ++++++------ .../Handlers/CompositeActionHandler.cs | 33 ++----------------- src/Runner.Worker/Handlers/ScriptHandler.cs | 8 ----- src/Runner.Worker/action_yaml.json | 6 ++-- 5 files changed, 18 insertions(+), 71 deletions(-) diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index eac9e1ea1..9abd05844 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -183,7 +183,7 @@ namespace GitHub.Runner.Worker var context = CreateContext(executionContext, extraExpressionValues); try { - var evaluateResult = TemplateEvaluator.Evaluate(context, "container-runs-env", token, 0, null, omitHeader: true); + var evaluateResult = TemplateEvaluator.Evaluate(context, "runs-env", token, 0, null, omitHeader: true); context.Errors.Check(); Trace.Info($"Environments evaluate result: {StringUtil.ConvertToJson(evaluateResult)}"); @@ -215,8 +215,6 @@ namespace GitHub.Runner.Worker return result; } - // TODO: Add Evaluate Composite Action Env Function Here - // Steps will be evaluated already in StepsRunner public Dictionary EvaluateCompositeActionEnvironment( IExecutionContext executionContext, MappingToken token, @@ -229,34 +227,19 @@ namespace GitHub.Runner.Worker var context = CreateContext(executionContext, extraExpressionValues); try { - var evaluateResult = TemplateEvaluator.Evaluate(context, "container-runs-env", token, 0, null, omitHeader: true); + var evaluateResult = TemplateEvaluator.Evaluate(context, "runs-env", token, 0, null, omitHeader: true); context.Errors.Check(); - if (evaluateResult != null) { - Trace.Info($"EvaluateCompositeActionEnvironment evaluateResult {evaluateResult}"); - } else { - Trace.Info($"EvaluateCompositeActionEnvironment evaluateResult is null"); - } - - - Trace.Info($"Composite Action Environments evaluate result: {StringUtil.ConvertToJson(evaluateResult)}"); // Mapping var mapping = evaluateResult.AssertMapping("composite env"); - if (mapping != null) { - Trace.Info($"EvaluateCompositeActionEnvironment Mapping {mapping}"); - } else { - Trace.Info($"EvaluateCompositeActionEnvironment Mapping is null"); - } foreach (var pair in mapping) { // Literal key var key = pair.Key.AssertString("composite env key"); - Trace.Info($"EvaluateCompositeActionEnvironment Key{key.Value}"); // Literal value var value = pair.Value.AssertString("composite env value"); - Trace.Info($"EvaluateCompositeActionEnvironment Value{value.Value}"); result[key.Value] = value.Value; Trace.Info($"Add env {key} = {value}"); diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 9cd7e6dbb..a8ee9c9b0 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -105,9 +105,8 @@ namespace GitHub.Runner.Worker // others void ForceTaskComplete(); void RegisterPostJobStep(IStep step); - IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData, PipelineContextData envData); + IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData, Dictionary envData); void EnqueueAllCompositeSteps(Queue steps); - ExecutionContext getParentExecutionContext(); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -272,7 +271,7 @@ namespace GitHub.Runner.Worker RegisterCompositeStep is a helper function used in CompositeActionHandler::RunAsync to add a child node, aka a step, to the current job to the front of the queue for processing. */ - public IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData, PipelineContextData envData) + public IStep RegisterCompositeStep(IStep step, DictionaryContextData inputsData, Dictionary envData) { // ~Brute Force Method~ // There is no way to put this current job in front of the queue in < O(n) time where n = # of elements in JobSteps @@ -297,10 +296,15 @@ namespace GitHub.Runner.Worker var newGuid = Guid.NewGuid(); step.ExecutionContext = Root.CreateChild(newGuid, step.DisplayName, newGuid.ToString("N"), null, null); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; - step.ExecutionContext.ExpressionValues["env"] = envData; - // TODO add environment variables for individual composite action steps - + // Add the composite action environment variables to each step. + // If the key already exists, we override it since the composite action env variables will have higher precedence + // Note that for each composite action step, it's environment variables will be set in the StepRunner automatically + foreach (var e in envData) + { + step.ExecutionContext.EnvironmentVariables[e.Key] = e.Value; + } + return step; } @@ -334,11 +338,6 @@ namespace GitHub.Runner.Worker } } - public ExecutionContext getParentExecutionContext() - { - return _parentExecutionContext; - } - public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null) { Trace.Entering(); diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 35ae41bb4..0902fafae 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -36,14 +36,6 @@ namespace GitHub.Runner.Worker.Handlers // Resolve action steps var actionSteps = Data.Steps; - if (actionSteps == null) - { - Trace.Error("Data.Steps in CompositeActionHandler is null"); - } - else - { - Trace.Info($"Data Steps Value for Composite Actions is: {actionSteps}."); - } // Create Context Data to reuse for each composite action step var inputsData = new DictionaryContextData(); @@ -52,31 +44,12 @@ namespace GitHub.Runner.Worker.Handlers inputsData[i.Key] = new StringContextData(i.Value); } - // Set up parent's environment data and then add on composite action environment data -#if OS_WINDOWS - var envData = ExecutionContext.ExpressionValues["env"].Clone() as DictionaryContextData; -#else - var envData = ExecutionContext.ExpressionValues["env"].Clone() as CaseSensitiveDictionaryContextData; -#endif - // Composite action will have already inherited the root env attributes. - // We evaluated the env simimilar to how ContainerActionHandler does it. - if (Data.Environment == null) { - Trace.Info($"Composite Env Mapping Token is null"); - } else { - Trace.Info($"Composite Env Mapping Token {Data.Environment}"); - } + + // Get Environment Data for Composite Action var extraExpressionValues = new Dictionary(StringComparer.OrdinalIgnoreCase); extraExpressionValues["inputs"] = inputsData; var manifestManager = HostContext.GetService(); - var evaluatedEnv = manifestManager.EvaluateCompositeActionEnvironment(ExecutionContext, Data.Environment, extraExpressionValues); - foreach (var e in evaluatedEnv) - { - // How to add to EnvironmentContextData - // We need to use IEnvironmentContextData because ScriptHandler uses this type for environment variables - Trace.Info($"Composite Action Env Key: {e.Key}"); - Trace.Info($"Composite Action Env Value: {e.Value}"); - envData[e.Key] = new StringContextData(e.Value); - } + var envData = manifestManager.EvaluateCompositeActionEnvironment(ExecutionContext, Data.Environment, extraExpressionValues); // Add each composite action step to the front of the queue var compositeActionSteps = new Queue(); diff --git a/src/Runner.Worker/Handlers/ScriptHandler.cs b/src/Runner.Worker/Handlers/ScriptHandler.cs index b93c40de8..051cd5fc7 100644 --- a/src/Runner.Worker/Handlers/ScriptHandler.cs +++ b/src/Runner.Worker/Handlers/ScriptHandler.cs @@ -255,14 +255,6 @@ namespace GitHub.Runner.Worker.Handlers Environment[env.Key] = env.Value; } } - - // if (context.Key == "env" && context.Value != null && !String.IsNullOrEmpty(System.Environment.GetEnvironmentVariable("TESTING_COMPOSITE_ACTIONS_ALPHA"))) - // { - // foreach (var env in context.Value as DictionaryContextData) - // { - // Environment[env.Key] = env.Value as StringContextData; - // } - // } } // dump out the command diff --git a/src/Runner.Worker/action_yaml.json b/src/Runner.Worker/action_yaml.json index f789edaf8..da937f2a9 100644 --- a/src/Runner.Worker/action_yaml.json +++ b/src/Runner.Worker/action_yaml.json @@ -43,7 +43,7 @@ "image": "non-empty-string", "entrypoint": "non-empty-string", "args": "container-runs-args", - "env": "container-runs-env", + "env": "runs-env", "pre-entrypoint": "non-empty-string", "pre-if": "non-empty-string", "post-entrypoint": "non-empty-string", @@ -56,7 +56,7 @@ "item-type": "container-runs-context" } }, - "container-runs-env": { + "runs-env": { "context": [ "inputs" ], @@ -88,7 +88,7 @@ "mapping": { "properties": { "using": "non-empty-string", - "env": "container-runs-env", + "env": "runs-env", "steps": "composite-steps" } }