-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding a proposal for existing hooks sidecar #252
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# Sidecar hooks | ||
|
||
## Overview | ||
|
||
A hooking mechanism that allows customizations to the VMI before it starts without changing KubeVirt | ||
core components. The common user of those hooks are Sidecars that will run in the same Pod that the | ||
VMI will be deployed. | ||
|
||
## Motivation | ||
|
||
A way to accommodate use cases that we might not want to bring and support in KubeVirt itself or to | ||
give room to test and develop features while they are still not mature enough. | ||
|
||
## Goals | ||
|
||
- Provide and maintain a set of gRPC APIs for users to do modifications to the VMI | ||
- Provide and maintain sidecars that are important for core functionalities | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am unsure if this merits a special goal. Both external and internal sidecars are expected to get the same assurance and stability for the integration point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, what you said is true but should be covered by the first bullet. What the second bullet introduces is that, we will provide some sidecars which add functionalities to KubeVirt and those sidecars we will be maintaining. We might want to take out or not the second bullet. I'm not sure where to draw the line in this document. |
||
|
||
## Non Goals | ||
|
||
- Provide and maintain a wide variety of sidecars that are not part of KubeVirt's core | ||
functionalities. | ||
|
||
## User Stories | ||
|
||
- As a VM owner, I want to apply custom configurations to QEMU command line | ||
victortoso marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- For debugability, by adding support or tweak [to run with debug tools][] | ||
victortoso marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- To test unsupported features or apply changes that [could workaround bugs][] | ||
- As a KubeVirt developer, I want to provide alternative solutions that might not be ready to core | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to provide a modular, plug-able solution, to Kubevirt without overloading the core components. It is not necessary a matter of readiness. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, do you disagree with it? I thought that |
||
KubeVirt components | ||
- For Network configurations like [Slirp][] and [Passt][] | ||
|
||
## Design | ||
|
||
KubeVirt should provide a versioned gRPC Hook module, with all the APIs that can be used including | ||
parameters and responses. | ||
|
||
The gRPC server runs in the Sidecar. The Sidecars will use the module to establish the communication | ||
with virt-launcher and exchange capabilities to define which Hook version is being used and what are | ||
the hooks that are implemented. | ||
|
||
Virt-launcher will be entitled with managing the hooks. It'll verify and connect to all the Sidecars | ||
that are running by checking the pre-defined folder where the unix sockets for gRPC communication | ||
will be created. | ||
|
||
Virt-launcher should apply the requested changes to the VMI. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unclear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I can do a better wording here. What happens is that virt-launcher has an XML ready to be applied, but provides this to the sidecar over hook API. The sidecar apply changes to it. Virt-launcher applies the result XML to libvirt. Changed to: -Virt-launcher should apply the requested changes to the VMI.
+Virt-launcher will apply the result data provided by the sidecars and then create the VMI. |
||
|
||
## Hooks API promotion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we are missing the API definition with all its details. There is also a need to explain what are the implications and considerations of a sidecar author regarding backward and forward compatibility. Perhaps starting with the virt-launcher side, which relates to the Kubevirt support of hooks and what are the expectations from the server side. It will be useful to elaborate on what happens when Kubevirt upgrades occur and what happens when the sidecar is upgraded. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I was in doubt about how deep I should go with the details. I'll go a little bit deeper. |
||
|
||
Should follow the same rules as stated in [API Graduation Guidelines][] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: missing link to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've used relative path line 54 (last line). It seems to have worked when looking from my personal branch |
||
|
||
[to run with debug tools]: https://kubevirt.io/user-guide/debug_virt_stack/launch-qemu-strace/ | ||
[could workaround bugs]: https://github.com/kubevirt/kubevirt/issues/8420 | ||
[Slirp]: https://github.com/kubevirt/kubevirt/pull/10272 | ||
[Passt]: https://github.com/kubevirt/kubevirt/pull/10425 | ||
[API Graduation Guidelines]: ../docs/api-graduation-guidelines.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are too specific, possibly even touching the implementation.
I guess that the main goal is to define a stable integration point for custom programs, that are looking to extend Kubevirt VM functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, yes. In the end, it is an interface to make changes. I've changed it to the following: