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 support for Filestore deletion protection #3183

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

tpdownes
Copy link
Member

@tpdownes tpdownes commented Oct 29, 2024

Filestore deletion protection ensures that instances are not unintentionally deleted. A typical lifecycle for a user will look like:

  1. Deploy a blueprint with deletion protection enabled
  2. Disable deletion protection in blueprint
  3. Redeploy blueprint
  4. Destroy deployment

In particular, enabling Filestore deletion protection does not prevent Terraform from destroying other resources. So a gcluster destroy command will destroy all resources except the Filestore and its dependencies. It will prevent modifications to a Filestore instance that require a destroy/create cycle.

We cannot yet add this to an integration test because we do not support tests that intentionally fail to destroy. Manual testing is described below:

  1. Add extra settings to examples/hpc-slurm.yaml and deploy:
  - id: homefs
    source: modules/file-system/filestore
    use: [network]
    settings:
      deletion_protection:
        enabled: true
        reason: Avoid data loss
      filestore_tier: ZONAL
      local_mount: /home
      size_gb: 1024
  1. Remove the extra settings and attempt to deploy. It proposes the plan you'd expect (replace the Filestore due to change of tier):
-/+ resource "google_filestore_instance" "filestore_instance" {
      ~ create_time                 = "2024-11-12T22:03:13.633956077Z" -> (known after apply)
      ~ deletion_protection_enabled = true -> false
      - deletion_protection_reason  = "Avoid data loss" -> null
      + etag                        = (known after apply)
      ~ id                          = "projects/toolkit-demo-zero-e913/locations/us-central1-a/instances/hpc-slurm-f5d5ec9b" -> (known after apply)
        name                        = "hpc-slurm-f5d5ec9b"
      ~ tier                        = "ZONAL" -> "BASIC_HDD" # forces replacement
      + zone                        = (known after apply)
        # (6 unchanged attributes hidden)

      ~ networks {
          ~ ip_addresses      = [
              - "10.103.225.2",
            ] -> (known after apply)
          + reserved_ip_range = (known after apply)
            # (3 unchanged attributes hidden)
        }

        # (1 unchanged block hidden)
    }

This fails with an error message:

Error: exit status 1

Error: Error when reading or editing Instance: googleapi: Error 400: instance "projects/toolkit-demo-zero-e913/locations/us-central1-a/instances/hpc-
slurm-f5d5ec9b" is protected from deletion: Avoid data loss

It also prints an unhelpful stacktrace that I have reported at hashicorp/terraform-provider-google#20311

If I then attempt to deliberately destroy the deployment with gcluster destroy hpc-slurm, it will progress through the destruction of non-Filestore infrastructure but leave the Filestore and its dependencies in place.

Summary of proposed changes: Plan: 0 to add, 0 to change, 46 to destroy.
...
Error: exit status 1

Error: Error when reading or editing Instance: googleapi: Error 400: instance "projects/toolkit-demo-zero-e913/locations/us-central1-a/instances/hpc-
slurm-f5d5ec9b" is protected from deletion: Avoid data loss

If I redeploy with a new blueprint that disables deletion protection:

module.homefs.google_filestore_instance.filestore_instance: Modifying... [id=projects/toolkit-demo-zero-e913/locations/us-central1-a/instances/hpc-slurm-f5d5ec9b]
...
module.homefs.google_filestore_instance.filestore_instance: Modifications complete after 11s [id=projects/toolkit-demo-zero-e913/locations/us-central1-a/instances/hpc-slurm-f5d5ec9b]

I can then run gcluster destroy hpc-slurm without difficulty.

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes
  • Ensure that unit test coverage remains above 80%
  • Update all applicable documentation
  • Follow Cluster Toolkit Contribution guidelines #

@tpdownes tpdownes self-assigned this Oct 29, 2024
@tpdownes tpdownes force-pushed the filestore_deletion branch 3 times, most recently from 091321e to 8261e06 Compare November 4, 2024 21:22
@tpdownes tpdownes force-pushed the filestore_deletion branch 3 times, most recently from 14a01a6 to 5326601 Compare November 11, 2024 17:04
@tpdownes tpdownes added the release-key-new-features Added to release notes under the "Key New Features" heading. label Nov 11, 2024
Filestore deletion protection ensures that instances are not
unintentionally deleted. A typical lifecycle for a user will look like:

1. Deploy a blueprint with deletion protection enabled
2. Disable deletion protection in blueprint
3. Redeploy blueprint
4. Destroy deployment

In particular, enabling Filestore deletion protection does not prevent
Terraform from destroying other resources. So a `gcluster destroy`
command will destroy all resources except the Filestore and its
dependencies.
@tpdownes tpdownes marked this pull request as ready for review November 12, 2024 22:21
Co-authored-by: Rachael Tamakloe <63983467+RachaelSTamakloe@users.noreply.github.com>
@tpdownes tpdownes enabled auto-merge November 13, 2024 04:29
@tpdownes tpdownes merged commit 5c357d4 into GoogleCloudPlatform:develop Nov 13, 2024
8 of 56 checks passed
@tpdownes tpdownes deleted the filestore_deletion branch November 15, 2024 22:57
@rohitramu rohitramu mentioned this pull request Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-key-new-features Added to release notes under the "Key New Features" heading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants