Network binding plugin: Support sidecar resource specification #309
Conversation
|
[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 |
kubevirt-bot
added
the
dco-signoff: yes
|
/uncc @cwilkers @aburdenthehand |
kubevirt-bot
requested review from
EdDev and
ormergi
and removed request for
aburdenthehand and
cwilkers
orelmisan
changed the title
Signed-off-by: Orel Misan <omisan@redhat.com>
orelmisan
force-pushed
the
nbp-sidecar-resources
branch
from
0064faf
to
ce11c57
Compare
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
kubevirt-bot
added
the
lifecycle/stale
kubevirt-bot
removed
the
lifecycle/stale
| the network binding plugin sidecar containers do not have CPU/Memory requests and limits. | ||
| The sidecar container can have a memory leak and may cause node's destabilization. | ||
|
|
||
| Suggested solution: |
| > | ||
| > Setting CPU/memory requests and/or limits for sidecar pods on the KubeVirt CR will apply uniformly on all [hook sidecar containers](https://kubevirt.io/user-guide/user_workloads/hook-sidecar/) and network binding plugin sidecar containers. | ||
|
|
||
| In the common scenario of a regular VM and no spacial configuration on the KubeVirt CR, |
There was a problem hiding this comment.
Did you mean special?
| In the common scenario of a regular VM and no spacial configuration on the KubeVirt CR, | |
| In the common scenario of a regular VM and no special configuration on the KubeVirt CR, |
| sidecarImage: quay.io/kubevirt/mynetbindingplugin | ||
| sidecarResources: | ||
| requests: | ||
| cpu: 200m | ||
| memory: 20Mi |
There was a problem hiding this comment.
How about introducing some kind of sidecar configuration type?
For example:
sidecar:
image:
resources:
requests:
...
limits:
....
Since API is in alpha stage I think this change can be done.
| > Setting CPU/memory requests and/or limits for sidecar pods on the KubeVirt CR will apply uniformly on all [hook sidecar containers](https://kubevirt.io/user-guide/user_workloads/hook-sidecar/) and network binding plugin sidecar containers. | ||
|
|
||
| In the common scenario of a regular VM and no spacial configuration on the KubeVirt CR, | ||
| the network binding plugin sidecar containers do not have CPU/Memory requests and limits. |
There was a problem hiding this comment.
Note that users may not realize its a must to prevent resource leak.
Sure we can say its the user responsibility, but in practice it may turn out to be bad UX.
Does is it make sense to change Kubevirt to set some arbitrary default req/limits for the sidecar (that will be documented) to prevent potential leak?
The suggested API will enable override it.
|
Pull requests that are marked with After that period the bot marks them with the label /label needs-approver-review |
kubevirt-bot
added
the
needs-approver-review
|
/sig network |
What this PR does / why we need it:
When registering a network binding plugin in the KubeVirt CR, it is possible to specify an optional sidecar container image.
The sidecar is currently created without resource specification, unless some spacial conditions apply.
This PR discusses the alternatives of specifying the sidecar container's resource requests.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
The contents of this PR was split from #303.
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note: