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

Bump minimum Go version to v1.22, rework CI matrix #667

Merged
merged 6 commits into from
Nov 5, 2024

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Nov 3, 2024

This bumps our CI to run the 4 most recent Postgres versions (v17, v16, v15, v14) against with the latest Go version (v1.23), and to run the previous Go release (v1.22) against the newest Postgres version. In doing so, we achieve better coverage on Postgres versions without having a bloated CI matrix of all possible combinations.

It also bumps our minimum Go version from v1.21 to v1.22. This continues our target of supporting the 2 most recent major Go versions, and expands CI to ensure we work on the 4 most recent PG versions.

@bgentry bgentry requested a review from brandur November 3, 2024 20:10
@mfridman
Copy link

mfridman commented Nov 4, 2024

Thanks for maintaining the version updates! I'm curious about the decision to drop postgres 14 support - is this driven by specific newer features you need, or a general policy of supporting the most recent 3 major versions?

Postges 14 is still in its supported lifecycle (roughly midway through) and remains a commonly deployed version across major cloud providers, which typically lag behind latest releases.

@bgentry
Copy link
Contributor Author

bgentry commented Nov 4, 2024

@mfridman There's no actual changes being made which will affect support for older Postgres versions and I don't believe any are coming soon. Honestly it's mainly a question of keeping the CI matrix in check. I should update the PR and description to make it clear this isn't actually dropping support for any Postgres versions, just changing CI.

In the case of Go, there are actually some things from newer Go versions that we'd like to start depending on soon.

@brandur
Copy link
Contributor

brandur commented Nov 4, 2024

Pulling PG 15 out of CI is effectively dropping support for it because you won't notice if you start using a new feature that 15 doesn't support, but 16+ does. Unlike Go, people still find it really hard to keep up-to-date on the latest versions of Postgres, so to keep more of them supported, it probably behooves us to keep running against Postgres versions we want to support.

To keep the build sustainable, I wonder if what we do is switch the build to use an N + M model instead of an N * M model. So you run all versions of supported Postgres on the latest Go version (1.23 currently), then all supported Go versions against Postgres 17.

Instead of having [1.21,1.22,1.23] * [14, 15, 16, 17] = 12 build jobs, you get [1.21, 1.22, 1.12] + [14, 15, 16, 17] = 7 jobs, which seems more manageable.

@bgentry
Copy link
Contributor Author

bgentry commented Nov 4, 2024

@brandur Yeah, that makes a lot of sense. It's pretty unlikely that we'll encounter a bug that only exists on a specific Go/Postgres version combo anyway. We may need to restructure the CI slightly to make this easy but I'm onboard with that.

@mfridman
Copy link

mfridman commented Nov 4, 2024

@brandur summarized it quite nicely. My main concern about dropping versions from CI was around keeping the project honest about the intended supported postgres versions. It's quite easy to pull in a new feature not to realize it means bumping the minimum version.

I was going to suggest pretty much the same approach, reducing the matrix from all Go/postgres versions toward an additive testing matrix.

ps. No issues around Go versions, mainly just the postgres ones ✌️.

@mfridman
Copy link

mfridman commented Nov 4, 2024

I see in the docs you do specify:

River is tested using the three most recent major versions of PostgreSQL.

So my comment may be moot in terms of keeping postgres 14 around in CI. I suppose it is up to the individual projects to test River to ensure compatibility with their minimum postgres version policy.

If River introduces features that depend on newer postgres features, downstream projects have options:

  1. Upgrade their postgres version requirement (often challenging, because it may be a dependency outside the project control)
  2. Pin to an earlier River version that matches their postgres constraints
  3. Skip/conditionally use features that require newer postgres versions

I appreciate this is the nature of software and support ya'll focusing River's testing on postgres versions that align with your maintenance goals.

@bgentry bgentry force-pushed the bg-bump-minimum-go-and-postgres-versions branch from ee57388 to 50d3080 Compare November 4, 2024 23:58
@bgentry bgentry changed the title Bump minimum Go version to v1.22, Postgres version to v15 Bump minimum Go version to v1.22, rework CI matrix Nov 4, 2024
@bgentry
Copy link
Contributor Author

bgentry commented Nov 5, 2024

This was my first time setting up an Actions matrix config that explicitly listed the chosen combinations: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow#expanding-or-adding-matrix-configurations

No other changes required 🙌

Copy link
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

Great! lgtm.

@bgentry bgentry merged commit cef2245 into master Nov 5, 2024
10 checks passed
@bgentry bgentry deleted the bg-bump-minimum-go-and-postgres-versions branch November 5, 2024 03:16
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
* make tidy

* run CI with the 4 most recent Postgres versions (v17, v16, v15, v14)
  against with the latest Go version (v1.23), and run the previous Go
  release (v1.22) against the newest Postgres version.

* use Go v1.22 or higher

* fix update-mod-go tests

* rework CI matrix to run 4 PG versions on latest Go, plus one prior Go release w/ latest PG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants