Skip to content

ci: add support for /retest command#2219

Merged
jpsim merged 3 commits intomainfrom
ci-add-support-for-retest-command
Apr 29, 2022
Merged

ci: add support for /retest command#2219
jpsim merged 3 commits intomainfrom
ci-add-support-for-retest-command

Conversation

@jpsim
Copy link
Contributor

@jpsim jpsim commented Apr 26, 2022

Commenting /retest on a PR will re-run failed jobs in its GitHub Actions workflows.

image

Copied from https://github.com/containernetworking/plugins.

Commenting `/retest` on a PR will re-run its GitHub Actions workflows.

Copied from https://github.com/containernetworking/plugins.

Signed-off-by: JP Simard <jp@jpsim.com>
@jpsim
Copy link
Contributor Author

jpsim commented Apr 26, 2022

The /retest command will only work once this PR is merged, so I can't test it here yet.

@jpsim jpsim marked this pull request as ready for review April 26, 2022 21:55
@jpsim
Copy link
Contributor Author

jpsim commented Apr 26, 2022

Right now, this will re-run all CI jobs, including ones that are already succeeding. I might look into making it just re-run failing jobs at a later time.

Update: now only re-runs failed jobs

@jpsim
Copy link
Contributor Author

jpsim commented Apr 26, 2022

In my testing, it takes 20 to 30 seconds for the command to take effect: https://github.com/jpsim/ZenTuner/actions/workflows/commands.yml

@keith
Copy link
Member

keith commented Apr 26, 2022

there is some custom stuff on envoy that we could reuse here probably

@jpsim
Copy link
Contributor Author

jpsim commented Apr 26, 2022

there is some custom stuff on envoy that we could reuse here probably

i looked at that but Envoy’s CI is all Azure Pipelines so it doesn’t really carry over.

@mattklein123
Copy link
Member

cc @itayd who helps us with RepoKitteh in the main repo and is working on "something better" which we can hopefully use here also when it is ready.

@itayd
Copy link

itayd commented Apr 27, 2022

don't we already have retest working here for azp?

@jpsim
Copy link
Contributor Author

jpsim commented Apr 27, 2022

Envoy Mobile uses GitHub Actions for all of its CI, there are no azp jobs. There's also one CircleCI job that deploys docs when new versions are pushed.

@Augustyniak
Copy link
Contributor

Speaking from the perspective of a person who did a few contributions in EM repo without being a maintainer I think that having a /retest command is a quality of life improvement - especially if we find a way to eventually make it smarter so that it does not rerun green jobs (that would've been pretty awesome).

Pretty often I found myself pushing multiple empty commits to a branch just to get rid jobs that decided to go red randomly. Having /retest would've allowed me to have to do less of a git work.

@goaway
Copy link
Contributor

goaway commented Apr 28, 2022

Now that GitHub Actions directly supports rerunning failed jobs only, I think that's almost always preferable to rerunning all of CI. We have finite resources, and CI jobs do get backed up. If this could be improved to rerun failed jobs only, I feel it would be a lot more compelling. That said, for the occasions where rerunning everything really is what you want (post-CI outage?), I don't see the harm in having this.

Unlike `rerun_url`, this endpoint isn't in the run API response, but we
can construct it manually easily.

Documentation for it is here:
https://docs.github.com/en/rest/actions/workflow-runs#re-run-failed-jobs-from-a-workflow-run

Signed-off-by: JP Simard <jp@jpsim.com>
@jpsim
Copy link
Contributor Author

jpsim commented Apr 29, 2022

Thanks for the feedback everyone. After spending some time looking at the API responses, and looking at GitHub's REST API documentation, I found an endpoint to rerun failed jobs from a workflow run.

Unlike rerun_url, this endpoint isn't in the run API response, but we can construct it manually easily.

Documentation for it is here: https://docs.github.com/en/rest/actions/workflow-runs#re-run-failed-jobs-from-a-workflow-run

I've tested it on a separate repo and it worked as expected: jpsim/ZenTuner#47

@jpsim jpsim requested review from Augustyniak and goaway April 29, 2022 16:58
@jpsim
Copy link
Contributor Author

jpsim commented Apr 29, 2022

@goaway @Augustyniak did my latest updates address your concerns about only re-running failing jobs?

@goaway
Copy link
Contributor

goaway commented Apr 29, 2022

Thanks @jpsim! This is fantastic, and should be a huge improvement over needing to navigate the UI to restart each failing individually - in addition to solving the permissions issue for external contributors.

Signed-off-by: JP Simard <jp@jpsim.com>
@jpsim jpsim requested a review from goaway April 29, 2022 20:15
@jpsim jpsim merged commit 13f9a6e into main Apr 29, 2022
@jpsim jpsim deleted the ci-add-support-for-retest-command branch April 29, 2022 23:52
jpsim added a commit that referenced this pull request May 2, 2022
* origin/main:
  Bump rules_apple to 0.34.2 (#2236)
  Bump Lyft Support Rotation (#2232)
  ci: add support for `/retest` command (#2219)
  format: add SwiftLint to check-format script (#2230)
  use 64 bit emulators for test (#2228)
  envoy: update to `d0befbb` & add `h2ExtendKeepaliveTimeout` (#2229)
  Add Ryan Hamilton to OWNERS.md (#2224)
  Android cert verifier: first import from chromium/net (#2222)

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit that referenced this pull request May 3, 2022
…rtion

* origin/main: (57 commits)
  network: add enableDrainPostDnsRefresh to iOS (#2242)
  envoy: update to em-cherry (#2241)
  network: support draining connections after triggered DNS refresh (#2225)
  Bump rules_apple to 0.34.2 (#2236)
  Bump Lyft Support Rotation (#2232)
  ci: add support for `/retest` command (#2219)
  format: add SwiftLint to check-format script (#2230)
  use 64 bit emulators for test (#2228)
  envoy: update to `d0befbb` & add `h2ExtendKeepaliveTimeout` (#2229)
  Add Ryan Hamilton to OWNERS.md (#2224)
  Android cert verifier: first import from chromium/net (#2222)
  Cleanup: remove the unused stats sink metrics_service (#2220)
  bazel: move back to symbol mapping table files (#2218)
  Add new version history section (#2209)
  build: simplify jnilib copy (#2214)
  ci: Update macOS version to macOS 12 (#2208)
  Update releasing.rst (#2200)
  support: Post Lyft support rotation changes to Slack (#2207)
  Don't run bump_support_rotation GitHub Action on forks (#2204)
  Release 0.4.6 (#2201)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants