Add support for restoring AlloyDB clusters via backup restore and PITR by GoogleMarcfont · Pull Request #8575 · GoogleCloudPlatform/magic-modules

GoogleMarcfont · 2023-08-07T21:26:46Z

Add support for restoring AlloyDB clusters via backup restore and PITR.

To restore a cluster directly from a backup, add the following line to your AlloyDB cluster:

restore_backup_source {
  backup_name = <fully qualified backup name>
}

To restore a cluster via PITR, add the following lines to your AlloyDB cluster:

restore_continuous_backup_source {
  cluster = <fully qualified source cluster name>
  point_in_time = <timestamp in RFC 3339 format>
}

Part of hashicorp/terraform-provider-google#13757. I am not fully closing the issue because I did not yet add support for data calls for backups as requested.

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).
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).
Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

alloydb: added `restore_backup_source` and `restore_continuous_backup_source` fields to support restore feature in `google_alloydb_cluster` resource.

…c-modules into marcfont/cbr Merge with main

modular-magician · 2023-08-07T21:27:29Z

Hello! I am a robot. It looks like you are a community contributor. @NickElliot, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

GoogleMarcfont · 2023-08-07T21:37:32Z

Hi Nick, I added an example file I am not quite sure how to validate that everything is set up the example correctly. Do you know if there's a way to see the markdown output, i.e. https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/alloydb_cluster?

modular-magician · 2023-08-07T21:42:16Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 1385 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 1385 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 30 insertions(+))
TF OiCS: Diff ( 4 files changed, 165 insertions(+))

modular-magician · 2023-08-07T21:51:11Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 897 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 4 files changed, 897 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 30 insertions(+))
TF OiCS: Diff ( 4 files changed, 165 insertions(+))

modular-magician · 2023-08-07T23:12:08Z

Tests analytics

Total tests: 2936
Passed tests 2632
Skipped tests: 302
Affected tests: 2

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccAlloydbCluster_restore|TestAccAlloydbCluster_alloydbClusterRestoreExample

Get to know how VCR tests work

modular-magician · 2023-08-17T02:56:19Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccAlloydbCluster_restore[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

shuyama1 · 2023-08-17T14:24:04Z

+  - !ruby/object:Api::Type::String
+    name: 'restoreSourceBackup'
+    ignore_read: true
+    immutable: true
+    conflicts:
+      - restore_source_cluster
+      - restore_point_in_time
+    description: |
+      The name of the backup that this cluster should be restored from. Conflicts with 'restore_source_cluster' and 'restore_point_in_time', both can't be set together.
+  - !ruby/object:Api::Type::String
+    name: 'restoreSourceCluster'
+    ignore_read: true
+    immutable: true
+    required_with:
+      - restore_point_in_time
+    conflicts:
+      - restore_source_backup
+    description: |
+      The name of the source cluster that this cluster should be restored from. The source cluster must have continuous
+      backup enabled for restore to succeed. Requires `restore_point_in_time` to also be set.
+
+      Conflicts with 'restore_source_backup', both can't be set together.
+  - !ruby/object:Api::Type::String
+    name: 'restorePointInTime'
+    ignore_read: true
+    immutable: true
+    required_with:
+      - restore_source_cluster
+    conflicts:
+      - restore_source_backup
+    description: |
+      The point in time that this cluster should be restored to, in RFC 3339 format. Requires `restore_source_cluster` to also be set.
+
+      Conflicts with 'restore_source_backup', both can't be set together.


Thanks for the explanation. I'd suggest that we keep the same structure as the API, as I feel that it may be slightly easier to read. Also it may be easier to keep in sync if more fields are added in the future.

shuyama1 · 2023-08-17T14:24:38Z

+
+if restoreBackup != nil || restoreSourceCluster != nil {
+	// Use restore API if this is a restore instead of a create cluster call
+	url = strings.Replace(url, "clusters?clusterId", "clusters:restore?clusterId", 1)


Sorry I was not clear earlier. By "users can also specify cluster settings via restore", I mean if users specify any fields in the cluster object, what will it happen if it conflicts with the source it restores from. I'm not super familiar with the resource, and it seems that you've already answered the question in your earlier response that "cluster has no overlap with the source field". Then I think it should be fine.

GoogleMarcfont · 2023-08-17T22:07:49Z

Replying to #8575 (comment) -

I understand your question now. The source cluster and destination cluster do not impact each other. Whatever is set in the destination cluster is what will be applied, and the restore doesn't care at all about the source cluster configuration. The restore will not copy any state from the source cluster.

modular-magician · 2023-08-17T22:12:41Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field backup_source.backup_name changed from optional to required on google_alloydb_cluster - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 882 insertions(+), 46 deletions(-))
Terraform Beta: Diff ( 4 files changed, 882 insertions(+), 46 deletions(-))
TF Conversion: Diff ( 1 file changed, 69 insertions(+))

modular-magician · 2023-08-17T22:17:44Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field backup_source.backup_name changed from optional to required on google_alloydb_cluster - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 882 insertions(+), 46 deletions(-))
Terraform Beta: Diff ( 4 files changed, 882 insertions(+), 46 deletions(-))
TF Conversion: Diff ( 1 file changed, 69 insertions(+))

modular-magician · 2023-08-17T23:23:58Z

Tests analytics

Total tests: 2969
Passed tests 2666
Skipped tests: 302
Affected tests: 1

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccAlloydbCluster_restore

