-
Notifications
You must be signed in to change notification settings - Fork 519
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
vmware-variants: migrate host-container versions to latest versions #1898
Merged
etungsten
merged 1 commit into
bottlerocket-os:develop
from
etungsten:vmware-migrations
Jan 19, 2022
+136
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains 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
13 changes: 13 additions & 0 deletions
13
sources/api/migration/migrations/v1.6.0/vmware-host-containers/Cargo.toml
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
[package] | ||
name = "vmware-host-containers" | ||
version = "0.1.0" | ||
authors = ["Erikson Tung <[email protected]>"] | ||
license = "Apache-2.0 OR MIT" | ||
edition = "2021" | ||
publish = false | ||
# Don't rebuild crate just because of changes to README. | ||
exclude = ["README.md"] | ||
|
||
[dependencies] | ||
serde_json = "1" | ||
migration-helpers = { path = "../../../migration-helpers", version = "0.1.0"} |
111 changes: 111 additions & 0 deletions
111
sources/api/migration/migrations/v1.6.0/vmware-host-containers/src/main.rs
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
#![deny(rust_2018_idioms)] | ||
|
||
use migration_helpers::{migrate, Migration, MigrationData, Result}; | ||
use std::process; | ||
|
||
const ADMIN_CONTAINER_SOURCE_SETTING_NAME: &str = "settings.host-containers.admin.source"; | ||
const ADMIN_CONTAINER_IMAGE_REPOSITORY: &str = "public.ecr.aws/bottlerocket/bottlerocket-admin"; | ||
const PREVIOUS_ADMIN_CONTAINER_VERSIONS: &[&str] = &["v0.7.0", "v0.7.1", "v0.7.2"]; | ||
const TARGET_ADMIN_CONTAINER_VERSION: &str = "v0.7.3"; | ||
|
||
const CONTROL_CONTAINER_SOURCE_SETTING_NAME: &str = "settings.host-containers.control.source"; | ||
const CONTROL_CONTAINER_IMAGE_REPOSITORY: &str = "public.ecr.aws/bottlerocket/bottlerocket-control"; | ||
const PREVIOUS_CONTROL_CONTAINER_VERSIONS: &[&str] = &["v0.5.0", "v0.5.1", "v0.5.2", "v0.5.3"]; | ||
const TARGET_CONTROL_CONTAINER_VERSION: &str = "v0.5.4"; | ||
|
||
pub struct VmwareHostContainerVersions; | ||
|
||
impl Migration for VmwareHostContainerVersions { | ||
fn forward(&mut self, mut input: MigrationData) -> Result<MigrationData> { | ||
// For admin container | ||
if let Some(data) = input.data.get_mut(ADMIN_CONTAINER_SOURCE_SETTING_NAME) { | ||
match data { | ||
serde_json::Value::String(source) => { | ||
for ver in PREVIOUS_ADMIN_CONTAINER_VERSIONS { | ||
let prev_source = format!("{}:{}", ADMIN_CONTAINER_IMAGE_REPOSITORY, ver); | ||
if *source == prev_source { | ||
*source = format!( | ||
"{}:{}", | ||
ADMIN_CONTAINER_IMAGE_REPOSITORY, TARGET_ADMIN_CONTAINER_VERSION | ||
); | ||
println!( | ||
"Changed value of '{}' from '{}' to '{}' on upgrade", | ||
ADMIN_CONTAINER_SOURCE_SETTING_NAME, prev_source, source | ||
); | ||
break; | ||
} | ||
} | ||
} | ||
_ => { | ||
println!( | ||
"'{}' is set to non-string value '{}'", | ||
ADMIN_CONTAINER_SOURCE_SETTING_NAME, data | ||
); | ||
} | ||
} | ||
} else { | ||
println!( | ||
"Found no '{}' to change on upgrade", | ||
ADMIN_CONTAINER_SOURCE_SETTING_NAME | ||
); | ||
} | ||
|
||
// For control container | ||
if let Some(data) = input.data.get_mut(CONTROL_CONTAINER_SOURCE_SETTING_NAME) { | ||
match data { | ||
serde_json::Value::String(source) => { | ||
for ver in PREVIOUS_CONTROL_CONTAINER_VERSIONS { | ||
let prev_source = format!("{}:{}", CONTROL_CONTAINER_IMAGE_REPOSITORY, ver); | ||
if *source == prev_source { | ||
*source = format!( | ||
"{}:{}", | ||
CONTROL_CONTAINER_IMAGE_REPOSITORY, | ||
TARGET_CONTROL_CONTAINER_VERSION | ||
); | ||
println!( | ||
"Changed value of '{}' from '{}' to '{}' on upgrade", | ||
CONTROL_CONTAINER_SOURCE_SETTING_NAME, prev_source, source | ||
); | ||
break; | ||
} | ||
} | ||
} | ||
_ => { | ||
println!( | ||
"'{}' is set to non-string value '{}'", | ||
CONTROL_CONTAINER_SOURCE_SETTING_NAME, data | ||
); | ||
} | ||
} | ||
} else { | ||
println!( | ||
"Found no '{}' to change on upgrade", | ||
CONTROL_CONTAINER_SOURCE_SETTING_NAME | ||
); | ||
} | ||
|
||
Ok(input) | ||
} | ||
|
||
fn backward(&mut self, input: MigrationData) -> Result<MigrationData> { | ||
// It's unclear what version of the host-containers we should downgrade to since it could | ||
// be any of the older host-container versions. | ||
// We can just stay on the latest host-container version since there are no breaking changes. | ||
println!("Vmware host-container versions migration has no work to do on downgrade"); | ||
Ok(input) | ||
} | ||
} | ||
|
||
fn run() -> Result<()> { | ||
migrate(VmwareHostContainerVersions) | ||
} | ||
|
||
// Returning a Result from main makes it print a Debug representation of the error, but with Snafu | ||
// we have nice Display representations of the error, so we wrap "main" (run) and print any error. | ||
// https://github.com/shepmaster/snafu/issues/110 | ||
fn main() { | ||
if let Err(e) = run() { | ||
eprintln!("{}", e); | ||
process::exit(1); | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to open this up for discussion.
The alternative to this is to create separate migrations between all the older versions of bottlerocket with incremental host-container version bumps instead of just having a single migration that bumps all versions to latest. This is feasible with how we push updates and how migrations work. But I'm not sure what reasons we may have for doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One reason might be for the 'enter-admin-container' script introduced in control-container v0.5.4. Since the script depends on
apicliet exec
which is a feature introduced in Bottlerocket version 1.4.0. The script wouldn't work on hosts older than v1.4.0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My vote: I think the bump from the existing version to the latest is fine. On downgrade, I would expect my host containers to go back to the version they were before the upgrade. If there is a specific reason for someone to use one of the incremental versions, that version could be supplied via setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately we can't definitively know what the previously-set host-container source is once we've migrated since it could be any of the older versions we migrated from. The only way to preserve that information is to have a chain of migrations, i.e. a migration between each Bottlerocket host version where we bumped the default host-container versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably the right thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. I have mixed feelings about this. I think in the instance that folks are downgrading and want a specific version, they should set it via user-data. The version of the container they had before would be dependent on the version when the host was initially set up, not necessarily the version they are rolling back to.