updog: change method of determining update availability #993

etungsten · 2020-07-15T16:29:32Z

Issue number:
N/A

Description of changes:

Author: Erikson Tung <[email protected]>
Date:   Tue Jul 14 15:58:23 2020 -0700

    updog: remove jitters, calculate wave position to determine availability
    
    This removes jittering in updog and instead use the seed to find a
    target time position within the wave to determine whether an update is
    available.
    
    This also removes the --timestamp option since it's no longer necessary
    without updog returning the jittered time.
    
    As a result of this change, updates will only be visible if they are
    available (host is past their defined update wave). To see all updates
    regardless of update waves, either --ignore-waves needs to be specified
    or settings.updates.ignore-waves needs to be set to true.

Author: Erikson Tung <[email protected]>
Date:   Wed Jul 15 17:54:46 2020 -0700

    waves: define a wave duration for the "last" wave
    
    To avoid having all the hosts in the last wave all updating at once when
    the last wave starts, we define an intermediary wave before the actual
    last wave to define its wave duration so updog can evenly spread the
    hosts throughout the "last" wave.

Testing done:
updog unit tests pass, newly created unit test for update_metadata passes.

Testing in an instance:

Confirmed that the seed value places the host in the last wave for v0.4.1 which starts on 7/19

bash-5.0# cat /etc/updog.toml 
metadata_base_url = "https://updates.bottlerocket.aws/2020-02-02/aws-k8s-1.15/x86_64/"
targets_base_url = "https://updates.bottlerocket.aws/targets/"
seed = 887
version_lock = "latest"
ignore_waves = false

Check update without ignoring waves, v0.4.1 does not show up since our wave hasn't started:

bash-5.0# updog check-update                                           aws-k8s-1.15 0.4.0
bash-5.0# updog check-update -a --json
[
  {
    "variant": "aws-k8s-1.15",
    "arch": "x86_64",
    "version": "0.4.0",
    "max_version": "0.4.1",
    "waves": {
      "0": "2020-06-29T20:00:00Z",
      "20": "2020-06-29T23:00:00Z",
      "102": "2020-06-30T19:00:00Z",
      "204": "2020-07-02T19:00:00Z",
      "512": "2020-07-05T19:00:00Z"
    },
    "images": {
      "boot": "bottlerocket-aws-k8s-1.15-x86_64-0.4.0-7303622-boot.ext4.lz4",
      "root": "bottlerocket-aws-k8s-1.15-x86_64-0.4.0-7303622-root.ext4.lz4",
      "hash": "bottlerocket-aws-k8s-1.15-x86_64-0.4.0-7303622-root.verity.lz4"
    }
  },
  {
    "variant": "aws-k8s-1.15",
    "arch": "x86_64",
    "version": "0.3.4",
    "max_version": "0.4.1",
    "waves": {
      "0": "2020-05-28T18:00:00Z",
      "20": "2020-05-28T21:00:00Z",
      "102": "2020-05-29T17:00:00Z",
      "204": "2020-05-31T17:00:00Z",
      "512": "2020-06-03T17:00:00Z"
    },
    "images": {
      "boot": "bottlerocket-aws-k8s-1.15-x86_64-0.3.4-85d09a8-boot.ext4.lz4",
      "root": "bottlerocket-aws-k8s-1.15-x86_64-0.3.4-85d09a8-root.ext4.lz4",
      "hash": "bottlerocket-aws-k8s-1.15-x86_64-0.3.4-85d09a8-root.verity.lz4"
    }
  }, 
 ...
]

Calling updog update gives me 0.4.0:

bash-5.0# updog update
Starting update to 0.4.0
^C

With --ignore-waves set, 0.4.1 becomes visible

bash-5.0# updog check-update --ignore-waves
aws-k8s-1.15 0.4.1
bash-5.0# updog check-update -a --ignore-waves       
aws-k8s-1.15 0.4.1
aws-k8s-1.15 0.4.0
aws-k8s-1.15 0.3.4
aws-k8s-1.15 0.3.3
aws-k8s-1.15 0.3.2
aws-k8s-1.15 0.3.1
aws-k8s-1.15 0.3.0

