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

chore(ci): Switch to PR Reviews for triggering CI #20892

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
# Comment Trigger
# CI Review Trigger
#
# This workflow is a central point for triggering workflow runs that normally run only as part of the merge queue,
# on demand by a comment. The exception being the integration tests, which have their own workflow file for
# comment triggers as the logic is a bit more complex.
#
# NOTE: This workflow runs on Pull Request Review Comments rather than normal comments to be able to
# capture the SHA that the comment is associated with.
#
# The available triggers are:
#
# /ci-run-all : runs all of the below
Expand All @@ -18,11 +21,11 @@
# /ci-run-regression : runs Regression Detection Suite
# /ci-run-k8s : runs K8s E2E Suite

name: Comment Trigger
name: CI Review Trigger

on:
issue_comment:
types: [created]
pull_request_review:
types: [submitted]

env:
DD_ENV: "ci"
Expand All @@ -37,30 +40,30 @@ env:

# The below concurrency group settings would let us cancel in progress runs that were triggered with the
# same comment on a given PR, which could save on time consuming runs.
# But GH does not currently support the github.event.comment.body as part of the concurrency name, this
# But GH does not currently support the github.event.review.body as part of the concurrency name, this
# appears to be due to the potential length of it.
#concurrency:
# group: ${{ github.workflow }}-${{ github.event.issue.id }}-${{ github.event.comment.body }}
# group: ${{ github.workflow }}-${{ github.event.issue.id }}-${{ github.event.review.body }}
# cancel-in-progress: true

jobs:
validate:
name: Validate comment
Copy link

Choose a reason for hiding this comment

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

do you need a step here to validate whether the commit it's running on is the latest in the PR? Some labelling and other actions might give a false sense of what has been tested...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it is necessary that the commit be the latest commit in the PR. The workflows will run again as part of the merge queue so if there are breakages they will be caught then. This comment trigger workflow is just to have the ability to trigger them without adding the PR to the merge queue.

An exception is the component features check which doesn't run on the merge queue, instead it runs nightly, but breakages to that are low urgency and can be resolved later.

name: Validate review
runs-on: ubuntu-latest
timeout-minutes: 5
if: |
github.event.issue.pull_request && (
contains(github.event.comment.body, '/ci-run-all')
|| contains(github.event.comment.body, '/ci-run-cli')
|| contains(github.event.comment.body, '/ci-run-misc')
|| contains(github.event.comment.body, '/ci-run-deny')
|| contains(github.event.comment.body, '/ci-run-component-features')
|| contains(github.event.comment.body, '/ci-run-cross')
|| contains(github.event.comment.body, '/ci-run-unit-mac')
|| contains(github.event.comment.body, '/ci-run-unit-windows')
|| contains(github.event.comment.body, '/ci-run-environment')
|| contains(github.event.comment.body, '/ci-run-regression')
|| contains(github.event.comment.body, '/ci-run-k8s')
startsWith(github.event.review.body, '/ci-run-all')
|| startsWith(github.event.review.body, '/ci-run-cli')
|| startsWith(github.event.review.body, '/ci-run-misc')
|| startsWith(github.event.review.body, '/ci-run-deny')
|| startsWith(github.event.review.body, '/ci-run-component-features')
|| startsWith(github.event.review.body, '/ci-run-cross')
|| startsWith(github.event.review.body, '/ci-run-unit-mac')
|| startsWith(github.event.review.body, '/ci-run-unit-windows')
|| startsWith(github.event.review.body, '/ci-run-environment')
|| startsWith(github.event.review.body, '/ci-run-regression')
|| startsWith(github.event.review.body, '/ci-run-k8s')
)
steps:
- name: Generate authentication token
Expand All @@ -69,14 +72,7 @@ jobs:
with:
app_id: ${{ secrets.GH_APP_DATADOG_VECTOR_CI_APP_ID }}
private_key: ${{ secrets.GH_APP_DATADOG_VECTOR_CI_APP_PRIVATE_KEY }}
- name: Get PR author
id: pr
uses: tspascoal/get-user-teams-membership@v3
with:
username: ${{ github.event.issue.user.login }}
team: 'Vector'
GITHUB_TOKEN: ${{ steps.generate_token.outputs.token }}
- name: Get PR comment author
- name: Get PR review author
Comment on lines -72 to -79
Copy link

Choose a reason for hiding this comment

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

Neat. Good to remove because this could have technically been vulnerable as others might push to another one's PR 💯

Copy link
Member Author

Choose a reason for hiding this comment

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

That is true though it would have required:

  • A maintainer to create a fork
  • The maintainer to give a non-maintainer write access to their fork
  • The maintainer to create a PR from their fork

Typically maintainers create branches in the main repository. Agreed though, it is nice to get rid of.

Copy link

Choose a reason for hiding this comment

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

Or steal a GitHub token with write permissions and push changes to a maintainer's branch and merge via comment 😉

id: comment
uses: tspascoal/get-user-teams-membership@v3
with:
Expand All @@ -85,65 +81,65 @@ jobs:
GITHUB_TOKEN: ${{ steps.generate_token.outputs.token }}

- name: Validate author membership
if: steps.pr.outputs.isTeamMember == 'false' || steps.comment.outputs.isTeamMember == 'false'
if: steps.comment.outputs.isTeamMember == 'false'
run: exit 1

cli:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-cli')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-cli')
uses: ./.github/workflows/cli.yml
secrets: inherit

misc:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-misc')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-misc')
uses: ./.github/workflows/misc.yml
secrets: inherit

deny:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-deny')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-deny')
uses: ./.github/workflows/deny.yml
secrets: inherit

component-features:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-component-features')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-component-features')
uses: ./.github/workflows/component_features.yml
secrets: inherit

cross:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-cross')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-cross')
uses: ./.github/workflows/cross.yml
secrets: inherit

unit-mac:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-unit-mac')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-unit-mac')
uses: ./.github/workflows/unit_mac.yml
secrets: inherit

unit-windows:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-unit-windows')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-unit-windows')
uses: ./.github/workflows/unit_windows.yml
secrets: inherit

environment:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-environment')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-environment')
uses: ./.github/workflows/environment.yml
secrets: inherit

regression:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-regression')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-regression')
uses: ./.github/workflows/regression.yml
secrets: inherit

k8s:
needs: validate
if: contains(github.event.comment.body, '/ci-run-all') || contains(github.event.comment.body, '/ci-run-k8s')
if: startsWith(github.event.review.body, '/ci-run-all') || contains(github.event.review.body, '/ci-run-k8s')
uses: ./.github/workflows/k8s_e2e.yml
secrets: inherit
25 changes: 10 additions & 15 deletions .github/workflows/cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,23 @@ jobs:
env:
CARGO_INCREMENTAL: 0
steps:
- name: (PR comment) Get PR branch
if: ${{ github.event_name == 'issue_comment' }}
uses: xt0rted/pull-request-comment-branch@v2
id: comment-branch

- name: (PR comment) Set latest commit status as pending
if: ${{ github.event_name == 'issue_comment' }}
- name: (PR review) Set latest commit status as pending
if: ${{ github.event_name == 'pull_request_review' }}
uses: myrotvorets/[email protected]
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: CLI - Linux
status: pending

- name: (PR comment) Checkout PR branch
if: ${{ github.event_name == 'issue_comment' }}
- name: (PR review) Checkout review SHA
if: ${{ github.event_name == 'pull_request_review' }}
uses: actions/checkout@v3
with:
ref: ${{ steps.comment-branch.outputs.head_ref }}
ref: ${{ github.events.review.commit_id }}

- name: Checkout branch
if: ${{ github.event_name != 'issue_comment' }}
if: ${{ github.event_name != 'pull_request_review' }}
Comment on lines -13 to +29
Copy link

Choose a reason for hiding this comment

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

you could avoid this construct and create a more hard-linked reference: set an input to this reusable workflow to provide the ref, emit a "validated commit ID" in the validate job and pass it as input to these workflows.

the pattern is repeated in several reusable workflows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, I did notice this boilerplate repeated across workflows that could be DRY'd up, but didn't want to attempt that refactoring with this change.

Copy link

Choose a reason for hiding this comment

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

ok, sounds good to me

uses: actions/checkout@v3

- name: Cache Cargo registry + index
Expand All @@ -54,11 +49,11 @@ jobs:
run: scripts/upload-test-results.sh
if: always()

- name: (PR comment) Set latest commit status as ${{ job.status }}
- name: (PR review) Set latest commit status as ${{ job.status }}
uses: myrotvorets/[email protected]
if: always() && github.event_name == 'issue_comment'
if: always() && github.event_name == 'pull_request_review'
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: CLI - Linux
status: ${{ job.status }}
29 changes: 12 additions & 17 deletions .github/workflows/component_features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#
# Runs on:
# - scheduled UTC midnight Tues-Sat
# - on PR comment (see comment-trigger.yml)
# - on PR review (see comment-trigger.yml)
# - on demand from github actions UI

