open-telemetry / opentelemetry.io

The OpenTelemetry website and documentation
https://opentelemetry.io
Creative Commons Attribution 4.0 International
534 stars 1.16k forks source link

[CI] Running `/fix:cmd` over a PR rebuilds but skips GH action checks #4339

Open chalin opened 5 months ago

chalin commented 5 months ago

For context, see:

Unless we get a separate bot to handle /fix:* commands, we're stuck with this behavior: GH doesn't rerun actions if a workflow over the same repo committed a change to a PR. I usually work around this by forcing a change (via rebase, or by making a trivial edit) over the PR, but that isn't always practical.

Other possible solutions:

Thoughts @open-telemetry/docs-maintainers?

svrnm commented 5 months ago

This discussion links to the official GH documentation about the topic:

When you use the repository's GITHUB_TOKEN to perform tasks, events triggered by the GITHUB_TOKEN, with the exception of workflow_dispatch and repository_dispatch, will not create a new workflow run. This prevents you from accidentally creating recursive workflow runs.

And:

If you do want to trigger a workflow from within a workflow run, you can use a GitHub App installation access token or a personal access token instead of GITHUB_TOKEN to trigger events that require a token.

Using a PAT seems to be a reasonable solution for me.

svrnm commented 5 months ago

Using a PAT seems to be a reasonable solution for me.

We have one already, and maybe using it consistently will help. Let me raise a PR for that.

svrnm commented 5 months ago

looks like #4345 did not solve it. I tested it with https://github.com/open-telemetry/opentelemetry.io/pull/4349 and now checks were run afterwards.

svrnm commented 5 months ago

So the "good" news is that we are not the only ones running in those issues, so there is some info out there that is helpful, but this is a rabbit hole, looks like the checkout action adds additional trouble:

https://github.com/orgs/community/discussions/26220

chalin commented 5 months ago

@svrnm - we're hitting permission issues now, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/8822774576/job/24221719586:

image
svrnm commented 5 months ago

@chalin that's odd because this step was not updated by any of the 2 latest changes:

https://github.com/open-telemetry/opentelemetry.io/blame/adb72161798991fe272251de4f6d9a8bfd8bc799/.github/workflows/pr-actions.yml#L105

chalin commented 5 months ago

Reverting the last change, as you did was what I had in mind too. Thanks for doing that and investigating further 🙏

svrnm commented 5 months ago

Reverting the last change, as you did was what I had in mind too. Thanks for doing that and investigating further 🙏

I am worried we are in a position where neither token gives us all the things we need, as it looks like the PAT is not allowed to push to forks. In my research I came across a page that listed solutions to this problem but I closed it so I have to find it once again will keep you posted.