|
| 1 | +--- |
| 2 | +description: > |
| 3 | + Coding guidelines for kcp projects. |
| 4 | +--- |
| 5 | + |
| 6 | +# Coding Guidelines & Conventions |
| 7 | + |
| 8 | +- Always be clear about what clients or client configs target. Never use an unqualified `client`. Instead, always qualify. For example: |
| 9 | + - `rootClient` |
| 10 | + - `orgClient` |
| 11 | + - `pclusterClient` |
| 12 | + - `rootKcpClient` |
| 13 | + - `orgKubeClient` |
| 14 | +- Configs intended for `NewForConfig` (i.e. today often called "admin workspace config") should uniformly be called `clusterConfig` |
| 15 | + - Note: with org workspaces, `kcp` will no longer default clients to the "root" ("admin") logical cluster |
| 16 | + - Note 2: sometimes we use clients for same purpose, but this can be harder to read |
| 17 | +- Cluster-aware clients should follow similar naming conventions: |
| 18 | + - `crdClusterClient` |
| 19 | + - `kcpClusterClient` |
| 20 | + - `kubeClusterClient` |
| 21 | +- `clusterName` is a kcp term. It is **NOT** a name of a physical cluster. If we mean the latter, use `pclusterName` or similar. |
| 22 | +- Qualify "namespace"s in code that handle up- and downstream, e.g. `upstreamNamespace`, `downstreamNamespace`, and also `upstreamObj`, `downstreamObj`. |
| 23 | +- Logging: |
| 24 | + - Use the `fmt.Sprintf("%s|%s/%s", clusterName, namespace, name` syntax. |
| 25 | + - Default log-level is 2. |
| 26 | + - Controllers should generally log (a) **one** line (not more) non-error progress per item with `klog.V(2)` (b) actions like create/update/delete via `klog.V(3)` and (c) skipped actions, i.e. what was not done for reasons via `klog.V(4)`. |
| 27 | +- When orgs land: `clusterName` or `fooClusterName` is always the fully qualified value that you can stick into obj.ObjectMeta.ClusterName. It's not necessarily the `(Cluster)Workspace.Name` from the object. For the latter, use `workspaceName` or `orgName`. |
| 28 | +- Generally do `klog.Errorf` or `return err`, but not both together. If you need to make it clear where an error came from, you can wrap it. |
| 29 | +- New features start under a feature-gate (`--feature-gate GateName=true`). (At some point in the future), new feature-gates are off by default *at least* until the APIs are promoted to beta (we are not there before we have reached MVP). |
| 30 | +- Feature-gated code can be incomplete. Also their e2e coverage can be incomplete. **We do not compromise on unit tests**. Every feature-gated code needs full unit tests as every other code-path. |
| 31 | +- Go Proverbs are good guidelines for style: https://go-proverbs.github.io/ – watch https://www.youtube.com/watch?v=PAAkCSZUG1c. |
| 32 | +- We use Testify's [require](https://pkg.go.dev/github.com/stretchr/testify/require) a |
| 33 | + lot in tests, and avoid |
| 34 | + [assert](https://pkg.go.dev/github.com/stretchr/testify/assert). |
| 35 | + |
| 36 | + Note this subtle distinction of nested `require` statements: |
| 37 | + ```Golang |
| 38 | + require.Eventually(t, func() bool { |
| 39 | + foos, err := client.List(...) |
| 40 | + require.NoError(err) // fail fast, including failing require.Eventually immediately |
| 41 | + return someCondition(foos) |
| 42 | + }, ...) |
| 43 | + ``` |
| 44 | + and |
| 45 | + ```Golang |
| 46 | + require.Eventually(t, func() bool { |
| 47 | + foos, err := client.List(...) |
| 48 | + if err != nil { |
| 49 | + return false // keep trying |
| 50 | + } |
| 51 | + return someCondition(foos) |
| 52 | + }, ...) |
| 53 | + ``` |
| 54 | + The first fails fast on every client error. The second ignores client errors and keeps trying. Either |
| 55 | + has its place, depending on whether the client error is to be expected (e.g. because of asynchronicity making the resource available), |
| 56 | + or signals a real test problem. |
| 57 | + |
| 58 | +### Using Kubebuilder CRD Validation Annotations |
| 59 | + |
| 60 | +All of the built-in types for `kcp` are `CustomResourceDefinitions`, and we generate YAML spec for them from our Go types using [kubebuilder](https://github.com/kubernetes-sigs/kubebuilder). |
| 61 | + |
| 62 | +When adding a field that requires validation, custom annotations are used to translate this logic into the generated OpenAPI spec. [This doc](https://book.kubebuilder.io/reference/markers/crd-validation.html) gives an overview of possible validations. These annotations map directly to concepts in the [OpenAPI Spec](https://swagger.io/specification/#data-type-format) so, for instance, the `format` of strings is defined there, not in kubebuilder. Furthermore, Kubernetes has forked the OpenAPI project [here](https://github.com/kubernetes/kube-openapi/tree/master/pkg/validation) and extends more formats in the extensions-apiserver [here](https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go#L27). |
| 63 | + |
| 64 | + |
| 65 | +### Replicated Data Types |
| 66 | + |
| 67 | +Some objects are replicated and cached amongst shards when `kcp` is run in a sharded configuration. When writing code to list or get these objects, be sure to reference both shard-local and cache informers. To make this more convenient, wrap the look up in a function pointer. |
| 68 | + |
| 69 | +For example: |
| 70 | + |
| 71 | +```Golang |
| 72 | + |
| 73 | +func NewController(ctx, |
| 74 | + localAPIExportInformer, cacheAPIExportInformer apisinformers.APIExportClusterInformer |
| 75 | +) (*controller, error) { |
| 76 | + ... |
| 77 | + return &controller{ |
| 78 | + listAPIExports: func(clusterName logicalcluster.Name) ([]apisv1apha1.APIExport, error) { |
| 79 | + exports, err := localAPIExportInformer.Cluster(clusterName).Lister().List(labels.Everything()) |
| 80 | + if err != nil { |
| 81 | + return cacheAPIExportInformer.Cluster(clusterName).Lister().List(labels.Everything()) |
| 82 | + } |
| 83 | + return exports, nil |
| 84 | + ... |
| 85 | + } |
| 86 | +} |
| 87 | +``` |
| 88 | +
|
| 89 | +A full list of replicated resources is currently outlined in the [replication controller](https://github.com/kcp-dev/kcp/blob/main/pkg/reconciler/cache/replication/replication_controller.go). |
0 commit comments