sundog: use JSON output to allow non-String generated model types #430

tjkirch · 2019-10-17T19:28:46Z

We couldn't use an integer for the bork-generated update seed because sundog
was taking the output of generators and serializing it as a string. This lets
the generator determine the type of the output by using JSON as an interchange
format.

Pluto and bork are updated to serialize their output appropriately.

(Note: bork can't use a u64 because JSON can't encode a u64; u32 is granular
enough for this purpose.)

Fixes #260.

Testing done:

updog config still OK, and the data store value is now an integer:

[ec2-user@ip-192-168-104-219 ~]$ cat /.thar/rootfs/etc/updog.toml 
metadata_base_url = "https://d25d9m6x9pxh9h.cloudfront.net/45efedef4afe/metadata/"
target_base_url = "https://d25d9m6x9pxh9h.cloudfront.net/45efedef4afe/targets/"
seed = 434
[ec2-user@ip-192-168-104-219 ~]$ cat /.thar/rootfs/var/lib/thar/datastore/current/live/settings/updates/seed; echo
434

sundog is happy:

bash-5.0# journalctl -u sundog
-- Logs begin at Thu 2019-10-17 19:08:26 UTC, end at Thu 2019-10-17 19:22:52 UTC. --
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal systemd[1]: Starting User-specified setting generators...
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal sundog[2978]: Oct 17 19:09:01.219  INFO sundog: Sundog started
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal sundog[2978]: Oct 17 19:09:01.220  INFO sundog: Retrieving setting generators
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal sundog[2978]: Oct 17 19:09:01.228  INFO sundog: Retrieving settings values
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal sundog[2978]: Oct 17 19:09:01.283  INFO sundog: Sending settings values to the API
Oct 17 19:09:01 ip-192-168-105-152.us-west-2.compute.internal systemd[1]: Started User-specified setting generators.

migration testing done:

Normal forward migration - u32-compatible string to u32:

$ echo '"42"' > ds/live/settings/updates/seed
$ cargo run -- --datastore-path ds --forward
$ cat ds/live/settings/updates/seed; echo
42

If we run again, it's a weird forward migration, already a u32-compatible number, but OK:

$ cargo run -- --datastore-path ds --forward
$ cat ds/live/settings/updates/seed; echo
42

If we have some other string we'll get a clear error and failure:

$ echo '"hi"' > ds/live/settings/updates/seed
$ cargo run -- --datastore-path ds --forward
Error: Migration { msg: "Existing update seed string is not a valid u32: invalid digit found in string" }

Other types are reported as unsupported and fail:

$ echo 'true' > ds/live/settings/updates/seed
$ cargo run -- --datastore-path ds --forward
Error: Migration { msg: "Unsupported type of existing update seed: \'Bool(true)\'" }

Now the backward migrations.

Normal backward migration - u32 to string.

$ echo '42' > ds/live/settings/updates/seed
$ cargo run -- --datastore-path ds --backward
$ cat ds/live/settings/updates/seed; echo
"42"

If we run again, it's a weird backward migration, already a u32-compatible number, but OK:

$ cargo run -- --datastore-path ds --backward
$ cat ds/live/settings/updates/seed; echo
"42"

If we have some other string we'll get a clear error and failure:

$ cat ds/live/settings/updates/seed; echo
"hi"
$ cargo run -- --datastore-path ds --backward
Error: Migration { msg: "Existing update seed string(!) is not a valid u32: invalid digit found in string" }

Other types are reported as unsupported and fail:

$ echo 'false' > ds/live/settings/updates/seed
$ cargo run -- --datastore-path ds --backward
Error: Migration { msg: "Unsupported type of existing update seed: \'Bool(false)\'" }

Live instance testing:

Started an older AMI, yeeted my update into slot B, copied over the migration, signpost-flipped, and rebooted. Started OK, migration ran OK, and the setting is an integer!

bash-5.0# signpost status
OS disk: /dev/xvda
Set A:   boot=/dev/xvda2 root=/dev/xvda3 hash=/dev/xvda4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/xvda6 root=/dev/xvda7 hash=/dev/xvda8 priority=2 tries_left=0 successful=false
Active:  Set B
Next:    Set A
bash-5.0# systemctl status migrator
● migrator.service - Thar data store migrator
   Loaded: loaded (/x86_64-thar-linux-gnu/sys-root/usr/lib/systemd/system/migrator.service; enabled; vendor preset: enabled)
   Active: active (exited) since Fri 2019-10-18 22:38:32 UTC; 1min 32s ago
 Main PID: 2633 (code=exited, status=0/SUCCESS)
    Tasks: 0
   Memory: 0B
   CGroup: /system.slice/migrator.service

