Multirack: Wicketd API for cluster join config#10465
Conversation
Add 2 new API methods to get and put a minimum configuration required for joining a new rack to an uninitialized cluster. This multirack join configuration is mutually exlcusive with the rss configuration, and so wicketd will only maintain one of them at a time. A wrapper enum, `RssOrMultirackJoinConfig`, was added to support this. A couple of new wrapper types: `BgpAuthKeys` and `SledInventory` were introduced to commonize logic between RSS and Mulitrack Join. RSS behavior should not have changed except in the case when a multirack join configuration has been uploaded and a user requests to get the current RSS config. In the old code, a default struct would be returned, now a `NotFound` is returned. The Join code follows the patterns of the RSS code as much as possible.
| use wicketd_api::SetBgpAuthKeyStatus; | ||
|
|
||
| pub(crate) struct CurrentMultirackJoinConfig { | ||
| inventory: SledInventory, |
There was a problem hiding this comment.
inventory, bootstrap_sleds, and bgp_auth_keys fields exist in both CurrentMultirackJoinConfig and CurrentRssConfig.
I'm wondering if there should be a wrapper struct around RssOrMultirackJoinConfig instead that centralizes those fields. That might help deduplicate things a bit more, but maybe not. The biggest win, if it matters, would be making it easier to maintain the bgp auth keys when the config changes. This could happen if, for example, a customer uploaded BGP keys first and then uploaded the MultirackJoinConfig. I could certainly make it so that we copy the fields across types, to fix this, but at that point I'd probably prefer a wrapper type with those fields.
| pub struct MultirackJoinConfigBaseUserInput { | ||
| /// List of slot numbers only. | ||
| pub bootstrap_slots: BTreeSet<u16>, | ||
| pub rack_network_config: UserSpecifiedRackNetworkConfig, |
There was a problem hiding this comment.
I think this will work for the scenario where racks are interconnected over BGP. However, for the DDM case where racks are either directly plugged into each other or are connected over a switch and are on the same broadcast domain, we need to identify what external switch ports to treat as underlay ports. One way to introduce this could be turning UserSpecifiedPortConfig into an enum like
enum UserSpecifiedPortConfig {
MultirackDdmPortConfig,
SoloRackPortConfig { ... }
}Where the SoloRackPortConfig contains what UserSpecifiedPortConfig currently contains. I think the multi rack DDM ports should be completely self configuring and not require explicit configuration.
There was a problem hiding this comment.
Ah, that's great. This is the turning on all the ports to listen for router announcements/solicitations option that we discussed, right?
There was a problem hiding this comment.
Listening for announcements/solicitations yes, but not necessarily all ports. If we take the above approach, just the ports the user indicates as having the MultirackDdmPortConfig.
There was a problem hiding this comment.
Make sense. Thanks. I had inferred from the lack of a data carrying enum variant on MultirackDdmPortConfig that it was all ports. But that's because I was referring to the UserSpecifiedRackNetworkConfig and not per port config. Poor reading comprehension on my part.
There was a problem hiding this comment.
@rcgoodfellow I'm looking closer at this as I'm about to start implementing, and I have a few questions.
Is the SoloRackPortConfig really only for the initial RSS rack? It seems like racks could have both types of port configs, and likely will.
What if instead we used something like the following:
enum UserSpecifiedPortConfig {
DdmAutoPortConfig,
BgpFullManualConfig
}There was a problem hiding this comment.
I went ahead and added some support for this in bb42a84
2 participants
Add 2 new API methods to get and put a minimum configuration required for joining a new rack to an uninitialized cluster. This multirack join configuration is mutually exlcusive with the rss configuration, and so wicketd will only maintain one of them at a time. A wrapper enum,
RssOrMultirackJoinConfig, was added to support this.A couple of new wrapper types:
BgpAuthKeysandSledInventorywere introduced to commonize logic between RSS and Mulitrack Join.RSS behavior should not have changed except in the case when a multirack join configuration has been uploaded and a user requests to get the current RSS config. In the old code, a default struct would be returned, now a
NotFoundis returned. The Join code follows the patterns of the RSS code as much as possible.Notes for reviewers