Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Conditional Steps in Composite Actions #1438

Merged
merged 22 commits into from
Oct 29, 2021
Merged

Support Conditional Steps in Composite Actions #1438

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f11df5b
conditional support for composite actions
thboop Oct 12, 2021
7278a46
Fix Conditional function evaluation
fhammerl Oct 12, 2021
0a79f90
Push launch.json temporarily
fhammerl Oct 12, 2021
9f6df00
Revert "Push launch.json temporarily"
thboop Oct 12, 2021
ef46c42
rename context
thboop Oct 12, 2021
3e068ec
Cleanup comments
thboop Oct 12, 2021
e820233
fix success/failure functions to run based on pre/main steps
thboop Oct 12, 2021
c41547d
idea of step_status
thboop Oct 13, 2021
6b139bf
change to use steps context, WIP
thboop Oct 13, 2021
6f683bd
add inputs to possible if condition expressions
thboop Oct 13, 2021
02bf28d
use action_status
thboop Oct 14, 2021
2386007
pr cleanup
thboop Oct 14, 2021
e6ac2cb
Added right stages
fhammerl Oct 27, 2021
8590f2e
Test on stage in conditional functions
fhammerl Oct 27, 2021
1fae86a
Fix naming and formatting
fhammerl Oct 27, 2021
9c57bff
Fix tests
fhammerl Oct 27, 2021
c412713
Add success and failure L0s
fhammerl Oct 28, 2021
7668c2f
Remove comment
fhammerl Oct 28, 2021
c8d6db8
Remove whitespace
fhammerl Oct 28, 2021
3e7f9a3
Undo formatting
fhammerl Oct 28, 2021
71949db
Add L0 for step-if parsing
fhammerl Oct 28, 2021
b2306dd
Add ADR
fhammerl Oct 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Runner.Worker/ActionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,6 @@ private ActionSetupInfo PrepareRepositoryActionAsync(IExecutionContext execution
}
}

// TODO: remove once we remove the DistributedTask.EnableCompositeActions FF
foreach (var step in compositeAction.Steps)
{
if (string.IsNullOrEmpty(executionContext.Global.Variables.Get("DistributedTask.EnableCompositeActions")) && step.Reference.Type != Pipelines.ActionSourceType.Script)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still need the FF in the runner? I guess the answer might be yes for older version GHES.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose when we do remove the FF we would search the runner for its references, not sure if leaving the TODO made sense.

Expand Down
15 changes: 9 additions & 6 deletions src/Runner.Worker/ExecutionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public interface IExecutionContext : IRunnerService
string ScopeName { get; }
string SiblingScopeName { get; }
string ContextName { get; }
ActionRunStage Stage { get; }
Task ForceCompleted { get; }
TaskResult? Result { get; set; }
TaskResult? Outcome { get; set; }
Expand Down Expand Up @@ -76,8 +77,8 @@ public interface IExecutionContext : IRunnerService
// Initialize
void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token);
void CancelToken();
IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, Dictionary<string, string> intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null);
IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary<string, string> intraActionState = null, string siblingScopeName = null);
IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, ActionRunStage stage, Dictionary<string, string> intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null);
IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, ActionRunStage stage, Dictionary<string, string> intraActionState = null, string siblingScopeName = null); // ARG
fhammerl marked this conversation as resolved.
Show resolved Hide resolved

