Add generics policy for kubernetes #6693
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Background | ||
|
|
||
| In general, Kubernetes has not restricted using new Go features, we've quickly | ||
| adopted new standard library types and methods and will continue to do so. | ||
|
|
||
| Generally the latest stable go release is in use on the main development branch. | ||
| This includes all of the staging libraries (client-go etc.) that originate in the | ||
| main [kubernetes/kubernetes](https://github.com/kubernetes/kubernetes) repository. | ||
|
|
||
| In Kubernetes v1.24 we shipped Go 1.18 (which adds support for generics) | ||
| rather late in the release cycle, so we temporarily prohibited using generics | ||
| in case we ran into other issues and needed to roll back to unblock the release. | ||
| Now that v1.24.0 is out, use of generics should be allowed. | ||
|
|
||
| # Generics Policy | ||
|
|
||
| Generics may be used in Kubernetes starting in v1.25, with the following restrictions | ||
| applying only until v1.24 is [out of support][version-support]: | ||
|
|
||
| - Generics should **not** be used in Kubernetes libraries used across multiple Kubernetes | ||
| versions, that is the non "staged" libraries like: | ||
| - [k8s.io/utils](https://github.com/kubernetes/utils) | ||
| - [sigs.k8s.io/yaml](https://github.com/kubernetes-sigs/yaml) | ||
| - [k8s.io/klog](https://github.com/kubernetes/klog) | ||
| - etc. | ||
|
|
||
| - Generics should be **avoided** when writing Kubernetes bug fixes that are likely to be backported, to streamline cherry-picking to older release branches. | ||
|
|
||
| These restrictions should be considered lifted when v1.24 is out of support. | ||
|
|
||
| ## Recommendations for Reviewers | ||
|
|
||
| - Consider if proposed generics pull requests improve maintainability and readability. | ||
|
|
||
| - The current generics implementation is known to have some performance issues | ||
| depending on usage: consider requesting benchmarks before / after the changes. | ||
|
There was a problem hiding this comment. does the scalability sig have any suggestions on this ? There might be regressions, if this is in the api-machinery code path or core serialize/deserialize codes or at this point we are saying its unrestricted and we will take issues as we find it ? There was a problem hiding this comment. This should be at the best judgement of reviewers to request golang benchmarks etc. the same they would any other change that might impact performance, scalability testing should catch serious regressions. In my experience, core serialization codepath changes (e.g. suggestion to use a different json or gzip library) are heavily scrutinized by the relevant approvers and benchmarks are preformed. There was a problem hiding this comment. are there any links to the known perf issues to give hints to where generics might not be appropriate yet? There was a problem hiding this comment. I didn't want to put information into this doc that would become out of date as go improves and measuring > "knowing", but in the initial go1.18 implementation at least generics are only partially monomorphized by GCShape and have an additional indirection at runtime for the method dictionaries. So they pretty universally cost more, but that may still be justified by type safety and maintainability if you can keep it out of hot paths => avoid developer pain from code generation. https://planetscale.com/blog/generics-can-make-your-go-code-slower I'd like to think a suitable usage for now looks like: There was a problem hiding this comment.
fair enough |
||
|
|
||
| [version-support]: https://kubernetes.io/releases/patch-releases/#support-period | ||
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.
should we have guidance for use of generics in our libraries consumed by multiple branches of kubernetes, like k8s.io/utils, k8s.io/kube-openapi, etc? xref discussion in kubernetes/utils#243 (comment)
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.
Great point, we only discussed this from the staging library angle previously.
We should add a note similar to the bug fixes guidance, but as a hard requirement, I think.
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 rewrote this a bit to position limitations until v1.24 is out of support and address this point.
Uh oh!
There was an error while loading. Please reload this page.
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.
looks fine as a starting point. we could maybe relax the guidance to allow use of generics in those repos in ways that would still support go1.17 (go-version conditional files, etc), but I'm not familiar with how possible/fiddly that is... we'd want to be really confident in the implications before doing so, I guess
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.
Even if technically possible (I think so with build tags?) I don’t think we want to allow this versus waiting a few more releases just because it’s very similar to starting to support multiple branches of these libraries from a support perspective and we don’t do that today in any I can think of.