Adjust API review process for CRDs and SIGs/subprojects

[jbeda]

There was an email thread to SIG-arch talking about this. Here are some concrete proposals that we can turn into action. I'm opting to have the discussion here in GitHub.

I agree with the start of that thread that having some clarity here would be good. I also agree with Tim that the process, as it stands today, isn’t scaling.

For reference, the (very verbose) document on the API review process is here: https://github.com/kubernetes/community/blob/master/sig-architecture/api-review-process.md.

I think that, in practicality, we already diverge from what is going on there. And the process clearly isn’t scaling.

I propose the following changes/updates:

• We have looser rules for features implemented in SIGs via CRDs. The whole goal of our extension points is to allow for faster movement and iteration. Introducing more gatekeeping runs counter to that.
• We introduce a new level of API review requirement (beyond mandatory and voluntary) called “As Available”. This would mean that the API review team would have (say) 2 weeks to give a review. If that time limit expires then the change can move forward.
• We remove the requirement for a KEP for API changes that are wholly within a SIG and implemented via CRDs. If the change is more far ranging than a SIG then it should require a KEP and be reviewed/reviewable by SIG-arch.
• The process to expand the reviewer pool clearly isn’t working. There are no new reviewers over the last year+. Until we have an expanded set of reviewers, most CRDs will probably not get a review.
• We ensure that API reviews are only for syntactic consistency. Other concern (such as the data model implied by the API) are within the purview of the SIG/sub-project itself. If there are concerns there and these are cross-cutting for the project, then this should be handled as a cross-cutting KEP review at the SIG-arch level and not as an API review. Most/all CRDs won’t have this type of impact.

[jbeda]

/sig architecture

[liggitt]

I propose the following changes/updates:
...
We have looser rules for features implemented in SIGs via CRDs. The whole goal of our extension points is to allow for faster movement and iteration. Introducing more gatekeeping runs counter to that.
We remove the requirement for a KEP for API changes that are wholly within a SIG and implemented via CRDs. ...

There are a few categories of uses of CRDs:

1. APIs that are experimental, alpha, or otherwise unsupported
2. APIs that are in support of "internal" tooling and are unsupported
3. APIs that intend to be published and supported

For the first two categories, we should find ways to clearly delineate the lack of support to avoid confusing users, and find a lightweight way to avoid accidental use of the k8s.io namespace, but otherwise not get too bogged down (xref https://github.com/kubernetes/enhancements/pull/1111/files#diff-6a82fa3b4e6aaa14971d14f41b95c812R113).

For the last category, the fact that they are made using a CRD is only an implementation detail. The concerns about consistency and compatibility are no less present than with built-in APIs. In many ways, it is easier to accidentally break compatibility with a CRD-backed API (and harder to recover from it), so the need for guardrails is even greater. Ideally, toolchains for creating CRDs (like kubebuilder) would add support for managing releases, saving off test data and structs to be able to automatically test the current schema against previous releases of the custom resource. Until tooling guardrails like that exist, I only see skipping review leading to worse outcomes for API consumers.

There are no new reviewers over the last year+.

22 reviewers, targeted by SIG area, were identified during the v1.15 release. See https://github.com/kubernetes/kubernetes/blob/master/OWNERS_ALIASES#L357-L405 and https://groups.google.com//forum/#!topic/kubernetes-sig-leads/xoMawT5WIgE for details.

And the process clearly isn’t scaling.

Sacrificing quality for velocity isn't appropriate for supported API surface. To help scale, I'd encourage investment in toolchains, and for SIGs to identify people to participate in the review process.

[dims]

/area api

[dims]

Also please see traffic here for context - kubernetes/enhancements#1111 (comment)

[jbeda]

First -- I didn't know that there were more reviewers per SIG area. That is good news. I'd expect he list of reviewers in the main doc to be updated with that. Perhaps a pointer to the OWNERS_ALIASES file? (or perhaps we should break this into smaller files?). But in any case, I mispoke -- there are no new approvers. That is still a long pole.

As for your categories -- I'd say that we can slice this finer. I'd break the last category into things that ship as part of the core and are exposed to end users of every k8s cluster. Those things that are optional and/or shipped outside of that core don't have the same implied support.

And when we say "supported" here, what exactly do we support? Is that something we have defined someplace?

"Sacrificing quality for velocity" is a stark way to frame this. I'd say that we need to make the appropriate tradeoffs between autonomy and centralized control. Right now, there are just too few people bottlenecking this process for parts of the project that may not need it.

[timothysc]

I'll try to PR the current policy to frame the discussion, and propose some options on guidelines.

/assign @timothysc

[thockin]

As an approver and sometimes bottleneck, I still agree with the general thrust of the proposal in the thread and in kubernetes/enhancements#1111 .

Everyone wants to be part of the kubernetes umbrella, and I keep telling them "this comes at a cost". This is part of the cost. If you don't want to be beholden to Kubernetes, don't be.

I do agree that these sorts of reviews should be about API, but the line between syntax/style and semantics is often blurry.

[timothysc]

"Everyone wants to be part of the kubernetes umbrella"

I think is a misnomer, and we made the clarifications in steering of the layers of core/sigs/associated with that intent in mind. What is missing in API review document is a set of guidelines and practices that echos this model and makes the expectations explicit to the end consumer.

For example, in some of our subprojects we explicitly state that it is a non-goal to be in core. They are opt-in extensions or services.

[cblecker]

Everyone wants to be part of the kubernetes umbrella, and I keep telling them "this comes at a cost". This is part of the cost. If you don't want to be beholden to Kubernetes, don't be.

+100.