Add Trunk resource controller implementation #581
Conversation
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Hi, @aldokimi! I've left a few comments in the API types. Thanks for submitting this pull request, it will be very important in achieving parity with CAPO.
|
|
||
| // segmentationType is the type of segmentation to use (e.g., "vlan"). | ||
| // +required | ||
| // +kubebuilder:validation:MaxLength=64 |
There was a problem hiding this comment.
Did you specify the max length as 64 for some specific reason? The Neutron schema defines the length as 32 bytes
https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L71
|
|
||
| // segmentationID is the segmentation identifier (e.g., VLAN ID). | ||
| // +required | ||
| SegmentationID int32 `json:"segmentationID"` |
There was a problem hiding this comment.
| SegmentationID int32 `json:"segmentationID"` | |
| SegmentationID int16 `json:"segmentationID"` |
I'd change to use integers with 16 bits, since in practice neutron only supports VLAN segmentation type (4094 possible IDs) for subports.
Wdyt?
To be honest, I don't know if guidelines restrict usage to only int32 and int64. Can you give an input, @mandre?
There was a problem hiding this comment.
sorry! kubeapi conventions says it MUST be used int32 or int64, so ORC may so.
ignore my suggestion :)
| // +required | ||
| PortRef KubernetesNameRef `json:"portRef"` | ||
|
|
||
| // segmentationType is the type of segmentation to use (e.g., "vlan"). |
There was a problem hiding this comment.
| // segmentationType is the type of segmentation to use (e.g., "vlan"). | |
| // segmentationType is the type of segmentation to use. | |
| // Possible values are "vlan" or "inherit". |
If the only two supported types are VLAN and inherit, I think we can quote them here.
|
|
||
| // TrunkFilter specifies a filter to select a trunk. At least one parameter must be specified. | ||
| // +kubebuilder:validation:MinProperties:=1 | ||
| type TrunkFilter struct { |
There was a problem hiding this comment.
Do we want to add more fields when filtering?
https://docs.openstack.org/api-ref/network/v2/index.html#request-parameters
| type Subport struct { | ||
| // portRef is a reference to the ORC Port which will be used as a subport. | ||
| // +required | ||
| PortRef KubernetesNameRef `json:"portRef"` |
There was a problem hiding this comment.
| PortRef KubernetesNameRef `json:"portRef"` | |
| PortRef KubernetesNameRef `json:"portRef,omitempty"` |
| PortID string `json:"portID,omitempty"` | ||
|
|
||
| // segmentationType is the type of segmentation used. | ||
| // +kubebuilder:validation:MaxLength=1024 |
There was a problem hiding this comment.
I believe this should match the same validation in spec for this field.
| Name string `json:"name,omitempty"` | ||
|
|
||
| // description is a human-readable description for the resource. | ||
| // +kubebuilder:validation:MaxLength=1024 |
There was a problem hiding this comment.
Since we didn't set a max length for the description at spec, I think this could lead to an error - you define a description with 1025+ characters and the controller tries to write the status, but the kube-apiserver fails when validating.
Same for the other fields on this struct.
2 participants
Summary
Adds complete Trunk resource controller implementation with subport implementation (which is necessary for the Trunk type), dependency handling, and comprehensive test coverage.
Key Features
Testing
E2E Testing
-All resources are created in the K8s side
-All resources are created in the Openstack side
-All CRUD functionalities are passing the tests
Checklist