Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't constrain baseboard when adding sled #4987

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Conversation

andrewjstone
Copy link
Contributor

We were artificially limiting how we added sleds to a rack by forcing them to be of type Baseboard::Gimlet. Instead of constructing a Baseboard inside nexus, we instead send down the serial and part numbers and use those to uniquely identify the sled. We ignore whether it's a PC or Gimlet as long as the ids match. This is similar to how the inventory works and allows adding a sled to a rack on the falcon a4x2 testbed.

We were artificially limiting how we added sleds to a rack by forcing
them to be of type `Baseboard::Gimlet`. Instead of constructing a
`Baseboard` inside nexus, we instead send down the serial and part
numbers and use those to uniquely identify the sled. We ignore whether
it's a PC or Gimlet as long as the ids match. This is similar to how
the inventory works and allows adding a sled to a rack on the falcon
a4x2 testbed.
@davepacheco
Copy link
Collaborator

This is probably fine but after the discussion in chat on Friday I'm not sure it's the right direction for the testbed. It sounded like the direction we wanted was "make the testbed look indistinguishable from real hardware" rather than changing the software to better support the testbed? (CC @rcgoodfellow)

@andrewjstone
Copy link
Contributor Author

This is probably fine but after the discussion in chat on Friday I'm not sure it's the right direction for the testbed. It sounded like the direction we wanted was "make the testbed look indistinguishable from real hardware" rather than changing the software to better support the testbed? (CC @rcgoodfellow)

I actually don't think this violates that constraint at all. The testbed code acts exactly like the omicron code. There was never a need to specify "Gimlet" at the nexus layer and so we don't anymore. The constraint is that the BaseboardId is unique, which is what we do in both hardware and testbed.

I also don't think that constraint means we would never change production code to run on the testbed, similar to how we would change it to work on new generations of hardware. Not all sleds will be gimlets in the future and so this change allows new hardware to work as well.

Additionally, this literally allows me to run our current update on the testbed now. I just tested it via adding a sled and generating blueprints via OMDB. That's a big win for a small change.

@davepacheco
Copy link
Collaborator

Ah, I think I understand this better now. We're not changing the Nexus external API here. It's just that before, Nexus was fetching the revision from the inventory, solely to fill in the "revision" that Bootstrap Agent wanted. Now Sled Agent accepts what we actually want, which is a part/serial to identify the sled (no revision -- it was not needed), and we only compare those two fields.

My only question then is whether there's any impact to existing systems. I'm guessing not since we still never run mixed versions of Nexus/Sled Agent?

@andrewjstone
Copy link
Contributor Author

Ah, I think I understand this better now. We're not changing the Nexus external API here. It's just that before, Nexus was fetching the revision from the inventory, solely to fill in the "revision" that Bootstrap Agent wanted. Now Sled Agent accepts what we actually want, which is a part/serial to identify the sled (no revision -- it was not needed), and we only compare those two fields.

You got it. I had this epiphany last night, and realized it would make my questions on friday moot! 😃

My only question then is whether there's any impact to existing systems. I'm guessing not since we still never run mixed versions of Nexus/Sled Agent?

Exactly. And in addition to that, the only time this path gets called is when adding a sled, which we haven't done yet anywhere.

@andrewjstone andrewjstone merged commit dd8d1aa into main Feb 6, 2024
21 checks passed
@andrewjstone andrewjstone deleted the add-sled-baseboard-id branch February 6, 2024 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants