enhancements: Propose a more formal yet lean enhancement process #288
Conversation
kubevirt-bot
added
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 |
|
@EdDev @alaypatel07 @davidvossel @vladikr @rmohr @xpivarc @alicefr @acardace please review |
|
@aburdenthehand please review as well (you get this as your very own comment to invite you for review :) ) |
|
/cc |
|
/cc |
design-proposals/enhancements.md
Outdated
| - VEP Author creates a PR to propose the design targeting a specific SIG | ||
| - SIG decides on an approver to sheppered the VEP |
There was a problem hiding this comment.
VEP Author creates a PR to propose the design targeting a specific SIG
How do we guarantee that SIGS are responsive to these PRs?
Said differently, how are we enforcing the PRs are triaged rather than rotting due to inactivity?
There was a problem hiding this comment.
Later on the document talks about mirroring the OWNERS files from core kubevirt/kubevirt so I would assume the core approval group would end up with this responsibility.
It might be cleaner to define a meta SIG like sig-architecture in k8s who manage the KEP process and keep PRs flowing there?
There was a problem hiding this comment.
Generally I think each SIG needs to have a representative in order to make sure someone from the SIG is reviewing PRs/proposals/issues, otherwise everyone can say the next one will have a look. I'll have a look what we have written down for now and will propose this.
It might be cleaner to define a meta SIG like sig-architecture in k8s who manage the KEP process and keep PRs flowing there?
I think this could be beneficial for more reasons, common solutions on common problems across SIGs and so on. Therefore +1 from me
There was a problem hiding this comment.
The SIG owns a PR. If the SIG is not responsible, then we should define a group which a VEP author can reach out to get some support.
I do not think that all PRs should run through i.e. something like sig-architecture.
A core principle of this proposal is to distribute the load.
At best a group like a throectical sig-architecture could act as a moderator between SIG and VEP author, but must not be directly involved with a VEP, unless the VEP is owned by this (arch) SIG.
@davidvossel would it be good enough for you to say: "In case of an unresponsive SIG, the VPE author can consult the KubeVirt Maintainers in order to resolve the situation."?
There was a problem hiding this comment.
I've added:
- KubeVirt Maintainers are responsible to support the owning SIG and VEP author in the case of conflicts and questions
There was a problem hiding this comment.
How about adding a label stating that a design proposal doesn't have an owning SIG yet? Something like triage/needs-owning-sig?
This way it would be more visible that the design proposal needs to be assigned + it would be easier querying such VEPs
There was a problem hiding this comment.
Yes, that's a good idea. How good are you in prow automation coding, Itamar?
There was a problem hiding this comment.
I have near-to-zero experience with it TBH :)
But maybe it's time to dive in. I'll try getting to it.
There was a problem hiding this comment.
Thanks :)
FWIW - I think that we need approvers and "potential chairs" to really look into automation to simply help the process. the appovers and pr authors.
design-proposals/enhancements.md
Outdated
|
|
||
| Require this process for KubeVirt v1.3+ | ||
|
|
There was a problem hiding this comment.
Previously a formal design was never required, but encouraged when the complexity of a feature was high.
Are we saying now that the design process is required? If so, where do we draw the line for what needs a design and what does not?
A part of the reason the guideline around what needs a design and what doesn't has been so relaxed in the past is that drawing the line is difficult. if we're too strict, than we've introduced needless bureaucracy. Too relaxed, and someone risks implementing a bunch of code that get's rejected.
There was a problem hiding this comment.
+1 I wouldn't change the requirements here and would reword this as something like strongly encourage while also highlighting the benefits of getting a design reviewed alongside a PR etc.
There was a problem hiding this comment.
What about requiring a proposal if an new API is being proposed? The API is the biggest burden to maintain ,imho, so making sure we don't overextend it seems reasonable. Also, nowadays we have 3 months dev cycles while in the past we had only one month, therefore proposals are less likely to stall progress(if we do good work reviewing them...).
There was a problem hiding this comment.
I wouldn't be against mandating this for enhancements touching the API for the 1.4 release cycle but I think our lives would be easier setting the bar a little lower until the process fully GAs.
There was a problem hiding this comment.
Let me slice it into two parts:
- When does this proposal become effective (a new way to process features)
- What features are - covered in Define the term "Feature" #285
I merely meant to say:
For whatever feature is proposed accroding to current standards, from 1.3 on, let's use this newly proposed process.
This proposal does not change the requirement for when a design is required.
Simply put: If we'd require a design today, then - in future - use this new process instead of the old one.
There was a problem hiding this comment.
FWIW this should also be clear from the doc itself, as the proposal itself does not touch what a feature is.
There was a problem hiding this comment.
|
|
||
| ## Repos | ||
|
|
||
| - https://github.com/kubevirt/enhancements |
There was a problem hiding this comment.
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.
Yes.
Please see https://github.com/fabiand/enhancements
This includes this proposal as a dry run, and I've got a PR to convert all existing - former - designs into this new format, specifically assigning primary sig owners for former designs.
design-proposals/enhancements.md
Outdated
| - VEP Author creates a PR to propose the design targeting a specific SIG | ||
| - SIG decides on an approver to sheppered the VEP |
There was a problem hiding this comment.
Later on the document talks about mirroring the OWNERS files from core kubevirt/kubevirt so I would assume the core approval group would end up with this responsibility.
It might be cleaner to define a meta SIG like sig-architecture in k8s who manage the KEP process and keep PRs flowing there?
design-proposals/enhancements.md
Outdated
|
|
||
| Require this process for KubeVirt v1.3+ | ||
|
|
There was a problem hiding this comment.
+1 I wouldn't change the requirements here and would reword this as something like strongly encourage while also highlighting the benefits of getting a design reviewed alongside a PR etc.
|
xlink #286 |
Signed-off-by: Fabian Deutsch <[email protected]>
Signed-off-by: Fabian Deutsch <[email protected]>
fabiand
force-pushed
the
propose-enhancements
branch
from
c250ba0
to
a5e328b
Compare
Signed-off-by: Fabian Deutsch <[email protected]>
|
Thanks for the review folks, Ive addressed most comments. |
| 2. SIG approvers are empowered to approve designs, increase the approvers pool | ||
| 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.
Can't we just call it KEP (Kubevirt Enhancement Proposal)?
I feel like using VEP will be confusing. WDYT?
There was a problem hiding this comment.
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.
@fabiand yeah this is a valid concern imo
| - VEP Author creates a GitHub Issue for getting a unique identifier and starting the process | ||
| - 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 |
There was a problem hiding this comment.
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.
@iholder101 very well. What would you add to make it clear that it is a living doucment?
BTW, in k8s the tracking issue remains open until the feature GAs.
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.
Ah-a. I see your comment below.
There was a problem hiding this comment.
WDYT about the following? I've highlighted new bullets
- VEP Author creates a GitHub Issue for getting a unique identifier and starting the process
- VEP Author creates a PR to propose the design targeting a specific SIG
- VEP Author is expected to list requirements and goals for Alpha
- Preferably, VEP Author also vaguely lists requirements for Beta/GA.
- 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
- VEP author should make sure that the VEP is the source of truth w.r.t. the VEP's status (design / implementations / goals / etc)
- VEP author should create more PRs to update the VEP whenever it is not reflecting the updated VEP's status
- VEP author updates the requirements for Beta/GA as they become clearer
- VEP author updates implementation phases for Beta/GA as they become clearer
- KubeVirt Maintainers are responsible to support the owning SIG and VEP author in the case of conflicts and questions
There was a problem hiding this comment.
I think this is going in the right direction, I'll update the doc
There was a problem hiding this comment.
This is still relevant I think
| None. | ||
|
|
||
| ## Scalability | ||
|
|
Signed-off-by: Fabian Deutsch <[email protected]>
| 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.
| 4. to make one SIG to own the feature once it has been merged. The SIG is responsible for maintaining, fixing, running, _everything-it_. | |
| 4. to make one SIG 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.
Yes, yes, it was more like a small (failed) word game:
maintain it
fix it
run it
everything it ....
| 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.
| the resulting code) from it's inception (design) all the way (fixes, maintenance) to it's end (removal)[^1] | |
| the resulting code) from its inception (design) all the way (fixes, maintenance) to its end (removal)[^1] |
| ## API Examples | ||
|
|
||
| None. | ||
|
|
There was a problem hiding this comment.
| ## API Examples | |
| None. |
| ## 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.
Unclear. SIG approvers can increase the approvers pool?
Or, the fact that SIG approvers can approve designs increases the approvers pool? In which case:
| 2. SIG approvers are empowered to approve designs, increase the approvers pool | |
| 2. SIG approvers are empowered to approve designs, increasing the approvers pool |
But does it really? Technically, most sigs have less approvers than kubevirt/community does today...
There was a problem hiding this comment.
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.
|
@jean-edouard @iholder101 @lyarwood @0xFelix @vladikr @xpivarc i'd appreciate your opinion to start moving fwd with this. |
|
|
||
| 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.
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.
Yeah I think that should be decrease.
| 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.
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.
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.
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.
Do you mean "once it GAs"?
If you'd forget about the feature before it GAs, it will eventually be dropped.
Forgetting about a feature after developing it to the point it GAs sounds fair to me TBH.
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.
I'm not against encouraging it, but not sure it's necessary TBH. These scenarios are completely valid IMO:
- Someone develops a feature under a certain SIG but is interested only in the feature's narrow scope, not the entire SIG. For example, someone can develop a network feature under sig-network, but care about this specific feature and not about SIG network generally.
- Someone can develop a feature, own and maintain it for a while, then stop maintaining it (due to switching his position, becoming interested in other stuff, etc). IOW, we should expect that people will move around. In any case the SIG always owns all of its features while people can stop maintaining them.
| 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) |
There was a problem hiding this comment.
To me these two items essentially are the same, maybe merge them?
|
|
||
| * 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.
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.
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.
@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.
|
|
||
| Process elements: | ||
|
|
||
| - VEP Author creates a GitHub Issue for getting a unique identifier and starting the process |
| 2. SIG approvers are empowered to approve designs, increase the approvers pool | ||
| 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 |
|
|
||
| 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.
Yeah I think that should be decrease.
|
|
||
| ## Functional Testing Approach | ||
|
|
||
| Require this process for KubeVirt v1.4 and onwards. |
|
|
||
| * 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.
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?
| 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.
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.
|
@fabiand @0xFelix @jean-edouard @lyarwood @davidvossel I think this proposal is very important and would serve as a perfect alternative to the current "design proposal" which is not owned by any SIG, doesn't serve as a live-document until the feature GAs, and so on. What's left in order to get this in? Does anyone have any objections? |
Honestly I'm a bit concerned about this. |
Man, that's an interesting thought! :) As I see it, this proposal has two main goals:
We aim to address (2) by introducing SIGs and giving them ownership and responsibility of the KEPs in their domain. This way SIGs can approve proposals without waiting for many "uber-approvers" to chime in the discussion, and have the responsibility to respond in a reasonable time frame.
I understand your point. However, I'm not sure it will hurt us in practice for a few reasons.
Let me know what you think |
|
@jean-edouard ping :) @fabiand WDYT about splitting this proposal into two - one for using a tracking issue with graduation requirements etc, and another one for the SIG ownership part? I support them both, but perhaps it'll be easier to merge this way. |
|
|
||
| Beta for KubeVirt v1.4 | ||
| GA in KubeVirt v1.5 | ||
|
|
There was a problem hiding this comment.
@fabiand Would you like some more help with the PR? perhaps adding collaborators?
What this PR does / why we need it:
Today it's a very small group
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 the heart this proposal is requiring
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:
Please see the "preview" at https://github.com/fabiand/enhancements
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.
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)Testing: New code requires new unit tests. New features and bug fixes require at least on e2e testRelease note: