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.

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

feat(ci): Add retest workflow. Fixes #12864 #13000

Merged
merged 1 commit into from
May 2, 2024

Conversation

miltalex
Copy link
Member

@miltalex miltalex commented May 1, 2024

Fixes #12864

Motivation

Due to flaky tests, it would be nice to allow Members to re-run failing workflows.

Modifications

Extend github workflows with retest.yaml. It adds a new workflow run which is triggered on issue_comment:[created,edited]. It requires the comment to contain exactly the /retest. It will execute a sequence of API calls to fetch the latest SHA, the latest run ID and trigger the rerun of the failed jobs.

Note: If the workflow doesn't contain any failed jobs it will fail.

Verification

Used a forked repository https://github.com/miltalex/argo-workflows . Merged the new workflow on main branch and opened PRs (miltalex#1 , miltalex#2 , miltalex#3) in order to:

  • Test new comment /retest → Trigger failed jobs
  • Test edit comment to retest → Retest workflow is skipped
  • Test edit comment to /retest → Trigger failed jobs
  • Test new comment without /retest → Retest workflow is skipped
  • Test new comment with /retest and no failing jobs → Retest workflow failed due to no failing jobs.

@miltalex miltalex force-pushed the feature/gh-wf-retest-comment branch 2 times, most recently from a4dd27f to 7675c06 Compare May 1, 2024 13:30
@miltalex miltalex marked this pull request as ready for review May 1, 2024 14:03
@agilgur5 agilgur5 self-assigned this May 1, 2024
@agilgur5 agilgur5 added the area/build Build or GithubAction/CI issues label May 1, 2024
@agilgur5 agilgur5 changed the title feat: Add retest workflow. Fixes #12864 feat(ci): Add retest workflow. Fixes #12864 May 1, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented May 1, 2024

  • Tested in a forked repository

Would be good to detail where and what you tested.

I know you were testing in miltalex#2 and miltalex#1 as I had been taking a look. It would be good to also add which scenarios you tested in a list. Can see my "Verification" section in #12006 as an example

Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Thanks for all the effort in testing this out and getting it working!

I left some comments below to optimize this a bit. Some token security settings also need adjustments

.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
@miltalex miltalex force-pushed the feature/gh-wf-retest-comment branch 3 times, most recently from 9d2ace5 to d757689 Compare May 2, 2024 13:09
Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

A few small nits, otherwise LGTM

.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
.github/workflows/retest.yaml Outdated Show resolved Hide resolved
Signed-off-by: Miltiadis Alexis <alexmiltiadis@gmail.com>
@miltalex miltalex force-pushed the feature/gh-wf-retest-comment branch from d757689 to ca3ed5d Compare May 2, 2024 16:39
Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Great work, this should be really helpful whenever we have test flakes!

@agilgur5 agilgur5 enabled auto-merge (squash) May 2, 2024 16:43
@agilgur5 agilgur5 merged commit b4af68b into argoproj:main May 2, 2024
15 checks passed
@miltalex miltalex deleted the feature/gh-wf-retest-comment branch May 2, 2024 16:55
@agilgur5
Copy link
Contributor

agilgur5 commented May 4, 2024

Hmm this seemed to have failed on my first try of it in #12732 (comment): https://github.com/argoproj/argo-workflows/actions/runs/8948441124/job/24581552898:

/home/runner/work/_temp/f4de4a64-b8ac-400d-a1ff-042b59deb995.sh: line 2: unexpected EOF while looking for matching `''

@agilgur5
Copy link
Contributor

agilgur5 commented May 4, 2024

^That run might actually be too old and may have been GC'd on GH's side. I didn't have a button to rerun the failed jobs myself.

We may want to add some error handling for a more clear error message in some scenarios

@agilgur5
Copy link
Contributor

agilgur5 commented May 4, 2024

Hmm failed on a more recent run in #12732 (comment) as well: https://github.com/argoproj/argo-workflows/actions/runs/8949025175/job/24582819712. Same error as above

@miltalex
Copy link
Member Author

miltalex commented May 4, 2024

I will have a look to amend it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Add GH Action for /retest comments to re-run failed jobs
2 participants