fix: disable HA and PITR on target instance during migration #294
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DMS migration fails when the source instance has High Availability or Point-in-Time Recovery enabled, because these settings are copied to the target via DeepCopy() in DefineInstance(). The target is then demoted to a read replica by DMS, which is incompatible with HA and PITR. Changes: - Strip HA and PITR from helper app spec in CreateInstance() - Disable PITR and set availability to ZONAL in PrepareTargetInstance() - Add warnings in ValidateSourceInstance() about temporarily disabled features After promotion, HA and PITR are automatically re-enabled when UpdateApplicationInstance() applies the original app spec (which still has these settings) via naiserator reconciliation. Fixes #240
Instead of relying on implicit re-enabling through naiserator reconciliation, explicitly restore AvailabilityType, backup and PITR settings from the source instance onto the target CNRM spec in UpdateTargetInstanceAfterPromotion(). This makes the restore visible in logs and less fragile.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Migration fails when the source Cloud SQL instance has any of these features enabled (#240):
Root cause
DefineInstance()usesDeepCopy()to copy the source instance spec to the target. This copies all settings, including HA and PITR. The target instance is then created with these settings via the helper app.During setup, DMS calls
demoteTargetInstance()to convert the target to a read replica. This fails because:Fix
Three changes in
instance.go:CreateInstance()– After callingDefineInstance(), strip HA and PITR from the helper app spec before creating the target instance. This prevents GCP from creating an instance with incompatible settings.PrepareTargetInstance()– Also explicitly disable PITR (pointInTimeRecoveryEnabled = false) and setavailabilityType = ZONALon the CNRM spec as a safety net.ValidateSourceInstance()– Log clear warnings when the source has HA or PITR enabled, explaining they will be temporarily disabled during migration.Re-enabling after migration
After promotion,
UpdateApplicationInstance()applies the original app spec (which still has HA/PITR enabled) via naiserator reconciliation, so these features are automatically re-enabled on the target instance.What about audit logging?
Issue #240 also mentions audit logging. The
cloudsql.enable_pgauditdatabase flag would also be copied viaDeepCopy(), but it is less clear whether this actually breaks DMS migration. This may need further investigation/testing.Fixes #240
/cc @mortenlj