magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Adding support for major version upgrade in cloud sql instance.

Open usingh83 opened this issue 1 year ago • 22 comments

If this PR is for Terraform, I acknowledge that I have:

  • [X] Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • [X] Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
  • [X] Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • [X] Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • [X] Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

sql: added support for major version upgrade to `google_cloud_sql` resource

usingh83 avatar Aug 04 '22 05:08 usingh83

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Aug 04 '22 05:08 google-cla[bot]

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @melinath, a repository maintainer, has been assigned to assist you and help review your changes.

:question: First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

  • Ensuring it's backwards compatible, covers common error cases, etc.
  • Summarizing the change into a user-facing changelog note.
  • Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.


modular-magician avatar Aug 04 '22 05:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 45 insertions(+), 24 deletions(-)) Terraform Beta: Diff ( 1 file changed, 45 insertions(+), 24 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 04 '22 05:08 modular-magician

Tests analytics

Total tests: 2126 Passed tests 1879 Skipped tests: 226 Failed tests: 21

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccSqlDatabaseInstance_basic_with_user_labels|TestAccSqlDatabaseInstance_authNets|TestAccSqlDatabaseInstance_PointInTimeRecoveryEnabled|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_settings_upgrade|TestAccSqlDatabaseInstance_basicMSSQL|TestAccSqlDatabaseInstance_settings_deletionProtection|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample|TestAccActiveDirectoryDomain_update|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician avatar Aug 04 '22 05:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 46 insertions(+), 24 deletions(-)) Terraform Beta: Diff ( 1 file changed, 46 insertions(+), 24 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 04 '22 06:08 modular-magician

Tests analytics

Total tests: 2126 Passed tests 1880 Skipped tests: 226 Failed tests: 20

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccSqlDatabaseInstance_settings_upgrade|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_settings_deletionProtection|TestAccSqlDatabaseInstance_basicMSSQL|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeInstance_networkPerformanceConfig|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample|TestAccActiveDirectoryDomain_update|TestAccSqlDatabaseInstance_basic_with_user_labels|TestAccSqlDatabaseInstance_PointInTimeRecoveryEnabled|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_authNets|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician avatar Aug 04 '22 07:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 28 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 1 file changed, 28 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 04 '22 07:08 modular-magician

Tests analytics

Total tests: 2126 Passed tests 1878 Skipped tests: 226 Failed tests: 22

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccSqlDatabaseInstance_authNets|TestAccSqlDatabaseInstance_settings_upgrade|TestAccSqlDatabaseInstance_basic_with_user_labels|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_settings_deletionProtection|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccSqlDatabaseInstance_basicMSSQL|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccActiveDirectoryDomain_update|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample|TestAccSqlUser_mysqlDisabled|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_PointInTimeRecoveryEnabled

modular-magician avatar Aug 04 '22 08:08 modular-magician