Calling updog update --ignore-waves gives me v0.4.1:

bash-5.0# updog update --ignore-waves         
Starting update to 0.4.1

I manually edited /etc/updog.toml to lower the seed value to fit a wave that has already passed:

[ec2-user@ip-192-168-21-225 ~]$ sudo vi /.bottlerocket/rootfs/etc/updog.toml 
[ec2-user@ip-192-168-21-225 ~]$ sudo sheltie
bash-5.0# cat /etc/updog.toml 
metadata_base_url = "https://updates.bottlerocket.aws/2020-02-02/aws-k8s-1.15/x86_64/"
targets_base_url = "https://updates.bottlerocket.aws/targets/"
seed = 200 
version_lock = "latest"
ignore_waves = false

And now v0.4.1 shows up as an available update even without the --ignore-waves flag.

bash-5.0# updog check-update --json
{
  "variant": "aws-k8s-1.15",
  "arch": "x86_64",
  "version": "0.4.1",
  "max_version": "0.4.1",
  "waves": {
    "0": "2020-07-13T19:00:00Z",
    "20": "2020-07-13T22:00:00Z",
    "102": "2020-07-14T18:00:00Z",
    "204": "2020-07-16T18:00:00Z",
    "512": "2020-07-19T18:00:00Z"
  },
  "images": {
    "boot": "bottlerocket-aws-k8s-1.15-x86_64-0.4.1-a29a1450-boot.ext4.lz4",
    "root": "bottlerocket-aws-k8s-1.15-x86_64-0.4.1-a29a1450-root.ext4.lz4",
    "hash": "bottlerocket-aws-k8s-1.15-x86_64-0.4.1-a29a1450-root.verity.lz4"
  }
}
bash-5.0# updog update
Starting update to 0.4.1

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.

sources/updater/update_metadata/src/lib.rs

webern · 2020-07-15T17:47:25Z

sources/updater/update_metadata/src/lib.rs

