From 81fe044a22ca40fef3c281ab1a0bfd4c5eb5f778 Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Tue, 5 Nov 2019 15:43:05 -0500 Subject: [PATCH] Cache Feature Bug: Job Container stops before post actions are run breaking Cache (#167) * stop job container after all post actions. * c * c --- src/Runner.Worker/ActionRunner.cs | 16 +++++++++- .../ContainerOperationProvider.cs | 9 ++++++ src/Runner.Worker/ExecutionContext.cs | 25 +++------------ src/Runner.Worker/JobExtension.cs | 32 +------------------ src/Test/L0/Worker/ExecutionContextL0.cs | 18 +++++++++-- 5 files changed, 45 insertions(+), 55 deletions(-) diff --git a/src/Runner.Worker/ActionRunner.cs b/src/Runner.Worker/ActionRunner.cs index 0ec510c66c8..d88e1910c7a 100644 --- a/src/Runner.Worker/ActionRunner.cs +++ b/src/Runner.Worker/ActionRunner.cs @@ -94,7 +94,21 @@ public async Task RunAsync() { postDisplayName = $"Post {this.DisplayName}"; } - ExecutionContext.RegisterPostJobAction(postDisplayName, handlerData.CleanupCondition, Action); + + var repositoryReference = Action.Reference as RepositoryPathReference; + var pathString = string.IsNullOrEmpty(repositoryReference.Path) ? string.Empty : $"/{repositoryReference.Path}"; + var repoString = string.IsNullOrEmpty(repositoryReference.Ref) ? $"{repositoryReference.Name}{pathString}" : + $"{repositoryReference.Name}{pathString}@{repositoryReference.Ref}"; + + ExecutionContext.Debug($"Register post job cleanup for action: {repoString}"); + + var actionRunner = HostContext.CreateService(); + actionRunner.Action = Action; + actionRunner.Stage = ActionRunStage.Post; + actionRunner.Condition = handlerData.CleanupCondition; + actionRunner.DisplayName = postDisplayName; + + ExecutionContext.RegisterPostJobStep($"{actionRunner.Action.Name}_post", actionRunner); } IStepHost stepHost = HostContext.CreateService(); diff --git a/src/Runner.Worker/ContainerOperationProvider.cs b/src/Runner.Worker/ContainerOperationProvider.cs index ab7278c6599..9085dcb0a39 100644 --- a/src/Runner.Worker/ContainerOperationProvider.cs +++ b/src/Runner.Worker/ContainerOperationProvider.cs @@ -11,6 +11,7 @@ using GitHub.Runner.Sdk; using GitHub.DistributedTask.Pipelines.ContextData; using Microsoft.Win32; +using GitHub.DistributedTask.Pipelines.ObjectTemplating; namespace GitHub.Runner.Worker { @@ -38,6 +39,14 @@ public async Task StartContainersAsync(IExecutionContext executionContext, objec List containers = data as List; ArgUtil.NotNull(containers, nameof(containers)); + var postJobStep = new JobExtensionRunner(runAsync: this.StopContainersAsync, + condition: $"{PipelineTemplateConstants.Always}()", + displayName: "Stop containers", + data: data); + + executionContext.Debug($"Register post job cleanup for stoping/deleting containers."); + executionContext.RegisterPostJobStep(nameof(StopContainersAsync), postJobStep); + // Check whether we are inside a container. // Our container feature requires to map working directory from host to the container. // If we are already inside a container, we will not able to find out the real working direcotry path on the host. diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 2f7de9e5b4c..be55b4c7815 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -98,7 +98,7 @@ public interface IExecutionContext : IRunnerService // others void ForceTaskComplete(); - void RegisterPostJobAction(string displayName, string condition, Pipelines.ActionStep action); + void RegisterPostJobStep(string refName, IStep step); } public sealed class ExecutionContext : RunnerService, IExecutionContext @@ -240,27 +240,10 @@ public void ForceTaskComplete() }); } - public void RegisterPostJobAction(string displayName, string condition, Pipelines.ActionStep action) + public void RegisterPostJobStep(string refName, IStep step) { - if (action.Reference.Type != ActionSourceType.Repository) - { - throw new NotSupportedException("Only action that has `action.yml` can define post job execution."); - } - - var repositoryReference = action.Reference as RepositoryPathReference; - var pathString = string.IsNullOrEmpty(repositoryReference.Path) ? string.Empty : $"/{repositoryReference.Path}"; - var repoString = string.IsNullOrEmpty(repositoryReference.Ref) ? $"{repositoryReference.Name}{pathString}" : - $"{repositoryReference.Name}{pathString}@{repositoryReference.Ref}"; - - this.Debug($"Register post job cleanup for action: {repoString}"); - - var actionRunner = HostContext.CreateService(); - actionRunner.Action = action; - actionRunner.Stage = ActionRunStage.Post; - actionRunner.Condition = condition; - actionRunner.DisplayName = displayName; - actionRunner.ExecutionContext = Root.CreatePostChild(displayName, $"{actionRunner.Action.Name}_post", IntraActionState); - Root.PostJobSteps.Push(actionRunner); + step.ExecutionContext = Root.CreatePostChild(step.DisplayName, refName, IntraActionState); + Root.PostJobSteps.Push(step); } public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary intraActionState = null, int? recordOrder = null) diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index d57e1c20108..2e7f857e7e5 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -110,9 +110,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel } } - // Build up 3 lists of steps, pre-job, job, post-job - var postJobStepsBuilder = new Stack(); - + // Build up 2 lists of steps, pre-job, job // Download actions not already in the cache Trace.Info("Downloading actions"); var actionManager = HostContext.GetService(); @@ -134,10 +132,6 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel condition: $"{PipelineTemplateConstants.Success}()", displayName: "Initialize containers", data: (object)containers)); - postJobStepsBuilder.Push(new JobExtensionRunner(runAsync: containerProvider.StopContainersAsync, - condition: $"{PipelineTemplateConstants.Always}()", - displayName: "Stop containers", - data: (object)containers)); } // Add action steps @@ -187,33 +181,9 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel } } - // Add post-job steps - Trace.Info("Adding post-job steps"); - while (postJobStepsBuilder.Count > 0) - { - postJobSteps.Add(postJobStepsBuilder.Pop()); - } - - // Create execution context for post-job steps - foreach (var step in postJobSteps) - { - if (step is JobExtensionRunner) - { - JobExtensionRunner extensionStep = step as JobExtensionRunner; - ArgUtil.NotNull(extensionStep, extensionStep.DisplayName); - Guid stepId = Guid.NewGuid(); - extensionStep.ExecutionContext = jobContext.CreateChild(stepId, extensionStep.DisplayName, stepId.ToString("N"), null, null); - } - } - List steps = new List(); steps.AddRange(preJobSteps); steps.AddRange(jobSteps); - steps.AddRange(postJobSteps); - - // Start agent log plugin host process - // var logPlugin = HostContext.GetService(); - // await logPlugin.StartAsync(context, steps, jobContext.CancellationToken); // Prepare for orphan process cleanup _processCleanup = jobContext.Variables.GetBoolean("process.clean") ?? true; diff --git a/src/Test/L0/Worker/ExecutionContextL0.cs b/src/Test/L0/Worker/ExecutionContextL0.cs index d39d6f2954a..d494bd79b10 100644 --- a/src/Test/L0/Worker/ExecutionContextL0.cs +++ b/src/Test/L0/Worker/ExecutionContextL0.cs @@ -206,8 +206,22 @@ public void RegisterPostJobAction_ShareState() var action2 = jobContext.CreateChild(Guid.NewGuid(), "action_2", "action_2", null, null); action2.IntraActionState["state"] = "2"; - action1.RegisterPostJobAction("post1", "always()", new Pipelines.ActionStep() { Name = "post1", DisplayName = "Test 1", Reference = new Pipelines.RepositoryPathReference() { Name = "actions/action" } }); - action2.RegisterPostJobAction("post2", "always()", new Pipelines.ActionStep() { Name = "post2", DisplayName = "Test 2", Reference = new Pipelines.RepositoryPathReference() { Name = "actions/action" } }); + + var postRunner1 = hc.CreateService(); + postRunner1.Action = new Pipelines.ActionStep() { Name = "post1", DisplayName = "Test 1", Reference = new Pipelines.RepositoryPathReference() { Name = "actions/action" } }; + postRunner1.Stage = ActionRunStage.Post; + postRunner1.Condition = "always()"; + postRunner1.DisplayName = "post1"; + + + var postRunner2 = hc.CreateService(); + postRunner2.Action = new Pipelines.ActionStep() { Name = "post2", DisplayName = "Test 2", Reference = new Pipelines.RepositoryPathReference() { Name = "actions/action" } }; + postRunner2.Stage = ActionRunStage.Post; + postRunner2.Condition = "always()"; + postRunner2.DisplayName = "post2"; + + action1.RegisterPostJobStep("post1", postRunner1); + action2.RegisterPostJobStep("post2", postRunner2); Assert.NotNull(jobContext.JobSteps); Assert.NotNull(jobContext.PostJobSteps);