Scaffold EVPN Fabric Resource and Controller#328
Conversation
9938145 to
c39b9f8
Compare
7242b1a to
2ac3738
Compare
2ac3738 to
718b995
Compare
718b995 to
05afbd9
Compare
05afbd9 to
3b85728
Compare
3b85728 to
9d64fe5
Compare
866f5e6 to
012e6c0
Compare
86621eb to
726ea7f
Compare
This high-level CRD will take care of creating and managing all low-level resources that are required to build a EVPN VXLAN resource. Its controller implementation is intentionally left empty to a large extend to be extended by future changes. The setup constructs a list of sub-reconciler functions that are called in sequence to build up the full lifecycle of the `Fabric`. Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
012e6c0 to
d78ee8b
Compare
726ea7f to
a73a136
Compare
There was a problem hiding this comment.
Looks good to me :) The API is self-contained and explanatory, conventions also (the Unnumbered bool had let me thinking for a bit), the reconciler logic is also fitting for the purpose of this PR.
Before we merge I would like to sync on this issue: I am wondering if we would want to have a more modular and extendable design for the fabric type. For example, by default we could have a "plain routed" fabric with just deviceSelector, loopbacks, and underlay. Then, we could extend it with references to a specialized type, similar to what we do with the core types. Like this we could have support different fabrics, ike the current EVPN-VXLAN, EVPN MPLS, etc. Some of these fabrics would be reusing some types.
Obviously the evpn self-contained group is quite appealing too and eventually also the straight-forward way to go.
Thanks for opening this PR, looking forward 🚀
| // FabricSpec defines the desired state of Fabric. | ||
| type FabricSpec struct { |
There was a problem hiding this comment.
I quite like this API definition, it is straightforward and self-explanatory. I think thought, that there are some assumptions being made and that are not clear/documented: e.g., one device and all sub-resources belong to just one fabric. Either if this holds of not, I think we would benefit from either adding a concept, or extending its documentation.
| // deviceSelector identifies which devices are members of this fabric. | ||
| // All devices whose labels match this selector will be enrolled. | ||
| // +required | ||
| DeviceSelector metav1.LabelSelector `json:"deviceSelector"` |
There was a problem hiding this comment.
We currently wouldn't prevent a user defining two fabrics, with label selectors that intersect on some devices. This would result in a competition for the sub-resources and would potentially lead to an outage. At the same time, I understand that if we support two fabrics, some devices might actually need to be in both. I think the question in that case is, how do we ensure that a single brick is indirectly managed by just one fabric? should we also label the sub resources?
This high-level CRD will take care of creating and managing all low-level resources that are required to build a EVPN VXLAN resource.
Its controller implementation is intentionally left empty to a large extend to be extended by future changes. The setup constructs a list of sub-reconciler functions that are called in sequence to build up the full lifecycle of the
Fabric.