-
Notifications
You must be signed in to change notification settings - Fork 105
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
enhancements: Propose a more formal yet lean enhancement process #288
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,109 @@ | ||||||||
# Overview | ||||||||
|
||||||||
Rework the existing KubeVirt design proposal process into a more | ||||||||
formal - but still lean - and SIG-aligned Enhancement Proposal process. | ||||||||
|
||||||||
## Motivation | ||||||||
|
||||||||
Today it's a [very small group](https://github.com/kubevirt/community/blob/main/OWNERS#L7-L14) | ||||||||
which can approve design-proposals. | ||||||||
While certain individuals can approve design proposals, once they are merged | ||||||||
it is not clear who is owning them post-merge. | ||||||||
|
||||||||
In this proposal we are proposing to change the process and shape them | ||||||||
around SIGs. | ||||||||
|
||||||||
At its core, this proposal requires: | ||||||||
|
||||||||
1. designs to be sponsored by a SIG | ||||||||
2. to make one SIG responsible for the design process (reviews of design, code, documentation) | ||||||||
3. to make one SIG responsible for managing the design process (collaboration with other SIGs as needed) | ||||||||
4. to make one SIG to own the feature once it has been merged. The SIG is responsible for maintaining, fixing, running, _everything-it_. | ||||||||
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.
Suggested change
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. Yes, yes, it was more like a small (failed) word game: maintain it 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. Shouldn't this process encourage a VEP author to become member of said SIG? Currently it sounds a bit like as a VEP author I can forget about the feature once it is merged and the SIG has to carry the burden of maintaining it. 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 would add that the original author is encouraged but not required to become a member of the SIG to help with on-going maintenance of the feature they are introducing. 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.
Do you mean "once it GAs"? Forgetting about a feature after developing it to the point it GAs sounds fair to me TBH.
I'm not against encouraging it, but not sure it's necessary TBH. These scenarios are completely valid IMO:
|
||||||||
|
||||||||
This has a few effects - see the following Goals section. | ||||||||
|
||||||||
## Goals | ||||||||
|
||||||||
1. The design process now has a mechanism to distribute designs among SIGs | ||||||||
2. SIG approvers are empowered to approve designs, increase the approvers pool | ||||||||
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. Unclear. SIG approvers can increase the approvers pool?
Suggested change
But does it really? Technically, most sigs have less approvers than kubevirt/community does today... 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. The second. Because we have sig approvers, we implicitly increase the pool. Yes, sigs have less approvers then root, but: the sum of sig approvers should venetually be larger than the amount of root approvers. |
||||||||
3. The ownership of an implemented feature becomes clear | ||||||||
4. Ensure that designs converge (accept, reject) | ||||||||
5. Formalize this process as an Virtualization Enhancement Proposal (VEP) process | ||||||||
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. Can't we just call it KEP (Kubevirt Enhancement Proposal)? 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. Yeah, I thought about this as well. My worry would be the confusion between kube and kubevirt KEPs. 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. KVEPs? 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. @fabiand yeah this is a valid concern imo |
||||||||
|
||||||||
## Non Goals | ||||||||
|
||||||||
1. Create unnecessary paperwork | ||||||||
|
||||||||
## Definition of Users | ||||||||
|
||||||||
* VEP Author - The person writing a design/enhancement proposal | ||||||||
* SIG - A formal KubeVirt SIG | ||||||||
* SIG Approver - A member of a SIG with approval permissions | ||||||||
|
||||||||
## User Stories | ||||||||
|
||||||||
* As a VEP Author I want to know who I can work with in order to move | ||||||||
my proposal forward | ||||||||
* As a SIG I want to have a say in what is getting pushed into my domain | ||||||||
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. Will this hurt adoption of new features in the long run if a SIG decides there is no capacity to maintain everything? I'm thinking of a constantly increasing maintenance load on the SIG while the count of members may not increase. 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. In an extreme scenario but the opposite is also possible if we continue to allow features to flow into the project without finer grain ownership of the design process right? 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. @lyarwood +1. It is better to understand that we don't have the capacity to maintain a feature. Now we can't even figure that out because there is no clear definition of ownership. |
||||||||
in order to make sure that we are able to maintain it | ||||||||
* As a SIG Approver I want to ensure that a design is sound before a | ||||||||
VEP Author is approaching an implementation. | ||||||||
fabiand marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
* As a KubeVirt developer, I want to have a single source-of-truth | ||||||||
w.r.t. the current status, design API of a feature. | ||||||||
|
||||||||
## Repos | ||||||||
|
||||||||
- https://github.com/kubevirt/enhancements | ||||||||
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. Can I suggest that we create and seed this repo with examples to help folks understand the proposal better? It can then also serve as a place to document follow ups or discussions we don't want to resolve prior to merging this document. Happy to help with this if you don't have the bandwidth. 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. Yes. Please see https://github.com/fabiand/enhancements |
||||||||
|
||||||||
# Design | ||||||||
|
||||||||
Key elements: | ||||||||
|
||||||||
- Ownership: SIGs own a _feature_ (which includes the process, but also | ||||||||
the resulting code) from it's inception (design) all the way (fixes, maintenance) to it's end (removal)[^1] | ||||||||
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.
Suggested change
|
||||||||
- Approvals: SIG approvers will be allowed to approve designs | ||||||||
- Responsibilities: SIG Approvers are responsible for driving a design, and connecting it to other SIGs as needed | ||||||||
|
||||||||
Process elements: | ||||||||
|
||||||||
- VEP Author creates a GitHub Issue for getting a unique identifier and starting the process | ||||||||
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: Add link to the repo? |
||||||||
- VEP Author creates a PR to propose the design targeting a specific SIG | ||||||||
- SIG decides on an approver to shepherd the VEP | ||||||||
- SIG collaborates with other SIGs to ensure its thoroughly reviewed | ||||||||
- SIG approves or rejects VEP | ||||||||
- Other SIG approvers can veto a proposal | ||||||||
- SIG owns future maintenance of the implementation | ||||||||
- KubeVirt Maintainers are responsible to support the owning SIG and VEP author in the case of conflicts and questions | ||||||||
Comment on lines
+69
to
+76
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. What's missing here IMO is the concept of treating the KEP/VEP as a "live-document" that's continuously being updated. That's how it works in k8s and I think it would benefit us as well. For example, after the KEP/VEP is merged and the implementation starts taking place the feature owners and/or SIG maintainers can decide to change the design (which could be APIs, behavior, etc). In such scenario the KEP/VEP should be updated with a PR. Another example where it is very important is updating the requirements and changes between alpha/beta/ga stages (which should be required in the future, see #286). When a feature is only being started it is very hard to think about the exact timeline or requirements for Beta/GA. It should be acceptable to leave these empty with the intention of updating them later when the feature matures and more feedback is granted. Another clear benefit from this approach is that the KEP/VEP remains the source of truth for the feature's accepted design. This is very different than the current situation where a DP can be merged, but usually gets irrelevant very quickly since design changes aren't being reflected back to it. BTW, in k8s the tracking issue remains open until the feature GAs. Maybe we want to document it here as well. 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. @iholder101 very well. What would you add to make it clear that it is a living doucment?
This is part of this proposal as well. If this is not clear, then it should be clarified. 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-a. I see your comment below. 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. WDYT about the following? I've highlighted new bullets
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 this is going in the right direction, I'll update the doc 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 still relevant I think |
||||||||
|
||||||||
Technical elements: | ||||||||
|
||||||||
- VEPs will live in a new dedicated repository `kubevirt/enhancements` | ||||||||
- `OWNER_ALIASES` will be mirrored from kubevirt/kubevirt in order to have the same SIGs in the EP repository | ||||||||
- Approvals and ownership is defined with `OWNERS` files in the `veps/sig-*` directories, tying into the general prow approval and merge flow | ||||||||
- GitHub Issues will be used to create unique identifiers | ||||||||
|
||||||||
## API Examples | ||||||||
|
||||||||
None. | ||||||||
|
||||||||
Comment on lines
+85
to
+88
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.
Suggested change
|
||||||||
## Scalability | ||||||||
|
||||||||
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: redundant newline |
||||||||
Instead of relying on a small approvers pool, now the process starts | ||||||||
with routing VEPs in the beginning of their life-time to SIGs. | ||||||||
This is expected to increase the time-to-merge-or-reject for VEPs. | ||||||||
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. Does this really increase the time? I would think spreading the load across a larger pool of possible approvers should speed up the process? 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. Yeah I think that should be decrease. |
||||||||
And to distribute the work better. | ||||||||
|
||||||||
## Update/Rollback Compatibility | ||||||||
|
||||||||
We move back to the community/design-proposals process. | ||||||||
|
||||||||
## Functional Testing Approach | ||||||||
|
||||||||
Require this process for KubeVirt v1.4 and onwards. | ||||||||
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. require from v1.5 |
||||||||
|
||||||||
# Implementation Phases | ||||||||
|
||||||||
Beta for KubeVirt v1.4 | ||||||||
GA in KubeVirt v1.5 | ||||||||
|
||||||||
Comment on lines
+105
to
+108
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. @fabiand Would you like some more help with the PR? perhaps adding collaborators? |
||||||||
[^1]: The exact feature life-cycle is under discussion in https://github.com/kubevirt/community/pull/251. This doc here should be updated once #251 got merged. |
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.
To me these two items essentially are the same, maybe merge them?