Support Self Service Maintenance feature #6131

2tricpony · 2022-06-14T21:36:16Z

Support Self Service Maintenance feature. This adds support for 2 API fields - maintenance_version and available_maintenance_version. Fixes hashicorp/terraform-provider-google#11881

maintenance_version feature summary:

can't be set during creation (API will return error if it's set during creation)
can only be set to versions in available_maintenance_version
versions set to an older one than the current one will be ignored
versions set to invalid newer versions will run into API errors

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

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).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
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).
[] Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

sql: added `maintenance_version` and `available_maintenance_versions` fields to `google_sql_database_instance` resource

modular-magician · 2022-06-14T21:36:30Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

sql: added maintenance_version and 'available_maintenance_versions' fields in google_sql_database_instance

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2022-06-14T21:37:13Z

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

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

❓ 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 · 2022-06-14T21:48:51Z

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

Diff report:

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

modular-magician · 2022-06-14T22:35:40Z

Tests analytics

Total tests: 2052
Passed tests 1823
Skipped tests: 226
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withAddons|TestAccSqlUser_mysqlDisabled

modular-magician · 2022-06-15T00:00:25Z

Tests passed during RECORDING mode:
TestAccSqlUser_mysqlDisabled[view]

Tests failed during RECORDING mode:
TestAccContainerCluster_withAddons[view]
TestAccContainerCluster_withConfidentialNodes[view]

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

shuyama1 · 2022-06-15T20:38:50Z

Question: Is available_maintenance_versions an output-only field while maintenance_version is not?
Just want to make sure if I understand correctly. Many thanks!

2tricpony · 2022-06-20T16:48:50Z

That is correct! availabe_maintenance_versions can never be updated. maintenance_version field can be updated to any of versions specified in the available_maintenance_versions field.

shuyama1

Would you mind adding a test or modifying an existing test for these two added fields?

shuyama1 · 2022-06-21T14:23:47Z

mmv1/products/sql/api.yaml

@@ -68,6 +68,22 @@ objects:
          - :FIRST_GEN
          - :SECOND_GEN
          - :EXTERNAL
+      - !ruby/object:Api::Type::String
+        name: 'maintenanceVersion'
+        output: true


Sounds like maintenanceVersion is not an output-only field.

That's right. It can be updated after instance creation.

shuyama1 · 2022-06-21T14:30:41Z

mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb

+                        "available_maintenance_versions": {
+                                Type:        schema.TypeList,
+                                Computed:    true,
+                                Optional:    true,


If this is an output-only field, we probably want to remove Optional: true.

modular-magician · 2022-07-19T22:03:51Z

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

Diff report:

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

modular-magician · 2022-07-19T22:45:42Z

Tests analytics

Total tests: 2112
Passed tests 1879
Skipped tests: 226
Failed tests: 7

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccPrivatecaCertificateAuthority_subordinateCaActivatedByFirstPartyIssuerOnCreation|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccSqlDatabaseInstance_maintenanceVersionWithOldVersion_suppressConfig|TestAccSqlDatabaseInstance_basicMSSQL|TestAccSqlDatabaseInstance_SqlServerAuditConfig

modular-magician · 2022-07-19T23:18:56Z

Tests passed during RECORDING mode:
TestAccSqlDatabaseInstance_basicMSSQL[view]
TestAccSqlDatabaseInstance_SqlServerAuditConfig[view]
TestAccSqlDatabaseInstance_maintenanceVersionWithOldVersion_suppressConfig[view]
TestAccFirebaserulesRelease_BasicRelease[view]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[view]
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[view]
TestAccPrivatecaCertificateAuthority_subordinateCaActivatedByFirstPartyIssuerOnCreation[view]

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

2tricpony · 2022-07-20T17:10:46Z

Hi shuyama1,

I have addressed the changes that were requested. Please take another look.

shuyama1 · 2022-07-26T16:41:22Z

@2tricpony Thanks for making the update! Sorry for the delay. I'll start reviewing the PR.

2tricpony · 2022-08-08T17:24:33Z

Gentle ping on this. :)

shuyama1 · 2022-08-09T16:52:45Z

@2tricpony Sorry, this fell off my radar. Thanks for the ping!

shuyama1

Based on the debug log, it seems that the maintenance_version is not included in the request body even if we config it explicitly. Should it throw error if we set this field in the creation step (as we discussed offline)?
Also would you mind adding an update step for this feature? I understand that the it can not really update the resource(as the newly created instance should have the most recent version if I understand correctly), but we can just add it to check if it can be sent correctly via the API call. Many thanks! Let me know if you got any questions.

mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb

mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown

mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown

