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

Remove host container migration #4324

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

vyaghras
Copy link
Contributor

@vyaghras vyaghras commented Dec 6, 2024

Issue number:

Closes #

Description of changes:

migrations: remove all the weak setting on downgrade 
migrations: Change public control host-container source metadata to object 
migrations: Change aws admin host-container source metadata to object 
migrations: Change public control host-container source metadata to object 
migrations: Change public admin host-container source metadata to object 
common_migrations: add new migrations to handle settings-generator as struct 
models: Add Strength enum and Setting-Generator struct 
apiclient: update README to document version 2 of /tx API 
shared-defaults: change public host container source setting-generator to table 
shared-defaults: change aws host container source setting-generator to table 
datastore: changes to match changes in Core kit datastore

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.

@vyaghras vyaghras force-pushed the remove_host_container_migration branch from f3531f7 to b77efc3 Compare December 6, 2024 19:02
@vyaghras
Copy link
Contributor Author

vyaghras commented Dec 6, 2024

☝️ Changed the Version in Twoliter.toml

@vyaghras vyaghras requested review from bcressey and cbgbt December 6, 2024 20:57
@vyaghras vyaghras mentioned this pull request Jan 2, 2025
7 tasks
@vyaghras vyaghras force-pushed the remove_host_container_migration branch 10 times, most recently from e9f7de6 to d1a7c76 Compare January 27, 2025 17:58
@vyaghras vyaghras requested review from cbgbt and bcressey January 27, 2025 18:09
…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
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
We rely that metadata can only be populated from defaults, and storewolf
will repopulate the metadata from defaults.
In this migration we will remove the data and metadata if it matches
with existing ones in datastore.
We will rely on storewolf to populate correct metadata and data.
In this migration we will remove the data and metadata if it matches
with existing ones in datastore.
We will rely on storewolf to populate correct metadata and data.
In this migration we will remove the data and metadata if it matches
with existing ones in datastore.
We will rely on storewolf to populate correct metadata and data.
In this migration we will remove the data and metadata if it matches
with existing ones in datastore.
We will rely on storewolf to populate correct metadata and data.
Keep the source same as:
public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14
Add a strength metadata as weak so that the setting can be deleted on
upgrade and downgrade.
… 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.

- RemoveSchnauzerMigration: This will remove metadata(always) and data
  if the value matches with the value in existing datastore.

- RemoveDataMigration: This will remove the data when value matches with
  existing data.

- 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 vyaghras force-pushed the remove_host_container_migration branch from d1a7c76 to afdc836 Compare January 28, 2025 17:11
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines +5 to +8
// Remove the weak settings on downgrade
fn run() -> Result<()> {
migrate(ResetMetadataMigration)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: ResetMetadataMigration could be embedded here since it's unlikely we'll make use of it as a common migration

Comment on lines +1943 to +1948
/// We use this migration to remove a setting string if it matches the old value.
pub struct RemoveOldData {
pub setting: &'static str,
pub old_val: &'static str,
pub new_val: &'static str,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

new_val not used. Maybe rename this to RemoveMatchingString

Comment on lines +5 to +6
const OLD_CONTROL_CTR: &str = "public.ecr.aws/bottlerocket/bottlerocket-control:v0.7.18";
const NEW_CONTROL_CTR: &str = "public.ecr.aws/bottlerocket/bottlerocket-control:v0.7.18";
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be the latest public control container

Comment on lines +5 to +6
const OLD_ADMIN_CTR: &str = "public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14";
const NEW_ADMIN_CTR: &str = "public.ecr.aws/bottlerocket/bottlerocket-admin:v0.11.14";
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to be v0.11.16

pub struct RemoveSchnauzerMigration {
pub setting: &'static str,
pub old_cmdline: &'static str,
pub new_cmdline: &'static str,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unused?

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