TEP-0090: Matrix [Proposal]

[jerop]

In TEP-0090: Matrix we described the problem statement for fanning out PipelineTasks. That is, enabling execution of a PipelineTask with different permutations of Parameters specified in a Matrix.

In this change, we add the proposal to solve the above problem. It includes the design details, implementation plan, promotion plan, design evalutation, future work, alternatives, and others.

In summary, we propose adding a matrix field to the PipelineTask specification that will be used to declare Parameters of type Array. The PipelineTask will be executed in parallel TaskRuns or Runs with its Parameters substituted with the permutations of Parameters in the Matrix.

POC Demo: https://drive.google.com/file/d/1wDYIl2Kqj5XiZzikF9878bcVb7_lmcoZ/view?t=31m11s (accessible to tekton-dev@)

/kind tep

[jerop]

/cc @bobcatfish @tlawrie @imjasonh @squee1945 @vdemeester @Tomcli @pritidesai @Oded-B

(notifying contributors who reviewed the pull request adding the problem statement for this TEP - #532)

[tekton-robot]

@jerop: GitHub didn't allow me to request PR reviews from the following users: tlawrie, squee1945, Tomcli, Oded-B.

Note that only tektoncd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @bobcatfish @tlawrie @imjasonh @squee1945 @vdemeester @Tomcli @pritidesai @Oded-B

(notifying contributors who reviewed the pull request adding the problem statement for this TEP - #532)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vdemeester]

/assign

[Oded-B]

@jerop I think this will work for my usecase.
Currently, I just create distinct pipelines using this bash code in one of the "main" pipeline steps:

            for RELEASE_PATH in "$LIST_OF_CHANGED_RELEASES"; do
              IFS='/' read -a RELEASE_PATH_ARRAY <<< "$RELEASE_PATH"
              RELEASE_ENV=${RELEASE_PATH_ARRAY[1]}
              RELEASE_SITE=${RELEASE_PATH_ARRAY[2]}
              RELEASE_CLUSTER=${RELEASE_PATH_ARRAY[3]}
              RELEASE_CHART=${RELEASE_PATH_ARRAY[4]}
              tkn pipeline start per-chart-diff-or-apply-pipeline --prefix-name=${PR_NUMBER}-${ACTION}-${RELEASE_ENV}-${RELEASE_SITE}-${RELEASE_CLUSTER}-${RELEASE_CHART} \
                --param action=$ACTION \
                --param prNumber=$PR_NUMBER \
                --param releasePath=$RELEASE_PATH \
                --param releaseEnv=$RELEASE_ENV \
                --param releaseSite=$RELEASE_SITE \
                --param releaseCluster=$RELEASE_CLUSTER \
                --param releaseChart=$RELEASE_CHART \
                --param gitUrl=$(params.gitUrl) \
                --param gitRev=$(params.gitSHA) \
                --labels sourcePipelineRun=$(params.sourcePipelineRun),sourceTaskRun=$(context.taskRun.name) \
                --workspace name=ssh-creds,secret=ssh-key \
                --workspace name=git-source,volumeClaimTemplateFile=/tmp/volumeClaimTemplateFile.yaml
            done

I don't think there should be an issue moving to the proposed configuration, and it will be so much readable and generally nicer, and I would also be able to use Ordering Dependencies functionality to report the status of the whole pipeline back to the Github PR.

The only complication is the need to move to a shared(ReadWriteMany) workspace, but it makes sense for my use case (clone the git repo once, and the shared volume from multiple tasks)

[jerop]

/assign @afrittoli

[ghost]

/assign

[vinamra28]

hi @jerop, the proposal looks very much interesting. There can be one more use case which can be solved maybe in a follow-up TEP or in this TEP is that we can try to implement matrix at Pipeline level. Possible use case of having matrix at Pipeline level can be - There is a Pipeline which clones a node-based project and then we want to test that repo on different versions of nodejs, ie, node-12, node-14 and node-16.

[afrittoli]

/test pull-community-teps-lint

[jerop]

@afrittoli - thank you for the detailed review!

Is this meant as a way to integrate the "loop" functionality into the core API, and if so does it replace the work of growing the custom task from experimental to stable and into the API?

How does this relate to the existing custom controller in the experimental repo?

yes, the aim is to support it in Tekton Pipelines API (behind the alpha feature gate initially)

One concern that I have is about the execution strategy for the matrix. If I understand the proposal correctly, the PipelineRun controller would create one TaskRun for each combination of values in the matrix. Since we have parallel execution as a requirement, we might run into issues if we have workspaces and we need anything other than readonly.

addressed this concern in https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#workspaces

It might be worth exploring overlaps with TEP-0044 or design a single pod execution at least for some of the use cases.

addressed the overlap with TEP-0044 in https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#tep-0044-data-locality-and-pod-overhead-in-pipelines

And discussed overlaps with other TEPs in https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#related-tekton-projects-and-proposals

If a pipeline is executed with a large number of inputs and it's allowed to start by a global concurrency controller, it will eat all the available resources (use all allowed taskruns) and prevent other pipelines to progress. Having a limit on the matrix would allow to contain that.

we have requirement 4 (formerly requirement 5) to address this: https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#requirements

and solved for it here: https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#concurrency-control

this means we should be able to use when expression as they exist today, and they should work when used in conjunction with matrix too - i.e. it should be possible for a when expression to refer to a single value out of the array which is passed to the specific TaskRun

when expressions do not accept variables from the PipelineTask itself, so it doesn't accept variables from either the params or matrix fields in the PipelineTask - only from Pipeline - so we can't read from the matrix

discussed further in: https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#when-expressions

meant to remove requirement 4, which is the same as non-goal 4 - updated the requirements to remove it

How does this related to existing Pipeline features like: finally, when, retries, timeout

addressed them here: https://github.com/jerop/community/blob/matrix-proposal/teps/0090-matrix.md#design

[ghost]

I don't have any specific feedback - the design as it stands now looks great to me, thanks @jerop!

/approve

[afrittoli]

Thank you for all the updates @jerop, the TEP looks great and very thorough!!

I have one question about the global flag and a proposal about the combinations IDs.

[afrittoli]

Looking great!

/approve

[lbernick]

/approve

[bobcatfish]

I'd like to discuss a bit more around how we want to structure results from fanned out tasks but I'm also happy to see this move forward as a whole as-is!

[jerop]

I'd like to discuss a bit more around how we want to structure results from fanned out tasks but I'm also happy to see this move forward as a whole as-is!

summarizing offline discussions with @bobcatfish about this for future reference - there were some design options for results from fanned out tasks in the TEP, removed them as they are out of scope and we will solve for it after TEP-0075 and TEP-0076 have landed (added a non-goal to make this explicit) - thank you @bobcatfish!

[vdemeester]

SGTM, just few questions.

[vdemeester]

/meow

[tekton-robot]

@vdemeester:

In response to this:

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, lbernick, sbwsg, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• teps/OWNERS [afrittoli,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[pritidesai]

API WG - ready to merge 🎉

/lgtm