Skip to content

feat: major inplace upgrade, deletion protection #583

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

Merged
merged 6 commits into from
Jul 18, 2025
Merged

Conversation

shemau
Copy link
Contributor

@shemau shemau commented Jul 8, 2025

Description

Add support for two ICD features:

Deletion protection

This is feature flag, that when set to true provides protection from accidentally deleting the ICD instance.

Conceptually the instance is created with this flag set to true and if/when a destroy is attempted the request fails. In addition to requesting a destroy the flag must be set to false.

Note: This does not prevent the resource being destroyed outside of terraform.

In place major version upgrade

This feature allows the major version, eg. 6.0 to upgraded to 7.0 by an in-place upgrade, retaining the original instance connection endpoints. During a major version upgrade, the instance is set to read-only (setUserWriteBlock mode), upgraded and set to accept writes again. There is a service outage during this time, since the service will not accept database updates.

There is an feature flag, version_upgrade_skip_backup. It is NOT recommended to use this feature, since an upgrade failure may result in data loss. This flag avoids taking a backup whilst in read-only mode to speed the process up.

Full documentation at: https://cloud.ibm.com/docs/databases-for-mongodb?topic=databases-for-mongodb-upgrading&interface=ui

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@shemau shemau requested a review from rajatagarwal-ibm as a code owner July 8, 2025 11:52
@shemau
Copy link
Contributor Author

shemau commented Jul 8, 2025

deletion protection is set to false for all examples and pipeline testing, but true by default. This allows the pipelines to destroy any instances that they create without encountering an error. Since the feature MUST be overridden in order to perform a terraform destroy, it is exposed as a variable in all situations, module, sub-module and DAs.

The feature was manually tested, without setting the flag to false and results in

TestRunfullyConfigurableSolutionIBMKeys 2025-07-08T11:17:13+01:00 retry.go:99: Returning due to fatal error: FatalError
{
Underlying: error while running command: exit status 1; 
Error: Deletion protection is enabled for resource mongo-key-botkfq-mongodb to prevent accidential deletion

Set deletion_protection to false, apply and then destroy if deletion should
proceed
}  

@shemau
Copy link
Contributor Author

shemau commented Jul 8, 2025

/run pipeline

@shemau
Copy link
Contributor Author

shemau commented Jul 8, 2025

Note: There is no major version upgrade test included in the pipeline tests. The main reason for mongodb is that 6.0 is EOL this month and work is already started on removing support. At this point, the feature will be untestable.

Version upgrade testing has been completed manually.

@shemau
Copy link
Contributor Author

shemau commented Jul 8, 2025

Following the deep dive:

destroy process

Confirmed this is a two step process, 'apply' deletion protection = false, 'destroy'.

A single step update and destroy is not supported. Terraform uses the state file for a destroy, so it is necessary to apply the change first.

upgrade process

The upgrade process across all plans (validated on Postgres version 15, plan standard, deletion protection true. Attempted upgrade to version 16 and 17 (unlikely to be supported) and got

│ Error: ---
│ id: terraform-23dab597
│ summary: No available upgrade versions for version 15
│ severity: error
│ resource: ibm_database
│ operation: CustomizeDiff
│ component:
│   name: github.com/IBM-Cloud/terraform-provider-ibm
│   version: 1.79.2
│ ---
│ 
│ 
│   with module.database.ibm_database.postgresql_db,
│   on ../../main.tf line 165, in resource "ibm_database" "postgresql_db":
│  165: resource "ibm_database" "postgresql_db" {
│ 

So it appears to be impossible to launch a destroy and recreate accidental update if the plan does not support an in-place upgrade.

Read only replicas

Delete protection applies to read only replicas independently of the main instance. Delete protection must be removed from a replica before it can be destroyed.

@shemau
Copy link
Contributor Author

shemau commented Jul 9, 2025

/run pipeline

tags = var.resource_tags
member_host_flavor = "multitenant"
backup_crn = data.ibm_database_backups.backup_database.backups[0].backup_id
resource_group_id = module.resource_group.resource_group_id
Copy link
Contributor

Choose a reason for hiding this comment

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

could we sync the order between all ICD repos? it will be easier for maintenance when doing diff

for example postgresql, redis has different as well:

resource_group_id   
  name              
  postgresql_version 
  region         
  tags             
  access_tags    
  deletion_protection
  member_host_flavor 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is feeling like to much change for this cycle. The backup-restore folders do not even have the same names. I agree, the backup/restore and pitr/restore examples should be closer to identical, but can this wait for a less time sensitive PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, we can address that in different PR

@@ -4,5 +4,6 @@
"resource_tags": $TAGS,
"name": $PREFIX,
"existing_resource_group_name": "geretain-test-mongo",
"existing_kms_instance_crn": $HPCS_US_SOUTH_CRN
"existing_kms_instance_crn": $HPCS_US_SOUTH_CRN,
"deletion_protection": false
Copy link
Contributor

Choose a reason for hiding this comment

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

redis and postgresql doesn't have deletion_protection inside catalogvalidation, is this okey? fully-configurable also doesn't have it. is ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes committed to all repos and both files.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks

@shemau
Copy link
Contributor Author

shemau commented Jul 17, 2025

/run pipeline

@shemau
Copy link
Contributor Author

shemau commented Jul 17, 2025

/run pipeline

@ocofaigh ocofaigh merged commit a4b9efa into main Jul 18, 2025
2 checks passed
@ocofaigh ocofaigh deleted the major-upgrade branch July 18, 2025 13:30
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants