feat: resolve steps referencing StepActions concurrently by Maximilien-R · Pull Request #8925 · tektoncd/pipeline

Maximilien-R · 2025-07-28T15:46:49Z

This PR improves the performance of TaskRun reconciliation by resolving StepActions concurrently and refactors the resolution logic for better efficiency.

The problem

Currently, when a Task contains multiple steps that reference StepActions, the resolution of these references is performed sequentially. This can lead to significant delays in starting a TaskRun, particularly when using remote resolvers like git, as each resolution adds to the total time.

Additionally, the existing code performs a deep copy of every step, regardless of whether it references a StepAction, leading to unnecessary memory allocations.

The changes

This pull request introduces two main improvements to StepAction resolution:

Concurrent resolution: StepActions are now resolved concurrently using an errgroup. This reduces the time required to process TaskRuns that contain multiple steps with remote StepAction references, such as those from a git repository.
Code refactoring: The resolution logic in taskspec.go has been refactored for clarity and maintainability. This includes:
- Introducing a HasStepRefs function for an early exit if no StepActions need to be resolved.
- Creating a resolveStepRef function to encapsulate the logic of resolving a single StepAction.
- Splitting the process into two phases: concurrent resolution and sequential merging of results.
- Adding a updateTaskRunProvenance function to handle status updates cleanly.
- Optimizing DeepCopy to only occur when a step.Ref is present.

/kind feature

The resolution of `StepActions` within a `TaskRun` is now performed concurrently, which can significantly reduce the time it takes for a `TaskRun` to start, especially when using multiple remote `StepActions`.

linux-foundation-easycla · 2025-07-28T15:46:56Z

The committers listed above are authorized under a signed CLA.

✅ login: Maximilien-R / name: Maximilien Raulic (9b1f2e7)

tekton-robot · 2025-07-28T15:57:03Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8
pkg/reconciler/taskrun/resources/taskspec.go	100.0%	97.1%	-2.9

Maximilien-R · 2025-07-28T19:14:24Z

/kind feature

tekton-robot · 2025-07-28T19:31:54Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8
pkg/reconciler/taskrun/resources/taskspec.go	100.0%	97.1%	-2.9

afrittoli

Thanks for this, it looks good!
A few minor comments, nothing blocking.
/approve

pkg/apis/config/testdata/config-defaults-step-action-parallelism-limit-err.yaml

pkg/apis/config/testdata/config-defaults-step-action-parallelism-limit.yaml

afrittoli · 2025-07-29T20:32:38Z

pkg/reconciler/taskrun/resources/taskspec.go

+}
+
+// HasStepRefs provides a fast check to see if any steps in a TaskSpec contain a reference to a StepAction.
+func HasStepRefs(taskSpec *v1.TaskSpec) bool {


Is this exported only so that it may have dedicated unit tests?

Indeed, initially the function was private but not wanting to modify the test file too much, I made it public, thinking that it could be a function that could be useful in other contexts.

However, I introduced this commit to make it private if that makes more sense to you.

Let me know what your preference is and I'd be happy to squash or remove that commit.

Thanks @Maximilien-R for the extra commit. Either way is probably ok.

We usually don't export functions unless we need to, but we also have a policy (which we don't always honour), to only tests for exported functions, meaning that other functions can only be tested indirectly through their calling function.

In this case, I'm not sure which of the two policies would win, it seems reasonable to have unit tests specifically for that function. @vdemeester any preference?

@afrittoli @vdemeester should we document this under the contribution guide if not documented already ?

pkg/reconciler/taskrun/resources/taskspec_test.go

waveywaves

The default-step-action-parallelism-limit doesn't specify exactly what is being parallelized, and don't think these would be running in parallel as your PR adds changes to throttle the concurrency of StepAction resolution go routines (g.Go() usage) which doesn't guarantee parallel CPU execution. I believe using the term concurrency here is much better.

What do you think about updating the config key to another identifier which reflects this better default-step-ref-concurrency-limit (considering this is to resolve the references), or default-step-action-concurrency-limit (replacing the existing parallemism from the key mentioned in this PR to concurrency) or maybe something else ...

waveywaves · 2025-07-29T21:28:32Z

/retest

tekton-robot · 2025-08-04T12:04:51Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8
pkg/reconciler/taskrun/resources/taskspec.go	100.0%	97.1%	-2.9

tekton-robot · 2025-08-04T12:15:43Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8
pkg/reconciler/taskrun/resources/taskspec.go	100.0%	97.1%	-2.9

Maximilien-R · 2025-08-04T12:18:08Z

@afrittoli, I have applied the various proposed corrections as well as converting the public HasStepRefs function into a private function, let me know if this seems more relevant to you.

@waveywaves As suggested, I replaced the various occurrences of the notion of "parallelism" with "concurrency" and "step action" with "step ref", indeed, this seems more coherent to me.

I also took the opportunity to expand the documentation around the configuration key to make things more explicit and detailed.

Note: I've pushed unit commits to make it easier to review and identify the changes made. When you're satisfied with the results, I'd be glad to squash everything into one commit and update the body and message of my initial commit.

waveywaves · 2025-08-15T07:35:17Z

/retest

waveywaves

Based on #8925 (comment), I see a -2.9% delta on one of the files wrt unit test coverage. Can we try having a smaller delta <0.3-0.5%?

Apart from that it looks good, I can lgtm after we have this one update 👼

tekton-robot · 2025-08-19T15:59:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, waveywaves

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [afrittoli,waveywaves]

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

tekton-robot · 2025-08-20T09:13:56Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8

