api: add PreferredAllocator interface #535
Conversation
Codecov Report
@@ Coverage Diff @@
## master #535 +/- ##
==========================================
- Coverage 57.00% 56.93% -0.07%
==========================================
Files 31 31
Lines 2114 2120 +6
==========================================
+ Hits 1205 1207 +2
- Misses 839 841 +2
- Partials 70 72 +2
Continue to review full report at Codecov.
|
pkg/deviceplugin/server.go
Outdated
| postAllocate func(*pluginapi.AllocateResponse) error | ||
| preStartContainer func(*pluginapi.PreStartContainerRequest) error | ||
| preferredAllocation func(*pluginapi.PreferredAllocationRequest) (*pluginapi.PreferredAllocationResponse, error) |
There was a problem hiding this comment.
These closure declarations have become too lengthy. Let's declare new functional types postAllocateFunc, preStartContainerFunc and getPreferredAllocationFunc in this file and use them instead of func(...).
There was a problem hiding this comment.
could you use the functional types in this file too?
pkg/deviceplugin/server.go
Outdated
| postAllocate func(*pluginapi.AllocateResponse) error | ||
| preStartContainer func(*pluginapi.PreStartContainerRequest) error | ||
| preferredAllocation func(*pluginapi.PreferredAllocationRequest) (*pluginapi.PreferredAllocationResponse, error) |
There was a problem hiding this comment.
could you use the functional types in this file too?
pkg/deviceplugin/manager.go
Outdated
| @@ -22,6 +22,10 @@ import ( | |||
| pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1" | |||
| ) | |||
|
|
|||
| type postAllocateFunc func(*pluginapi.AllocateResponse) error | |||
| type preStartContainerFunc func(*pluginapi.PreStartContainerRequest) error | |||
| type preferredAllocationFunc func(*pluginapi.PreferredAllocationRequest) (*pluginapi.PreferredAllocationResponse, error) | |||
There was a problem hiding this comment.
I'd prefer to rename preferredAllocationFunc to getPreferredAllocationFunc for consistency.
Also all the other variables, parameters and struct fields preferredAllocation should be better renamed to become a verb to signal they contain a func, that is to getPreferedAllocation.
There was a problem hiding this comment.
I would not have preferred to prefix every preferred with get since not even the pluginapi gets it everywhere, but I get it, preferred or not.
There was a problem hiding this comment.
I agree. The interface name doesn't need to be prefixed indeed. Could you please revert it?
There was a problem hiding this comment.
I would prefer I got the gets before preferred preferably right now
uniemimu
changed the title
This adds a PreferredAllocator interface so that plugins can optionally implement the API. Signed-off-by: Ukri Niemimuukko <ukri.niemimuukko@intel.com>
rojkov
changed the title
This adds a GetPreferredAllocator interface so that plugins can
optionally implement the API.
Signed-off-by: Ukri Niemimuukko ukri.niemimuukko@intel.com