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 deployment templates for Aurora Serverless v2 #3623

Conversation

tclark-innovim
Copy link
Contributor

@tclark-innovim tclark-innovim commented Apr 8, 2024

Summary: Summary of changes

Addresses CUMULUS-3669: Update deployment templates for Aurora Serverless v2

Changes

  • Update tf-modules/cumulus-rds-tf for serverless v2.
  • Update example/rds-cluster-tf for serverless v2.

PR Checklist

  • Update CHANGELOG
  • Unit tests
  • Ad-hoc testing - Deploy changes and test manually
  • Integration tests

@tclark-innovim tclark-innovim self-assigned this Apr 8, 2024
Copy link
Contributor

@npauzenga npauzenga left a comment

Choose a reason for hiding this comment

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

Looks good @tclark-innovim! I have a couple questions just for context. This will also take a bit of testing so apologies if there's some lag.

tf-modules/cumulus-rds-tf/main.tf Show resolved Hide resolved
example/rds-cluster-tf/variables.tf Show resolved Hide resolved
@@ -103,7 +113,7 @@ variable "lambda_timeouts" {
variable "parameter_group_family" {
description = "Database family to use for creating database parameter group"
type = string
default = "aurora-postgresql11"
default = "aurora-postgresql13"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have a separate parameter_group_family_v13 below? This change updates the default?

Copy link
Member

Choose a reason for hiding this comment

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

Yes - I believe this change is incorrect, this is an artifact of a mediocre upgrade path. This release should remove this parameter group, but once that is done will make it such that users coming from releases prior to the PG11->PG13 update cannot upgrade without hitting a version with this first.

If you're curious what I mean, take a look at the release notes for the PG upgrade and/or we can tag up on this.

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 agree that we no longer need "parameter_group_family" and "parameter_group_family_v13". Do you recommend removing "parameter_group_family" and leaving "parameter_group_family_v13" as part of this PR?

tf-modules/cumulus-rds-tf/outputs.tf Show resolved Hide resolved
tf-modules/cumulus-rds-tf/main.tf Show resolved Hide resolved
@Jkovarik
Copy link
Member

Taking a look 👀

@@ -6,6 +6,10 @@ output "rds_endpoint" {
value = module.rds_cluster.rds_endpoint
}

output "rds_reader_endpoint" {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might as well add this output now, we know we'll need it soon.

@@ -94,11 +95,11 @@ resource "aws_rds_cluster" "cumulus" {
preferred_backup_window = var.backup_window
db_subnet_group_name = aws_db_subnet_group.default.id
apply_immediately = var.apply_immediately
storage_encrypted = true
Copy link
Member

Choose a reason for hiding this comment

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

It's worth noting - this is required as the default changed from v1 to v2. 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, v2 default is storage_encrypted = false, so we need to add it.

tf-modules/cumulus-rds-tf/main.tf Show resolved Hide resolved
Copy link
Member

@Jkovarik Jkovarik left a comment

Choose a reason for hiding this comment

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

@tclark-innovim really great work! I tested deployment of a new cluster, and a few modifications thereof and this looks really good. That said, a couple of things:

I believe the following are needed prior to merging this:

  • Fix parameter group change noted in the PR
  • Discuss if this is merge-able without user documentation (team)
  • Demo discover-pdrs name collision #5/Start Using Lerna to organize packages #6 for the units/integration tests
  • Review test or demo that the module supports upgrading a cluster deployed with the prior version of the module.
  • Add basic upgrade instructions in the CHANGELOG upgrade section if we merge prior to user doc story

Things that are needed prior to closing the ticket:

  • Determine an upgrade path for our dev database cluster (if we merge it, we should be running on our module latest, in theory)
  • Author PR to update template deploy repo

@tclark-innovim tclark-innovim changed the base branch from master to feature/CUMULUS-3536-serverless-v2-epic April 18, 2024 13:58
Copy link
Member

@Jkovarik Jkovarik left a comment

Choose a reason for hiding this comment

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

Code changes look good, team discussed in the arch meeting re: deployment strategy and a feature branch addresses the other commentary. Nice work!

@tclark-innovim tclark-innovim merged commit 424989e into feature/CUMULUS-3536-serverless-v2-epic Apr 18, 2024
2 checks passed
tclark-innovim added a commit that referenced this pull request Sep 24, 2024
* Update deployment templates for Aurora Serverless v2 (#3623)

* update CL

* update terraform templates to serverless v2

* add terraform variable validation

* remove upgrade variables

* add prevent_destroy = true

* add prevent_destroy = true

* CUMULUS-3670 Develop upgrade/migration process Aurora Serverless v1 to v2 (#3643)

* remove prevent_destroy to allow automated CI migrations

* set force_ssl = 0 (#3658)

Co-authored-by: Tim Clark <tim.clark@nasa.gov>

* [CUMULUS-3671]: Update docs for Serverless V2 (#3666)

* initial commit

* serverless v2 doc updates

* Update serverless V2 docs

* Fix lint issue

* set DISABLE_PG_SSL: true to support CI

* fix lint error

* set disableSSL = true

* remove DISABLE_PG_SSL

* set rejectUnauthorized: 'false'

* update CL for v2 changes

* fix changelog

* add migration notes to changelog, add v2 docs to sidebar

* fix changelog

---------

Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>
etcart added a commit that referenced this pull request Sep 25, 2024
* Update deployment templates for Aurora Serverless v2 (#3623)

* update CL

* update terraform templates to serverless v2

* add terraform variable validation

* remove upgrade variables

* add prevent_destroy = true

* add prevent_destroy = true

* CUMULUS-3670 Develop upgrade/migration process Aurora Serverless v1 to v2 (#3643)

* remove prevent_destroy to allow automated CI migrations

* set force_ssl = 0 (#3658)

Co-authored-by: Tim Clark <tim.clark@nasa.gov>

* [CUMULUS-3671]: Update docs for Serverless V2 (#3666)

* initial commit

* serverless v2 doc updates

* Update serverless V2 docs

* Fix lint issue

* set DISABLE_PG_SSL: true to support CI

* fix lint error

* set disableSSL = true

* remove DISABLE_PG_SSL

* set rejectUnauthorized: 'false'

* update CL for v2 changes

* fix changelog

* add migration notes to changelog, add v2 docs to sidebar

* fix changelog

---------

Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>
paulpilone added a commit that referenced this pull request Oct 4, 2024
* CUMULUS-3536 Update Cumulus Core from Aurora Serverless V1 to V2 (#3632)

* Update deployment templates for Aurora Serverless v2 (#3623)

* update CL

* update terraform templates to serverless v2

* add terraform variable validation

* remove upgrade variables

* add prevent_destroy = true

* add prevent_destroy = true

* CUMULUS-3670 Develop upgrade/migration process Aurora Serverless v1 to v2 (#3643)

* remove prevent_destroy to allow automated CI migrations

* set force_ssl = 0 (#3658)

Co-authored-by: Tim Clark <tim.clark@nasa.gov>

* [CUMULUS-3671]: Update docs for Serverless V2 (#3666)

* initial commit

* serverless v2 doc updates

* Update serverless V2 docs

* Fix lint issue

* set DISABLE_PG_SSL: true to support CI

* fix lint error

* set disableSSL = true

* remove DISABLE_PG_SSL

* set rejectUnauthorized: 'false'

* update CL for v2 changes

* fix changelog

* add migration notes to changelog, add v2 docs to sidebar

* fix changelog

---------

Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>

* Ecarton/hailian fix sf event sqs to db records CUMULUS-3901 (#3799)

* Fix isThrottlingException function to check error name

* update changelog and add name/code check in errors

* linter fix

* changelog

* typo fix

---------

Co-authored-by: Hailiang Zhang <hailiang.zhang@nasa.gov>
Co-authored-by: etcart <amberhosen@gmail.com>

* reintroduce Migration Count Report to migrations with async_operations_table (#3805)

Co-authored-by: etcart <amberhosen@gmail.com>

* Updates CL 18.5.0 release date, removes 19.0, fixes some lint stuff

---------

Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>
Co-authored-by: cumulus-bot <141277837+cumulus-bot@users.noreply.github.com>
Co-authored-by: Hailiang Zhang <hailiang.zhang@nasa.gov>
Co-authored-by: Paul Pilone <paul@element84.com>
paulpilone added a commit that referenced this pull request Oct 4, 2024
….0 release (#3814)

* CUMULUS-3536 Update Cumulus Core from Aurora Serverless V1 to V2 (#3632)

* Update deployment templates for Aurora Serverless v2 (#3623)

* update CL

* update terraform templates to serverless v2

* add terraform variable validation

* remove upgrade variables

* add prevent_destroy = true

* add prevent_destroy = true

* CUMULUS-3670 Develop upgrade/migration process Aurora Serverless v1 to v2 (#3643)

* remove prevent_destroy to allow automated CI migrations

* set force_ssl = 0 (#3658)

Co-authored-by: Tim Clark <tim.clark@nasa.gov>

* [CUMULUS-3671]: Update docs for Serverless V2 (#3666)

* initial commit

* serverless v2 doc updates

* Update serverless V2 docs

* Fix lint issue

* set DISABLE_PG_SSL: true to support CI

* fix lint error

* set disableSSL = true

* remove DISABLE_PG_SSL

* set rejectUnauthorized: 'false'

* update CL for v2 changes

* fix changelog

* add migration notes to changelog, add v2 docs to sidebar

* fix changelog

---------

Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>

* Ecarton/hailian fix sf event sqs to db records CUMULUS-3901 (#3799)

* Fix isThrottlingException function to check error name

* update changelog and add name/code check in errors

* linter fix

* changelog

* typo fix

---------

Co-authored-by: Hailiang Zhang <hailiang.zhang@nasa.gov>
Co-authored-by: etcart <amberhosen@gmail.com>

* reintroduce Migration Count Report to migrations with async_operations_table (#3805)

Co-authored-by: etcart <amberhosen@gmail.com>

* Updates CL 18.5.0 release date, removes 19.0, fixes some lint stuff

* Bumps packages versions to 18.5.0

* Adds 18.5.0 docs to website

---------

Co-authored-by: etcart <amberhosen@gmail.com>
Co-authored-by: Tim Clark <tim.clark@nasa.gov>
Co-authored-by: Nate Pauzenga <npauzenga@gmail.com>
Co-authored-by: cumulus-bot <141277837+cumulus-bot@users.noreply.github.com>
Co-authored-by: Hailiang Zhang <hailiang.zhang@nasa.gov>
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