updater: exposes updata functions to add migrations and updates, add/remove waves #577

etungsten · 2019-12-06T18:30:58Z

Issue #, if available: Fixes #574

Description of changes:

Exposes updata's add_migration, add_update, add_wave, remove_wave functionality as Manifest methods in the update_metadata library.

Also tries to reconcile some duplicate errors between updog/error.rs and update_metadata/error.rs

Testing:

updog and updata unit tests pass

Running updata:

$ updata init test-manifest
$ ls
debug  test-manifest
$ updata add-migration test-manifest --from 0.0.1 --to 0.0.2
$ cat test-manifest 
{
  "updates": [],
  "migrations": {
    "(0.0,0.0)": []
  },
  "datastore_versions": {}
}
$ updata add-migration test-manifest --from 0.1.1 --to 0.2.0
$ cat test-manifest 
{
  "updates": [],
  "migrations": {
    "(0.0,0.0)": [],
    "(0.1,0.2)": []
  },
  "datastore_versions": {}
}
$ updata add-update test-manifest --arch x86_64 --boot ~/thar/PRIVATE-thar/build/thar-x86_64-boot.ext4 --data-version 0.1 --flavor eks --hash blah --version 0.1.6 --root ~/thar/PRIVATE-thar/build/thar-x86_64-root.ext4
19:15:50 [ INFO] Maximum version set to 0.1.6
$ cat test-manifest 
{
  "updates": [
    {
      "flavor": "eks",
      "arch": "x86_64",
      "version": "0.1.6",
      "max_version": "0.1.6",
      "waves": {},
      "images": {
        "boot": "/home/ANT.AMAZON.COM/etung/thar/PRIVATE-thar/build/thar-x86_64-boot.ext4",
        "root": "/home/ANT.AMAZON.COM/etung/thar/PRIVATE-thar/build/thar-x86_64-root.ext4",
        "hash": "blah"
      }
    }
  ],
  "migrations": {
    "(0.0,0.0)": [],
    "(0.1,0.2)": []
  },
  "datastore_versions": {
    "0.1.6": "0.1"
  }
}

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 · 2019-12-06T22:14:14Z

workspaces/updater/update_metadata/src/lib.rs

+pub fn load_file(path: &Path) -> Result<Manifest> {
+    let file = File::open(path).context(error::ManifestRead { path })?;
+    serde_json::from_reader(file).context(error::ManifestParse)
+}
+
+pub fn write_file(path: &Path, manifest: &Manifest) -> Result<()> {
+    let manifest = serde_json::to_string_pretty(&manifest).context(error::UpdateSerialize)?;
+    fs::write(path, &manifest).context(error::ConfigWrite { path })?;
+    Ok(())
+}


These should probably live in the caller (updata, unnamed-lambda-function) instead of in the library.

If these utility functions are going to only be used for reading and writing manifest files (as in, if manifests are always going to be read/written this way with serde_json), I feel like they can stay here close to the Manifest struct and the rest of its methods.

Looking for more opinions on this!

sam-aws · 2019-12-06T22:17:43Z

workspaces/updater/updog/src/bin/updata.rs

+                Ok(_) => Ok(()),
+                Err(e) => Err(error::Error::UpdateMetadata { source: e }),
+            }
+        }
        Command::AddUpdate(args) => args.run(),
        Command::AddWave(args) => args.add(),


add-wave, remove-wave, and the associated validate function should also move to update_metadata as unamed-lambda-function will need them too.

sam-aws · 2019-12-06T22:19:56Z

workspaces/updater/update_metadata/src/de.rs

