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

Fix upgrade paths for different postgres versions #321

Merged
merged 2 commits into from
May 20, 2024
Merged

Conversation

nb1701
Copy link
Contributor

@nb1701 nb1701 commented May 20, 2024

It would seem that RDS will let 12.17 ONLY upgrade to 16.1, and 12.18 ONLY upgrade to 16.2, not 16.1. This means we have to maintain a table of valid upgrade paths here until everyone has made it onto 16, at which point RDS will upgrade minor versions automatically.

This changes the deploy logic, making it a bit less generic in order to handle this additional complexity of specific major.minor valid upgrade paths.

Checklist

General

  • Added the correct label
  • Assigned to a specific person or civiform/deployment-system
  • Performed manual testing (at a minimum run bin/setup without your changes and then bin/deploy with your changes to ensure your changes don't break existing deployments)

Instructions for manual testing

You can just deploy, say, 1.62.0 which is on 12, which should set you up with 12.18, and verify this tool upgrades you to 16.2. The failure we noted before was someone on 12.17, so to do this, you would need to edit main.tf to deploy 12.17 specifically when you're on something like 1.62.0, then perform the upgrade.

Issue(s) this completes

civiform/civiform#7516

@nb1701 nb1701 added the bug Something isn't working label May 20, 2024
@nb1701 nb1701 requested review from a team and rockycodes and removed request for a team May 20, 2024 22:14
@nb1701
Copy link
Contributor Author

nb1701 commented May 20, 2024

Tested 12.18 -> 16.2. Tested all the error paths by messing with the versions in the code directly to trigger them.

@nb1701 nb1701 requested a review from gwendolyngoetz May 20, 2024 22:17
It would seem that RDS will let 12.17 ONLY upgrade to 16.1, and 12.18
ONLY upgrade to 16.2, not 16.1. This means we have to maintain a table
of valid upgrade paths here until everyone has made it onto 16, at which
point RDS will upgrade minor versions automatically.

This changes the deploy logic, making it a bit less generic in order to
handle this additional complexity of specific major.minor valid upgrade
paths.
@nb1701 nb1701 force-pushed the nb1701/pg_upgrade_fix branch from a48ba91 to f00715c Compare May 20, 2024 22:18
print(
f'{Color.YELLOW}This version of the deployment tool does not have information about if {current_major}.{current_minor} is sufficiently new enough to upgrade to version {to_apply}. Check https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.PostgreSQL.html and verify this is a valid upgrade path.{Color.END}'
f'{Color.YELLOW}This version of the deployment tool does not have information about if {current_version} is sufficiently new enough to upgrade to the latest minor version of version PostgreSQL {major_to_apply}. Check https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.PostgreSQL.html and verify this is a valid upgrade path.{Color.END}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
f'{Color.YELLOW}This version of the deployment tool does not have information about if {current_version} is sufficiently new enough to upgrade to the latest minor version of version PostgreSQL {major_to_apply}. Check https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.PostgreSQL.html and verify this is a valid upgrade path.{Color.END}'
f'{Color.YELLOW}The CiviForm deployment tool does not yet know if {current_version} is sufficiently new enough to upgrade to the latest minor release of PostgreSQL version {major_to_apply}. Check https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.PostgreSQL.html and verify this is a valid upgrade path.{Color.END}'

This was hard to read with the multiple ambiguous use of the work version. Does this still mean what you wanted?

Copy link
Contributor Author

@nb1701 nb1701 May 20, 2024

Choose a reason for hiding this comment

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

Yeah, I was struggling with the wording there. See if this makes more sense.
This version of the CiviForm deployment tool has no information about your current PostgreSQL version of {current_version} and thus cannot choose the correct version to upgrade to. If you proceed, we will attempt to upgrade to the latest {major_to_apply}.x version, but this may not succeed. Check https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.PostgreSQL.html to verify that this is a valid upgrade path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also avoided telling them they could set export POSTGRESQL_VERSION to a specific version to get past this, because if they're seeing it, they're probably doing something wrong, and leaving that in your config file is a recipe for future pain.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like your new copy much more. Ship it!

@nb1701 nb1701 requested a review from gwendolyngoetz May 20, 2024 23:12
@nb1701 nb1701 merged commit 49a1bd2 into main May 20, 2024
5 checks passed
@nb1701 nb1701 deleted the nb1701/pg_upgrade_fix branch May 20, 2024 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants