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

Update workflow to be used in merge queue. #2420

Closed
wants to merge 1 commit into from
Closed
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
30 changes: 8 additions & 22 deletions .github/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,21 @@ We have two types of files in the `/workflows` directory:
- they shouldn't (and can't) be used in isolation, as they don't take care of the code being checked out
- 📄 starting with `workflow_` indicate a collection of jobs that are automatically triggered by certain events or actions

## Pipelines
## Pipelines and workflows

We have two types of workflows triggered by Pull Requests:
We have one workflow triggered by Pull Requests. When a Pull Request is opened, we run selected jobs based on the location and changes made in the code. We have two sets of jobs:

1. When a Pull Request is opened, we run selected jobs based on the location and changes made in the code. We have two sets of jobs:
- if changes are made inside the `/coral` directory, we run all [coral jobs](./workflows/jobs-coral.yaml)
- if are made outside in `/core` or `/cluster-api` directories or `pom.xml`, we run all [maven jobs](./workflows/jobs-maven.yaml). This jobs are only run when the Pull Request is ready for review, excluding "Draft" PRs.
2. When a Pull Request is approved, the [`workflow-merge-to-main`](./workflows/workflow-merge-to-main.yaml) is triggered.
- if changes are made inside the `/coral` directory, we run all [coral jobs](./workflows/jobs-coral.yaml)
- if are made outside in `/core` or `/cluster-api` directories or `pom.xml`, we run all [maven jobs](./workflows/jobs-maven.yaml).

- This workflow runs all [coral jobs](./workflows/jobs-coral.yaml) and [maven jobs](./workflows/jobs-maven.yaml)
- After they are successful, it runs a job called `merge-to-main`. This job is defined in the [Branch protection rule](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/managing-a-branch-protection-rule) for `main` and is and is necessary to enable merging on our default branch.
These jobs are only run when the Pull Request is ready for review, excluding "Draft" PRs.

**Note**: This workflow is also triggered when a Pull Request targeting main is synchronized. This way we make sure that the checks are always automatically run on the latest code.
We have one workflow [`workflow-merge-to-main`](./workflows/workflow-merge-to-main.yaml) that we run in a merge queue before merging an approved PR branch to `main`.

In addition, we have a workflow that is triggered when changes are made to the `openapi.yaml file`. We check whether the changes affect the TypeScript file in `/coral`. If the changes do affect the TypeScript file, the `api.d.ts` file is automatically generated, and the TypeScript compiler is run. If the TypeScript compiler finds no errors, the file is committed. Otherwise the job fails.

## Drawbacks

We have identified two potential drawbacks with this strategy:
- This workflow runs all [coral jobs](./workflows/jobs-coral.yaml) and [maven jobs](./workflows/jobs-maven.yaml)

1. There will be a delay between approving a Pull Request and merging it because of the required job on approval. We can mitigate this in the future:

- by adding automatic notification about a PR being ready to merge.
- consider auto-merging it in the future. Currently, we prefer manual merges to maintain more control over the process.

2. Depending on the workflow, some job runs may be redundant and increase the time between giving a Pull Request for review and merging it.
In addition, we have a workflow that is triggered when changes are made to the `openapi.yaml file`. We check whether the changes affect the TypeScript file in `/coral`. If the changes do affect the TypeScript file, the `api.d.ts` file is automatically generated, and the TypeScript compiler is run. If the TypeScript compiler finds no errors, the file is committed. Otherwise the job fails.

- We plan to iterate over our workflows to reduce redundancy and improve efficiency and will pay attention if this slows us down,
- We are using the `merge-to-main` workflow as a temporary solution until we move to GitHub merge queues, when they become mores stable.

## Background of this strategy

Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
name: "CodeQL"

on:
push:
branches: [ "main" ]
merge_group:
pull_request:
# The branches below must be a subset of the branches above
branches: [ "main" ]
Expand Down
42 changes: 14 additions & 28 deletions .github/workflows/workflow-merge-to-main.yaml
Original file line number Diff line number Diff line change
@@ -1,28 +1,16 @@
# This workflow is called when a pull request is approved
# that targets the protected branch `main`.
# The job `merge-to-main` ("Status check for enabling merging to main")
# is a required status check.
# (https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28)
# We can only merge to main when this job has run and is green.
# The workflow can be triggered manually, too.
name: Checks enabling merge to main
# This workflow is used in a merge queue
# when a branch targets the protected branch `main`.

name: Enabling merge to main in merge queue

on:
pull_request_review:
types:
- submitted
branches:
- main
pull_request:
types:
- synchronize
branches:
- main
merge_group:
workflow_dispatch:

jobs:
checkout-code:
if: (github.event_name == 'pull_request' && github.event.action == 'synchronize') || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved')
if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || github.event_name == 'merge_group'
runs-on: ubuntu-latest
steps:
- name: Checkout code
Expand All @@ -32,33 +20,31 @@ jobs:
fetch-depth: 0

run-coral-jobs:
if: (github.event_name == 'pull_request' && github.event.action == 'synchronize') || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved')
name: Run coral workflow before enabling merge to main
if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || github.event_name == 'merge_group'
name: Coral workflow
needs: checkout-code
uses: ./.github/workflows/jobs-coral.yaml


run-maven-jobs:
if: (github.event_name == 'pull_request' && github.event.action == 'synchronize') || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved')
name: Run maven workflow before enabling merge to main
if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || github.event_name == 'merge_group'
name: Maven workflow
needs: checkout-code
uses: ./.github/workflows/jobs-maven.yaml

run-e2e-tests:
if: (github.event_name == 'pull_request' && github.event.action == 'synchronize') || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved')
name: Run Playwright e2e test before enabling merge to main
if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || github.event_name == 'merge_group'
name: Playwright e2e test
needs: checkout-code
permissions:
actions: write
uses: ./.github/workflows/end-to-end-tests.yaml

merge-to-main:
name: Status check for enabling merging to main
if: (github.event_name == 'pull_request' && github.event.action == 'synchronize') || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved')
if: github.event_name != 'pull_request' || github.event.action == 'enqueued' || github.event_name == 'merge_group'
runs-on: ubuntu-latest
needs: [ run-maven-jobs, run-coral-jobs, run-e2e-tests ]

steps:
- name: Confirm check
run: echo "🎉 this was successful"

run: echo "🎉 this was successful"
1 change: 0 additions & 1 deletion .github/workflows/workflow-pull-requests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ jobs:
echo "changes-maven=true" >> $GITHUB_OUTPUT
fi


run-maven-jobs:
needs: check-changes
# the condition makes sure that changes on a draft PR do not trigger the job, except it's a "ready_for_review" event
Expand Down
Loading