…e.html.markdown Co-authored-by: Shuya Ma <[email protected]>

modular-magician · 2022-09-20T00:58:40Z

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]
TestAccSqlDatabaseInstance_maintenanceVersion[Error message] [Debug log]

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

shuyama1 · 2022-09-20T03:15:16Z

mmv1/third_party/terraform/tests/resource_sql_database_instance_test.go.erb

+			resource.TestStep{
+				Config: fmt.Sprintf(
+					testGoogleSqlDatabaseInstance_maintenanceVersionWithOldVersion, databaseName),
+			},
+			resource.TestStep{
+				ResourceName:      "google_sql_database_instance.instance",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},


I believe this is a copy error and should be removed?

2tricpony · 2022-09-21T16:41:03Z

Added the suppressDiffFunc back.

modular-magician · 2022-09-21T16:45:34Z

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

Diff report:

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

modular-magician · 2022-09-21T17:45:04Z

Tests analytics

Total tests: 2177
Passed tests 1936
Skipped tests: 238
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample

modular-magician · 2022-09-21T17:50:24Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

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

shuyama1 · 2022-09-26T15:20:58Z

mmv1/third_party/terraform/resources/resource_sql_database_instance.go.erb

@@ -1428,6 +1477,16 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{})
 	return resourceSqlDatabaseInstanceRead(d, meta)
 }

