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

migrations: host-containers default version migration redux and new common migration 'ReplaceTemplateMigration' #717

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Feb 8, 2020

Issue #, if available: Partially addresses #685

Description of changes:
Bumps the default version of the host container images.
Adds new common migration helper ReplaceTemplateMigration.
Adds new migration binary for bumping the default host containers version from v0.2 to v0.3.

Testing done:
Forward migration:

Updating template and value of 'settings.host-containers.admin.source' on upgrade

Changing template of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-admin:v0.2' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-admin:v0.3'

Changing value of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.2' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-admin:v0.3'

Updating template and value of 'settings.host-containers.control.source' on upgrade

Changing template of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-control:v0.2' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-control:v0.3'

Changing value of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-control:v0.2' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-control:v0.3'

Backward migration:

Changing template of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-admin:v0.3' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-admin:v0.2'

Changing value of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-admin:v0.3' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.2'

Updating template and value of 'settings.host-containers.control.source' on downgrade

Changing template of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-control:v0.3' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-control:v0.2'

Changing value of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-control:v0.3' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-control:v0.2'

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@etungsten etungsten requested review from tjkirch and zmrow February 8, 2020 01:04
workspaces/Cargo.toml Outdated Show resolved Hide resolved
@etungsten
Copy link
Contributor Author

@tjkirch Now that the host-containers source is templated. What do you think is the best way to account for other possible regions on backwards migration? (Currently the images are only available in us-west-2)

@tjkirch
Copy link
Contributor

tjkirch commented Feb 8, 2020

@etungsten Unfortunately we can't just check the version/suffix because customers could use the same versioning. We need to make sure the source account and version match, at least. One way would be a pattern match that reuses the source region. A simpler way would be confirming the source account matches, then just replacing the suffix. (We have all the data, so we could regenerate from the pattern using schnauzer, but that seems a bit much and assume some things...)

@tjkirch
Copy link
Contributor

tjkirch commented Feb 8, 2020

@etungsten Actually, using schnauzer may not be such a bad idea. Through the MigrationData, we already have all of the data and metadata we would need to generate the old value (to check for a match) and the new value, without needing the API. We've rebooted if we're running migrations, so we know we have the region setting. Worth a try?

Something like:

const NEW_TEMPLATE = "<same as existing template but with new version>";
if let Some(existing_source) = input.data.get("settings.host-containers.control.source") {
  if let Some(metadata) = input.metadata.get("settings.host-containers.control.source") {
    if let Some(template) = metadata.get("template") {
      let registry = schnauzer::build_template_registry();
      let old = registry.render_template(template, input.data)?;
      if old == existing_source {
        let new = registry.render_template(NEW_TEMPLATE, input.data)?;
        println!("{}", new);
      } // elses to handle missing data
...

@iliana iliana mentioned this pull request Feb 11, 2020
10 tasks
@etungsten etungsten force-pushed the host-container-version-migrate-again branch from 164022a to 0b1f7ab Compare February 12, 2020 00:40
@etungsten
Copy link
Contributor Author

etungsten commented Feb 12, 2020

Adds new common migration helper ReplaceTemplateMigration. Thanks @tjkirch, for all the help with this!
Renames host-containers migrations binary.

@etungsten etungsten force-pushed the host-container-version-migrate-again branch 2 times, most recently from 904d323 to 8c1c601 Compare February 12, 2020 00:47
@tjkirch tjkirch mentioned this pull request Feb 12, 2020
@etungsten etungsten marked this pull request as ready for review February 12, 2020 00:53
@etungsten etungsten changed the title migrations: host-containers default version migration redux migrations: host-containers default version migration redux and new common migration 'ReplaceTemplateMigration' Feb 12, 2020
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.

🍔

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Side note: whether we include the concrete migration for v0.3 depends on out-of-band discussions today regarding the breaking-ness of our next release. (If it's a full break release with no upgrade, we won't need the migration.) We still want the common migration code because we'll need it soon regardless.

@etungsten etungsten force-pushed the host-container-version-migrate-again branch from 8c1c601 to deea57e Compare February 12, 2020 22:26
@etungsten
Copy link
Contributor Author

etungsten commented Feb 12, 2020

Addresses @tjkirch 's comments.

  • Created new helper function update_template_and_data

Tested and things still work.

@etungsten etungsten force-pushed the host-container-version-migrate-again branch from deea57e to 261b934 Compare February 12, 2020 23:53
@etungsten
Copy link
Contributor Author

Drop dependency on simplelog

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Would you please confirm/update the testing?

@etungsten etungsten force-pushed the host-container-version-migrate-again branch from 261b934 to daf8dc1 Compare February 13, 2020 01:30
@etungsten
Copy link
Contributor Author

etungsten commented Feb 13, 2020

Addresses @tjkirch 's comments.

Tested new changes and things still work as expected:
Forward

Updating template and value of 'settings.host-containers.admin.source' on upgrade

Changing template of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-admin:v0.3' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-admin:v0.2'

Changing value of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-admin:v0.3' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.2'

Updating template and value of 'settings.host-containers.control.source' on upgrade

Changing template of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-control:v0.3' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-control:v0.2'

Changing value of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-control:v0.3' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-control:v0.2'

Backward:

Updating 'settings.host-containers.admin.source''s template and value on downgrade

Changing template of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-admin:v0.2' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-admin:v0.3'

Changing value of 'settings.host-containers.admin.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.2' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-admin:v0.3'

Updating 'settings.host-containers.control.source''s template and value on downgrade

Changing template of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/thar-control:v0.2' to '328549459982.dkr.ecr.{{ settings.aws.region }}.amazonaws.com/bottlerocket-control:v0.3'

Changing value of 'settings.host-containers.control.source' from '328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-control:v0.2' to '328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-control:v0.3'

@etungsten etungsten force-pushed the host-container-version-migrate-again branch from daf8dc1 to 599f0d8 Compare February 13, 2020 02:07
@tjkirch
Copy link
Contributor

tjkirch commented Feb 13, 2020

It looks like the forward/backward testing sections are backwards? And the messages aren't symmetrical (forward was updated but not backward, or vice versa, not sure because the header seems backwards), can we fix that?

[edit] Ah, I see, the force-push after the testing fixed that message. Would be nice to keep the testing in the description up to date, if it's changed, and just confirm in update-related comments that it was retested.

@etungsten etungsten force-pushed the host-container-version-migrate-again branch from 599f0d8 to 02a951a Compare February 13, 2020 18:11
@etungsten
Copy link
Contributor Author

Addressed @tjkirch 's comments.
Updated PR description with new testing results

Adds new common migration helper 'ReplaceTemplateMigration' that updates
a setting's template and then use that template to generate new data
which is then set.
Adds new migration binary for bumping the default host containers
version from v0.2 to v0.3.
v0.3 contains IMDSv2 support and API transaction support.
The migration also includes a name change for the host-container images
@etungsten etungsten force-pushed the host-container-version-migrate-again branch from 02a951a to 463222f Compare February 13, 2020 22:50
@etungsten
Copy link
Contributor Author

Force push above rebases onto develop

Bumps the default versions of admin and control container to version 0.3
@etungsten etungsten requested a review from tjkirch February 13, 2020 22:52
@etungsten etungsten merged commit 8363987 into develop Feb 13, 2020
@etungsten etungsten deleted the host-container-version-migrate-again branch February 13, 2020 23:10
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.

3 participants