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.

Sign up for GitHub

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

Jump to bottom

ci: sent ci-triggers comment only once on PR open or reopen #737

Merged
merged 1 commit into from
Jul 16, 2024
Merged

ci: sent ci-triggers comment only once on PR open or reopen #737

merged 1 commit into from
Jul 16, 2024

Conversation

tobiasgiese
Copy link
Contributor

If we specify a type we can stop the Github Actions bot from resending the
ci-triggers list to open PRs on every HEAD change.

[...] if no activity types are specified, the workflow runs when a pull request
is opened or reopened or when the head branch of the pull request is updated.

source: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

zeeke
zeeke approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@zeeke zeeke left a comment

Choose a reason for hiding this comment

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

LGTM

@tobiasgiese tobiasgiese force-pushed the pr-ci-triggers-once branch from 0a7f458 to 7f3c087 Compare July 16, 2024 08:49
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@tobiasgiese
Copy link
Contributor Author

tobiasgiese commented Jul 16, 2024 •
edited
Loading

Pushed a new HEAD to the branch but the bot is still commenting the ci-triggers list.
Or do we need to merge this to master to take affect? AFAIK the workflows are per branch, right?

According to https://docs.github.com/en/actions/using-workflows/triggering-a-workflow:

GitHub searches the .github/workflows directory in the root of your repository for workflow files that are present in the associated commit SHA or Git ref of the event.

So this should work 🤔

@zeeke
Copy link
Member

zeeke commented Jul 16, 2024

This might explain why it is not working:

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow. This event allows your workflow to do things like label or comment on pull requests from forks. Avoid using this event if you need to build or run code from the pull request.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

@tobiasgiese
Copy link
Contributor Author

tobiasgiese commented Jul 16, 2024 •
edited
Loading

This might explain why it is not working:

Ah, that explains it. So we have 2 options:

  • set it to pull_request and keep it if it works
  • test it with pull_request and if it works we can reset it to pull_request_target

WDYT?

Edit: IMO is pull_request for the ci-triggers list totally fine

@tobiasgiese tobiasgiese force-pushed the pr-ci-triggers-once branch from 7f3c087 to a0033f6 Compare July 16, 2024 09:11
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@tobiasgiese
Copy link
Contributor Author

... but sure, simply changing it to pull_request does not work, that would completely undermine the security mechanism 🤦🏻

@tobiasgiese
ci: sent ci-triggers comment only once on PR open or reopen
ee7a04a 
If we specify a type we can stop the Github Actions bot from resending the
ci-triggers list to open PRs on every HEAD change.

> [...] if no activity types are specified, the workflow runs when a pull request
is opened or reopened or when the head branch of the pull request is updated.

Signed-off-by: Tobias Giese <[email protected]>
@tobiasgiese tobiasgiese force-pushed the pr-ci-triggers-once branch from a0033f6 to ee7a04a Compare July 16, 2024 09:12
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jul 16, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9954046539

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 40.909%
Totals Coverage Status
Change from base Build 9939865280: 0.008%
Covered Lines: 5600
Relevant Lines: 13689
💛 - Coveralls

@zeeke
Copy link
Member

zeeke commented Jul 16, 2024

It is way simpler to merge this, test it in another PR, and create a revert PR if the final situation is worse than the actual one.

@adrianchiris
Copy link
Collaborator

merging :)

@adrianchiris adrianchiris merged commit 4daeede into k8snetworkplumbingwg:master Jul 16, 2024
13 checks passed
@tobiasgiese tobiasgiese deleted the pr-ci-triggers-once branch July 16, 2024 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrianchiris adrianchiris adrianchiris approved these changes

@zeeke zeeke zeeke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tobiasgiese @zeeke @coveralls @adrianchiris