+func maintenanceVersionDiffSuppress(_, old, new string, _ *schema.ResourceData) bool {
+	// Ignore the database version part and only compare the last part of the maintenance version which represents the release date of the version.
+	if len(old) > 14 && len(new) > 14 && old[len(old)-14:] > new[len(new)-14:] {


Question: Do you think if we should suppress when old[len(old)-14:] = new[len(new)-14:]?

Good question. If the version is same, it's a no-op on the backend. Having said that, I think we can also suppress on Terraform side when it's equal. I have added that logic.

modular-magician · 2022-09-26T17:32:14Z

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

Diff report:

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

modular-magician · 2022-09-26T18:25:16Z

Tests analytics

Total tests: 2182
Passed tests 1942
Skipped tests: 238
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample

modular-magician · 2022-09-26T18:27:13Z

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

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

shuyama1

Tested it locally and LGTM in general! Just some nitpicks on documentation. Also please rebase the PR if conflicts occur. Thank you!

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown

2tricpony · 2022-09-26T23:37:43Z

Thank you for verifying! I have updated the PR and I didn't find any conflicts.

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown

…e.html.markdown Co-authored-by: Shuya Ma <[email protected]>

modular-magician · 2022-09-26T23:51:54Z

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

Diff report:

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

modular-magician · 2022-09-27T01:02:37Z

Tests analytics

Total tests: 2182
Passed tests 1926
Skipped tests: 238
Failed tests: 18

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccComputeVpnTunnel_regionFromGateway|TestAccComputePacketMirroring_computePacketMirroringFullExample|TestAccComputeRoute_routeIlbVipExample|TestAccComputeRoute_routeIlbExample|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeGlobalForwardingRule_labels|TestAccComputeGlobalForwardingRule_ipv6|TestAccComputeGlobalForwardingRule_updateTarget|TestAccComputeGlobalForwardingRule_privateServiceConnectGoogleApisExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleHybridExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeForwardingRule_serviceDirectoryRegistrations|TestAccComputeForwardingRule_update|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample

modular-magician · 2022-09-27T01:11:17Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentBasicExample[Debug log]
TestAccComputeVpnTunnel_regionFromGateway[Debug log]
TestAccComputePacketMirroring_computePacketMirroringFullExample[Debug log]
TestAccComputeRoute_routeIlbVipExample[Debug log]
TestAccComputeRoute_routeIlbExample[Debug log]
TestAccComputeGlobalForwardingRule_internalLoadBalancing[Debug log]
TestAccComputeGlobalForwardingRule_labels[Debug log]
TestAccComputeGlobalForwardingRule_ipv6[Debug log]
TestAccComputeGlobalForwardingRule_updateTarget[Debug log]
TestAccComputeGlobalForwardingRule_privateServiceConnectGoogleApisExample[Debug log]
TestAccComputeGlobalForwardingRule_globalForwardingRuleHybridExample[Debug log]
TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample[Debug log]
TestAccComputeForwardingRule_serviceDirectoryRegistrations[Debug log]
TestAccComputeForwardingRule_update[Debug log]
TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

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

2tricpony and others added 3 commits June 14, 2022 13:31

Changes for SSM terraform

2f21578

Change description of some fields.

cb08b34

Merge branch 'GoogleCloudPlatform:main' into main

6e0a876

modular-magician requested a review from shuyama1 June 14, 2022 21:37

shuyama1 requested changes Jun 21, 2022

View reviewed changes

modular-magician requested a review from shuyama1 July 14, 2022 22:54

shuyama1 removed their request for review July 15, 2022 04:26

modular-magician requested a review from shuyama1 July 15, 2022 05:01

shuyama1 removed their request for review July 15, 2022 05:03

2tricpony added 2 commits July 19, 2022 14:46

Add a test case for maintenance version suppress diff functionality

f890112

Add test case for suppress diff functionality

a036a42

modular-magician requested a review from shuyama1 July 19, 2022 21:47

Make availableMaintenanceVersions optional=false

d15bed2

shuyama1 requested changes Aug 9, 2022

View reviewed changes

2tricpony and others added 2 commits August 9, 2022 10:05

Update mmv1/third_party/terraform/website/docs/r/sql_database_instanc…

d126eb5

…e.html.markdown Co-authored-by: Shuya Ma <[email protected]>

Update mmv1/third_party/terraform/website/docs/r/sql_database_instanc…

b5f5b45

…e.html.markdown Co-authored-by: Shuya Ma <[email protected]>

shuyama1 reviewed Sep 20, 2022

View reviewed changes

Rollforward changes for suppressDiffFunc

d77cb6f

modular-magician requested a review from shuyama1 September 21, 2022 16:34

shuyama1 reviewed Sep 26, 2022

View reviewed changes

Fix to suppress diff for equal maintenance version.

81ca557

modular-magician requested a review from shuyama1 September 26, 2022 17:19

shuyama1 approved these changes Sep 26, 2022

View reviewed changes

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown Outdated Show resolved Hide resolved

2tricpony and others added 2 commits September 26, 2022 16:35

Doc changes

3e313ed

Merge branch 'GoogleCloudPlatform:main' into main

294bad0

modular-magician requested a review from shuyama1 September 26, 2022 23:36

shuyama1 reviewed Sep 26, 2022

View reviewed changes

mmv1/third_party/terraform/website/docs/r/sql_database_instance.html.markdown Outdated Show resolved Hide resolved

Update mmv1/third_party/terraform/website/docs/r/sql_database_instanc…

767fef9

…e.html.markdown Co-authored-by: Shuya Ma <[email protected]>

modular-magician requested a review from shuyama1 September 26, 2022 23:39

shuyama1 approved these changes Sep 27, 2022

View reviewed changes

shuyama1 merged commit e9ec4dc into GoogleCloudPlatform:main Sep 27, 2022

Support Self Service Maintenance feature #6131

Support Self Service Maintenance feature #6131

Conversation

2tricpony commented Jun 14, 2022 • edited by shuyama1 Loading

modular-magician commented Jun 14, 2022

modular-magician commented Jun 14, 2022

modular-magician commented Jun 14, 2022

Diff report:

modular-magician commented Jun 14, 2022

Tests analytics

Action taken

modular-magician commented Jun 15, 2022

shuyama1 commented Jun 15, 2022

2tricpony commented Jun 20, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

shuyama1 Jun 21, 2022

Choose a reason for hiding this comment

2tricpony Jul 19, 2022

Choose a reason for hiding this comment

shuyama1 Jun 21, 2022

Choose a reason for hiding this comment

2tricpony Jul 19, 2022

Choose a reason for hiding this comment

modular-magician commented Jul 19, 2022

Diff report:

modular-magician commented Jul 19, 2022

Tests analytics

Action taken

modular-magician commented Jul 19, 2022

2tricpony commented Jul 20, 2022

shuyama1 commented Jul 26, 2022

2tricpony commented Aug 8, 2022

shuyama1 commented Aug 9, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

modular-magician commented Sep 20, 2022

shuyama1 Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

2tricpony commented Sep 21, 2022

modular-magician commented Sep 21, 2022

Diff report:

modular-magician commented Sep 21, 2022

Tests analytics

Action taken

modular-magician commented Sep 21, 2022

shuyama1 Sep 26, 2022

Choose a reason for hiding this comment

2tricpony Sep 26, 2022

Choose a reason for hiding this comment

modular-magician commented Sep 26, 2022

Diff report:

modular-magician commented Sep 26, 2022

Tests analytics

Action taken

modular-magician commented Sep 26, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

2tricpony commented Sep 26, 2022

modular-magician commented Sep 26, 2022

Diff report:

modular-magician commented Sep 27, 2022

Tests analytics

Action taken

modular-magician commented Sep 27, 2022

2tricpony commented Jun 14, 2022 •

edited by shuyama1

Loading

shuyama1 Sep 20, 2022 •

edited

Loading