✨ Implement support for EFA interface type #5275
Conversation
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @RadekManak! |
|
Hi @RadekManak. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
needs-ok-to-test
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| @@ -153,6 +163,11 @@ type AWSMachineSpec struct { | |||
| // +kubebuilder:validation:MaxItems=2 | |||
| NetworkInterfaces []string `json:"networkInterfaces,omitempty"` | |||
|
|
|||
| // NetworkInterfaceType is the interface type of the primary network Interface. | |||
There was a problem hiding this comment.
Maybe mention here that the default is interface and that "" and interface are equivalent.
There was a problem hiding this comment.
Maybe we can have explicit default here? E.g. // +kubebuilder:default=interface
There was a problem hiding this comment.
Is it possible in the future that we might want to change the default? For example, what if Amazon deprecates the current interface type and makes EFA the choice, we would want to move folks across to EFA. If we add an explicit default here, then they would have to manually update their configs to move across should things ever change
|
Overall looks good, just one small comment. /assign @damdo |
| // NetworkInterfaceType is the type of network interface. | ||
| type NetworkInterfaceType string | ||
|
|
||
| const ( |
There was a problem hiding this comment.
Should we follow Kube conventions and use PascalCase rather than these lowercase version, so Interface and EFA
| // NetworkInterfaceType is the interface type of the primary network Interface. | ||
| // +kubebuilder:validation:Enum=interface;efa | ||
| // +optional | ||
| NetworkInterfaceType *NetworkInterfaceType `json:"networkInterfaceType,omitempty"` |
There was a problem hiding this comment.
Does this need to be a pointer? There's no difference between "" and a nil value right?
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds a new enum field to AWSMachineSpec called NetworkInterfaceType that configures the type of the first interface (also called primary interface) on the created instance.
The two values are:
NetworkInterfaceTypeENI: This is the default value, same as nil. The network interface will be ENI.
NetworkInterfaceTypeEFAWithENAInterface: This option sets up both ENA (for IP networking) and EFA interface for HPC connections.
Fixes #3035
Special notes for your reviewer:
Checklist:
Release note: