Adding a proposal for existing hooks sidecar #252
Conversation
kubevirt-bot
added
the
dco-signoff: yes
|
[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 |
design-proposals/sidecar-hooks.md
Outdated
|
|
||
| ## Non Goals | ||
|
|
||
| - Maintain a wide variety of user's applications in KubeVirt codebase |
There was a problem hiding this comment.
Maybe expand on the non-goals: What types of user applications are considered "wide variety," and why is it not a goal to maintain them in the KubeVirt codebase?
There was a problem hiding this comment.
That's a good point. I think the important factor is core functionalities for KubeVirt that can be helped with sidecars. We do provide and maintain Today sidecars that configure slirp and passt, named "network pluggins".
I think it is reasonable that we improve hooks API as needed for core functionalities like that, but not necessarily to all cases.
If we do have a lot of sidecars with custom features that users want to have in a single place, we should simply create another repository for it. It does not need to be in KubeVirt repo, nor maintained by KubeVirt developers.
|
|
||
| ## Hooks API promotion | ||
|
|
||
| Should follow the same rules as stated in [API Graduation Guidelines][] |
There was a problem hiding this comment.
nit: missing link to API Graduation Guidelines on Line 48
There was a problem hiding this comment.
I've used relative path line 54 (last line). It seems to have worked when looking from my personal branch
https://github.com/victortoso/kubevirt-community/blob/hooks-formal-proposal/design-proposals/sidecar-hooks.md
Signed-off-by: Victor Toso <victortoso@redhat.com>
victortoso
force-pushed
the
hooks-formal-proposal
branch
from
15958c2
to
6981c86
Compare
There was a problem hiding this comment.
Thank you for starting this effort, we are lacking intent and design thoughts on the sidecar hooks.
Recent work on sidecars & hooks exposed challenges on how to evolve the protocol API, keep old sidecars working and a clear view forward about the API stability so it can be trusted.
|
|
||
| ## Goals | ||
|
|
||
| - Provide and maintain a set of gRPC APIs for users to do modifications to the VMI |
There was a problem hiding this comment.
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.
Hum, yes. In the end, it is an interface to make changes. I've changed it to the following:
-- Provide and maintain a set of gRPC APIs for users to do modifications to the VMI
+- Provide and maintain an interface for custom applications to do modifications to the VMI| ## 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.
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.
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.
| - As a VM owner, I want to apply custom configurations to QEMU command line | ||
| - For debugability, by adding support or tweak [to run with debug tools][] | ||
| - 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.
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.
Ah, do you disagree with it? I thought that network binding plugins was an example of this. Of course, I can be wrong.
| 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.
This is unclear.
I think virt-launcher is suppose to proxy domain creation and changes through the hook point before applying it to the domain manager (libvirt).
There was a problem hiding this comment.
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.|
|
||
| Virt-launcher should apply the requested changes to the VMI. | ||
|
|
||
| ## Hooks API promotion |
There was a problem hiding this comment.
I think we are missing the API definition with all its details.
There is a need to express all existing versions and describe what each functionality does.
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.
Migration also needs to be taken into account, to explain the implications.
There was a problem hiding this comment.
Thanks, I was in doubt about how deep I should go with the details. I'll go a little bit deeper.
|
@victortoso I think we are missing what happen when we upgrade and migrate virt-launcher pods with sidecars. I'm thinking to the case when we upgrade virt-launcher with a deprecated hooks API. Do we have a mechanism to change the sidecar image on migration? Like, I want to upgrade my sidecar at the same time I upgrade KubeVirt |
@alicefr there is none. The image is defined on annotation and that's it. That's a great use case. The problem is that, this and other suggestions are improvements that should likely be implemented for the beta version. Should we define this now in this PR? Perhaps I can add a section of Future improvements? Or we keep it simple with only alpha related APIs / limitations and we update this when we agree on all improvements we want for Beta. |
|
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
|
/remove-lifecycle stale I'll get back to this. Its just that there are related ongoing discussions/events around hook sidecars that would merit being added here too. |
kubevirt-bot
removed
the
lifecycle/stale
|
@victortoso I think that when multiple hook sidecars have the same hook point, we can sort those hook points based on priority to determine the execution order. kubevirt/kubevirt#11941 |
|
/cc |
|
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
|
/remove-lifecycle stale Promise to get back to this soon. |
kubevirt-bot
removed
the
lifecycle/stale
|
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
Adding a proposal based on @EdDev recommendation [here](kubevirt/kubevirt#10842 (comment).
The goal is to define a clear path for API graduation of the hooks. I'm not sure how much technical detail I should add in the proposal, so feel free to let me know what I have missing.