Add backup_config attribute to cockroach_cluster resource #242

fantapop · 2024-10-25T18:41:02Z

The ability to manage backup configurations for clusters is added here.

Note that this is an alternate approach to #241 . Here we add the backup_config as an attribute on the cluster resource instead of giving it a dedicated resource. This pattern is more in line with our other resources because conceptually, the backup configuration for a cluster exists whether or not it is being managed in terraform.

@kathancox I've copied most of the documentation over directly from the other PR that you reviewed. There were some changes in format because now that the backup config is attached to an attribute, there wasn't as much room to put the large block of text necessary for documenting the work arounds for setting retention days. Instead, I created a separate page as a "guide" for this purpose.

Commit checklist

This brings in sdk changes for controlling backup configuration.

kathancox

The markdown files lgtm! All comments non-blocking, optional nits.

kathancox · 2024-10-25T20:53:07Z

docs/data-sources/cluster.md

@@ -28,6 +28,7 @@ data "cockroach_cluster" "cockroach" {
 ### Read-Only

 - `account_id` (String) The cloud provider account ID that hosts the cluster. Needed for CMEK and other advanced features.
+- `backup_config` (Attributes) (see [below for nested schema](#nestedatt--backup_config))


Suggested change

- `backup_config` (Attributes) (see [below for nested schema](#nestedatt--backup_config))

- `backup_config` (Attributes) Refer to [Nested Schema for `backup_config`](#nestedatt--backup_config).

That change might be overly doc-sy.
Do you need the anchor link <a id="nestedatt--backup_config"></a>? Would it work to just use the heading as the link, so it would probably be (#nested-schema-for-backup_config). I'm assuming this works the same as other .md files, but maybe I'm wrong!

This is all generated automatically. We could customize it, but I'm unclear if we could do it in a way that wouldn't provide ongoing maintenance. It may be possible but It would need to be considered for a separate project and would need to be prioritized.

Got it, makes sense!

kathancox · 2024-10-25T20:56:05Z

docs/guides/updating-backup-retention.md

+
+Changing the value of `backup_config.retention_days` after using your one change
+will be a multi-step operation. Here are two workflows that will work.  Both of
+these options assume you already have modified retention_days once and as a


Suggested change

these options assume you already have modified retention_days once and as a

these options assume you already have modified `retention_days` once and as a

kathancox · 2024-10-25T20:56:18Z

docs/guides/updating-backup-retention.md

+these options assume you already have modified retention_days once and as a
+result must open a support ticket:
+
+* Change it and then open a ticket before applying:


Suggested change

* Change it and then open a ticket before applying:

* Change it, and then open a ticket before applying the change to Terraform. To do this:

Optional nit; just trying to connect the numbered steps to this sentence.

kathancox · 2024-10-25T20:56:41Z

docs/guides/updating-backup-retention.md

+	1. Update `backup_config.retention_days` to the new value in Terraform.
+	1. Before applying the run, contact support to change
+	   `backup_config.retention_days` to the new value.
+	1. Apply the changes in Terraform. A Terraform READ operation will complete,


Suggested change

1. Apply the changes in Terraform. A Terraform READ operation will complete,

1. Apply the changes in Terraform. A Terraform `READ` operation will complete,

kathancox · 2024-10-25T20:58:19Z

docs/guides/updating-backup-retention.md

+	1. Apply the changes in Terraform. A Terraform READ operation will complete,
+	   recognize the existing value, and update the tf state.
+* Temporarily remove management of the `backup_config.retention_days` from
+  Terraform, update it via ticket, and then add it back.


Suggested change

Terraform, update it via ticket, and then add it back.

Terraform, update it via ticket, and then add it back. To do this:

kathancox · 2024-10-25T21:02:49Z

CHANGELOG.md

+- management of cluster backup settings is now supported using the
+  `backup_config` attribute on the `cockroach_cluster` resource.  See
+  [Cluster Backups](https://www.cockroachlabs.com/docs/cockroachcloud/managed-backups)
+  for more information.


Suggested change

- management of cluster backup settings is now supported using the

`backup_config` attribute on the `cockroach_cluster` resource. See

[Cluster Backups](https://www.cockroachlabs.com/docs/cockroachcloud/managed-backups)

for more information.

- Management of cluster backup settings is now supported using the

`backup_config` attribute on the `cockroach_cluster` resource. For more information, refer to

[Cluster Backups](https://www.cockroachlabs.com/docs/cockroachcloud/managed-backups).

kathancox · 2024-10-25T21:03:41Z

templates/guides/updating-backup-retention.md

+these options assume you already have modified retention_days once and as a
+result must open a support ticket:
+
+* Change it and then open a ticket before applying:


Suggested change

* Change it and then open a ticket before applying:

* Change it, and then open a ticket before applying the change to Terraform. To do this:

kathancox · 2024-10-25T21:04:06Z

templates/guides/updating-backup-retention.md

+* Temporarily remove management of the `backup_config.retention_days` from
+  Terraform, update it via ticket, and then add it back.


Suggested change

* Temporarily remove management of the `backup_config.retention_days` from

Terraform, update it via ticket, and then add it back.

* Temporarily remove management of the `backup_config.retention_days` from

Terraform, update it via ticket, and then add it back. To do this:

kathancox · 2024-10-25T21:04:18Z

templates/guides/updating-backup-retention.md

+
+Changing the value of `backup_config.retention_days` after using your one change
+will be a multi-step operation. Here are two workflows that will work.  Both of
+these options assume you already have modified retention_days once and as a


Suggested change

these options assume you already have modified retention_days once and as a

these options assume you already have modified `retention_days` once and as a

kathancox · 2024-10-25T21:04:29Z

templates/guides/updating-backup-retention.md

+	1. Update `backup_config.retention_days` to the new value in Terraform.
+	1. Before applying the run, contact support to change
+	   `backup_config.retention_days` to the new value.
+	1. Apply the changes in Terraform. A Terraform READ operation will complete,


Suggested change

1. Apply the changes in Terraform. A Terraform READ operation will complete,

1. Apply the changes in Terraform. A Terraform `READ` operation will complete,

fantapop · 2024-10-28T16:42:30Z

Thanks for your attention on this @kathancox . I've made the requested changes here.

The ability to manage backup configurations for clusters is added here.

rgcase

LGTM looks great, I think this looks much better than having it as its own resource.

Update cockroach-cloud-sdk-go to version v4.1.0

11a9369

This brings in sdk changes for controlling backup configuration.

fantapop requested review from rgcase, kathancox and marksoper October 25, 2024 18:41

kathancox approved these changes Oct 25, 2024

View reviewed changes

fantapop force-pushed the fantapop/CC-27591-backup-configuration-in-cluster branch from 8b59700 to 29282c1 Compare October 28, 2024 16:41

Add backup_config attribute to cockroach_cluster resource

25b0dd4

The ability to manage backup configurations for clusters is added here.

fantapop force-pushed the fantapop/CC-27591-backup-configuration-in-cluster branch from 29282c1 to 25b0dd4 Compare October 28, 2024 17:25

rgcase approved these changes Oct 28, 2024

View reviewed changes

fantapop mentioned this pull request Oct 28, 2024

Add backup_config resource #241

Closed

5 tasks

fantapop merged commit 9cc0df1 into main Oct 28, 2024
4 checks passed

fantapop deleted the fantapop/CC-27591-backup-configuration-in-cluster branch October 28, 2024 22:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add backup_config attribute to cockroach_cluster resource #242

Add backup_config attribute to cockroach_cluster resource #242

fantapop commented Oct 25, 2024

kathancox left a comment

kathancox Oct 25, 2024 •

edited

Loading

fantapop Oct 28, 2024

kathancox Oct 28, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

kathancox Oct 25, 2024

fantapop commented Oct 28, 2024

rgcase left a comment

	- `backup_config` (Attributes) (see [below for nested schema](#nestedatt--backup_config))
	- `backup_config` (Attributes) Refer to [Nested Schema for `backup_config`](#nestedatt--backup_config).

	these options assume you already have modified retention_days once and as a
	these options assume you already have modified `retention_days` once and as a

	* Change it and then open a ticket before applying:
	* Change it, and then open a ticket before applying the change to Terraform. To do this:

	1. Apply the changes in Terraform. A Terraform READ operation will complete,
	1. Apply the changes in Terraform. A Terraform `READ` operation will complete,

	Terraform, update it via ticket, and then add it back.
	Terraform, update it via ticket, and then add it back. To do this:

		* Temporarily remove management of the `backup_config.retention_days` from
		Terraform, update it via ticket, and then add it back.

Add backup_config attribute to cockroach_cluster resource #242

Add backup_config attribute to cockroach_cluster resource #242

Conversation

fantapop commented Oct 25, 2024

kathancox left a comment

Choose a reason for hiding this comment

kathancox Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fantapop commented Oct 28, 2024

rgcase left a comment

Choose a reason for hiding this comment

kathancox Oct 25, 2024 •

edited

Loading