search

oxidecomputer / omicron

Omicron: Oxide control plane
Mozilla Public License 2.0
251 stars 39 forks source link

`UplinkConfig` needs to include autonegotiation parameter #3809

Closed rcgoodfellow closed 11 months ago

rcgoodfellow commented 1 year ago

https://github.com/oxidecomputer/omicron/blob/ed70f1c8c6a83fed0d6b5fed2ba6c033bdc18c5b/common/src/api/internal/shared.rs#L84-L100

jgallagher commented 1 year ago

Now that these settings are stored in the bootstore, adding fields may be a little tricky - we'll probably have to chat with @andrewjstone about the best way to do that in a backwards-compatible way.

andrewjstone commented 1 year ago

Now that these settings are stored in the bootstore, adding fields may be a little tricky - we'll probably have to chat with @andrewjstone about the best way to do that in a backwards-compatible way.

TLDR: Check the generation number of bootstore::NetworkConfig and deserialize appropriately.

It shouldn't be bad at all. The bootstore transfers opaque blobs attached to generation numbers and chooses the higher of the two it sees. The main issue is that you need to check the generation number before you deserialize and ensure that all versions of sled-agent are updated before you add update the bootstore with the new field (at a new generation).

You'll probably have two different versions of EarlyNetworkConfig and then a deserialize method that returns the right version based on the generation. This may also be wrapped in an enum. Alternatively, you could have one new version of EarlyNetworkConfig with an optional new field, and then default that for the old generation.

This is really just a general upgrade issue and not related to bootstore specifically.

rcgoodfellow commented 11 months ago

Fixed in #4564