Maximilien-R · 2025-08-20T09:21:32Z

Hi @waveywaves,

I added two more commits:

This one to add missing test cases to cover missing branches.
This one to remove unnecessary too conservative conditions which, actually, can't be hit with the current implementation of the underlying functions.

I also took the opportunity to rebase my branch on main and squash my previous commits into one while modifying the content of the commit message to replace occurrences of parallel with concurrent.

If the two added commits are fine with you I can squash them too 👍

waveywaves · 2025-08-20T09:40:11Z

@Maximilien-R thank you for your work, I'll review it soon, if you can squash it that would be great

Avoids unnecessary DeepCopy operations on steps that do not reference a StepAction. Introduces concurrent resolution of steps that reference StepActions to improve the performance of TaskRun reconciliation, especially when using remote resolvers like git. The key changes include: - `hasStepRefs` function: A new function that quickly checks if a `TaskSpec` contains any steps referencing `StepActions`. This allows for an early exit if no resolution is needed, avoiding unnecessary work. - `resolveStepRef` function: This new function encapsulates the logic for resolving a single `StepAction` reference. It handles fetching the remote resource, merging the `StepAction` with the step's specification, and returning the resolved step - Two-phase resolution: The `GetStepActionsData` function is now split into two distinct phases: - Concurrent Resolution: All `StepAction` references are resolved concurrently using an `errgroup`. - Sequential Merging: The resolved steps and their provenance are merged into the final step list and the `TaskRun` status sequentially. - `updateTaskRunProvenance` function: A dedicated function for updating the TaskRun's status with provenance information. The maximum number of StepActions that can be resolved concurrently is defined by the default config and its `default-step-ref-concurrency-limit` key.

tekton-robot · 2025-08-20T10:24:46Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/config/default.go	87.5%	88.3%	0.8

waveywaves · 2025-08-21T14:24:46Z

/ok-to-test

waveywaves · 2025-08-21T14:29:08Z

/lgtm

thank you for your work on this very useful feature!

JordanGoasdoue · 2025-08-26T09:27:13Z

/lgtm

thank you for your work on this very useful feature!

@waveywaves It seems the e2e-tests failed, is it possible to rerun it ?
We would like to have this feature in the next release 🙏

Thank you

waveywaves · 2025-08-26T09:39:28Z

/retest

github-project-automation bot added this to Tekton Community Roadmap Jul 28, 2025

github-project-automation bot moved this to Todo in Tekton Community Roadmap Jul 28, 2025

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 28, 2025

tekton-robot requested review from dibyom and twoGiants July 28, 2025 15:46

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 28, 2025

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 28, 2025

Maximilien-R force-pushed the feat/parallel-stepaction-resolution branch from fbe1cd7 to e48f5d1 Compare July 28, 2025 19:21

waveywaves self-assigned this Jul 29, 2025

afrittoli reviewed Jul 29, 2025

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2025

waveywaves requested changes Jul 29, 2025

View reviewed changes

Maximilien-R force-pushed the feat/parallel-stepaction-resolution branch from c3c4e4a to c2ae737 Compare August 4, 2025 12:04

Maximilien-R requested review from afrittoli and waveywaves August 4, 2025 12:18

waveywaves approved these changes Aug 19, 2025

View reviewed changes

Maximilien-R force-pushed the feat/parallel-stepaction-resolution branch from c2ae737 to 72fc43f Compare August 20, 2025 09:02

Maximilien-R changed the title ~~feat: resolve steps referencing StepActions in parallel~~ feat: resolve steps referencing StepActions concurrently Aug 20, 2025

Maximilien-R force-pushed the feat/parallel-stepaction-resolution branch from 72fc43f to 9b1f2e7 Compare August 20, 2025 10:12

tekton-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Aug 21, 2025

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 21, 2025

tekton-robot merged commit 8669ca1 into tektoncd:main Aug 26, 2025
47 of 48 checks passed

github-project-automation bot moved this from Todo to Done in Tekton Community Roadmap Aug 26, 2025

Conversation

Maximilien-R commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The problem

The changes

Uh oh!

linux-foundation-easycla bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tekton-robot commented Jul 28, 2025

Uh oh!

Maximilien-R commented Jul 28, 2025

Uh oh!

tekton-robot commented Jul 28, 2025

Uh oh!

afrittoli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

afrittoli Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Maximilien-R Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

afrittoli Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waveywaves Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waveywaves left a comment

Choose a reason for hiding this comment

Uh oh!

waveywaves commented Jul 29, 2025

Uh oh!

tekton-robot commented Aug 4, 2025

Uh oh!

tekton-robot commented Aug 4, 2025

Uh oh!

Maximilien-R commented Aug 4, 2025

Uh oh!

waveywaves commented Aug 15, 2025

Uh oh!

waveywaves left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Aug 19, 2025

Uh oh!

tekton-robot commented Aug 20, 2025

Uh oh!

Maximilien-R commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waveywaves commented Aug 20, 2025

Uh oh!

tekton-robot commented Aug 20, 2025

Uh oh!

waveywaves commented Aug 21, 2025

Uh oh!

waveywaves commented Aug 21, 2025

Uh oh!

JordanGoasdoue commented Aug 26, 2025

Uh oh!

waveywaves commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Maximilien-R commented Jul 28, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jul 28, 2025 •

edited

Loading

afrittoli Aug 4, 2025 •

edited

Loading

waveywaves left a comment •

edited

Loading

Maximilien-R commented Aug 20, 2025 •

edited

Loading