Get to know how VCR tests work

modular-magician · 2023-08-17T23:26:56Z

Tests analytics

Total tests: 2969
Passed tests 2664
Skipped tests: 302
Affected tests: 3

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccAlloydbCluster_restore|TestAccDataLossPreventionStoredInfoType_dlpStoredInfoTypeGroupIndexUpdate|TestAccDataLossPreventionJobTrigger_dlpJobTriggerInspectCustomInfoTypes

Get to know how VCR tests work

modular-magician · 2023-08-17T23:37:34Z

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccAlloydbCluster_restore[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

modular-magician · 2023-08-17T23:39:35Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataLossPreventionStoredInfoType_dlpStoredInfoTypeGroupIndexUpdate[Debug log]
TestAccDataLossPreventionJobTrigger_dlpJobTriggerInspectCustomInfoTypes[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccAlloydbCluster_restore[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

modular-magician · 2023-08-18T16:52:47Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 882 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 4 files changed, 882 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 69 insertions(+))

modular-magician · 2023-08-18T18:12:45Z

Tests analytics

Total tests: 2969
Passed tests 2666
Skipped tests: 302
Affected tests: 1

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccAlloydbCluster_restore

Get to know how VCR tests work

modular-magician · 2023-08-18T18:31:54Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccAlloydbCluster_restore[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

modular-magician · 2023-08-18T23:16:00Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 4 files changed, 882 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 4 files changed, 882 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 69 insertions(+))

modular-magician · 2023-08-19T00:28:16Z

Tests analytics

Total tests: 2980
Passed tests 2682
Skipped tests: 295
Affected tests: 3

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccDataplexDatascan_dataplexDatascanFullProfileExample|TestAccDataplexDatascan_dataplexDatascanBasicQualityExample|TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

modular-magician · 2023-08-19T00:34:36Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataplexDatascan_dataplexDatascanFullProfileExample[Debug log]
TestAccDataplexDatascan_dataplexDatascanBasicQualityExample[Debug log]
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

shuyama1

Thanks for adding thorough tests!

GoogleCloudPlatform#8575) Co-authored-by: Nick Elliot <nickelliot@google.com> Co-authored-by: Shuya Ma <87669292+shuyama1@users.noreply.github.com>

GoogleMarcfont added 21 commits July 11, 2023 16:42

Properly handle ABP midnight (hours = 0)

a705911

Add ExpectNonEmptyPlan to test

145121a

Add continuous backup config to alloydb cluster

01bdf49

Properly handle ABP midnight (hours = 0)

1698b7b

add more unit tests

a0a2a68

Merge branch 'marcfont/cbr' of https://github.com/GoogleMarcfont/magi…

cd32e8d

…c-modules into marcfont/cbr Merge with main

Adding more tests

e886c86

Merge branch 'main' into marcfont/cbr

50611a2

Fix cluster.yaml lint

22dcd5c

Add default value and update cmek test

6910f0f

Merge branch 'main' into marcfont/cbr

ce93ae5

Update test to use kms key bootstrapping util

9c2350c

condense update tests

95b98e6

Add restore fields to cluster

8c6cbba

ongoing

ef517d6

Restore API working

f37f890

make fields immutable and ignored on read

8c47937

Add required_with field

5fa7584

working E2E test for restoring backup and PITR

43565fd

Added tests for invalid use cases

a06326f

Added example for how to use cluster restore

3c49109

modular-magician requested a review from NickElliot August 7, 2023 21:27

Merge branch 'main' into marcfont/restore

a0a9c6e

fix bad merge

ed9730b

remove test comment in favor of inline commenting

81dd01f

shuyama1 requested changes Aug 17, 2023

View reviewed changes

Update cluster.yaml to use same structure as API

944b4bb

modular-magician requested a review from shuyama1 August 17, 2023 22:05

GoogleMarcfont closed this Aug 17, 2023

GoogleMarcfont reopened this Aug 17, 2023

rename backup and continuous backup source

f9a4379

shuyama1 reviewed Aug 18, 2023

View reviewed changes

Apply suggestions from code review

22d6b07

modular-magician requested a review from shuyama1 August 18, 2023 23:08

shuyama1 approved these changes Aug 21, 2023

View reviewed changes

shuyama1 merged commit 6dfa322 into GoogleCloudPlatform:main Aug 21, 2023

Uh oh!

Conversation

GoogleMarcfont commented Aug 7, 2023 • edited by shuyama1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modular-magician commented Aug 7, 2023

Uh oh!

GoogleMarcfont commented Aug 7, 2023

Uh oh!

modular-magician commented Aug 7, 2023

Diff report

Uh oh!

modular-magician commented Aug 7, 2023

Diff report

Uh oh!

modular-magician commented Aug 7, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 17, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

shuyama1 Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

shuyama1 Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GoogleMarcfont commented Aug 17, 2023

Uh oh!

modular-magician commented Aug 17, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 17, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 17, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 17, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 17, 2023

Uh oh!

modular-magician commented Aug 17, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

modular-magician commented Aug 18, 2023

Diff report

Uh oh!

modular-magician commented Aug 18, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 18, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

modular-magician commented Aug 18, 2023

Diff report

Uh oh!

modular-magician commented Aug 19, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 19, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

GoogleMarcfont commented Aug 7, 2023 •

edited by shuyama1

Loading