Add EthernetSegment resource and NX-OS provider implementation#378
Conversation
felix-kaestner
force-pushed
the
evpn-multihoming
branch
3 times, most recently
from
e562595 to
6dc0ffb
Compare
felix-kaestner
force-pushed
the
evpn-multihoming
branch
from
6dc0ffb to
5ce4943
Compare
nikatza
left a comment
nikatza
left a comment
There was a problem hiding this comment.
Nice, many thanks for this PR! There are some issues that can we could resolve quickly, so I hope we can merge it soon.
We may want to include the ethernet-segment routes in this API, however it would only mean an extension (and preferably in a follow-up).
Looking forward! 🔥
felix-kaestner
force-pushed
the
evpn-multihoming
branch
from
5ce4943 to
10d7c14
Compare
felix-kaestner
force-pushed
the
evpn-multihoming
branch
3 times, most recently
from
ee5879c to
cd9e8a1
Compare
|
|
||
| // ESIType is the ESI derivation type parsed from the first byte of ESI. | ||
| // +optional | ||
| ESIType ESIType `json:"esiType,omitempty"` |
There was a problem hiding this comment.
Wouldn't it make more sense to use a mutating webhook for this? After all this is immutable after creation, right?
There was a problem hiding this comment.
Also saw this thread now indicating that the device may not always support the requested state. I am wondering if we shouldn't go into configured=False then because intent!=configured.
There was a problem hiding this comment.
Status.ESIType is populated from the device state (read-back from the operational ESI's first byte), not defaulted from user input. So a mutating webhook wouldn't apply here. The spec field ESIType is already immutable via CEL validation.
Regarding Configured=False for unsupported states: the provider returns an error for unsupported ESI types, which the controller surfaces as a terminal error and sets Configured=False. So this case is already covered.
| mh.State = AdminStEnabled | ||
| mh.EadEviRoute = true | ||
| mh.DfElectionMode = DfElectModeModulo | ||
| mh.DfElectionTime = "3.0" |
There was a problem hiding this comment.
Is this the device's default?
There was a problem hiding this comment.
Yes, 3 seconds is the NX-OS default for the DF election hold timer. We set it explicitly so that the desired-state payload is fully declarative — otherwise a gNMI GET would return "3.0" from the device but our desired state would have no value, causing a perpetual reconcile diff.
Only DfElectionMode=Modulo is not device default, as it would be the Cisco specific PerFlow. However, the mode is contained in the spec, with a default value of Modulo, so we use that here (adhering to the spec).
There was a problem hiding this comment.
EadEviRoute is also not device default, but we set it always to true as the cross-vendor default set by this project.
| return fmt.Errorf("ESI type %s is not supported by this provider", req.EthernetSegment.Spec.ESIType) | ||
| } | ||
|
|
||
| return p.Patch(ctx, f, mh, mm, es) |
There was a problem hiding this comment.
Do you see any option to not patch all of these entities for each ES? Everything but es are singeltons, aren't they? I am not seeing them being cleaned up. Do you think a (vendored) global ESI Multi-Homing config would make sense?
There was a problem hiding this comment.
You are right that some of these entities are global singletons on NX-OS and not per ES. However, the options like DF in openconfig are per ES, hence we added it also the spec.
/openconfig-ethernet-segments
ethernet-segments
ethernet-segment[name]
name
config
state
df-election
config
df-election-method
preference
revertive
election-wait-time
Things like the feature activation or the EvpnMulticastItems are static values, same for all ES, so setting those paths in every ES's reconcilation is not a conflict. Only the DfElectionTime in the MultihomingItems would lead to conflict between multiple ES resources.
felix-kaestner
force-pushed
the
evpn-multihoming
branch
2 times, most recently
from
b77f315 to
ffd6194
Compare
Introduce the EthernetSegment API type for EVPN multi-homing (RFC 7432 Section 5) with controller, CRD, and NX-OS provider. The controller validates that the referenced interface is an aggregate with switchport configuration and watches for switchport state transitions. The NX-OS provider enables EVPN multihoming globally, configures the ESI per port-channel via a subtree patch, and rejects SingleActive mode and vPC coexistence with typed terminal errors. Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
felix-kaestner
force-pushed
the
evpn-multihoming
branch
from
ffd6194 to
9d8f9be
Compare
Merging this branch changes the coverage (1 decrease, 1 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
3 participants
Introduce the EthernetSegment API type for EVPN multi-homing (RFC 7432 Section 5) with controller, CRD, and NX-OS provider.
The controller validates that the referenced interface is an aggregate with switchport configuration and watches for switchport state transitions. The NX-OS provider enables EVPN multihoming globally, configures the ESI per port-channel via a subtree patch, and rejects SingleActive mode and vPC coexistence with typed terminal errors.