Skip to content

Commit

Permalink
Cache Feature Bug: Job Container stops before post actions are run br…
Browse files Browse the repository at this point in the history 
…eaking Cache (#167)

* stop job container after all post actions.

* c

* c
  • Loading branch information
@thboop
thboop authored Nov 5, 2019
1 parent 3ba55f8 commit 81fe044
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 55 deletions.
16 changes: 15 additions & 1 deletion src/Runner.Worker/ActionRunner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<IActionRunner>();
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<IDefaultStepHost>();
Expand Down
9 changes: 9 additions & 0 deletions src/Runner.Worker/ContainerOperationProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -38,6 +39,14 @@ public async Task StartContainersAsync(IExecutionContext executionContext, objec
List<ContainerInfo> containers = data as List<ContainerInfo>;
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.
Expand Down
25 changes: 4 additions & 21 deletions src/Runner.Worker/ExecutionContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<IActionRunner>();
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<string, string> intraActionState = null, int? recordOrder = null)
Expand Down
32 changes: 1 addition & 31 deletions src/Runner.Worker/JobExtension.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,7 @@ public async Task<List<IStep>> InitializeJob(IExecutionContext jobContext, Pipel
}
}

// Build up 3 lists of steps, pre-job, job, post-job
var postJobStepsBuilder = new Stack<IStep>();

// Build up 2 lists of steps, pre-job, job
// Download actions not already in the cache
Trace.Info("Downloading actions");
var actionManager = HostContext.GetService<IActionManager>();
Expand All @@ -134,10 +132,6 @@ public async Task<List<IStep>> 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
Expand Down Expand Up @@ -187,33 +181,9 @@ public async Task<List<IStep>> 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<IStep> steps = new List<IStep>();
steps.AddRange(preJobSteps);
steps.AddRange(jobSteps);
steps.AddRange(postJobSteps);

// Start agent log plugin host process
// var logPlugin = HostContext.GetService<IAgentLogPlugin>();
// await logPlugin.StartAsync(context, steps, jobContext.CancellationToken);

// Prepare for orphan process cleanup
_processCleanup = jobContext.Variables.GetBoolean("process.clean") ?? true;
Expand Down
18 changes: 16 additions & 2 deletions src/Test/L0/Worker/ExecutionContextL0.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<IActionRunner>();
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<IActionRunner>();
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);
Expand Down

0 comments on commit 81fe044

Please sign in to comment.