Network binding plugin: Support sidecar resource specification #309

orelmisan · 2024-07-09T15:42:24Z

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.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note:

None

kubevirt-bot · 2024-07-09T15:42:27Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cwilkers for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

orelmisan · 2024-07-09T15:43:05Z

/uncc @cwilkers @aburdenthehand
/cc @EdDev @ormergi

Signed-off-by: Orel Misan <omisan@redhat.com>

kubevirt-bot · 2024-10-07T15:55:31Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

aburdenthehand · 2024-10-14T14:00:31Z

/remove-lifecycle stale
@EdDev @ormergi please review

EdDev

This is an important security improvement to protect such sidecards from leaking resources beyond what the plugin author and/or the cluster admin expected it to.

EdDev · 2024-10-14T17:03:58Z

design-proposals/network-binding-plugin/network-binding-plugin.md

+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:


nit: "Solution:" is enough.

ormergi · 2024-10-15T10:26:57Z

design-proposals/network-binding-plugin/network-binding-plugin.md

+>
+> 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, 


Did you mean special?

Suggested change

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,

ormergi · 2024-10-15T10:32:23Z

design-proposals/network-binding-plugin/network-binding-plugin.md

+          sidecarImage: quay.io/kubevirt/mynetbindingplugin
+          sidecarResources:
+            requests:
+              cpu: 200m
+              memory: 20Mi


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.

ormergi · 2024-10-15T10:38:35Z

design-proposals/network-binding-plugin/network-binding-plugin.md

+> 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.


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.

kubevirt-bot · 2024-10-22T11:25:53Z

Pull requests that are marked with lgtm should receive a review
from an approver within 1 week.

After that period the bot marks them with the label needs-approver-review.

/label needs-approver-review

xpivarc · 2024-11-18T15:24:15Z

/sig network

aburdenthehand · 2024-12-11T12:48:07Z

@orelmisan Please take a look at the current feedback from Or to help move this forward

kubevirt-bot · 2025-03-11T13:14:54Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

kubevirt-bot · 2025-04-10T13:42:03Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

orelmisan · 2025-04-20T16:05:48Z

/remove-lifecycle rotten

kubevirt-bot · 2025-07-19T16:20:15Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

orelmisan · 2025-07-20T06:15:08Z

/remove-lifecycle stale

aburdenthehand · 2025-08-25T12:49:24Z

@orelmisan What's happening with this one? Looks like it is still waiting to resolve feedback provided.
Should we migrate it to the enhancements repo?

orelmisan · 2025-09-17T12:45:21Z

@orelmisan What's happening with this one? Looks like it is still waiting to resolve feedback provided. Should we migrate it to the enhancements repo?

Yes, it would make sense.
Apologies for the delayed response.

kubevirt-bot · 2025-12-16T13:40:36Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

kubevirt-bot · 2026-01-15T14:05:59Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jul 9, 2024

kubevirt-bot requested review from aburdenthehand and cwilkers July 9, 2024 15:42

kubevirt-bot added the size/L label Jul 9, 2024

kubevirt-bot requested review from EdDev and ormergi and removed request for aburdenthehand and cwilkers July 9, 2024 15:43

orelmisan mentioned this pull request Jul 9, 2024

Network binding plugin: Support compute container resource overhead #303

Open

8 tasks

orelmisan changed the title ~~Network binding plugin: Address sidecar resource specification~~ Network binding plugin: Support sidecar resource specification Jul 9, 2024

Network binding plugin: Support sidecar resource spec

ce11c57

Signed-off-by: Orel Misan <omisan@redhat.com>

orelmisan force-pushed the nbp-sidecar-resources branch from 0064faf to ce11c57 Compare July 9, 2024 15:50

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 7, 2024

kubevirt-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 14, 2024

EdDev approved these changes Oct 14, 2024

View reviewed changes

kubevirt-bot assigned EdDev Oct 14, 2024

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2024

ormergi reviewed Oct 15, 2024

View reviewed changes

kubevirt-bot added the needs-approver-review Indicates that a PR requires a review from an approver. label Oct 22, 2024

kubevirt-bot added the sig/network label Nov 18, 2024

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 11, 2025

kubevirt-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 10, 2025

kubevirt-bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Apr 20, 2025

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 19, 2025

kubevirt-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 20, 2025

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 16, 2025

kubevirt-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 15, 2026

	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,

Network binding plugin: Support sidecar resource specification #309

Are you sure you want to change the base?

Network binding plugin: Support sidecar resource specification #309

Uh oh!

Conversation

orelmisan commented Jul 9, 2024

Uh oh!

kubevirt-bot commented Jul 9, 2024

Uh oh!

orelmisan commented Jul 9, 2024

Uh oh!

kubevirt-bot commented Oct 7, 2024

Uh oh!

aburdenthehand commented Oct 14, 2024

Uh oh!

EdDev left a comment

Choose a reason for hiding this comment

Uh oh!

EdDev Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

ormergi Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

ormergi Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

ormergi Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

kubevirt-bot commented Oct 22, 2024

Uh oh!

xpivarc commented Nov 18, 2024

Uh oh!

aburdenthehand commented Dec 11, 2024

Uh oh!

kubevirt-bot commented Mar 11, 2025

Uh oh!

kubevirt-bot commented Apr 10, 2025

Uh oh!

orelmisan commented Apr 20, 2025

Uh oh!

kubevirt-bot commented Jul 19, 2025

Uh oh!

orelmisan commented Jul 20, 2025

Uh oh!

aburdenthehand commented Aug 25, 2025

Uh oh!

orelmisan commented Sep 17, 2025

Uh oh!

kubevirt-bot commented Dec 16, 2025

Uh oh!

kubevirt-bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants