wip: Run less e2e matrix by default #9109

vdemeester · 2025-10-28T14:23:55Z

Changes

But add a way to run all of them with a comment.

I wonder if this is going to work 🤔

/kind misc

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

vdemeester · 2025-10-28T15:30:59Z

/run-backcompat-e2e

vdemeester · 2025-10-29T11:22:01Z

.github/workflows/e2e-matrix-extras.yaml

+
+jobs:
+  run-if-requested:
+    if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/run-extra-tests') }}


So this is a bit more efficient than slash command as it doesn't require to run a workflow if the comment doesn't contain /run-extra-tests

Does this circumvent rules regarding who can run/trigger tests?

I think we should stick to one way of implementing slash commands.

Maybe we could use a slash command to add a label, and check for the label in matrix - only run certain instances if the label is set.

So the only reason I did not go with slash command was, aside from experimenting, that this approach doesn't consume "run" — aka it won't start a workflow unless the comment on the PR has this content. The slash command approaches runs on any comments on any PRs (or issues I think), which tends to be a lot (and sometimes it may take over some other workflows).

But in general I agree with you, we should stick to one way of implementing slash commands indeed.

vdemeester · 2025-10-29T11:22:10Z

/run-extra-tests

vdemeester · 2025-10-29T11:22:25Z

^^ this won't work for that PR, but this would be the "incatation"

vdemeester · 2025-10-29T11:22:44Z

Example of it vdemeester/experiments#3, working with forks.

tekton-robot · 2025-10-29T13:04:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: twoGiants
To complete the pull request process, please ask for approval from vdemeester after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

OWNERS

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

vdemeester · 2025-10-29T14:21:31Z

Note: the idea is then to mark oldest and latest (including the arm one) as required, and the rest would be optionnal. If they run and fail, by default, tide will refuse to merge anyway.

aThorp96

I like this idea a lot, I think it will reduce flakiness for any iterative pull requests. Though I wonder if we should ensure that the matrix extras have passed before a PR is merged, in case a backwards compatibility regression is introduced and /run-extra-tests was never commented on the PR initially I missed that the required e2e tests still test latest and oldest. I think that should effectively cover our bases for regressions.

Two concerns I wanted to discuss as well:

We will need to make sure this doesn't enable circumventing CI protection rules, who can run or rerun CI
This duplicates a lot of the existing e2e-matrix.yml workflow. Since that workflow is already setup to be dispatched from another workflow and , does it makes sense to rewrite the e2e test workflow to receive the os, k8s-name, k8s-version, feature-flags, etc as inputs? If we do this, the business logic for setting up and running the e2e tests would all be in one place, and the various workflows which trigger e2e tests would be responsible for specifying their matrixes

aThorp96 · 2025-10-29T14:26:14Z

.github/workflows/e2e-matrix-extras.yaml

+
+jobs:
+  run-if-requested:
+    if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/run-extra-tests') }}


Does this circumvent rules regarding who can run/trigger tests?

aThorp96 · 2025-10-29T14:27:22Z

.github/workflows/e2e-matrix-extras.yaml

+    outputs:
+      sha: ${{ steps.pr.outputs.sha }}
+
+  backcompat-e2e:


For clarity

Suggested change

backcompat-e2e:

backwards-compatibility-e2e:

aThorp96 · 2025-10-29T14:27:59Z

.github/workflows/e2e-matrix-extras.yaml

+    concurrency:
+      group: ${{ github.workflow }}-${{ matrix.os }}-${{ matrix.k8s-name }}-${{ matrix.feature-flags }}-${{ github.event.issue.number }}
+      cancel-in-progress: true
+    name: e2e tests (backcompat)


For clarity

Suggested change

name: e2e tests (backcompat)

name: e2e tests (backwards compatibility)

aThorp96 · 2025-10-29T14:49:11Z

Also, it looks like while multiple /run-extra-tests comments will only result in one Check showing up on the PR, each comment will still run the full test suite without cancelling. We'll need to find a way to ensure the previous runs are cancelled when a new run is started on the same PR

vdemeester · 2025-10-29T14:50:57Z

Also, it looks like while multiple /run-extra-tests comments will only result in one Check showing up on the PR, each comment will still run the full test suite without cancelling. We'll need to find a way to ensure the previous runs are cancelled when a new run is started on the same PR

Yes, cancelling will be handled for this PR as we have the cancelling group, that I didn't have in my example repository.

We will need to make sure this doesn't enable circumventing CI protection rules, who can run or rerun CI

Yes, this is something missing, a check that would check if you can or not run/rerun the CI indeed.

vdemeester · 2025-10-29T14:51:59Z

Yeah the name isn't correct, I should use extra-tests or something instead of backward..

vdemeester · 2025-10-29T14:58:23Z

I think I can use something similar to https://github.com/peter-evans/slash-command-dispatch/blob/main/src/github-helper.ts#L58.

But add a way to run all of them with a comment. Signed-off-by: Vincent Demeester <[email protected]>

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

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

tekton-robot added kind/misc Categorizes issue or PR as a miscellaneuous one. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesnt merit a release note. labels Oct 28, 2025

tekton-robot requested review from pritidesai and twoGiants October 28, 2025 14:24

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 28, 2025

vdemeester force-pushed the gh-workflow-on-demand branch from fa89ad4 to a3243e7 Compare October 28, 2025 15:03

vdemeester force-pushed the gh-workflow-on-demand branch 2 times, most recently from b0bbe04 to 654bf50 Compare October 29, 2025 11:18

vdemeester commented Oct 29, 2025

View reviewed changes

twoGiants approved these changes Oct 29, 2025

View reviewed changes

aThorp96 reviewed Oct 29, 2025

View reviewed changes

wip: Run less e2e matrix by default

65b5748

But add a way to run all of them with a comment. Signed-off-by: Vincent Demeester <[email protected]>

vdemeester force-pushed the gh-workflow-on-demand branch from 654bf50 to 65b5748 Compare November 6, 2025 08:49

	name: e2e tests (backcompat)
	name: e2e tests (backwards compatibility)

wip: Run less e2e matrix by default #9109

Are you sure you want to change the base?

wip: Run less e2e matrix by default #9109

Uh oh!

Conversation

vdemeester commented Oct 28, 2025

Changes

Submitter Checklist

Release Notes

Uh oh!

vdemeester commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

tekton-robot commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

aThorp96 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aThorp96 commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

vdemeester commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aThorp96 left a comment •

edited

Loading