[reconfigurator] RoT bootloader planner support #8664
Conversation
| @@ -2139,12 +2139,15 @@ INFO sufficient InternalDns zones exist in plan, desired_count: 3, current_count | |||
| INFO sufficient ExternalDns zones exist in plan, desired_count: 3, current_count: 3 | |||
| INFO sufficient Nexus zones exist in plan, desired_count: 3, current_count: 3 | |||
| INFO sufficient Oximeter zones exist in plan, desired_count: 0, current_count: 0 | |||
| WARN cannot configure RoT bootloader update for board (missing sign in stage0 caboose from inventory), serial_number: serial0, part_number: model0 | |||
There was a problem hiding this comment.
Like the RoT, we need to have a SIGN in the caboose. Will write reconfigurator-cli tests for RoT bootloader as part of #8798
There was a problem hiding this comment.
This looks good to me. Sorry for the conflicts with #8836 - I think some of the changes here would be clearer when combined with the changes on that branch? But I think we could land these in either order without too much pain.
| // We try MGS-driven update components in a hardcoded priority order until | ||
| // any of them returns `Some`. The order is described in RFD 565 section | ||
| // "Update Sequence". For now, we only plan SP, RoT and RoT bootloader | ||
| // updates. When implemented, host OS updates will be the first to try. |
There was a problem hiding this comment.
Should this be "last to try"? It'd probably also be fine to drop the last two sentences, since host OS planning is just around the corner.
| // update. For `expected_updates`, each component update counts as an | ||
| // update, so the amount of `all_updates` should be a third of | ||
| // `expected_updates`. | ||
| assert_eq!(all_updates.len(), expected_updates.len() / 3); |
There was a problem hiding this comment.
I don't think this comment was right before (and is not right with these changes either). We're only seeing a third of the updates because we haven't actually done all of the expected_updates; we only did all of the bootloaders. I made some changes to this test in #8836 to try to make it more clear. We might want to do the same here, although I think this test is claiming we'd try to stage updates for all the bootloader concurrently. That's a thing we're supposed to never do; it's currently enforced by only actually staging one pending MGS update a time regardless of the target component, but maybe this test shouldn't actually work with the bootloader at all? (I.e., even if we pass usize::MAX to plan_mgs_updates, maybe we should only allow at most one bootloader update?)
There was a problem hiding this comment.
huh 🤔 Guess I misunderstood what was happening here, thanks!
|
Thanks @jgallagher! I think I'd like to merge this one first since it'll be easier to iterate on the best way to handle testing in your PR. Additionally, it breaks #8835, so I'd rather pause on merging that one and fix up the reconfigurator-cli tests there based off of this one. |
Analogous to #8421 for RoT bootloader. Reconfigurator CLI support has already been implemented #8620
Closes: #8668