Oct 18 22:38:31 localhost migrator[2633]: Oct 18 22:38:31.958 TRACE data_store_version: Parsed major '0' and minor '1'
Oct 18 22:38:31 localhost migrator[2633]: Oct 18 22:38:31.958  INFO migrator: Found applicable forward migration 'migrate_v0.1_borkseed': v0.0 < (v0.1) <= v0.1
Oct 18 22:38:31 localhost migrator[2633]: Oct 18 22:38:31.958 DEBUG migrator: Sorted migrations: ["migrate_v0.1_borkseed"]
Oct 18 22:38:31 localhost migrator[2633]: Oct 18 22:38:31.958  INFO migrator: Copying datastore from /var/lib/thar/datastore/v0.0_TCfFxkFQ0xQviRgt to work location /var/lib/thar/datastore/v0.1_wUeZUfKRfsEF17Em
Oct 18 22:38:32 localhost migrator[2633]: Oct 18 22:38:32.022  INFO migrator: Running migration command: "/var/lib/thar/datastore/migrations/migrate_v0.1_borkseed" "--forward" "--datastore-path" "/var/lib/thar/datastore/v0.1_wUeZUfKRfsEF17Em"
Oct 18 22:38:32 localhost migrator[2633]: Oct 18 22:38:32.135 DEBUG migrator: Migration stdout:
Oct 18 22:38:32 localhost migrator[2633]: Oct 18 22:38:32.135 DEBUG migrator: Migration stderr:
Oct 18 22:38:32 localhost migrator[2633]: Oct 18 22:38:32.135  INFO migrator: Flipping /var/lib/thar/datastore/v0.1 to point to v0.1_wUeZUfKRfsEF17Em
Oct 18 22:38:32 localhost migrator[2633]: Oct 18 22:38:32.135  INFO migrator: Flipping /var/lib/thar/datastore/v0 to point to v0.1
Oct 18 22:38:32 localhost systemd[1]: Started Thar data store migrator.
bash-5.0# cat /var/lib/thar/datastore/current/live/settings/updates/seed; echo
1074

sam-aws · 2019-10-17T20:35:35Z

We should probably update Updog to s/u64/u32 as well but that can come separately since it won't break without it.

workspaces/api/bork/src/main.rs

tjkirch · 2019-10-18T18:48:14Z

We should probably update Updog to s/u64/u32 as well but that can come separately since it won't break without it.

This push adds a commit that changes updog to use u32 instead of u64 for seed, to match the other changes in the PR. @sam-aws and I talked in person to confirm the changes were appropriate, though he said he'll have to update another PR to match.

tjkirch · 2019-10-18T18:49:06Z

(oops; also pushed the work-in-progress migration commit, which I just removed.)

workspaces/api/sundog/src/main.rs

tjkirch · 2019-10-18T21:16:15Z

This push adds a migration for the data type change of the seed. The data-store-version is bumped to 0.1, our first change.

We couldn't use an integer for the bork-generated update seed because sundog was taking the output of generators and serializing it as a string. This lets the generator determine the type of the output by using JSON as an interchange format. Pluto and bork are updated to serialize their output appropriately. (Note: bork can't use a u64 because JSON can't encode a u64; u32 is granular enough for this purpose.)

tjkirch · 2019-10-18T21:50:22Z

Had to rebase on develop to fix all of the paths that changed with #428.

sam-aws

Looks good - it would be nice to try updating into this by eg. manually dd-ing this to the inactive side, putting the migration in the migration directory, and running updog update-apply --reboot to see it survive.

workspaces/api/migration/migrations/v0.1/borkseed/src/main.rs

tjkirch · 2019-10-18T22:11:40Z

This push addresses @sam-aws 's comment about not changing the seed value in backward migrations where we already found a string that's a valid u32.

tjkirch · 2019-10-18T22:42:04Z

Looks good - it would be nice to try updating into this by eg. manually dd-ing this to the inactive side, putting the migration in the migration directory, and running updog update-apply --reboot to see it survive.

Done and successful! Updated description with testing details.

sam-aws · 2019-10-18T22:43:43Z

yeeted my update into slot B

workspaces/api/migration/migrations/v0.1/borkseed/Cargo.toml

jahkeup · 2019-10-21T19:15:37Z

