Clean up and remove stale code#10518
Open
andrewjstone wants to merge 2 commits into
Open
Conversation
Clean up the bootstrap agent in preparation for multirack support. This ended up requiring a few other significant changes in other crates. As a first step, we should recognize that the bootstrap agent dropshot API is never going to support TLS. This is why we have sprockets running over the bootstrap network in the first place. All cross sled interactions should use sprockets for security. However, we need to bootstrap the system and so we allow wicketd to talk to the local bootstrap agent on it's own sled. This occurs over the lockstep API. With some further changes, we were able to remove the bootstrap agent API altogether. Additionally, some sprockets related names were changed to enhance clarity. For historical reasons, we used `Baseboard` throughout wicket, but with the changes made to remove the bootstrap agent dropshot API, we only have access to `BaseboardId`s, as that's what the majority of the system works with. Therefore, we migrated many uses of `Baseboard` to use `BaseboardId` instead. This is a much more straightforward type to use. As part of the change to using `BaseboardId`s in wicketd, we wanted to remove optionality for it. The switch zone should always know its own Baseboard through the injected file, and so now we fail immediately if the file does not exist. In order to get a non-optional `BaseboardId` we wanted to make conversion from `Baseboard` to `BaseboardId` infallible. I had started doing this by removing the `Unknown` variant from `Baseboard` and reworking all users. When I had finished I had discovered that this changes a few APIs in a non-backwards compatible manner. As this PR is already very large, I didn't want to make that change. So I went back and re-added the `Unknown` variant, panicking if it ever gets used. I will clean this up in a follow up that also removes the bootstore which is a primary user. The bootstore itself cannot ever have an `Unknown` variant in production, so this is fine for now. To fix a chicken and egg problem where the now gone bootstrap agent api was needed for wicketd to learn its bootstrap-agent-lockstep-api address, we now plumb that address through SMF directly, since the bootstrap agent already knows it when creating the switch zone. Since rack reset doesn't actually work, I also went ahead and removed all the related code. This simplifies things immensely. We can add back working reset functionality in the future, that will work a lot more like the clean slate script. There was also an old, unused `UpdateManager`, that was only called from an unused bootstrap agent API. That has also been removed.
Contributor
Author
|
This needs to be tested on a racklette and on a4x2. |
3dc634a to
2723254
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clean up the bootstrap agent in preparation for multirack support. This
ended up requiring a few other significant changes in other crates.
As a first step, we should recognize that the bootstrap agent dropshot
API is never going to support TLS. This is why we have sprockets
running over the bootstrap network in the first place. All cross sled
interactions should use sprockets for security. However, we need to
bootstrap the system and so we allow wicketd to talk to the local
bootstrap agent on it's own sled. This occurs over the lockstep API.
With some further changes, we were able to remove the bootstrap agent
API altogether. Additionally, some sprockets related names were changed
to enhance clarity.
For historical reasons, we used
Baseboardthroughout wicket, but withthe changes made to remove the bootstrap agent dropshot API, we only
have access to
BaseboardIds, as that's what the majority of the systemworks with. Therefore, we migrated many uses of
Baseboardto useBaseboardIdinstead. This is a much more straightforward type to use.As part of the change to using
BaseboardIds in wicketd, we wanted toremove optionality for it. The switch zone should always know its own
Baseboard through the injected file, and so now we fail immediately if
the file does not exist.
In order to get a non-optional
BaseboardIdwe wanted to makeconversion from
BaseboardtoBaseboardIdinfallible. I had starteddoing this by removing the
Unknownvariant fromBaseboardandreworking all users. When I had finished I had discovered that this
changes a few APIs in a non-backwards compatible manner. As this PR is
already very large, I didn't want to make that change. So I went back
and re-added the
Unknownvariant, panicking if it ever gets used.I will clean this up in a follow up that also removes the bootstore
which is a primary user. The bootstore itself cannot ever have an
Unknownvariant in production, so this is fine for now.To fix a chicken and egg problem where the now gone bootstrap agent
api was needed for wicketd to learn its bootstrap-agent-lockstep-api
address, we now plumb that address through SMF directly, since the
bootstrap agent already knows it when creating the switch zone.
Since rack reset doesn't actually work, I also went ahead and removed
all the related code. This simplifies things immensely. We can add back
working reset functionality in the future, that will work a lot more
like the clean slate script.
There was also an old, unused
UpdateManager, that was only called froman unused bootstrap agent API. That has also been removed.