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

@@ -94,7 +94,21 @@ namespace GitHub.Runner.Worker
                 {
                     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>();

src/Runner.Worker/ContainerOperationProvider.cs

@@ -11,6 +11,7 @@ using GitHub.Runner.Common;
 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 @@ namespace GitHub.Runner.Worker
             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.

src/Runner.Worker/ExecutionContext.cs

@@ -98,7 +98,7 @@ namespace GitHub.Runner.Worker
 
         // 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 @@ namespace GitHub.Runner.Worker
             });
         }
 
-        public void RegisterPostJobAction(string displayName, string condition, Pipelines.ActionStep action)
+        public void RegisterPostJobStep(string refName, IStep step)
         {
-            if (action.Reference.Type != ActionSourceType.Repository)
+            step.ExecutionContext = Root.CreatePostChild(step.DisplayName, refName, IntraActionState);
-            {
+            Root.PostJobSteps.Push(step);
-                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);
         }
 
         public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary<string, string> intraActionState = null, int? recordOrder = null)

src/Runner.Worker/JobExtension.cs

@@ -110,9 +110,7 @@ namespace GitHub.Runner.Worker
                         }
                     }
 
-                    // Build up 3 lists of steps, pre-job, job, post-job
+                    // Build up 2 lists of steps, pre-job, job
-                    var postJobStepsBuilder = new Stack<IStep>();
-
                     // Download actions not already in the cache
                     Trace.Info("Downloading actions");
                     var actionManager = HostContext.GetService<IActionManager>();
@@ -134,10 +132,6 @@ namespace GitHub.Runner.Worker
                                                                           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 @@ namespace GitHub.Runner.Worker
                         }
                     }
 
-                    // 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;

src/Test/L0/Worker/ExecutionContextL0.cs

@@ -206,8 +206,22 @@ namespace GitHub.Runner.Common.Tests.Worker
                 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);