workspaces/api/migration/migrations/v0.1/borkseed/src/main.rs

+use std::convert::TryFrom;
+
+/// We moved from String to u32 for the seed value generated by bork and used by updog.
+struct BorkSeedIntMigration;


Should we instead name the crate and this migration type consistently to be more descriptive than "borkseed", I think "bork-seed-int" is a much more descriptive and helpful name.

That short name (without the "-migration" suffix used for the binary) only shows up as a path component that follows 'migrations' so I chose to keep it short since its purpose was already obvious.

Are there or do namespaces prevent conflicts?

I'm thinking about this along the lines of traditional db migrations where a short but descriptive name make its activities clearer.

I think we'll have a better idea when we have more than one. It's easy to change; migrations are referenced from the repo, so we can change them at any time.

jahkeup

tjkirch · 2019-10-21T22:40:57Z

This push removes one line of the change where I had changed u64 to u32, but the u64 was unrelated to the seed value. Filed #447 for fixing that.

tjkirch requested review from zmrow, patraw and sam-aws October 17, 2019 19:28

sam-aws reviewed Oct 17, 2019

View reviewed changes

workspaces/api/bork/src/main.rs Show resolved Hide resolved

tjkirch force-pushed the num-generation branch from 81acf70 to 5f4a3f3 Compare October 18, 2019 18:48

zmrow reviewed Oct 18, 2019

View reviewed changes

workspaces/api/sundog/src/main.rs Show resolved Hide resolved

tjkirch marked this pull request as ready for review October 18, 2019 21:45

tjkirch force-pushed the num-generation branch from 0444641 to 6a98ee6 Compare October 18, 2019 21:49

tjkirch requested review from sam-aws and zmrow October 18, 2019 21:51

sam-aws approved these changes Oct 18, 2019

View reviewed changes

workspaces/api/migration/migrations/v0.1/borkseed/src/main.rs Outdated Show resolved Hide resolved

workspaces/api/migration/migrations/v0.1/borkseed/src/main.rs Show resolved Hide resolved

tjkirch force-pushed the num-generation branch from 6a98ee6 to bbc8a66 Compare October 18, 2019 22:11

jahkeup reviewed Oct 21, 2019

View reviewed changes

tjkirch mentioned this pull request Oct 21, 2019

Include bork seed migration in repo #446

Closed

tjkirch added 2 commits October 21, 2019 15:36

updog: use u32 seed to match new definition

1b855ef

Add migration for bork-generated update seed to be u32

3ee3f89

tjkirch force-pushed the num-generation branch from bbc8a66 to 3ee3f89 Compare October 21, 2019 22:37

bcressey approved these changes Oct 21, 2019

View reviewed changes

tjkirch merged commit ef8a005 into develop Oct 21, 2019

tjkirch deleted the num-generation branch October 21, 2019 22:47

tjkirch mentioned this pull request Oct 21, 2019

Migration: migration helper functions #87

Closed

sam-aws mentioned this pull request Oct 22, 2019

updata: Update metadata generation tool #265

Merged

iliana added this to the v0.2.0 milestone Nov 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sundog: use JSON output to allow non-String generated model types #430

sundog: use JSON output to allow non-String generated model types #430

tjkirch commented Oct 17, 2019 •

edited

sam-aws commented Oct 17, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

sam-aws left a comment

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

sam-aws commented Oct 18, 2019

jahkeup Oct 21, 2019

tjkirch Oct 21, 2019

jahkeup Oct 21, 2019

tjkirch Oct 21, 2019

jahkeup left a comment •

edited

tjkirch commented Oct 21, 2019

sundog: use JSON output to allow non-String generated model types #430

sundog: use JSON output to allow non-String generated model types #430

Conversation

tjkirch commented Oct 17, 2019 • edited

sam-aws commented Oct 17, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

sam-aws left a comment

Choose a reason for hiding this comment

tjkirch commented Oct 18, 2019

tjkirch commented Oct 18, 2019

sam-aws commented Oct 18, 2019

jahkeup Oct 21, 2019

Choose a reason for hiding this comment

tjkirch Oct 21, 2019

Choose a reason for hiding this comment

jahkeup Oct 21, 2019

Choose a reason for hiding this comment

tjkirch Oct 21, 2019

Choose a reason for hiding this comment

jahkeup left a comment • edited

Choose a reason for hiding this comment

tjkirch commented Oct 21, 2019

tjkirch commented Oct 17, 2019 •

edited

jahkeup left a comment •

edited