Composite Actions Support for Multiple Run Steps

[ethanchewy]

In this PR, we support executing multiple run steps with custom inputs in a composite action. You can use this feature by writing using: "composite" in the action.yml file. Our goal for our first version is to be able to run simple, multiple steps in an action.yml file. This first step allows for users to run a list of steps in the action.yml file rather than invoking a javascript entry point. This will eventually allow us to support nesting of actions within an action, etc.

Here is an in-depth example:

ethanchewy/test-composite/action.yml:

name: "Composite Action Test v2"
description: "Run a simple composite action"
inputs:
  your_name:
    description: 'lol'
    default: 'Ethan'
runs:
    using: "composite"
    steps: 
        - run: echo hello ${{inputs.your_name}}
        - run: echo sha ${{github.sha}}
        - run: echo os ${{runner.os}}
        - run: |
            echo job index ${{ strategy.job-index }}

workflow.yml

name: Composite Action
on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]
jobs:
  build:
    runs-on: self-hosted
    - uses: actions/checkout@v2
    - uses: ethanchewy/test-composite@v5
      with:
        your_name: "Octocat"
    - name: step 1
      run: echo Workflow step 1
    - name: step 2
      run: echo Workflow step 2

Github UI Action View: https://github.com/ethanchewy/testing-actions/runs/778225962?check_suite_focus=true

What this doesn't support yet (for future PRs):

• Input, Output, Env flow (ex: All nested steps should have access to its parents' input variables and nested steps can overwrite the input variables)
• Nesting other actions in composite actions (supporting in a step the terminology "using: __")
• etc.

How this was implemented at a high level:
We have one queue that keeps track of the composite actions transformed into a job step (in the process, it’s added to the Root node) => then we requeue in a way where these composite actions are executed first.
None of the steps are actually evaluated by the CompositeActionsHandler but they are pushed to the front of the job step queue and handled by other handlers depending on its type (ex: Script handler).

This is an experimental feature that can only be turned on via turning the feature flag on:
export TESTING_COMPOSITE_ACTIONS_ALPHA=1
./run.sh

[ericsciple]

I think this will print Action steps: Object

[ericsciple]

If the json representation is desired (too noisy? esp with large inline scripts?) then see StringUtil or IOUtil (has function to convert to/from json)

[ethanchewy]

@TingluoHuang thoughts?

[ethanchewy]

Also, compositeAction.Steps is of type List<Pipelines.ActionStep>. If we were to output this, what would be the best way to output the values in a json string format?

Right now, this Trace.info statement returns the object string representation: GitHub.DistributedTask.Pipelines.ActionStep

If we were to print out the steps, it might make more sense to print them out in the LoadCompositeSteps function

[ethanchewy]

From @TingluoHuang (thanks Ting!!) :

Verbose will shows up only in dev mode (you build the runner from source code.)
Trace.Info($"Load {compositeAction.Steps.Count} action steps.");
Trace.Verbose($"Details: {StringUtil.ConvertToJson(compositeAction.Steps}");

So, I'll do that.

[ericsciple]

does this error give the user enough context to know where the problem is? Like does it convey that an error happened when loading the path-to-action.yml

[ericsciple]

i guess we would have the same problem with the other place too (in this file)

[ethanchewy]

I think this error makes sense and is appropriate.

At the top of ConvertRuns:
var stepsLoaded = default(List<Pipelines.ActionStep>);

Then, we check in the action.yml file to see if there is any steps token:

case "steps":
                        if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("TESTING_COMPOSITE_ACTIONS_ALPHA")))
                        {
                            var steps = run.Value.AssertSequence("steps");
                            var evaluator = executionContext.ToPipelineTemplateEvaluator();
                            stepsLoaded = evaluator.LoadCompositeSteps(steps);
                            break;
                        }
                        throw new Exception("You aren't supposed to be using Composite Actions yet!");

If there are any issues with loading the steps, there would be an error reported in the LoadCompositeSteps() or LoadStep() function in the pipeline template evaulator. If the steps attribute is empty, there would be no error and LoadCompositeSteps would return the default value of List which is equivalent to null.

Thus, if stepsLoaded is null, it must be because there was no steps attribute written in the action.yml by the user.

Thus, I believe throwing this message is appropriate.

[ethanchewy]

TODO: Add a more helpful error message + including file name, etc. to show user that it's because of their yaml file

(Just add TODO for now)

[ericsciple]

I'm worried a little bit because we're not adding the contexts from the job message, like we normally do when creating steps.

I know this is what Post does, but wouldnt surprise me if there is a bug there too.

It may be safe, since we're copying the root node (maybe job message contexts copied there first so safe?)

Let's add a TODO comment in the code like "// TODO: confirm whether not copying message contexts is safe".

I can help look into this more. It's tedious because values are delay expanded sometimes (sometimes evaluated early, like DisplayName). I wish the code were designed simpler... hmm thinking about alternatives.

[ethanchewy]

Added TODO. Leaving open for visibility.

[ethanchewy]

After more testing for the env flow, I realized that the display name does not evaluate the env variables. This has to do with not passing the context. See: #557 (comment)
Ex: https://github.com/ethanchewy/testing-actions/runs/795652488?check_suite_focus=true
Building in support will occur in the env flow PR: #557

Never mind, in both the composite action and workflow steps, the environment contexts are evaluated correctly: https://github.com/ethanchewy/testing-actions/actions/runs/143774560

Still investigating but for now I think the contexts are handled correctly for the composite action steps.

[ethanchewy]

tl;dr The context is exposed correctly in env flow PR and solves this concern

In the env flow PR, I handle the contexts for each correctly: https://github.com/actions/runner/pull/557/files#diff-e3d54b1d0cbdce4d358272df29857cf2R225 by setting the step.EnvironmentVariables to the environment variables.

Demo: https://github.com/ethanchewy/testing-actions/actions/runs/143774560

[ethanchewy]

Also, since we add each of our composite steps to JobSteps, eventually each of the steps will be run via the ScriptHandler.

In the ScriptHandler, it exposes the context to the step/job: https://github.com/actions/runner/blob/master/src/Runner.Worker/Handlers/ScriptHandler.cs#L248-L258

[ericsciple]

sort

[ericsciple]

system first

[ericsciple]

named at the bottom

[ethanchewy]

TODO: Replace 0 with location so that the steps are ordered.

[ericsciple]

        "always(0,0)",
        "failure(0,0)",
        "cancelled(0,0)",
        "success(0,0)",
        "hashFiles(1,255)"

[ethanchewy]

Maybe switch to false when going to server (and also switch the logic in the function to reflect this). We'll put these changes in the server side.