From da3cb5506fe80c2f540c74172524483c0bbe4341 Mon Sep 17 00:00:00 2001 From: David Kale Date: Wed, 22 Jul 2020 14:55:49 -0400 Subject: [PATCH 1/8] Fold logs for intermediate docker commands (#608) --- src/Runner.Worker/ActionManager.cs | 6 ++++-- src/Runner.Worker/ContainerOperationProvider.cs | 16 ++++++++++++++-- .../Handlers/ContainerActionHandler.cs | 5 ++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 0a942bf79..1a730130a 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -468,7 +468,7 @@ namespace GitHub.Runner.Worker ArgUtil.NotNull(setupInfo, nameof(setupInfo)); ArgUtil.NotNullOrEmpty(setupInfo.Container.Image, nameof(setupInfo.Container.Image)); - executionContext.Output($"Pull down action image '{setupInfo.Container.Image}'"); + executionContext.Output($"##[group]Pull down action image '{setupInfo.Container.Image}'"); // Pull down docker image with retry up to 3 times var dockerManger = HostContext.GetService(); @@ -492,6 +492,7 @@ namespace GitHub.Runner.Worker } } } + executionContext.Output("##[endgroup"); if (retryCount == 3 && pullExitCode != 0) { @@ -511,7 +512,7 @@ namespace GitHub.Runner.Worker ArgUtil.NotNull(setupInfo, nameof(setupInfo)); ArgUtil.NotNullOrEmpty(setupInfo.Container.Dockerfile, nameof(setupInfo.Container.Dockerfile)); - executionContext.Output($"Build container for action use: '{setupInfo.Container.Dockerfile}'."); + executionContext.Output($"##[group]Build container for action use: '{setupInfo.Container.Dockerfile}'."); // Build docker image with retry up to 3 times var dockerManger = HostContext.GetService(); @@ -541,6 +542,7 @@ namespace GitHub.Runner.Worker } } } + executionContext.Output("##[endgroup]"); if (retryCount == 3 && buildExitCode != 0) { diff --git a/src/Runner.Worker/ContainerOperationProvider.cs b/src/Runner.Worker/ContainerOperationProvider.cs index 2a27a731a..8ec8a9644 100644 --- a/src/Runner.Worker/ContainerOperationProvider.cs +++ b/src/Runner.Worker/ContainerOperationProvider.cs @@ -91,7 +91,10 @@ namespace GitHub.Runner.Worker #endif // Check docker client/server version + executionContext.Output("##[group]Checking docker version"); DockerVersion dockerVersion = await _dockerManger.DockerVersion(executionContext); + executionContext.Output("##[endgroup]"); + ArgUtil.NotNull(dockerVersion.ServerVersion, nameof(dockerVersion.ServerVersion)); ArgUtil.NotNull(dockerVersion.ClientVersion, nameof(dockerVersion.ClientVersion)); @@ -111,7 +114,7 @@ namespace GitHub.Runner.Worker } // Clean up containers left by previous runs - executionContext.Debug($"Delete stale containers from previous jobs"); + executionContext.Output("##[group]Clean up resources from previous jobs"); var staleContainers = await _dockerManger.DockerPS(executionContext, $"--all --quiet --no-trunc --filter \"label={_dockerManger.DockerInstanceLabel}\""); foreach (var staleContainer in staleContainers) { @@ -122,18 +125,20 @@ namespace GitHub.Runner.Worker } } - executionContext.Debug($"Delete stale container networks from previous jobs"); int networkPruneExitCode = await _dockerManger.DockerNetworkPrune(executionContext); if (networkPruneExitCode != 0) { executionContext.Warning($"Delete stale container networks failed, docker network prune fail with exit code {networkPruneExitCode}"); } + executionContext.Output("##[endgroup]"); // Create local docker network for this job to avoid port conflict when multiple runners run on same machine. // All containers within a job join the same network + executionContext.Output("##[group]Create local container network"); var containerNetwork = $"github_network_{Guid.NewGuid().ToString("N")}"; await CreateContainerNetworkAsync(executionContext, containerNetwork); executionContext.JobContext.Container["network"] = new StringContextData(containerNetwork); + executionContext.Output("##[endgroup]"); foreach (var container in containers) { @@ -141,10 +146,12 @@ namespace GitHub.Runner.Worker await StartContainerAsync(executionContext, container); } + executionContext.Output("##[group]Waiting for all services to be ready"); foreach (var container in containers.Where(c => !c.IsJobContainer)) { await ContainerHealthcheck(executionContext, container); } + executionContext.Output("##[endgroup]"); } public async Task StopContainersAsync(IExecutionContext executionContext, object data) @@ -173,6 +180,10 @@ namespace GitHub.Runner.Worker Trace.Info($"Container name: {container.ContainerName}"); Trace.Info($"Container image: {container.ContainerImage}"); Trace.Info($"Container options: {container.ContainerCreateOptions}"); + + var groupName = container.IsJobContainer ? "Starting job container" : $"Starting {container.ContainerNetworkAlias} service container"; + executionContext.Output($"##[group]{groupName}"); + foreach (var port in container.UserPortMappings) { Trace.Info($"User provided port: {port.Value}"); @@ -304,6 +315,7 @@ namespace GitHub.Runner.Worker container.ContainerRuntimePath = DockerUtil.ParsePathFromConfigEnv(containerEnv); executionContext.JobContext.Container["id"] = new StringContextData(container.ContainerId); } + executionContext.Output("##[endgroup]"); } private async Task StopContainerAsync(IExecutionContext executionContext, ContainerInfo container) diff --git a/src/Runner.Worker/Handlers/ContainerActionHandler.cs b/src/Runner.Worker/Handlers/ContainerActionHandler.cs index 573010030..6e8624510 100644 --- a/src/Runner.Worker/Handlers/ContainerActionHandler.cs +++ b/src/Runner.Worker/Handlers/ContainerActionHandler.cs @@ -49,8 +49,9 @@ namespace GitHub.Runner.Worker.Handlers // ensure docker file exist var dockerFile = Path.Combine(ActionDirectory, Data.Image); ArgUtil.File(dockerFile, nameof(Data.Image)); - ExecutionContext.Output($"Dockerfile for action: '{dockerFile}'."); + ExecutionContext.Output($"##[group]Building docker image"); + ExecutionContext.Output($"Dockerfile for action: '{dockerFile}'."); var imageName = $"{dockerManger.DockerInstanceLabel}:{ExecutionContext.Id.ToString("N")}"; var buildExitCode = await dockerManger.DockerBuild( ExecutionContext, @@ -58,6 +59,8 @@ namespace GitHub.Runner.Worker.Handlers dockerFile, Directory.GetParent(dockerFile).FullName, imageName); + ExecutionContext.Output("##[endgroup]"); + if (buildExitCode != 0) { throw new InvalidOperationException($"Docker build failed with exit code {buildExitCode}"); From 005f1c15b1af69e34ce342005c1c32f1a23f0746 Mon Sep 17 00:00:00 2001 From: Steven Maude Date: Wed, 22 Jul 2020 21:10:57 +0100 Subject: [PATCH 2/8] Fix "propogate" typo in ADR 0549 (#600) --- docs/adrs/0549-composite-run-steps.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adrs/0549-composite-run-steps.md b/docs/adrs/0549-composite-run-steps.md index b86bf856a..c4d6ff983 100644 --- a/docs/adrs/0549-composite-run-steps.md +++ b/docs/adrs/0549-composite-run-steps.md @@ -176,7 +176,7 @@ See the paragraph below for a rudimentary approach (thank you to @cybojenix for The `if` statement in the parent (in the example above, this is the `workflow.yml`) shows whether or not we should run the composite action. So, our composite action will run since the `if` condition for running the composite action is `always()`. -**Note that the if condition on the parent does not propogate to the rest of its children though.** +**Note that the if condition on the parent does not propagate to the rest of its children though.** In the child action (in this example, this is the `action.yml`), it starts with a clean slate (in other words, no imposing if conditions). Similar to the logic in the paragraph above, `echo "I will run, as my current scope is succeeding"` will run since the `if` condition checks if the previous steps **within this composite action** has not failed. `run: echo "I will not run, as my current scope is now failing"` will not run since the previous step resulted in an error and by default, the if expression is set to `success()` if the if condition is not set for a step. From bd1f245aace715a0a0879859a2b19f893d3326d4 Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Wed, 22 Jul 2020 16:11:55 -0400 Subject: [PATCH 3/8] Clarify details for defaults, shell, and working-dir (#607) --- docs/adrs/0549-composite-run-steps.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/adrs/0549-composite-run-steps.md b/docs/adrs/0549-composite-run-steps.md index c4d6ff983..09e8ed763 100644 --- a/docs/adrs/0549-composite-run-steps.md +++ b/docs/adrs/0549-composite-run-steps.md @@ -14,7 +14,7 @@ An important step towards meeting this goal is to build in functionality for act We don't want the workflow author to need to know how the internal workings of the action work. Users shouldn't know the internal workings of the composite action (for example, `default.shell` and `default.workingDir` should not be inherited from the workflow file to the action file). When deciding how to design certain parts of composite run steps, we want to think one logical step from the consumer. -A composite action is treated as **one** individual job step (aka encapsulation). +A composite action is treated as **one** individual job step (this is known as encapsulation). ## Decision @@ -251,8 +251,27 @@ If any of the steps fail in the composite action and the `continue-on-error` is For the composite action steps, it follows the same logic as above. In this example, `"Hello World 2"` will be outputted because the previous step has `continue-on-error` set to `true` although that previous step errored. ### Defaults +We will not support "defaults" in a composite action. -The composite action author will be required to set the `shell` and `workingDir` of the composite action. Moreover, the composite action author will be able to explicitly set the shell for each composite run step. The workflow author will not have the ability to change these attributes. +### Shell and Working-directory +For each run step in a composite action, the action author can set the `shell` and `working-directory` attributes for that step. These attributes are optional for each run step - by default, the `shell` is set to whatever default value is associated with the runner os (ex: bash =\> Mac). Moreover, the composite action author can map in values from the `inputs` for it's `shell` and `working-directory` attributes at the step level for an action. + +For example, + +`action.yml` + + +```yaml +inputs: + shell_1: + description: 'Your name' + default: 'pwsh' +steps: + - run: echo 1 + shell: ${{ inputs.shell_1 }} +``` + +Note, the workflow file and action file are treated as separate entities. **So, the workflow `defaults` will never change the `shell` and `working-directory` value in the run steps in a composite action.** Note, `defaults` in a workflow only apply to run steps not "uses" steps (steps that use an action). ### Visualizing Composite Action in the GitHub Actions UI We want all the composite action's steps to be condensed into the original composite action node. From 3d0147d322d6be1424e73dce97ebf5aaa2ebcbae Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Wed, 22 Jul 2020 17:34:40 -0400 Subject: [PATCH 4/8] Improve Debugging Messages for Empty Tokens (#609) * Improve Debugging Messages for Empty Tokens * fix tests --- src/Runner.Worker/ActionManifestManager.cs | 10 +++++----- src/Test/L0/Worker/ActionManagerL0.cs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index 5be77c174..aa6637f85 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -135,7 +135,7 @@ namespace GitHub.Runner.Worker // Evaluate Runs Last if (actionRunValueToken != null) { - actionDefinition.Execution = ConvertRuns(executionContext, templateContext, actionRunValueToken, actionOutputs); + actionDefinition.Execution = ConvertRuns(executionContext, templateContext, actionRunValueToken, fileRelativePath, actionOutputs); } } catch (Exception ex) @@ -359,6 +359,7 @@ namespace GitHub.Runner.Worker IExecutionContext executionContext, TemplateContext templateContext, TemplateToken inputsToken, + String fileRelativePath, MappingToken outputs = null) { var runsMapping = inputsToken.AssertMapping("runs"); @@ -442,7 +443,7 @@ namespace GitHub.Runner.Worker { if (string.IsNullOrEmpty(imageToken?.Value)) { - throw new ArgumentNullException($"Image is not provided."); + throw new ArgumentNullException($"You are using a Container Action but an image is not provided in {fileRelativePath}."); } else { @@ -463,7 +464,7 @@ namespace GitHub.Runner.Worker { if (string.IsNullOrEmpty(mainToken?.Value)) { - throw new ArgumentNullException($"Entry javascript file is not provided."); + throw new ArgumentNullException($"You are using a JavaScript Action but there is not an entry JavaScript file provided in {fileRelativePath}."); } else { @@ -481,8 +482,7 @@ namespace GitHub.Runner.Worker { if (steps == null) { - // TODO: Add a more helpful error message + including file name, etc. to show user that it's because of their yaml file - throw new ArgumentNullException($"No steps provided."); + throw new ArgumentNullException($"You are using a composite action but there are no steps provided in {fileRelativePath}."); } else { diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 0481d9894..0a343ebc1 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -846,7 +846,7 @@ namespace GitHub.Runner.Common.Tests.Worker { var traceFile = Path.GetTempFileName(); File.Copy(_hc.TraceFileName, traceFile, true); - Assert.Contains("Entry javascript file is not provided.", File.ReadAllText(traceFile)); + Assert.Contains("You are using a JavaScript Action but there is not an entry JavaScript file provided in", File.ReadAllText(traceFile)); } } finally @@ -2466,7 +2466,7 @@ runs: { var traceFile = Path.GetTempFileName(); File.Copy(_hc.TraceFileName, traceFile, true); - Assert.Contains("Entry javascript file is not provided.", File.ReadAllText(traceFile)); + Assert.Contains("You are using a JavaScript Action but there is not an entry JavaScript file provided in", File.ReadAllText(traceFile)); } } finally From d5a55506494fa3cadb4252a92f87c64a94366368 Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Wed, 22 Jul 2020 18:01:50 -0400 Subject: [PATCH 5/8] Fix Timeout-minutes for Whole Composite Action Step (#599) * Exploring child Linked Cancellation Tokens * Preliminary Timeout-minutes fix * Final Solution for resolving cancellation token's timeout vs. cancellation * Clean up + Fix error handling * Use linked tokens instead * Clean up * one liner * Remove JobExecutionContext => Replace with public Root accessor * Move CreateLinkedTokenSource in the CreateCompositeStep Function --- src/Runner.Worker/ExecutionContext.cs | 12 +++--- .../Handlers/CompositeActionHandler.cs | 42 +++---------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 6f6279c6b..5385490d7 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -60,10 +60,12 @@ namespace GitHub.Runner.Worker bool EchoOnActionCommand { get; set; } + ExecutionContext Root { get; } + // 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); + IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, CancellationTokenSource cancellationTokenSource = null); // logging long Write(string tag, string message); @@ -180,7 +182,7 @@ namespace GitHub.Runner.Worker } } - private ExecutionContext Root + public ExecutionContext Root { get { @@ -254,7 +256,7 @@ namespace GitHub.Runner.Worker DictionaryContextData inputsData, Dictionary envData) { - step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger); + step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token)); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; step.ExecutionContext.ExpressionValues["steps"] = Global.StepsContext.GetScope(step.ExecutionContext.GetFullyQualifiedContextName()); @@ -273,7 +275,7 @@ namespace GitHub.Runner.Worker return step; } - public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null) + public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, CancellationTokenSource cancellationTokenSource = null) { Trace.Entering(); @@ -298,7 +300,7 @@ namespace GitHub.Runner.Worker { child.ExpressionFunctions.Add(item); } - child._cancellationTokenSource = new CancellationTokenSource(); + child._cancellationTokenSource = cancellationTokenSource ?? new CancellationTokenSource(); child._parentExecutionContext = this; child.EchoOnActionCommand = EchoOnActionCommand; diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 80e4d13a5..3ab7957cf 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; using GitHub.DistributedTask.ObjectTemplating.Tokens; using GitHub.DistributedTask.Pipelines.ContextData; @@ -197,19 +198,11 @@ namespace GitHub.Runner.Worker.Handlers 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) + // Directly after the step, check if the step has failed or cancelled + // If so, return that to the output + if (step.ExecutionContext.Result == TaskResult.Failed || step.ExecutionContext.Result == TaskResult.Canceled) { ExecutionContext.Result = step.ExecutionContext.Result; break; @@ -245,7 +238,8 @@ namespace GitHub.Runner.Worker.Handlers } catch (OperationCanceledException ex) { - if (step.ExecutionContext.CancellationToken.IsCancellationRequested) + if (step.ExecutionContext.CancellationToken.IsCancellationRequested && + !ExecutionContext.Root.CancellationToken.IsCancellationRequested) { Trace.Error($"Caught timeout exception from step: {ex.Message}"); step.ExecutionContext.Error("The action has timed out."); @@ -253,7 +247,6 @@ namespace GitHub.Runner.Worker.Handlers } 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; @@ -273,29 +266,6 @@ namespace GitHub.Runner.Worker.Handlers 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. From e7b084477245559a64eb1c68aff724669ada71af Mon Sep 17 00:00:00 2001 From: Tingluo Huang Date: Thu, 23 Jul 2020 00:34:36 -0400 Subject: [PATCH 6/8] Add manual trigger --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2ff3b6e49..041670d6c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,6 +1,7 @@ name: Runner CI on: + workflow_dispatch: push: branches: - main From 2e50dffb37286b0ddbe3c80fd94518af2e896c77 Mon Sep 17 00:00:00 2001 From: Ethan Chiu <17chiue@gmail.com> Date: Thu, 23 Jul 2020 09:45:00 -0400 Subject: [PATCH 7/8] Clean Up Composite UI (#610) * Remove redundant code (display name is already evaluated in ActionRunner beforehand for each step) * remove * Remove nesting information for composite steps. * put messages in debug logs if composite. if not, put these messages as outputs * Fix group issue * Fix end group issue --- src/Runner.Worker/ExecutionContext.cs | 12 ++++++--- .../Handlers/CompositeActionHandler.cs | 6 ----- src/Runner.Worker/Handlers/ScriptHandler.cs | 27 ++++++++++++++----- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 5385490d7..1aad95c9c 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -60,12 +60,14 @@ namespace GitHub.Runner.Worker bool EchoOnActionCommand { get; set; } + bool InsideComposite { get; } + ExecutionContext Root { get; } // 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, CancellationTokenSource cancellationTokenSource = null); + IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool insideComposite = false, CancellationTokenSource cancellationTokenSource = null); // logging long Write(string tag, string message); @@ -152,6 +154,8 @@ namespace GitHub.Runner.Worker public bool EchoOnActionCommand { get; set; } + public bool InsideComposite { get; private set; } + public TaskResult? Result { get @@ -256,7 +260,7 @@ namespace GitHub.Runner.Worker DictionaryContextData inputsData, Dictionary envData) { - step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token)); + step.ExecutionContext = Root.CreateChild(_record.Id, step.DisplayName, _record.Id.ToString("N"), scopeName, step.Action.ContextName, logger: _logger, insideComposite: true, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token)); step.ExecutionContext.ExpressionValues["inputs"] = inputsData; step.ExecutionContext.ExpressionValues["steps"] = Global.StepsContext.GetScope(step.ExecutionContext.GetFullyQualifiedContextName()); @@ -275,7 +279,7 @@ namespace GitHub.Runner.Worker return step; } - public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, CancellationTokenSource cancellationTokenSource = null) + public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool insideComposite = false, CancellationTokenSource cancellationTokenSource = null) { Trace.Entering(); @@ -322,6 +326,8 @@ namespace GitHub.Runner.Worker child._logger.Setup(_mainTimelineId, recordId); } + child.InsideComposite = insideComposite; + return child; } diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 3ab7957cf..50015a574 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -215,12 +215,6 @@ namespace GitHub.Runner.Worker.Handlers 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}"); diff --git a/src/Runner.Worker/Handlers/ScriptHandler.cs b/src/Runner.Worker/Handlers/ScriptHandler.cs index f7d90dc45..397d9a176 100644 --- a/src/Runner.Worker/Handlers/ScriptHandler.cs +++ b/src/Runner.Worker/Handlers/ScriptHandler.cs @@ -23,6 +23,19 @@ namespace GitHub.Runner.Worker.Handlers public override void PrintActionDetails(ActionRunStage stage) { + // We don't want to display the internal workings if composite (similar/equivalent information can be found in debug) + void writeDetails(string message) + { + if (ExecutionContext.InsideComposite) + { + ExecutionContext.Debug(message); + } + else + { + ExecutionContext.Output(message); + } + } + if (stage == ActionRunStage.Post) { throw new NotSupportedException("Script action should not have 'Post' job action."); @@ -39,7 +52,7 @@ namespace GitHub.Runner.Worker.Handlers firstLine = firstLine.Substring(0, firstNewLine); } - ExecutionContext.Output($"##[group]Run {firstLine}"); + writeDetails(ExecutionContext.InsideComposite ? $"Run {firstLine}" : $"##[group]Run {firstLine}"); } else { @@ -50,7 +63,7 @@ namespace GitHub.Runner.Worker.Handlers foreach (var line in multiLines) { // Bright Cyan color - ExecutionContext.Output($"\x1b[36;1m{line}\x1b[0m"); + writeDetails($"\x1b[36;1m{line}\x1b[0m"); } string argFormat; @@ -109,23 +122,23 @@ namespace GitHub.Runner.Worker.Handlers if (!string.IsNullOrEmpty(shellCommandPath)) { - ExecutionContext.Output($"shell: {shellCommandPath} {argFormat}"); + writeDetails($"shell: {shellCommandPath} {argFormat}"); } else { - ExecutionContext.Output($"shell: {shellCommand} {argFormat}"); + writeDetails($"shell: {shellCommand} {argFormat}"); } if (this.Environment?.Count > 0) { - ExecutionContext.Output("env:"); + writeDetails("env:"); foreach (var env in this.Environment) { - ExecutionContext.Output($" {env.Key}: {env.Value}"); + writeDetails($" {env.Key}: {env.Value}"); } } - ExecutionContext.Output("##[endgroup]"); + writeDetails(ExecutionContext.InsideComposite ? "" : "##[endgroup]"); } public async Task RunAsync(ActionRunStage stage) From 48ac96307cea09bf51b55a3589c66526c0ea4e80 Mon Sep 17 00:00:00 2001 From: Christopher Johnson Date: Thu, 23 Jul 2020 17:46:48 -0400 Subject: [PATCH 8/8] Add ability to register a runner to the non-default self-hosted runner group (#613) Co-authored-by: Christopher Johnson --- src/Runner.Common/Constants.cs | 2 +- src/Runner.Listener/CommandSettings.cs | 11 +++++++- .../Configuration/ConfigurationManager.cs | 27 +++++++++++++++---- .../DTWebApi/WebApi/TaskAgentPoolReference.cs | 11 ++++++++ .../Configuration/ConfigurationManagerL0.cs | 12 +++++---- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 8533e931a..4f8694d53 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -90,7 +90,7 @@ namespace GitHub.Runner.Common public static readonly string Labels = "labels"; public static readonly string MonitorSocketAddress = "monitorsocketaddress"; public static readonly string Name = "name"; - public static readonly string Pool = "pool"; + public static readonly string RunnerGroup = "runnergroup"; public static readonly string StartupType = "startuptype"; public static readonly string Url = "url"; public static readonly string UserName = "username"; diff --git a/src/Runner.Listener/CommandSettings.cs b/src/Runner.Listener/CommandSettings.cs index 0d80c1d5e..364962610 100644 --- a/src/Runner.Listener/CommandSettings.cs +++ b/src/Runner.Listener/CommandSettings.cs @@ -42,7 +42,7 @@ namespace GitHub.Runner.Listener Constants.Runner.CommandLine.Args.Labels, Constants.Runner.CommandLine.Args.MonitorSocketAddress, Constants.Runner.CommandLine.Args.Name, - Constants.Runner.CommandLine.Args.Pool, + Constants.Runner.CommandLine.Args.RunnerGroup, Constants.Runner.CommandLine.Args.StartupType, Constants.Runner.CommandLine.Args.Token, Constants.Runner.CommandLine.Args.Url, @@ -169,6 +169,15 @@ namespace GitHub.Runner.Listener validator: Validators.NonEmptyValidator); } + public string GetRunnerGroupName(string defaultPoolName = null) + { + return GetArgOrPrompt( + name: Constants.Runner.CommandLine.Args.RunnerGroup, + description: "Enter the name of the runner group to add this runner to:", + defaultValue: defaultPoolName ?? "default", + validator: Validators.NonEmptyValidator); + } + public string GetToken() { return GetArgOrPrompt( diff --git a/src/Runner.Listener/Configuration/ConfigurationManager.cs b/src/Runner.Listener/Configuration/ConfigurationManager.cs index ce0863e1a..8e499adc7 100644 --- a/src/Runner.Listener/Configuration/ConfigurationManager.cs +++ b/src/Runner.Listener/Configuration/ConfigurationManager.cs @@ -159,17 +159,34 @@ namespace GitHub.Runner.Listener.Configuration _term.WriteSection("Runner Registration"); - //Get all the agent pools, and select the first private pool + // If we have more than one runner group available, allow the user to specify which one to be added into + string poolName = null; + TaskAgentPool agentPool = null; List agentPools = await _runnerServer.GetAgentPoolsAsync(); - TaskAgentPool agentPool = agentPools?.Where(x => x.IsHosted == false).FirstOrDefault(); + TaskAgentPool defaultPool = agentPools?.Where(x => x.IsInternal).FirstOrDefault(); - if (agentPool == null) + if (agentPools?.Where(x => !x.IsHosted).Count() > 1) { - throw new TaskAgentPoolNotFoundException($"Could not find any private pool. Contact support."); + poolName = command.GetRunnerGroupName(defaultPool?.Name); + _term.WriteLine(); + agentPool = agentPools.Where(x => string.Equals(poolName, x.Name, StringComparison.OrdinalIgnoreCase) && !x.IsHosted).FirstOrDefault(); } else { - Trace.Info("Found a private pool with id {1} and name {2}", agentPool.Id, agentPool.Name); + agentPool = defaultPool; + } + + if (agentPool == null && poolName == null) + { + throw new TaskAgentPoolNotFoundException($"Could not find any self-hosted runner groups. Contact support."); + } + else if (agentPool == null && poolName != null) + { + throw new TaskAgentPoolNotFoundException($"Could not find any self-hosted runner group named \"{poolName}\"."); + } + else + { + Trace.Info("Found a self-hosted runner group with id {1} and name {2}", agentPool.Id, agentPool.Name); runnerSettings.PoolId = agentPool.Id; runnerSettings.PoolName = agentPool.Name; } diff --git a/src/Sdk/DTWebApi/WebApi/TaskAgentPoolReference.cs b/src/Sdk/DTWebApi/WebApi/TaskAgentPoolReference.cs index bbef3809f..714860c1d 100644 --- a/src/Sdk/DTWebApi/WebApi/TaskAgentPoolReference.cs +++ b/src/Sdk/DTWebApi/WebApi/TaskAgentPoolReference.cs @@ -29,6 +29,7 @@ namespace GitHub.DistributedTask.WebApi this.PoolType = referenceToBeCloned.PoolType; this.Size = referenceToBeCloned.Size; this.IsLegacy = referenceToBeCloned.IsLegacy; + this.IsInternal = referenceToBeCloned.IsInternal; } public TaskAgentPoolReference Clone() @@ -67,6 +68,16 @@ namespace GitHub.DistributedTask.WebApi set; } + /// + /// Gets or sets a value indicating whether or not this pool is internal and can't be modified by users + /// + [DataMember] + public bool IsInternal + { + get; + set; + } + /// /// Gets or sets the type of the pool /// diff --git a/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs b/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs index 9cf3ad5ad..706a11cd7 100644 --- a/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs +++ b/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs @@ -39,10 +39,12 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration private string _expectedToken = "expectedToken"; private string _expectedServerUrl = "https://codedev.ms"; private string _expectedAgentName = "expectedAgentName"; - private string _expectedPoolName = "poolName"; + private string _defaultRunnerGroupName = "defaultRunnerGroup"; + private string _secondRunnerGroupName = "secondRunnerGroup"; private string _expectedAuthType = "pat"; private string _expectedWorkFolder = "_work"; - private int _expectedPoolId = 1; + private int _defaultRunnerGroupId = 1; + private int _secondRunnerGroupId = 2; private RSACryptoServiceProvider rsa = null; private RunnerSettings _configMgrAgentSettings = new RunnerSettings(); @@ -97,7 +99,7 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration _serviceControlManager.Setup(x => x.GenerateScripts(It.IsAny())); #endif - var expectedPools = new List() { new TaskAgentPool(_expectedPoolName) { Id = _expectedPoolId } }; + var expectedPools = new List() { new TaskAgentPool(_defaultRunnerGroupName) { Id = _defaultRunnerGroupId, IsInternal = true }, new TaskAgentPool(_secondRunnerGroupName) { Id = _secondRunnerGroupId } }; _runnerServer.Setup(x => x.GetAgentPoolsAsync(It.IsAny(), It.IsAny())).Returns(Task.FromResult(expectedPools)); var expectedAgents = new List(); @@ -155,7 +157,7 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration "configure", "--url", _expectedServerUrl, "--name", _expectedAgentName, - "--pool", _expectedPoolName, + "--runnergroup", _secondRunnerGroupName, "--work", _expectedWorkFolder, "--auth", _expectedAuthType, "--token", _expectedToken, @@ -175,7 +177,7 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration Assert.NotNull(s); Assert.True(s.ServerUrl.Equals(_expectedServerUrl)); Assert.True(s.AgentName.Equals(_expectedAgentName)); - Assert.True(s.PoolId.Equals(_expectedPoolId)); + Assert.True(s.PoolId.Equals(_secondRunnerGroupId)); Assert.True(s.WorkFolder.Equals(_expectedWorkFolder)); // validate GetAgentPoolsAsync gets called twice with automation pool type