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 migration to remove region on downgrade #643

Merged
merged 1 commit into from
Feb 1, 2020
Merged

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Jan 13, 2020

Older versions don't know about this setting and would fail to deserialize the
model if it's present. (We currently need to write a migration for any
added/removed settings, but this will be automated away.)

Region is a generated setting, so we're not throwing away anything important.


Fixes #642.
Builds on #638 and #644.

Testing done:

Saved contents to make sure we write everything out:

$ find datastore-sample/current/ > v0.1-contents

Migrate to v0.2. We can see the migration running and saying it has nothing to do.

$ cargo run -- --log-level trace --datastore-path datastore-sample/current --migration-directories ./migration-binaries --migrate-to-version 0.2
...
21:41:19 [DEBUG] (1) migrator: Sorted migrations: ["migrate_v0.2_remove-region"]
21:41:19 [ INFO] New data store is being built at work location /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.2_Oq7CIMsaiEfnCNL3
21:41:19 [ INFO] Running migration command: "./migration-binaries/migrate_v0.2_remove-region" "--forward" "--source-datastore" "/home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.1_AmdROiQykcX0vQRy" "--target-datastore" "/home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.2_Oq7CIMsaiEfnCNL3"
21:41:19 [DEBUG] (1) migrator: Migration stdout: RemoveRegionMigration has no work to do on upgrade.
RemoveRegionMigration has no work to do on upgrade.

21:41:19 [DEBUG] (1) migrator: Migration stderr: 
21:41:19 [ INFO] Flipping /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.2 to point to v0.2_Oq7CIMsaiEfnCNL3
21:41:19 [ INFO] Flipping /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0 to point to v0.2

When v0.2 (of aws-k8s flavor) runs, it will set settings.aws.region:

$ mkdir datastore-sample/current/live/settings/aws && echo -n '"us-west-2"' > datastore-sample/current/live/settings/aws/region

Downgrade back to v0.1. We can see the migration running and removing the region. (It removes it from live settings; it wasn't set in pending, hence the second message.)

$ cargo run -- --log-level trace --datastore-path datastore-sample/current --migration-directories ./migration-binaries --migrate-to-version 0.1
...
21:42:30 [DEBUG] (1) migrator: Sorted migrations: ["migrate_v0.2_remove-region"]
21:42:30 [ INFO] New data store is being built at work location /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.1_kZNHRvDkARjcHinw
21:42:30 [ INFO] Running migration command: "./migration-binaries/migrate_v0.2_remove-region" "--backward" "--source-datastore" "/home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.2_Oq7CIMsaiEfnCNL3" "--target-datastore" "/home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.1_kZNHRvDkARjcHinw"
21:42:30 [DEBUG] (1) migrator: Migration stdout: Removed settings.aws.region, which was set to '"us-west-2"'
Found no settings.aws.region to remove

21:42:30 [DEBUG] (1) migrator: Migration stderr: 
21:42:30 [ INFO] Flipping /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0.1 to point to v0.1_kZNHRvDkARjcHinw
21:42:30 [ INFO] Flipping /home/ANT.AMAZON.COM/tjk/work/thar/workspaces/api/migration/migrator/datastore-sample/v0 to point to v0.1

Region was removed:

$ ll datastore-sample/current/live/settings/aws/region
ls: cannot access 'datastore-sample/current/live/settings/aws/region': No such file or directory

...and so the file list is the same as before:

$ find datastore-sample/current/ > v0.1-contents-after
$ diff v0.1-contents{,-after}

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 13, 2020

This push rebases on #644 which only had an updated commit message.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 14, 2020

Per the notes in #644, I need to modify this to actually remove the file from the filesystem, since the migrator in old versions (before #644 is merged) doesn't allow removal through the standard interface.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 21, 2020

This push has no change in the implementation, it just rebases on the change from #644 that allows this migration to work even when downgrading to versions before the migrator interface change.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 23, 2020

This push just rebases on the changes from #644.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 28, 2020

This push is just a rebase onto the changes from #637.

@tjkirch tjkirch requested review from zmrow and removed request for bcressey and zmrow January 28, 2020 17:29
Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🔎

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 28, 2020

This push just rebases on a typo fix from #636.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 29, 2020

This push is just a rebase onto changes from #637.

@tjkirch
Copy link
Contributor Author

tjkirch commented Jan 31, 2020

Just a rebase onto changes from #644.

@tjkirch
Copy link
Contributor Author

tjkirch commented Feb 1, 2020

This push was just a rebase onto changes from #644.

Older versions don't know about this setting and would fail to deserialize the
model if it's present.  (We currently need to write a migration for any
added/removed settings, but this will be automated away.)

Region is a generated setting, so we're not throwing away anything important.
@tjkirch
Copy link
Contributor Author

tjkirch commented Feb 1, 2020

Rebase on develop; removed first-party exception from cargo-deny, and added migration to workspaces Cargo.toml and RELEASE.toml. (Conflict was with another migration added for 0.2.)

@tjkirch tjkirch changed the base branch from migrator-remove-setting to develop February 1, 2020 00:30
@tjkirch tjkirch merged commit 08fb4f1 into develop Feb 1, 2020
@tjkirch tjkirch deleted the region-migration branch February 1, 2020 00:36
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.

Migration for region setting
3 participants