[nexus-types] more validation for PlanningInput #5644
Merged
Conversation
This file contains 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
Created using spr 1.3.6-beta.1
2 tasks
sunshowers
changed the title
sunshowers
commented
Apr 27, 2024
| /// - A given external IP or vNIC is only associated with a single Omicron | ||
| /// zone. | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct PlanningInput { |
There was a problem hiding this comment.
Decided to move this up-top because this file is called planning_input.rs, and generally I think the main part of a struct should live at the top. (Also I was getting really confused :) )
The only differences here are that we now have a network_resources field and method.
sunshowers
commented
Apr 27, 2024
| use derive_where::derive_where; | ||
| use serde::{Deserialize, Serialize, Serializer}; | ||
|
|
||
| /// An append-only 1:1:1 (trijective) map for three keys and a value. |
There was a problem hiding this comment.
This is append-only for now -- we may add the ability to remove entries in the future if really necessary. (I think at least at the moment it isn't necessary either here or in BlueprintBuilder.)
jgallagher
approved these changes
Apr 29, 2024
Created using spr 1.3.6-beta.1
sunshowers
deleted the
sunshowers/spr/rfc-nexus-types-more-validation-for-planninginput-1
branch
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.
(Re-do of #5541.)
While reconciling the PlanningInput with the blueprint, I was a bit concerned
about getting nonsensical inputs (e.g. multiple zones get the same IPs) and the
system going haywire. In particular, I was concerned about the Serialize and
Deserialize impls which were seemingly added for reconfigurator-cli. However,
they were liabilities that didn't check for internal invariants.
To address this, this PR introduces the notion of a
TriMap. ATriMapisa 1:1:1 map which can be looked up by any one of three keys. Instead of storing
just a regular map, the
PlanningInputnow stores a couple ofTriMapinstances via an intermediate
OmicronZoneNetworkResourcesstruct.A
TriMapis implemented as a vector with three indexes. It's a prettystraightforward implementation, and I've also added property-based tests
to ensure that a
TriMapis always valid (including always deserializingto a valid structure).
At the moment, a
TriMapdoes not allow removing entries. If necessary,removals can be implemented by just marking the entry as dead and removing
the keys from the indexes. This is fine for our use case, since
TriMapsaren't long-lived.
I've also factored out the omicron zone IP and NIC code into an
OmicronZoneNetworkResourcesstruct. In the future, we'll make theBlueprintBuilderuse this struct, so that blueprints are validatedwhenever they're being mutated. We can also add this to the general code
that validates blueprints.