Remove host container migration #4324
Open
Conversation
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
vyaghras
force-pushed
the
remove_host_container_migration
branch
from
f3531f7 to
b77efc3
Compare
|
☝️ Changed the Version in Twoliter.toml |
bcressey
reviewed
Dec 19, 2024
sources/settings-migrations/v1.29.0/change-public-control-container-to-set-gen/src/main.rs
Outdated
Show resolved
Hide resolved
sources/settings-migrations/v1.29.0/change-public-control-container-to-set-gen/src/main.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
7 tasks
cbgbt
reviewed
Jan 16, 2025
Comment on lines
6
to
13
| [settings.host-containers.admin] | ||
| enabled = false | ||
| superpowered = true | ||
| source = "public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14" | ||
|
|
||
| [metadata.settings.host-containers.admin.source.setting-generator] | ||
| command = "schnauzer-v2 render --template 'public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14'" | ||
| strength = "weak" | ||
| skip-if-populated = true |
There was a problem hiding this comment.
Since these are static strings and don't require templates, can we disregard the setting generator and accomplish the same thing like this?
Suggested change
| [settings.host-containers.admin] | |
| enabled = false | |
| superpowered = true | |
| source = "public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14" | |
| [metadata.settings.host-containers.admin.source.setting-generator] | |
| command = "schnauzer-v2 render --template 'public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14'" | |
| strength = "weak" | |
| skip-if-populated = true | |
| [settings.host-containers.admin] | |
| enabled = false | |
| superpowered = true | |
| source = "public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14" | |
| [metadata.settings.host-containers.admin.source] | |
| strength = "weak" |
Comment on lines
18
to
22
|
|
||
| [metadata.settings.host-containers.control.source.setting-generator] | ||
| command = "schnauzer-v2 render --template 'public.ecr.aws/bottlerocket/bottlerocket-control:v0.7.18'" | ||
| strength = "weak" | ||
| skip-if-populated = true |
There was a problem hiding this comment.
Same feedback as the admin container source for public ECR.
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
Comment on lines
1267
to
1041
| pub struct MetadataStringToStructMigration { | ||
| pub setting: &'static str, | ||
| pub old_cmdline: &'static str, | ||
| pub new_cmdline: &'static str, | ||
| pub metadata_type: &'static str, | ||
| pub binary_for_generator: &'static str, | ||
| pub skip_if_populated: bool, | ||
| } |
There was a problem hiding this comment.
This seems highly specific to setting-generators, I doubt we'd use it elsewhere. I'd suggest renaming it to be more specific and dropping metadata_type as an argument.
We're already using Shlex ("shell lexer") in this migrator. Do we need binary_for_generator if we can just parse it out? The binary should always be the first element of the lexed command line.
Finally, I wonder if the settings for the "new" generator should just be a SettingsGenerator struct?
vyaghras
force-pushed
the
remove_host_container_migration
branch
from
b77efc3 to
95c4bac
Compare
This migration will change the public control host-container source to new source. In backward migration, it will be NO-Op.
This migration will change the public control host-container source to new source. In backward migration, it will be NO-Op as other migration will delete the weak settings and all metadata on downgrade.
This migration will change the aws admin host-container source setting-generator from string to object setting-generator metadata (that will generate source using sundog) on forward migration. We will also add a strength file with value "weak". In backward migration, it will reset the value of the source setting generator as string and delete the strength metadata.
This migration will change the aws control host-container source setting-generator from string to object setting-generator metadata (that will generate source using sundog) on forward migration. We will also add a strength file with value "weak". In backward migration, it will reset the value of the source setting generator as string and delete the strength metadata.
We need this migration that will delete all the weak settings and setting-generators.
We rely that metadata is always populated from defaults, and storewolf will repopulate the metadata that is already not present in datastore.
…r to table Update the source from string like: public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14 To setting generator as object, that contains: - command - strength - skip-if-populated
…o table
Update the source from string like:
schnauzer-v2 render --requires 'aws@v1(helpers=[ecr-prefix])' --template
'{{ ecr-prefix settings.aws.region }}/bottlerocket-admin:v0.11.14'
To setting generator as object, that contains:
- command
- strength
- skip-if-populated
We need to add these to match changes in Bottlerocket-core-kit Refer commit: bottlerocket-os/bottlerocket-core-kit@a72f6bd PR: bottlerocket-os/bottlerocket-core-kit#294
Updated the `commit_transaction` function to enable committing metadata from pending transactions. In commit transaction we will first commit metadata and then pending keys to correctly perform the check to identify if key exists or not. The strength handling among pending and committed transaction is as: If pending metadata is strong and committed metadata is weak, commit the pending setting. If pending metadata is weak, committed metadata is strong and the setting is already available, do not downgrade from strong to weak. Otherwise add the strength file with value as weak. If pending and committed metadata are the same, no action is performed. Additionally, made minor changes to metadata functions for improved access and flexibility: Introduced a `committed` field to dynamically access metadata in pending transactions. Replaced the hardcoded use of live committed metadata with this committed variable ans pass Committed::Live from previous usages. Refer commit: bottlerocket-os/bottlerocket-core-kit@20a435e Refer PR: bottlerocket-os/bottlerocket-core-kit#294
vyaghras
force-pushed
the
remove_host_container_migration
branch
from
95c4bac to
88c1fe4
Compare
… struct - RemoveWeakSettingsMigration: When we downgrade multiple version to a version where migrator is not aware of deleting the setting-generator as struct or the strength file. This migration will help us to delete the strength and setting generator metadata. - MetadataStringToStructMigration: migration to change host-containers source metadata to struct from string. - ReplaceSettingWithSettingGeneratorMigration: Migration to replace a setting with setting generator. - ResetMetadataMigration: Migration to remove all metadata on upgrade and downgrade. This metadata will be rewritten by storewolf. This migration will be used to convert a strong setting to a weak setting. Also deleted the test test_replaces_nothing for ReplaceSchnauzerMigration because we are deleting all the metadata as first step of the migrator, hence we will not have any metadata available in existing datastore to compare. We will assume that the settings generator in input metadata is same as Old schanuzer string and will always replace that with incoming setting generator string.
vyaghras
force-pushed
the
remove_host_container_migration
branch
from
88c1fe4 to
3e850ca
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue number:
Closes #
Description of changes:
Testing done:
Refer to testing in bottlerocket-os/bottlerocket-core-kit#294
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.