+                    return time >= target_time;
+                } else {
+                    // Check if last wave has started
+                    return time >= start;


The last wave does not have any jittering or ramping. We could avoid this by making our last wave always start with MAX_SEED in the manifest (I think). Otherwise I'm not sure how we could avoid all hosts in the last wave hitting at the same time.

Yeah, even before this change, the hosts part of the last wave will always all update at once.

That would mean we would always need to define the duration of the last wave in our waves.toml which currently we do not.

I think I'll go ahead and define the duration of the last wave to be 1 day.

So for example, if the last wave has 2048-512=1536 seeds allocated to it (going off of our default-waves.toml), then the time increment for each succeeding seed value would be (86400 seconds / 1536) ~= 1 minute. Does that sound reasonable? @tjkirch

I don't love the idea of using an arbitrary time period for the last wave - I'd rather clean up our understanding of wave definitions, which is currently fairly murky. I've already seen people interpreting the wave times differently. The "initial" wave is weird because it shouldn't actually get used, the final wave is weird because of this jump, intermediate waves still have questions about whether the time is start vs. end, etc.

Until we fix that, I think we could set an explicit ramp for the last wave in our wave TOML files by adding another wave at the end for 99/100%.

sources/updater/update_metadata/src/lib.rs

tjkirch

Some comments on update_metadata; I'll go through updog soon!

This also removes the --timestamp option since it's no longer necessary without updog returning the jittered time.

This is a breaking change, would you please add this part to the commit message too? (Personally I'd put the next paragraph of the PR description into the commit message, too; people sometimes look there for reasoning and usage of changes.)

Because of that and the other changes to pub functions/structs, we need to make sure it goes into a breaking release like 0.5.0.

sources/updater/README.md

sources/updater/update_metadata/src/lib.rs

etungsten · 2020-07-15T22:44:59Z

Push above addresses @webern and @tjkirch 's comments.

Moved test_update_ready and test_final_wave from updog/src/main.rs to update_metadata/src/lib.rs.
Updated Wave enum to include the seed of the wave, and I use that to calculate the seeds allocated to a particular wave.
Fixed updog: change method of determining update availability #993 (comment)
For the purpose of determining a host's position in the last wave, the last wave duration is set to 1 day by default.
Updated commit message to include more details about the changes.

etungsten · 2020-07-15T22:46:56Z

https://github.com/bottlerocket-os/bottlerocket/compare/3b88ec5fd72cab228cf14cd6b0ec50b68bc37816..0dcc5235684461cf1d08170e3ac3976b32a980dd

This force push fixes test_update_ready. Hopefully it's more clear.

etungsten · 2020-07-15T23:02:13Z

Made some small changes to the test_update_ready.

sources/updater/update_metadata/src/lib.rs

etungsten · 2020-07-16T01:00:46Z

Force push above addresses the following:

updog: change method of determining update availability #993 (comment)
Removes the default 1 day duration for the last wave.
Takes the suggestion from updog: change method of determining update availability #993 (comment) to update our preset wave structures so the last wave has a defined wave duration.
Updates test_update_ready again... Tried to make it more robust.

etungsten · 2020-07-16T16:02:40Z

Push above adds a few more test cases
Push below has minor fixes for those test cases

etungsten · 2020-07-17T17:28:46Z

Push above adds some comments to the wave toml files.
Fixes a comment in update_metadata

tjkirch

Woohoo!

One other thing I'd be interested in seeing, in your PR testing, is that the update becomes available when you adjust your seed downward to fit into the current wave. It's not quite 7/19 yet, so there's a little window to check :)

sources/updater/update_metadata/src/lib.rs

sources/updater/waves/accelerated-waves.toml

sources/updater/update_metadata/src/lib.rs

sources/updater/waves/accelerated-waves.toml

sources/updater/updog/src/main.rs

etungsten · 2020-07-18T00:32:06Z

Rebased onto develop to fix some minor conflicts.

Addresses @tjkirch 's review comments.

One other thing I'd be interested in seeing, in your PR testing, is that the update becomes available when you adjust your seed downward to fit into the current wave. It's not quite 7/19 yet, so there's a little window to check :)

Updated PR description to include this testing.

sources/updater/waves/default-waves.toml

sources/updater/waves/ohno.toml

sources/updater/waves/accelerated-waves.toml

sources/updater/update_metadata/src/lib.rs

etungsten · 2020-07-20T18:00:26Z

Push above addresses @tjkirch 's comments.

Separated out the test_update_ready... test into multiple ones.

Push below removes some redundant wave set up.

This removes jittering in updog and instead use the seed to find a target time position within the wave to determine whether an update is available. This also removes the --timestamp option since it's no longer necessary without updog returning the jittered time. As a result of this change, updates will only be visible if they are available (host is past their defined update wave). To see all updates regardless of update waves, either --ignore-waves needs to be specified or settings.updates.ignore-waves needs to be set to true.

To avoid having all the hosts in the last wave all updating at once when the last wave starts, we define an intermediary wave before the actual last wave to define its wave duration so updog can evenly spread the hosts throughout the "last" wave.

etungsten · 2020-07-20T18:31:48Z

Push above refactors the tests a little.

zmrow

etungsten mentioned this pull request Jul 15, 2020

updog: change heuristics for determining update availability #992

Closed

etungsten force-pushed the updog-availability branch from f24b877 to 0b89188 Compare July 15, 2020 16:34

etungsten requested review from webern, tjkirch, zmrow and bcressey July 15, 2020 16:38

etungsten force-pushed the updog-availability branch 2 times, most recently from 8890335 to 7dcc339 Compare July 15, 2020 16:43

tjkirch requested a review from sam-aws July 15, 2020 18:13

webern approved these changes Jul 15, 2020

View reviewed changes

etungsten commented Jul 15, 2020

View reviewed changes

sources/updater/update_metadata/src/lib.rs Outdated Show resolved Hide resolved

tjkirch reviewed Jul 15, 2020

View reviewed changes

etungsten force-pushed the updog-availability branch 2 times, most recently from 3b88ec5 to 0dcc523 Compare July 15, 2020 22:41

etungsten force-pushed the updog-availability branch from 0dcc523 to 7cdfe90 Compare July 15, 2020 23:01

etungsten requested review from tjkirch and webern July 15, 2020 23:02

etungsten commented Jul 15, 2020

View reviewed changes

sources/updater/update_metadata/src/lib.rs Outdated Show resolved Hide resolved

webern approved these changes Jul 16, 2020

View reviewed changes

etungsten force-pushed the updog-availability branch from 7cdfe90 to 8abc253 Compare July 16, 2020 00:57

etungsten force-pushed the updog-availability branch 2 times, most recently from bb88195 to 6eac239 Compare July 16, 2020 15:59

etungsten force-pushed the updog-availability branch 2 times, most recently from 90db2cd to e68f46a Compare July 17, 2020 17:27

tjkirch reviewed Jul 17, 2020

View reviewed changes

etungsten force-pushed the updog-availability branch from e68f46a to 2683364 Compare July 18, 2020 00:30

etungsten requested a review from tjkirch July 18, 2020 00:32

tjkirch reviewed Jul 18, 2020

View reviewed changes

etungsten force-pushed the updog-availability branch from 2683364 to 429cc38 Compare July 20, 2020 17:57

etungsten force-pushed the updog-availability branch from 429cc38 to cca416e Compare July 20, 2020 18:00

etungsten requested a review from tjkirch July 20, 2020 18:01

etungsten added 2 commits July 20, 2020 11:30

etungsten force-pushed the updog-availability branch from cca416e to a9ee7a9 Compare July 20, 2020 18:31

tjkirch approved these changes Jul 20, 2020

View reviewed changes

zmrow approved these changes Jul 24, 2020

View reviewed changes

etungsten added this to the v0.5.0 milestone Jul 27, 2020

etungsten merged commit 9c46dd2 into bottlerocket-os:develop Jul 27, 2020

etungsten deleted the updog-availability branch July 27, 2020 18:03

etungsten mentioned this pull request Aug 21, 2020

Updog: Report update jitter to caller #264

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updog: change method of determining update availability #993

updog: change method of determining update availability #993

etungsten commented Jul 15, 2020 •

edited

Loading

webern Jul 15, 2020 •

edited

Loading

etungsten Jul 15, 2020 •

edited

Loading

tjkirch Jul 15, 2020

tjkirch left a comment

etungsten commented Jul 15, 2020 •

edited

Loading

etungsten commented Jul 15, 2020

etungsten commented Jul 15, 2020

etungsten commented Jul 16, 2020

etungsten commented Jul 16, 2020

etungsten commented Jul 17, 2020

tjkirch left a comment

etungsten commented Jul 18, 2020

etungsten commented Jul 20, 2020

etungsten commented Jul 20, 2020

zmrow left a comment

updog: change method of determining update availability #993

updog: change method of determining update availability #993

Conversation

etungsten commented Jul 15, 2020 • edited Loading

webern Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

etungsten Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

tjkirch Jul 15, 2020

Choose a reason for hiding this comment

tjkirch left a comment

Choose a reason for hiding this comment

etungsten commented Jul 15, 2020 • edited Loading

etungsten commented Jul 15, 2020

etungsten commented Jul 15, 2020

etungsten commented Jul 16, 2020

etungsten commented Jul 16, 2020

etungsten commented Jul 17, 2020

tjkirch left a comment

Choose a reason for hiding this comment

etungsten commented Jul 18, 2020

etungsten commented Jul 20, 2020

etungsten commented Jul 20, 2020

zmrow left a comment

Choose a reason for hiding this comment

etungsten commented Jul 15, 2020 •

edited

Loading

webern Jul 15, 2020 •

edited

Loading

etungsten Jul 15, 2020 •

edited

Loading

etungsten commented Jul 15, 2020 •

edited

Loading