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

schnauzer: add a helper for ecr repos #1032

Merged
merged 2 commits into from
Aug 19, 2020
Merged

schnauzer: add a helper for ecr repos #1032

merged 2 commits into from
Aug 19, 2020

Conversation

webern
Copy link
Contributor

@webern webern commented Aug 12, 2020

Issue number:

N/A

Description of changes:

In order to expand Bottlerocket to certain AWS regions, we need to change the way the ECR URL is constructed. Initially we intended to use a single ECR registry ID. For some regions this will not work. So we added a helper function and a map to Schnauzer so that we can look up the registry ID from a map.

Additionally, this allows us to introduce the concept of a fallback ID/region pair. This will prove useful if a new region is added and, for example, an older version of Bottlerocket tries to run on it.

NOTE we need to update the migration versionized-directrory and the container tag versions that it hard-codes based on where things stand when we release these changes.

Testing done:

Ran an instance in me-south-1, the admin container worked, and I found these container URLs in the settings:

509306038620.dkr.ecr.me-south-1.amazonaws.com/bottlerocket-control:v0.4.1
509306038620.dkr.ecr.me-south-1.amazonaws.com/bottlerocket-admin:v0.5.2

Created a fake future version of Bottlerocket and ran an upgrade in us-west-2. Observed that the migrations ran and the URLs were still good.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
@iliana
Copy link
Contributor

iliana commented Aug 12, 2020

Can/should we combine this migration with #1030?

@webern
Copy link
Contributor Author

webern commented Aug 12, 2020

Can/should we combine this migration with #1030?

It's possible, but I'm not sure I'll be ready in time. I need to test this with confidence, which would be today (8/12) EOD/late at best.

@webern
Copy link
Contributor Author

webern commented Aug 12, 2020

force-pushed the webern:ecr-repos branch from 95538ce to 1961962

Fixed some wording, have not addressed PR comments yet.

@webern
Copy link
Contributor Author

webern commented Aug 12, 2020

force-pushed the webern:ecr-repos branch from 1961962 to 7b5acbf now

separates the refactors from the new code (two commits)

@webern
Copy link
Contributor Author

webern commented Aug 12, 2020

force-pushed the webern:ecr-repos branch from 7b5acbf to 3ee106c

Rebase develop with conflict resolution over the container version bumps.

@webern
Copy link
Contributor Author

webern commented Aug 12, 2020

webern force-pushed the webern:ecr-repos branch from 3ee106c to 356423d

Change design to return the whole fqdn instead of registry and region separately. Also addressed all other PR comments to-date.

sources/api/schnauzer/src/helpers.rs Outdated Show resolved Hide resolved
sources/api/schnauzer/src/helpers.rs Show resolved Hide resolved
@webern
Copy link
Contributor Author

webern commented Aug 13, 2020

webern force-pushed the webern:ecr-repos branch from 356423d to a2cba99
webern force-pushed the webern:ecr-repos branch from a2cba99 to dc87e69

Addresses @tjkirch comments: fixup refactor commit to remove unrelated errors, use 0.5.1 in the test string to make it more clear when this change took place.

@webern
Copy link
Contributor Author

webern commented Aug 18, 2020

webern force-pushed the webern:ecr-repos branch from dc87e69 to 92a8348

Rebase, resolve conflict for admin container version bump.

@webern
Copy link
Contributor Author

webern commented Aug 18, 2020

webern force-pushed the webern:ecr-repos branch from 92a8348 to 783374d

Missed a spot with the admin container bump.

Release.toml Outdated Show resolved Hide resolved
@webern
Copy link
Contributor Author

webern commented Aug 18, 2020

webern force-pushed the webern:ecr-repos branch from 783374d to b0944cf

Bump the container version in a comment that was missed.

@webern
Copy link
Contributor Author

webern commented Aug 18, 2020

Finished testing, exiting draft status...

@webern webern marked this pull request as ready for review August 18, 2020 22:59
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

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

🐳

@webern webern merged commit 4caf5a6 into bottlerocket-os:develop Aug 19, 2020
@webern webern deleted the ecr-repos branch January 13, 2021 19:47
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.

5 participants