// logging
long Write(string tag, string message);
Expand Down Expand Up @@ -144,6 +145,7 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext
public string ScopeName { get; private set; }
public string SiblingScopeName { get; private set; }
public string ContextName { get; private set; }
public ActionRunStage Stage { get; private set; }
public Task ForceCompleted => _forceCompleted.Task;
public CancellationToken CancellationToken => _cancellationTokenSource.Token;
public Dictionary<string, string> IntraActionState { get; private set; }
Expand Down Expand Up @@ -285,7 +287,7 @@ public void RegisterPostJobStep(IStep step)
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, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null)
public IExecutionContext CreateChild(Guid recordId, string displayName, string refName, string scopeName, string contextName, ActionRunStage stage, Dictionary<string, string> intraActionState = null, int? recordOrder = null, IPagingLogger logger = null, bool isEmbedded = false, CancellationTokenSource cancellationTokenSource = null, Guid embeddedId = default(Guid), string siblingScopeName = null)
{
Trace.Entering();

Expand All @@ -294,6 +296,7 @@ public void RegisterPostJobStep(IStep step)
child.Global = Global;
child.ScopeName = scopeName;
child.ContextName = contextName;
child.Stage = stage;
child.EmbeddedId = embeddedId;
child.SiblingScopeName = siblingScopeName;
child.JobTelemetry = JobTelemetry;
Expand Down Expand Up @@ -344,9 +347,9 @@ public void RegisterPostJobStep(IStep step)
/// An embedded execution context shares the same record ID, record name, logger,
/// and a linked cancellation token.
/// </summary>
public IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, Dictionary<string, string> intraActionState = null, string siblingScopeName = null)
public IExecutionContext CreateEmbeddedChild(string scopeName, string contextName, Guid embeddedId, ActionRunStage stage, Dictionary<string, string> intraActionState = null, string siblingScopeName = null)
{
return Root.CreateChild(_record.Id, _record.Name, _record.Id.ToString("N"), scopeName, contextName, logger: _logger, isEmbedded: true, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token), intraActionState: intraActionState, embeddedId: embeddedId, siblingScopeName: siblingScopeName);
return Root.CreateChild(_record.Id, _record.Name, _record.Id.ToString("N"), scopeName, contextName, stage, logger: _logger, isEmbedded: true, cancellationTokenSource: CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.Token), intraActionState: intraActionState, embeddedId: embeddedId, siblingScopeName: siblingScopeName);
}

public void Start(string currentOperation = null)
Expand Down Expand Up @@ -937,7 +940,7 @@ private IExecutionContext CreatePostChild(string displayName, Dictionary<string,
}

var newGuid = Guid.NewGuid();
return CreateChild(newGuid, displayName, newGuid.ToString("N"), null, null, intraActionState, _childTimelineRecordOrder - Root.PostJobSteps.Count, siblingScopeName: siblingScopeName);
return CreateChild(newGuid, displayName, newGuid.ToString("N"), null, null, ActionRunStage.Post, intraActionState, _childTimelineRecordOrder - Root.PostJobSteps.Count, siblingScopeName: siblingScopeName);
}
}

Expand Down
15 changes: 13 additions & 2 deletions src/Runner.Worker/Expressions/FailureFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,19 @@ protected sealed override object EvaluateCore(EvaluationContext evaluationContex
ArgUtil.NotNull(templateContext, nameof(templateContext));
var executionContext = templateContext.State[nameof(IExecutionContext)] as IExecutionContext;
ArgUtil.NotNull(executionContext, nameof(executionContext));
ActionResult jobStatus = executionContext.JobContext.Status ?? ActionResult.Success;
return jobStatus == ActionResult.Failure;

// Decide based on 'action_status' for composite MAIN steps and 'job.status' for pre, post and job-level steps
var isCompositeMainStep = executionContext.IsEmbedded && executionContext.Stage == ActionRunStage.Main;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some L0 for these new functions.
Do we need to change other ExpressionFunctions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than Success and Failure, we've got

AlwaysFunction: It's a constant return true
CancelledFunction : dependent on jobStatus == ActionResult.Cancelled, so no change required
HashFunction: irrelevant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added L0s for Success and Failure

if (isCompositeMainStep)
{
ActionResult actionStatus = EnumUtil.TryParse<ActionResult>(executionContext.GetGitHubContext("action_status")) ?? ActionResult.Success;
return actionStatus == ActionResult.Failure;
}
else
{
ActionResult jobStatus = executionContext.JobContext.Status ?? ActionResult.Success;
return jobStatus == ActionResult.Failure;
}
}
}
}
15 changes: 13 additions & 2 deletions src/Runner.Worker/Expressions/SuccessFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,19 @@ protected sealed override object EvaluateCore(EvaluationContext evaluationContex
ArgUtil.NotNull(templateContext, nameof(templateContext));
var executionContext = templateContext.State[nameof(IExecutionContext)] as IExecutionContext;
ArgUtil.NotNull(executionContext, nameof(executionContext));
ActionResult jobStatus = executionContext.JobContext.Status ?? ActionResult.Success;
return jobStatus == ActionResult.Success;

// Decide based on 'action_status' for composite MAIN steps and 'job.status' for pre, post and job-level steps
var isCompositeMainStep = executionContext.IsEmbedded && executionContext.Stage == ActionRunStage.Main;
if (isCompositeMainStep)
{
ActionResult actionStatus = EnumUtil.TryParse<ActionResult>(executionContext.GetGitHubContext("action_status")) ?? ActionResult.Success;
return actionStatus == ActionResult.Success;
}
else
{
ActionResult jobStatus = executionContext.JobContext.Status ?? ActionResult.Success;
return jobStatus == ActionResult.Success;
}
}
}
}
Loading