From 5ed80c753b56b6d2325ae57f4a02e0f7e71ac15e Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Thu, 19 Aug 2021 08:56:06 -0400 Subject: [PATCH] Port master to release branch (#1277) * For Main Steps, just run the step, don't check condition (#1273) * For Main Steps, just run the step, don't check condition * fix whitespace * pr feedback * Runner 2.280.3 Release (#1276) Co-authored-by: Tingluo Huang Co-authored-by: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> --- releaseNote.md | 4 +- .../Handlers/CompositeActionHandler.cs | 189 +++++++++--------- src/runnerversion | 2 +- 3 files changed, 101 insertions(+), 94 deletions(-) diff --git a/releaseNote.md b/releaseNote.md index 3f5f5f9ea..f871f4403 100644 --- a/releaseNote.md +++ b/releaseNote.md @@ -2,12 +2,10 @@ ## Bugs -- Send Path when resolving actions so we can correctly validate Policy for Composite Actions (#1250) +- Fixed an issue where composite steps would not run on `failure()` or `always()` when the job failed (#1273) ## Misc -- Allows flags instead of parameters when configuring the runner (#1220) - ## Windows x64 We recommend configuring the runner in a root folder of the Windows drive (e.g. "C:\actions-runner"). This will help avoid issues related to service identity folder permissions and long file path restrictions on Windows. diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs index 89cc43d4e..fa95180cd 100644 --- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs +++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs @@ -295,99 +295,108 @@ namespace GitHub.Runner.Worker.Handlers CancellationTokenRegistration? jobCancelRegister = null; try { - // Register job cancellation call back only if job cancellation token not been fire before each step run - if (!ExecutionContext.Root.CancellationToken.IsCancellationRequested) - { - // Test the condition again. The job was canceled after the condition was originally evaluated. - jobCancelRegister = ExecutionContext.Root.CancellationToken.Register(() => - { - // Mark job as cancelled - ExecutionContext.Root.Result = TaskResult.Canceled; - ExecutionContext.Root.JobContext.Status = ExecutionContext.Root.Result?.ToActionResult(); - - step.ExecutionContext.Debug($"Re-evaluate condition on job cancellation for step: '{step.DisplayName}'."); - var conditionReTestTraceWriter = new ConditionTraceWriter(Trace, null); // host tracing only - var conditionReTestResult = false; - if (HostContext.RunnerShutdownToken.IsCancellationRequested) - { - step.ExecutionContext.Debug($"Skip Re-evaluate condition on runner shutdown."); - } - else - { - try - { - var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(conditionReTestTraceWriter); - var condition = new BasicExpressionToken(null, null, null, step.Condition); - conditionReTestResult = templateEvaluator.EvaluateStepIf(condition, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, step.ExecutionContext.ToExpressionState()); - } - catch (Exception ex) - { - // Cancel the step since we get exception while re-evaluate step condition - Trace.Info("Caught exception from expression when re-test condition on job cancellation."); - step.ExecutionContext.Error(ex); - } - } - - if (!conditionReTestResult) - { - // Cancel the step - Trace.Info("Cancel current running step."); - step.ExecutionContext.CancelToken(); - } - }); - } - else - { - if (ExecutionContext.Root.Result != TaskResult.Canceled) - { - // Mark job as cancelled - ExecutionContext.Root.Result = TaskResult.Canceled; - ExecutionContext.Root.JobContext.Status = ExecutionContext.Root.Result?.ToActionResult(); - } - } - // Evaluate condition - step.ExecutionContext.Debug($"Evaluating condition for step: '{step.DisplayName}'"); - var conditionTraceWriter = new ConditionTraceWriter(Trace, step.ExecutionContext); - var conditionResult = false; - var conditionEvaluateError = default(Exception); - if (HostContext.RunnerShutdownToken.IsCancellationRequested) - { - step.ExecutionContext.Debug($"Skip evaluate condition on runner shutdown."); - } - else - { - try - { - var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(conditionTraceWriter); - var condition = new BasicExpressionToken(null, null, null, step.Condition); - conditionResult = templateEvaluator.EvaluateStepIf(condition, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, step.ExecutionContext.ToExpressionState()); - } - catch (Exception ex) - { - Trace.Info("Caught exception from expression."); - Trace.Error(ex); - conditionEvaluateError = ex; - } - } - if (!conditionResult && conditionEvaluateError == null) - { - // Condition is false - Trace.Info("Skipping step due to condition evaluation."); - step.ExecutionContext.Result = TaskResult.Skipped; - continue; - } - else if (conditionEvaluateError != null) - { - // Condition error - step.ExecutionContext.Error(conditionEvaluateError); - step.ExecutionContext.Result = TaskResult.Failed; - ExecutionContext.Result = TaskResult.Failed; - break; - } - else + // For main steps just run the action + if (stage == ActionRunStage.Main) { await RunStepAsync(step); } + // We need to evaluate conditions for pre/post steps + else + { + // Register job cancellation call back only if job cancellation token not been fire before each step run + if (!ExecutionContext.Root.CancellationToken.IsCancellationRequested) + { + // Test the condition again. The job was canceled after the condition was originally evaluated. + jobCancelRegister = ExecutionContext.Root.CancellationToken.Register(() => + { + // Mark job as cancelled + ExecutionContext.Root.Result = TaskResult.Canceled; + ExecutionContext.Root.JobContext.Status = ExecutionContext.Root.Result?.ToActionResult(); + + step.ExecutionContext.Debug($"Re-evaluate condition on job cancellation for step: '{step.DisplayName}'."); + var conditionReTestTraceWriter = new ConditionTraceWriter(Trace, null); // host tracing only + var conditionReTestResult = false; + if (HostContext.RunnerShutdownToken.IsCancellationRequested) + { + step.ExecutionContext.Debug($"Skip Re-evaluate condition on runner shutdown."); + } + else + { + try + { + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(conditionReTestTraceWriter); + var condition = new BasicExpressionToken(null, null, null, step.Condition); + conditionReTestResult = templateEvaluator.EvaluateStepIf(condition, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, step.ExecutionContext.ToExpressionState()); + } + catch (Exception ex) + { + // Cancel the step since we get exception while re-evaluate step condition + Trace.Info("Caught exception from expression when re-test condition on job cancellation."); + step.ExecutionContext.Error(ex); + } + } + + if (!conditionReTestResult) + { + // Cancel the step + Trace.Info("Cancel current running step."); + step.ExecutionContext.CancelToken(); + } + }); + } + else + { + if (ExecutionContext.Root.Result != TaskResult.Canceled) + { + // Mark job as cancelled + ExecutionContext.Root.Result = TaskResult.Canceled; + ExecutionContext.Root.JobContext.Status = ExecutionContext.Root.Result?.ToActionResult(); + } + } + // Evaluate condition + step.ExecutionContext.Debug($"Evaluating condition for step: '{step.DisplayName}'"); + var conditionTraceWriter = new ConditionTraceWriter(Trace, step.ExecutionContext); + var conditionResult = false; + var conditionEvaluateError = default(Exception); + if (HostContext.RunnerShutdownToken.IsCancellationRequested) + { + step.ExecutionContext.Debug($"Skip evaluate condition on runner shutdown."); + } + else + { + try + { + var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(conditionTraceWriter); + var condition = new BasicExpressionToken(null, null, null, step.Condition); + conditionResult = templateEvaluator.EvaluateStepIf(condition, step.ExecutionContext.ExpressionValues, step.ExecutionContext.ExpressionFunctions, step.ExecutionContext.ToExpressionState()); + } + catch (Exception ex) + { + Trace.Info("Caught exception from expression."); + Trace.Error(ex); + conditionEvaluateError = ex; + } + } + if (!conditionResult && conditionEvaluateError == null) + { + // Condition is false + Trace.Info("Skipping step due to condition evaluation."); + step.ExecutionContext.Result = TaskResult.Skipped; + continue; + } + else if (conditionEvaluateError != null) + { + // Condition error + step.ExecutionContext.Error(conditionEvaluateError); + step.ExecutionContext.Result = TaskResult.Failed; + ExecutionContext.Result = TaskResult.Failed; + break; + } + else + { + await RunStepAsync(step); + } + } } finally { diff --git a/src/runnerversion b/src/runnerversion index bb559261d..8b1daf227 100644 --- a/src/runnerversion +++ b/src/runnerversion @@ -1 +1 @@ -2.280.2 +2.280.3