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

updata: Update metadata generation tool #265

Merged
merged 10 commits into from
Nov 6, 2019
Merged

updata: Update metadata generation tool #265

merged 10 commits into from
Nov 6, 2019

Conversation

sam-aws
Copy link
Contributor

@sam-aws sam-aws commented Sep 20, 2019

Issue #, if available:
#447

Description of changes:
Add a binary to Updog, "updata", which can generate and validate update manifest files. Along with tuftool this can be used to deploy an update.
For example, add an update:

cargo run --bin updata -- add-update $INPUT_DIRECTORY/manifest.json -v 0.1.2 -m 0.1.2 -d 0.0 -a x86_64 --flavor aws-k8s --boot thar-x86_64-0.1.2-boot.ext4.lz4 --root thar-x86_64-0.1.2-root.ext4.lz4 —hash thar-x86_64-0.1.2-root.verity.lz4

Add a wave:

cargo run --bin updata -- add-wave $INPUT_DIRECTORY/manifest.json -v 0.1.2 -a x86_64 --flavor aws-k8s --bound-id 0 —start-time 2019-09-27T10:55:03-07:00

Add a migration:

cargo run --bin updata -- add-migration $INPUT_DIRECTORY/manifest.json --from 0.0 --to 0.1 migrate_0.1

This also addresses #447 by validating the ordering of wave timestamps at the time they are added.

Tested by creating metadata files, validating them via Updata and against working examples, and using Updata to deploy the last few updates.

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

@sam-aws sam-aws force-pushed the metadata branch 3 times, most recently from ecff12c to 6c7fe75 Compare September 24, 2019 18:42
@sam-aws sam-aws marked this pull request as ready for review September 24, 2019 21:45
@sam-aws
Copy link
Contributor Author

sam-aws commented Sep 24, 2019

Chasing up some loose ends but this is largely sane enough now to use for metadata creation.

@sam-aws
Copy link
Contributor Author

sam-aws commented Sep 26, 2019

Added add-mapping so that there's a way to set the datastore version for the 'first' image - otherwise you may get errors about a missing mapping when trying to update.

@sam-aws sam-aws requested a review from iliana September 26, 2019 20:28
@sam-aws
Copy link
Contributor Author

sam-aws commented Oct 22, 2019

Updated to rebase on top of #430 and add two commits to address #447

workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/update_metadata/Cargo.toml Outdated Show resolved Hide resolved
workspaces/updater/update_metadata/src/se.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/Cargo.toml Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
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.

The only remaining comment from before is using log constructs like info! instead of prints :)

workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/api/data_store_version/src/se.rs Outdated Show resolved Hide resolved
workspaces/updater/updog/Cargo.toml Show resolved Hide resolved
workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
From: iliana destroyer of worlds <[email protected]>
Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
@sam-aws
Copy link
Contributor Author

sam-aws commented Oct 31, 2019

Updated, had to rebase again unfortunately to catch the logging updates.
Fixes up the logging usage, a few commenting spots, consolidates the serde magic for data_store_version, and fixes/simplifies some commenting/logic bits.

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.

LGTM, thanks! A couple suggestions left but nothing major.

workspaces/updater/updog/src/create.rs Outdated Show resolved Hide resolved
workspaces/updater/update_metadata/src/lib.rs Outdated Show resolved Hide resolved
@sam-aws
Copy link
Contributor Author

sam-aws commented Nov 1, 2019

Dropped the println! in update_metadata, and cleaned up if let (Some) mig and another similar case with OptionExt.

Copy link
Contributor

@iliana iliana left a comment

Choose a reason for hiding this comment

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

updata looks fine to me, just some nits.

Comment on lines 103 to 113
impl<'de> Deserialize<'de> for Version {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let input = String::deserialize(deserializer)?;
let version = Version::from_str(&input)
.map_err(|e| D::Error::custom(format!("Invalid datastore version: {}", e)))?;
Ok(version)
}
}

impl Serialize for Version {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("{}.{}", self.major, self.minor))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

serde_plain has macros to do effectively this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've discovered derive_deserialize_from_str!() and my life is transformed, but deriving the serialization side doesn't look as easy because data_store_version's implementation of Display uses the "vX.Y" format, not "X.Y", and I think we have uses for the former format elsewhere.

Comment on lines 33 to 39
[[bin]]
name = "updog"
path = "src/main.rs"

[[bin]]
name = "updata"
path = "src/create.rs"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer we drop these sections from Cargo.toml and instead have src/main.rs for updog (this is automatic) and src/bin/updata.rs for updata.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why that layout? If you want updog to be the default just because it's the updog package and don't want to separate them, Cargo supports setting a default binary now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't care if there's a default binary, I just don't expect a binary named "updata" to be created from "create.rs".

Add the set-max-version command to update the maximum version field
across all updates in the manifest, optionally filtering by arch and
flavor.

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
If a removed update was the final reference to a datastore version and
--cleanup is set, remove that version:datastore mapping from the
manifest.

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
Include `add-mapping` to manually insert a
image-version:datastore-version mapping. This is helpful in particular
for adding a mapping for images that may be running but not in the
update manifest.

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
Use checked_sub() instead of casting the difference between the end and
start of the wave. This avoids a potential underflow that could cause
Updog to jitter itself to a point very far in the future.

Fixes #447

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
Before committing changes to wave metadata check that each successive
wave occurs after the previous one.

Fixes #447

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
Update all users of the semver and data_store_version crates to
consitently refer to them as "SemVer" and "DataVersion" respectively.

Also includes some small fixups to Updata, including:
- Allow --max-version to be optional and implicitly set
- Display changes to max version
- Error if migration marked for removal doesn't exist

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
Add the appropriate impls for data_store_version, removing the need for
doing it "manually" in update_metadata.

Signed-off-by: Samuel Mendoza-Jonas <[email protected]>
@sam-aws
Copy link
Contributor Author

sam-aws commented Nov 6, 2019

Updated to call updata updata, and simplified the deserialize side of data_store_version.

@sam-aws sam-aws merged commit 228420e into develop Nov 6, 2019
@sam-aws sam-aws deleted the metadata branch November 6, 2019 20:36
iliana added a commit that referenced this pull request Nov 7, 2019
Signed-off-by: iliana destroyer of worlds <[email protected]>
iliana added a commit that referenced this pull request Nov 7, 2019
@iliana iliana added this to the v0.2.0 milestone Nov 19, 2019
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.

4 participants