feat: adding support for multiqueue#650
Conversation
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
drew-viles
requested review from
65278,
mcbenjemaa,
pborn-ionos and
wikkyk
as code owners
TestsPlease note that running unit and e2e tests requires manual approval from a team member. e2e testsWe use labels to control which e2e tests contexts are run:
ℹ️ Ask a team member to add the requested labels if you don't have enough permissions. |
|
Nice and simple change but we'll hold it for now until after we have released 0.8.0 and v1alpha2. |
wikkyk
left a comment
wikkyk
left a comment
There was a problem hiding this comment.
Please port this to v1alpha2!
Remember to update the tests with the correct type, too.
| // Queues is the Multiqueue field in Proxmox to assign to the NIC | ||
| // +optional | ||
| // +kubebuilder:validation:Minimum=1 | ||
| Queues *uint16 `json:"queues,omitempty"` |
There was a problem hiding this comment.
Fields must be signed. If you're expecting 65535 to be a valid value, then make it an int32 with a Maximum rule, if 65534 then int16 is fine.
| // +kubebuilder:validation:Maximum=4094 | ||
| VLAN *uint16 `json:"vlan,omitempty"` | ||
|
|
||
| // Queues is the Multiqueue field in Proxmox to assign to the NIC |
There was a problem hiding this comment.
It'd be nice to make this comment a bit more informative:
| // Queues is the Multiqueue field in Proxmox to assign to the NIC | |
| // Queues is the number of queues assigned to the device. | |
| // This value is passed to the Multiqueue field in PROXMOX. |
| } | ||
|
|
||
| // extractNetworkQueue returns the queue out of net device input e.g. virtio=A6:23:64:4D:84:CB,bridge=vmbr1,mtu=1500,tag=100,queue=4. | ||
| func extractNetworkQueue(input string) uint16 { |
There was a problem hiding this comment.
Remember to fix the return value to match the Queues field.
This increases the importance of #653 :-)
| if v.Queues != nil { | ||
| queue := extractNetworkQueue(net) | ||
| if queue != *v.Queues { |
There was a problem hiding this comment.
This could be done with ptr.Deref.
| // example 'virtio,bridge=vmbr0,tag=100,queue=4'. | ||
| func formatNetworkDevice(model, bridge string, mtu *uint16, vlan *uint16, queue *uint16) string { |
There was a problem hiding this comment.
| // example 'virtio,bridge=vmbr0,tag=100,queue=4'. | |
| func formatNetworkDevice(model, bridge string, mtu *uint16, vlan *uint16, queue *uint16) string { | |
| // example 'virtio,bridge=vmbr0,tag=100,queues=4'. | |
| func formatNetworkDevice(model, bridge string, mtu *uint16, vlan *uint16, queues *uint16) string { |
|
|
Sorry absolutely slammed atm, I will pick this up again asap though |
2 participants
Issue #, if available:
#501
Description of changes:
Right now, users cannot add queues to the network NIC. With this PR, users will now be able to optionally supply a value into the CAPI provider to configure queues.
Testing performed:
Added tests for
make testand ran with success. I'm currently trying to get access to a proxmox with shared storage as it appears to be required for this where as right now, I'm using local storage.If you want to wait for that testing to be done on my side I totally get it! However, the change seems pretty straight forward on the whole so I'll let you make the call from your side.