Skip to content

Prepare for merge queues #3437

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
Merged
merged 3 commits into from
Sep 9, 2025
Merged

Prepare for merge queues #3437

merged 3 commits into from
Sep 9, 2025

Conversation

stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Jun 19, 2025 •
edited
Loading

We'd like to enable merge queues so that we can ensure all jobs are run on a PR prior to merging that PR. To do this, we need to mark most of our jobs as required. However, we are currently only running those jobs on changes to specific files e.g.:

name: functional-baremetal
on:
  pull_request:
    paths:
      - 'openstack/auth_env.go'
      - 'openstack/client.go'
      - 'openstack/endpoint.go'
      - 'openstack/endpoint_location.go'
      - 'openstack/config/provider_client.go'
      - 'openstack/utils/choose_version.go'
      - 'openstack/utils/discovery.go'
      - '**baremetal**'
      - '.github/workflows/functional-baremetal.yaml'
jobs:
    # ... rest of job definition

However, this causes jobs to never run rather than be "skipped". If the job is required for e.g. a merge queue, then the PR will never merge because it will wait forever for the job result.

This PR adds a local plugin to allow us to filter on files and skip steps when none of the affected files are checked. This only happens on pull requests, since during scheduled jobs we expect all jobs to run. Once this merges, we can mark all but the ironic and octavia master jobs as required.

@github-actions github-actions bot added semver:patch No API change backport-v2 This PR will be backported to v2 edit:actions This PR updates GitHub Actions code edit:testinfra This PR updates testing infrastructure code labels Jun 19, 2025
@coveralls
Copy link

coveralls commented Jun 19, 2025 •
edited
Loading

Coverage Status

coverage: 63.81%. remained the same
when pulling 23eae2c on shiftstack:merge-queues
into 7c53a81 on gophercloud:main.

mandre
mandre approved these changes Jun 20, 2025
View reviewed changes
Copy link
Contributor

@mandre mandre left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This commit doesn't enable merge queues, but prepares for enabling merge queues. We'll still have to configure it in the branch protection as explained here.

As I understand it, we also need to start making more jobs required for it to have any meaningful effect.

mandre
mandre previously requested changes Jun 20, 2025
View reviewed changes
Copy link
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

Actually, we'll also need to trigger the hold workflow on merge_group events, since this job is required.

@stephenfin stephenfin changed the title Enable merge queues Prepare for merge queues Aug 11, 2025
@stephenfin
Copy link
Contributor Author

This commit doesn't enable merge queues, but prepares for enabling merge queues. We'll still have to configure it in the branch protection as explained here.

Good point. Done.

As I understand it, we also need to start making more jobs required for it to have any meaningful effect.

Yeah, and for that to happen we need to ensure we always "run" all unit test and functional jobs and simply make them no-ops if they don't apply.

@stephenfin stephenfin force-pushed the merge-queues branch 6 times, most recently from 3403e4c to b1f9e44 Compare August 12, 2025 14:01
@stephenfin stephenfin marked this pull request as draft August 12, 2025 14:10
@stephenfin
Copy link
Contributor Author

stephenfin commented Aug 12, 2025 •
edited
Loading

This will be a good deal easier to do once #3490 is landed.

Or not. I've reworked this PR to take learnings from that one.

@stephenfin stephenfin force-pushed the merge-queues branch 3 times, most recently from ff190bf to f3280dd Compare August 13, 2025 09:57
@stephenfin
Add local GitHub action
5fb0f17 
To allow us to retrieve the list of changed files in the PR and compare
them against a whitelist. This filtering only happens for non-scheduled
jobs. For scheduled jobs, we should always run.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin stephenfin force-pushed the merge-queues branch 4 times, most recently from c747b95 to 19f97e3 Compare September 1, 2025 16:44
@stephenfin
Always run functional tests
23eae2c 
Inspired by [1].

[1] https://brunoscheufler.com/blog/2022-04-24-required-github-actions-jobs-in-a-monorepo

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin stephenfin marked this pull request as ready for review September 5, 2025 14:21
@stephenfin
Copy link
Contributor Author

stephenfin commented Sep 5, 2025 •
edited
Loading

@kayrus @mandre I think this is ready to go now. Once we merge this, we can make all but the currently failing job required and turn on merge queues.

kayrus
kayrus approved these changes Sep 8, 2025
View reviewed changes
Copy link
Contributor

@kayrus kayrus left a comment

Choose a reason for hiding this comment

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

LGTM, let's give it a try

@stephenfin stephenfin dismissed mandre’s stale review September 9, 2025 15:34

Comments addressed.

@stephenfin stephenfin merged commit 1abff10 into gophercloud:main Sep 9, 2025
78 of 80 checks passed
@stephenfin stephenfin deleted the merge-queues branch September 9, 2025 16:38
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 9, 2025

Failed to backport PR to v2 branch. See logs for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kayrus kayrus kayrus approved these changes

@mandre mandre mandre left review comments

Assignees
No one assigned
Labels
backport-v2 This PR will be backported to v2 edit:actions This PR updates GitHub Actions code edit:testinfra This PR updates testing infrastructure code semver:patch No API change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stephenfin @coveralls @mandre @kayrus