From 6d248850ecd979a8b8b7767b085c560f5493c475 Mon Sep 17 00:00:00 2001 From: Aldo Culquicondor Date: Thu, 17 Aug 2023 15:10:02 -0400 Subject: [PATCH 1/2] Add style and technical guidelines for SIG Scheduling --- sig-scheduling/CONTRIBUTING.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/sig-scheduling/CONTRIBUTING.md b/sig-scheduling/CONTRIBUTING.md index e9b3b38e3ce..5d222d03ed7 100644 --- a/sig-scheduling/CONTRIBUTING.md +++ b/sig-scheduling/CONTRIBUTING.md @@ -72,6 +72,26 @@ to the issue/PR, which helps to preserve the context. * Always open an issue for a TODO or a follow-up just in case you forget it. +### Technical and style guidelines + +The following guidelines apply primarily to kube-scheduler, but some subprojects +might also adhere to them. + +- Follow [effective go](https://go.dev/doc/effective_go) guidelines. +- Naming: + - Length: As a rule-of-thumb, the length of a variable name should be + proportional to the size of the scope where it is used and inversely + proportional to the number of times that it is used. +- In tests: + - Follow a [DAMP principle](https://stackoverflow.com/a/11837973). + - Use `cmp.Diff` instead of `reflect.Equal`, to provide useful comparisons. + - Avoid creating or using assertion libraries. + Use standard `t.Error` or `t.Fatal`, as necessary. + +Note that some existing code might be in violation of these guidelines, as it +might have been written before these guidelines were established. Feel free to +open PRs to get the code up to the standard. + ## Use of @mentions * @kubernetes/sig-scheduling-api-reviews - API Changes and Reviews From 89e913202520dd3a04bbbe8a2f0ee98f5d30f197 Mon Sep 17 00:00:00 2001 From: Aldo Culquicondor Date: Fri, 18 Aug 2023 10:36:13 -0400 Subject: [PATCH 2/2] Add more design and testing guidelines --- sig-scheduling/CONTRIBUTING.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/sig-scheduling/CONTRIBUTING.md b/sig-scheduling/CONTRIBUTING.md index 5d222d03ed7..1584b9ec046 100644 --- a/sig-scheduling/CONTRIBUTING.md +++ b/sig-scheduling/CONTRIBUTING.md @@ -37,7 +37,8 @@ also follow the requirements of `Bug Report`, it will help us a lot when analyzi * If there's any debate on the rationalities, you can bring it to the [SIG meeting](https://github.com/kubernetes/community/tree/master/sig-scheduling#meetings). * If there are multiple implementation options, consider creating a doc with Pros and Cons, and gather some feedback. You can do this by sharing the doc with the SIG's [mailing list](https://groups.google.com/forum/#!forum/kubernetes-sig-scheduling). - * If necessary, open an issue in kubernetes/enhancements and write a [KEP](https://github.com/kubernetes/enhancements/tree/master/keps/sig-scheduling) for it. + * Any feature that requires an API change or significant refactoring + should be preceded by a [Kubernetes Enhancement Proposal (KEP)](https://github.com/kubernetes/enhancements/tree/master/keps/sig-scheduling). * If you find any out-of-date documentation, please help the community correct that by either sending a PR to update the docs or open an issue if you are not sure about how to fix it. @@ -77,16 +78,39 @@ to the issue/PR, which helps to preserve the context. The following guidelines apply primarily to kube-scheduler, but some subprojects might also adhere to them. +When designing a feature, think about components that depend on kube-scheduler +code, such as [cluster-autoscaler](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler) +or [scheduler-plugins](https://github.com/kubernetes-sigs/scheduler-plugins). +Also consider interactions with other core components such as [kubelet](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/). + +When coding: - Follow [effective go](https://go.dev/doc/effective_go) guidelines. +- Use [contextual logging](https://git.k8s.io/community/contributors/devel/sig-instrumentation/migration-to-structured-logging.md#contextual-logging-in-kubernetes). + Some packages might still be using [structured logging](https://git.k8s.io/community/contributors/devel/sig-instrumentation/logging.md). +- When writing APIs, follow [k8s API conventions](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md) - Naming: - Length: As a rule-of-thumb, the length of a variable name should be proportional to the size of the scope where it is used and inversely proportional to the number of times that it is used. -- In tests: + +Testing: +- Unit tests: every change should have high coverage by unit tests. +- Integration tests: should cover interactions between the different components + of kube-scheduler (event handlers, queue, cache, scheduling cycles) and + kube-apiserver. +- E2E tests: should cover interactions with other components, such as kubelet, + kube-controller-manager, etc. +- [Perf tests](https://github.com/kubernetes/kubernetes/tree/master/test/integration/scheduler_perf): + should be considered for critical and/or CPU intensive operations. +- General guidelines: - Follow a [DAMP principle](https://stackoverflow.com/a/11837973). - Use `cmp.Diff` instead of `reflect.Equal`, to provide useful comparisons. + - Compare errors using `errors.Is` (`cmpopts.EquateErrors` when using + `cmp.Diff`) instead of comparing the error strings. + - Leverage existing utility functions from `pkg/scheduler/testing`. - Avoid creating or using assertion libraries. Use standard `t.Error` or `t.Fatal`, as necessary. + - `gomega` and `ginkgo` should only be used in E2E tests. Note that some existing code might be in violation of these guidelines, as it might have been written before these guidelines were established. Feel free to