name: Component Features - Linux
Expand All @@ -20,42 +20,37 @@ jobs:
check-component-features:
# use free tier on schedule and 8 core to expedite results on demand invocation
runs-on: ${{ github.event_name == 'schedule' && 'ubuntu-latest' || 'ubuntu-20.04-8core' }}
if: github.event_name == 'issue_comment' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule'
if: github.event_name == 'pull_request_review' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule'
steps:
- name: (PR comment) Get PR branch
if: github.event_name == 'issue_comment'
uses: xt0rted/pull-request-comment-branch@v2
id: comment-branch

- name: (PR comment) Set latest commit status as pending
if: github.event_name == 'issue_comment'
- name: (PR review) Set latest commit status as pending
if: github.event_name == 'pull_request_review'
uses: myrotvorets/[email protected]
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: Component Features - Linux
status: pending

- name: (PR comment) Checkout PR branch
if: github.event_name == 'issue_comment'
- name: (PR review) Checkout PR branch
if: github.event_name == 'pull_request_review'
uses: actions/checkout@v3
with:
ref: ${{ steps.comment-branch.outputs.head_ref }}
ref: ${{ github.events.review.commit_id }}

- name: Checkout branch
if: github.event_name != 'issue_comment'
if: github.event_name != 'pull_request_review'
uses: actions/checkout@v3

- run: sudo -E bash scripts/environment/bootstrap-ubuntu-20.04.sh
- run: bash scripts/environment/prepare.sh
- run: echo "::add-matcher::.github/matchers/rust.json"
- run: make check-component-features

- name: (PR comment) Set latest commit status as ${{ job.status }}
if: always() && github.event_name == 'issue_comment'
- name: (PR review) Set latest commit status as ${{ job.status }}
if: always() && github.event_name == 'pull_request_review'
uses: myrotvorets/[email protected]
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: Component Features - Linux
status: ${{ job.status }}
36 changes: 13 additions & 23 deletions .github/workflows/cross.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,23 @@ jobs:
- arm-unknown-linux-gnueabi
- arm-unknown-linux-musleabi
steps:

- name: (PR comment) Get PR branch
if: ${{ github.event_name == 'issue_comment' }}
uses: xt0rted/pull-request-comment-branch@v2
id: comment-branch

- name: (PR comment) Set latest commit status as pending
if: ${{ github.event_name == 'issue_comment' }}
- name: (PR review) Set latest commit status as pending
if: ${{ github.event_name == 'pull_request_review' }}
uses: myrotvorets/[email protected]
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: Cross
status: pending

- name: (PR comment) Checkout PR branch
if: ${{ github.event_name == 'issue_comment' }}
- name: (PR review) Checkout PR branch
if: ${{ github.event_name == 'pull_request_review' }}
uses: actions/checkout@v3
with:
ref: ${{ steps.comment-branch.outputs.head_ref }}
ref: ${{ github.events.review.commit_id }}

- name: Checkout branch
if: ${{ github.event_name != 'issue_comment' }}
if: ${{ github.event_name != 'pull_request_review' }}
uses: actions/checkout@v3

- uses: actions/cache@v4
Expand All @@ -70,30 +64,26 @@ jobs:
name: "vector-debug-${{ matrix.target }}"
path: "./target/${{ matrix.target }}/debug/vector"

- name: (PR comment) Set latest commit status as failed
- name: (PR review) Set latest commit status as failed
uses: myrotvorets/[email protected]
if: failure() && github.event_name == 'issue_comment'
if: failure() && github.event_name == 'pull_request_review'
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
token: ${{ secrets.GITHUB_TOKEN }}
context: Cross
status: 'failure'

update-pr-status:
name: (PR comment) Signal result to PR
name: (PR review) Signal result to PR
runs-on: ubuntu-20.04
timeout-minutes: 5
needs: cross-linux
if: needs.cross-linux.result == 'success' && github.event_name == 'issue_comment'
if: needs.cross-linux.result == 'success' && github.event_name == 'pull_request_review'
steps:
- name: (PR comment) Get PR branch
uses: xt0rted/pull-request-comment-branch@v2
id: comment-branch

- name: (PR comment) Submit PR result as success
- name: (PR review) Submit PR result as success
uses: myrotvorets/[email protected]
with:
sha: ${{ steps.comment-branch.outputs.head_sha }}
sha: ${{ github.events.review.commit_id }}
token: ${{ secrets.GITHUB_TOKEN }}
context: Cross
status: 'success'
Loading
Loading