Tests passed during RECORDING mode: TestAccFirebaserulesRelease_BasicRelease[view] TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view] TestAccCGCSnippet_eventarcWorkflowsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[view] TestAccCloudFunctions2Function_fullUpdate[view] TestAccSqlDatabaseInstance_basicMSSQL[view] TestAccSqlUser_mysqlDisabled[view] TestAccSqlDatabaseInstance_settings_deletionProtection[view] TestAccSqlDatabaseInstance_authNets[view] TestAccSqlDatabaseInstance_basic_with_user_labels[view] TestAccSqlDatabaseInstance_PointInTimeRecoveryEnabled[view] TestAccSqlDatabaseInstance_settings_upgrade[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeClone[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeReplica[view]

Tests failed during RECORDING mode: TestAccComputeInstance_networkPerformanceConfig[view] TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample[view] TestAccComputeGlobalForwardingRule_internalLoadBalancing[view] TestAccComputeInstance_soleTenantNodeAffinities[view] TestAccCloudRunService_cloudRunServiceStaticOutboundExample[view] TestAccSqlDatabaseInstance_SqlServerAuditConfig[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[view] TestAccActiveDirectoryDomain_update[view] TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample[view]

Please fix these to complete your PR View the build log or the debug log for each test

modular-magician avatar Aug 04 '22 09:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 28 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 1 file changed, 28 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 04 '22 11:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 27 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 1 file changed, 27 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 04 '22 11:08 modular-magician

Tests analytics

Total tests: 2126 Passed tests 1884 Skipped tests: 226 Failed tests: 16

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudFunctions2Function_fullUpdate|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryDomain_update|TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample|TestAccSqlUser_mysqlDisabled|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange

modular-magician avatar Aug 04 '22 12:08 modular-magician

Tests passed during RECORDING mode: TestAccFirebaserulesRelease_BasicRelease[view] TestAccCGCSnippet_eventarcWorkflowsExample[view] TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[view] TestAccCloudFunctions2Function_fullUpdate[view] TestAccSqlUser_mysqlDisabled[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeClone[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeReplica[view]

Tests failed during RECORDING mode: TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample[view] TestAccComputeGlobalForwardingRule_internalLoadBalancing[view] TestAccComputeInstance_networkPerformanceConfig[view] TestAccComputeInstance_soleTenantNodeAffinities[view] TestAccCloudRunService_cloudRunServiceStaticOutboundExample[view] TestAccSqlDatabaseInstance_SqlServerAuditConfig[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[view] TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample[view] TestAccActiveDirectoryDomain_update[view]

Please fix these to complete your PR View the build log or the debug log for each test

modular-magician avatar Aug 04 '22 13:08 modular-magician

It looks like you are adding support for a new field called "database_version", which would presumably allow users to specify a specific major version of the database to use?

If that's right, could you do the following:

  • update the release note to reflect that you're adding a new field
  • add this field to an existing test if possible, or add a new test that includes this field

Hi Stephen, Thanks for reviewing, that's not quite right. The database_version field is already there, but currently updating it will force a destroy and create in terraform. I have added the logic to handle the database_version updates in the resource update function, so that it never forces a new resource when updated.

usingh83 avatar Aug 05 '22 03:08 usingh83

This LGTM in terms of implementation - could you add comments explaining the implementation details as described above?

Also, please add a test that updates this field (or modify an existing test to update this field.)

Added test for Updation of database version

usingh83 avatar Aug 09 '22 20:08 usingh83

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 09 '22 21:08 modular-magician

Tests analytics

Total tests: 2129 Passed tests 1892 Skipped tests: 227 Failed tests: 10

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_SqlServerAuditConfig

modular-magician avatar Aug 09 '22 22:08 modular-magician

Tests passed during RECORDING mode: TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[view] TestAccCloudFunctions2Function_fullUpdate[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeClone[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeReplica[view]

Tests failed during RECORDING mode: TestAccComputeInstance_networkPerformanceConfig[view] TestAccComputeGlobalForwardingRule_internalLoadBalancing[view] TestAccComputeInstance_soleTenantNodeAffinities[view] TestAccCloudRunService_cloudRunServiceStaticOutboundExample[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[view] TestAccSqlDatabaseInstance_SqlServerAuditConfig[view]

Please fix these to complete your PR View the build log or the debug log for each test

modular-magician avatar Aug 09 '22 23:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 12 '22 07:08 modular-magician

one small change to make this more readable; otherwise seems probably good but I'll need to figure out why the new test isn't running in CI.

Hi Stephen, it seems the new test is running fine, here are the logs i found for my test in the replays https://pantheon.corp.google.com/storage/browser/_details/ci-vcr-logs/beta/refs/heads/auto-pr-6374/artifacts/d0e86738-fc00-4ae0-8b64-2781ac4a7c1d/replaying/TestAccSqlDatabaseInstance_mysqlMajorVersionUpgrade.log;tab=live_object

usingh83 avatar Aug 12 '22 07:08 usingh83

Tests analytics

Total tests: 2130 Passed tests 1892 Skipped tests: 227 Failed tests: 11

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccFirebaserulesRelease_BasicRelease|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician avatar Aug 12 '22 08:08 modular-magician

Tests passed during RECORDING mode: TestAccFirebaserulesRelease_BasicRelease[view] TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[view] TestAccCloudFunctions2Function_fullUpdate[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeClone[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeReplica[view] TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view]

Tests failed during RECORDING mode: TestAccComputeGlobalForwardingRule_internalLoadBalancing[view] TestAccComputeInstance_networkPerformanceConfig[view] TestAccComputeInstance_soleTenantNodeAffinities[view] TestAccCloudRunService_cloudRunServiceStaticOutboundExample[view] TestAccSqlDatabaseInstance_SqlServerAuditConfig[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[view]

Please fix these to complete your PR View the build log or the debug log for each test

modular-magician avatar Aug 12 '22 09:08 modular-magician

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) Terraform Beta: Diff ( 2 files changed, 72 insertions(+), 5 deletions(-)) TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar Aug 16 '22 19:08 modular-magician

Tests analytics

Total tests: 2135 Passed tests 1895 Skipped tests: 227 Failed tests: 13

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests
TestAccSqlUser_mysqlDisabled|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccComputeInstance_networkPerformanceConfig|TestAccFirebaserulesRelease_BasicRelease|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudFunctions2Function_fullUpdate|TestAccComputeInstance_soleTenantNodeAffinities|TestAccDataSourceGoogleServiceAccountIdToken_basic

modular-magician avatar Aug 16 '22 19:08 modular-magician

Tests passed during RECORDING mode: TestAccSqlUser_mysqlDisabled[view] TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeClone[view] TestAccFirebaserulesRelease_BasicRelease[view] TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[view] TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[view] TestAccCloudFunctions2Function_fullUpdate[view] TestAccDataSourceGoogleServiceAccountIdToken_basic[view]

Tests failed during RECORDING mode: TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[view] TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRangeReplica[view] TestAccSqlDatabaseInstance_SqlServerAuditConfig[view] TestAccComputeInstance_networkPerformanceConfig[view] TestAccCloudRunService_cloudRunServiceStaticOutboundExample[view] TestAccComputeGlobalForwardingRule_internalLoadBalancing[view] TestAccComputeInstance_soleTenantNodeAffinities[view]

Please fix these to complete your PR View the build log or the debug log for each test

modular-magician avatar Aug 16 '22 21:08 modular-magician

manual run of TestAccSqlDatabaseInstance_mysqlMajorVersionUpgrade: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloudBeta_ProviderGoogleCloudBetaMmUpstream/327532

melinath avatar Aug 18 '22 16:08 melinath