@@ -88,7 +88,7 @@ where
                error::DuplicateVersionKey { key }
            );
        } else {
-            return error::BadDataVersion {
+            return error::BadVersion {


~~Why the rename here?~~ Ohh because of the other error sourced from data_store_version. This error should still convey that we're trying to parse a data store version specifically.

sam-aws · 2019-12-06T22:20:38Z

workspaces/updater/update_metadata/Cargo.toml

@@ -15,6 +15,9 @@ serde = { version = "1.0.100", features = ["derive"] }
 serde_json = "1.0.40"
 serde_plain = "0.3.0"
 snafu = "0.5.0"
+toml = "0.5.1"
+tough = "0.2.0"


Does update_metadata use tough?

sam-aws · 2019-12-06T22:23:34Z

workspaces/updater/update_metadata/src/error.rs

+    #[snafu(display("Failed to write config file {}: {}", path.display(), source))]
+    ConfigWrite {
+        path: PathBuf,
+        source: std::io::Error,
+        backtrace: Backtrace,
+    },


I think this was originally from when Updog modified its own config file; this could be rename to "ManifestWrite" for example, and update the message.

Exposes `updata` functions to add migrations, updates, add/remove waves as `Manifest` methods in the `update_metadata` library. Also tries to reconcile some duplicate errors between `updog/error.rs` and `update_metadata/error.rs`

etungsten · 2019-12-06T23:30:25Z

Addresses subset of @sam-aws 's comments:

Moves add_wave and remove_wave, and validate_updates from updata to update_metadata
Clean up/rename errors

sam-aws

Looks good, if we want to ensure that we're always reading from/to JSON then having the load/write methods in update_metadata seems ok.
Have you tried constructing a manifest with updata with these changes?

zmrow

🍧

etungsten · 2019-12-11T19:18:07Z

Looks good, if we want to ensure that we're always reading from/to JSON then having the load/write methods in update_metadata seems ok.
Have you tried constructing a manifest with updata with these changes?

Tested updata, update PR description with test result.

etungsten requested review from iliana, zmrow and sam-aws December 6, 2019 18:30

etungsten force-pushed the expose-updata-functions branch 2 times, most recently from 76971a2 to 9577a92 Compare December 6, 2019 21:00

sam-aws reviewed Dec 6, 2019

View reviewed changes

etungsten force-pushed the expose-updata-functions branch from 9577a92 to 3cbc322 Compare December 6, 2019 23:29

etungsten changed the title ~~updater: exposes updata functions to add migrations and updates~~ updater: exposes updata functions to add migrations and updates, add/remove waves Dec 6, 2019

sam-aws approved these changes Dec 11, 2019

View reviewed changes

zmrow approved these changes Dec 11, 2019

View reviewed changes

etungsten merged commit 91e0008 into develop Dec 11, 2019

etungsten deleted the expose-updata-functions branch December 11, 2019 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updater: exposes updata functions to add migrations and updates, add/remove waves #577

updater: exposes updata functions to add migrations and updates, add/remove waves #577

etungsten commented Dec 6, 2019 •

edited

Loading

sam-aws Dec 6, 2019

etungsten Dec 6, 2019 •

edited

Loading

sam-aws Dec 6, 2019

sam-aws Dec 6, 2019

sam-aws Dec 6, 2019

sam-aws Dec 6, 2019

etungsten commented Dec 6, 2019

sam-aws left a comment

zmrow left a comment

etungsten commented Dec 11, 2019

updater: exposes updata functions to add migrations and updates, add/remove waves #577

updater: exposes updata functions to add migrations and updates, add/remove waves #577

Conversation

etungsten commented Dec 6, 2019 • edited Loading

sam-aws Dec 6, 2019

Choose a reason for hiding this comment

etungsten Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

sam-aws Dec 6, 2019

Choose a reason for hiding this comment

sam-aws Dec 6, 2019

Choose a reason for hiding this comment

sam-aws Dec 6, 2019

Choose a reason for hiding this comment

sam-aws Dec 6, 2019

Choose a reason for hiding this comment

etungsten commented Dec 6, 2019

sam-aws left a comment

Choose a reason for hiding this comment

zmrow left a comment

Choose a reason for hiding this comment

etungsten commented Dec 11, 2019

etungsten commented Dec 6, 2019 •

edited

Loading

etungsten Dec 6, 2019 •

edited

Loading