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

Add docs for managed backup Terraform provider attribute #19182

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

kathancox
Copy link
Contributor

@kathancox kathancox commented Nov 26, 2024

Fixes DOC-11779

This PR adds a section to the Standard and Advanced Managed backup pages on using the backup_config attribute under the cockroach_cluster resource for the CC Terraform Provider.

Also, added a short supplement to the TF Configuration step on the general Cloud Terraform docs to point out that users can configure managed backups using TF and linking to the full details on the Managed Backups page.

Rendered Preview

See the standard and advanced tabs for both pages.

Provision a cluster with Terraform
Managed Backups

Copy link

github-actions bot commented Nov 26, 2024

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 1cc7960
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/6751d58e7de7b9000876e2f5

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 1cc7960
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/6751d58e31e23f0008288f00

Copy link

netlify bot commented Nov 26, 2024

Netlify Preview

Name Link
🔨 Latest commit 1cc7960
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/6751d58ef2146c0008071ae2
😎 Deploy Preview https://deploy-preview-19182--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kathancox kathancox force-pushed the cc-terraform-managed-backups branch 2 times, most recently from 5146f74 to 9c022bd Compare November 26, 2024 20:16
@kathancox kathancox marked this pull request as ready for review November 26, 2024 20:30
Copy link

@alicia-l2 alicia-l2 left a comment

Choose a reason for hiding this comment

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

lgtm! had one question. thank you!

- If the initial value of the `retention_days` attribute is the default value `30`, you'll be able to modify the backup retention setting once more.
- If the initial value is not the default, you will not be able to modify `retention_days` again. You can refrain from including `retention_days` in the Terraform configuration and instead manage the retention in the Cloud Console.

For more details on modifying `retention_days` more than once, refer to the [Updating Backup Retention](https://github.com/cockroachdb/terraform-provider-cockroach/blob/main/docs/guides/updating-backup-retention.md) documentation in the Terraform provider for CockroachDB {{ site.data.products.cloud }} Repository.

Choose a reason for hiding this comment

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

should this just be to reach out to support since we dont let you do it on cloud api anyway?

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 kept this sentence so users can find the terraform provider docs, and also added the link to contact support for modifying the setting again. Hope that's good.

@kathancox
Copy link
Contributor Author

@fantapop Would appreciate your review of this content too, thanks so much!

@kathancox kathancox requested a review from fantapop December 3, 2024 19:05
Copy link

@fantapop fantapop left a comment

Choose a reason for hiding this comment

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

nice. Looks good to me.

@kathancox kathancox requested a review from rmloveland December 3, 2024 21:47
Copy link
Contributor

@rmloveland rmloveland left a comment

Choose a reason for hiding this comment

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

LGTM had some non-blocking things, take or leave

@@ -0,0 +1,9 @@
Set the following for the `backup_config` attribute:

- `enabled` controls whether managed backups are enabled or disabled. If you modify the `retention_days` setting even when managed backups are disabled, this will use the one possible change for `retention_days`. Possible values for the `enabled` setting are: `true` or `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

if by "one possible change" you are referring to the other callout where we say you can modify the retention policy only once, is it possible to make this a link to that info?

i see it's in an include but perhaps it could be done with an anchor tag just above the include

if it's too hard because each include is moving around i totally get it

however i think "the one possible change" as standalone text may be too subtle, i'm not a careful enough reader of docs when I use a thing, I only noticed because i just read the other thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, so I linked the one possible change to the general section on Retention at the top of the page, because this details the "one possible change".

@kathancox kathancox force-pushed the cc-terraform-managed-backups branch from 09d5db1 to 1cc7960 Compare December 5, 2024 16:32
@kathancox
Copy link
Contributor Author

TFTRs!!

@kathancox kathancox merged commit d6527b3 into main Dec 5, 2024
6 checks passed
@kathancox kathancox deleted the cc-terraform-managed-backups branch December 5, 2024 16:42
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.

4 participants