[wicket] Allow CLI-style RSS configuration#3326
Conversation
sunshowers
left a comment
There was a problem hiding this comment.
Looks really good overall, the basic logic seems sound.
| for sled in sleds { | ||
| // We should never get a non-sled from wicketd; if we do, filter it out. | ||
| if sled.id.type_ != SpType::Sled { | ||
| continue; |
There was a problem hiding this comment.
Warning as in printing something to the wicket user, or warn!ing to the wicket log that's only available to support personnel?
| // We have to attach the comment for each sled on the _next_ item in the | ||
| // array, so here we set our prefix to be the previous item's details. |
| } | ||
|
|
||
| let Some(config) = config else { | ||
| return; |
There was a problem hiding this comment.
When is this going to happen?
If config is None then we're going to just have those default values in the template returned. That doesn't seem right.
There was a problem hiding this comment.
This will happen if someone asks for the config before wicket has been able to get the current config from wicketd, which is unlikely (but not impossible) in practice. Returning the default values in the template was intentional, but assumed we fixed the default values to be reasonable (possibly just empty strings with comments explaining them).
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use omicron_common::api::internal::shared::RackNetworkConfig as InternalRackNetworkConfig; |
There was a problem hiding this comment.
Can this and PortSpeed/InternalPortSpeed share a type via progenitor's replace? Seems like a lot of extra ceremony converting between the types that isn't always necessary.
There was a problem hiding this comment.
Maybe! I'm still uneasy about when replace is dangerous or not, so didn't reach for it here...
7582ccc to
b2fee2f
Compare
RSS wants to know the bootstrap addresses of the sleds to include; this PR teaches wicketd how to find them. It also updates the wicket TOML config introduced in #3326 to include the sled's bootstrap address (if known) in a comment in the sled list; I'm unsure if this is something we'll want to keep once customers are running RSS, but it seems handy for debugging for now. Builds on #3326.
The bulk of code in this PR is the new `rack_setup` wicket pane, which includes a bunch of popup-related things (which are all fairly verbose for now; I followed what the update pane does structurally, and am very thankful it exists!). On a fresh (uninitialized and unconfigured) system, the pane should look roughly like this, although hopefully not with `UNKNOWN` bootstrap addresses (those are because these screenshots are from a simulated dev environment that doesn't have a bootstrap network):  If we attempt to start an update, we'll get a confirm popup; pressing "Yes" will fail with the error from wicketd (in this case, "you need to configure things!"):  After filling in all the configuration options via the CLI added in #3326, we see the options reflected in the UI:  at which point starting RSS should succeed, and the `Current rack status: ` line will change to `Initializing`. This will eventually progress to a new state once RSS completes (whether successful or not).
This is the first of 4 (ish?) PRs building toward configuring and running RSS via wicket. It adds a handful of endpoints to wicketd for various RSS configuration settings, and a preliminary CLI interface with which a user can supply those settings.
This isn't 100% correct or complete and will be tweaked in some of the followup PRs, but the majority of it is close enough to right that I think it's ready for review on its own.