Skip to content

Latest commit

 

History

History
232 lines (162 loc) · 13.5 KB

CONTRIBUTING.md

File metadata and controls

232 lines (162 loc) · 13.5 KB
Raw

Contributing to tmc

tmc is Apache 2.0 licensed and we accept contributions via GitHub pull requests.

Please read the following guide if you're interested in contributing to tmc.

Certificate of Origin

By contributing to this project you agree to the Developer Certificate of Origin (DCO). This document was created by the Linux Kernel community and is a simple statement that you, as a contributor, have the legal right to make the contribution. See the DCO file for details.

Getting started

Prerequisites

  1. Clone this repository.
  2. Install Go (1.19+).
  3. Install kubectl.

Build & verify

  1. In one terminal, build and start tmc:
go run ./cmd/tmc start
  1. In another terminal, tell kubectl where to find the kubeconfig:
export KUBECONFIG=.tmc/admin.kubeconfig
  1. Confirm you can connect to tmc:
kubectl api-resources

Finding areas to contribute

Starting to participate in a new project can sometimes be overwhelming, and you may not know where to begin. Fortunately, we are here to help! We track all of our tasks here in GitHub, and we label our issues to categorize them. Here are a couple of handy links to check out:

You're certainly not limited to only these kinds of issues, though! If you're comfortable, please feel free to try working on anything that is open.

We do use the assignee feature in GitHub for issues. If you find an unassigned issue, comment asking if you can be assigned, and ideally wait for a maintainer to respond. If you find an assigned issue and you want to work on it or help out, please reach out to the assignee first.

Sometimes you might get an amazing idea and start working on a huge amount of code. We love and encourage excitement like this, but we do ask that before you embarking on a giant pull request, please reach out to the community first for an initial discussion. You could file an issue, send a discussion to our mailing list, and/or join one of our community meetings. For now we are still part of KCP community and we are using their community meeting for now.

Finally, we welcome and value all types of contributions, beyond "just code"! Other types include triaging bugs, tracking down and fixing flaky tests, improving our documentation, helping answer community questions, proposing and reviewing designs, etc.

Priorities & milestones

We prioritize issues and features both synchronously (during community meetings) and asynchronously (Slack/GitHub conversations).

We group issues together into milestones. Each milestone represents a set of new features and bug fixes that we want users to try out. We aim for each milestone to take about a month from start to finish.

You can see the current list of milestones in GitHub.

For a given issue or pull request, its milestone may be:

  • unset/unassigned: we haven't looked at this yet, or if we have, we aren't sure if we want to do it and it needs more community discussion
  • assigned to a named milestone
  • assigned to TBD - we have looked at this, decided that it is important and we eventually would like to do it, but we aren't sure exactly when

If you are confident about the target milestone for your issue or PR, please set it. If you don’t have permissions, please ask & we’ll set it for you.

Story tasks

Story tasks in an epic should generally represent an independent chunk of work that can be implemented. These don't necessarily need to be copied to standalone GitHub issues; it's ok if we just track the story in the epic as a task. On a case by case basis, if a story seems large enough that it warrants its own issue, we can discuss creating one.

Please tag yourself using your GitHub handle next to a story task you plan to work on. If you don't have permission to do this, please let us know by either commenting on the issue, or reaching out in Slack, and we'll assist you.

When you open a PR for a story task, please edit the epic description and add a link to the PR next to your task.

When the PR has been merged, please make sure the task is checked off in the epic.

Tracking work

Issue status and project board

We use the Github projects beta for project management, compare our project board. Please add issues and PRs into the kcp project and update the status (new, in-progress, ...) for those you are actively working on.

Unplanned/untracked work

If you find yourself working on something that is unplanned and/or untracked (i.e., not an open GitHub issue or story task in an epic), that's 100% ok, but we'd like to track this type of work too! Please file a new issue for it, and when you have a PR ready, mark the PR as fixing the issue.

Coding guidelines & conventions

  • Always be clear about what clients or client configs target. Never use an unqualified client. Instead, always qualify. For example:

    • rootClient
    • orgClient
    • pclusterClient
    • rootKcpClient
    • orgKubeClient

  • Configs intended for NewForConfig (i.e. today often called "admin workspace config") should uniformly be called clusterConfig

    • Note: with org workspaces, kcp will no longer default clients to the "root" ("admin") logical cluster
    • Note 2: sometimes we use clients for same purpose, but this can be harder to read

  • Cluster-aware clients should follow similar naming conventions:

    • crdClusterClient
    • kcpClusterClient
    • tmcClusterClient
    • kubeClusterClient

  • clusterName is a kcp term. It is NOT a name of a physical cluster. If we mean the latter, use pclusterName or similar.

  • In the syncer: upstream = kcp, downstream = pcluster. Depending on direction, "from" and "to" can have different meanings. source and sink are synonyms for upstream and downstream.

  • Qualify "namespace"s in code that handle up- and downstream, e.g. upstreamNamespace, downstreamNamespace, and also upstreamObj, downstreamObj.

  • Logging:

    • Use the fmt.Sprintf("%s|%s/%s", clusterName, namespace, name syntax.
    • Default log-level is 2.
    • 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).

  • 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.

  • 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.

  • 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).

  • 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.

  • Go Proverbs are good guidelines for style: https://go-proverbs.github.io/ – watch https://www.youtube.com/watch?v=PAAkCSZUG1c.

  • We use Testify's require a lot in tests, and avoid assert.

    Note this subtle distinction of nested require statements:

    require.Eventually(t, func() bool {
  foos, err := client.List(...)
  require.NoError(err) // fail fast, including failing require.Eventually immediately
  return someCondition(foos)
}, ...)

    and

    require.Eventually(t, func() bool {
  foos, err := client.List(...)
  if err != nil {
     return false // keep trying
  }
  return someCondition(foos)
}, ...)

    The first fails fast on every client error. The second ignores client errors and keeps trying. Either has its place, depending on whether the client error is to be expected (e.g. because of asynchronicity making the resource available), or signals a real test problem.

Using Kubebuilder CRD Validation Annotations

All of the built-in types for tmc-kcp are CustomResourceDefinitions, and we generate YAML spec for them from our Go types using kubebuilder.

When adding a field that requires validation, custom annotations are used to translate this logic into the generated OpenAPI spec. This doc gives an overview of possible validations. These annotations map directly to concepts in the OpenAPI Spec so, for instance, the format of strings is defined there, not in kubebuilder. Furthermore, Kubernetes has forked the OpenAPI project here and extends more formats in the extensions-apiserver here.

Getting your PR Merged

The tmc project uses OWNERS files to denote the collaborators who can assist you in getting your PR merged. There are two roles: reviewer and approver. Merging a PR requires sign off from both a reviewer and an approver.

Continuous Integration

tmc uses a combination of GitHub Actions

Debugging flakes

Tests that sometimes pass and sometimes fail are known as flakes. Sometimes, there is only an issue with the test, while other times, there is an actual bug in the main code. Regardless of the root cause, it's important to try to eliminate as many flakes as possible.

Unit test flakes

If you're trying to debug a unit test flake, you can try to do something like this:

go test -race ./pkg/reconciler/apis/apibinding -run TestReconcileBinding -count 100 -failfast

This tests one specific package, running only a single test case by name, 100 times in a row. It fails as soon as it encounters any failure. If this passes, it may still be possible there is a flake somewhere, so you may need to run it a few times to be certain. If it fails, that's a great sign - you've been able to reproduce it locally. Now you need to dig into the test condition that is failing. Work backwards from the condition and try to determine if the condition is correct, and if it should be that way all the time. Look at the code under test and see if there are any reasons things might not be deterministic.

End to end test flakes

Debugging an end-to-end (e2e) test that is flaky can be a bit trickier than a unit test. Our e2e tests run in one of two modes:

  1. Tests share a single kcp server
  2. Tests in a package share a single kcp server

The e2e-shared-server CI job uses mode 1, and the e2e CI job uses mode 2.

There are also a handful of tests that require a fully isolated kcp server, because they manipulate some configuration aspects that are system-wide and would break all the other tests. These tests run in both e2e and e2e-shared-server, separate from the other kcp instance(s).

You can use the same run, -count, and -failfast settings from the unit test section above for trying to reproduce e2e flakes locally. Additionally, if you would like to operate in mode 1 (all tests share a single kcp server), you can start a kcp instance locally in a separate terminal or tab:

bin/test-server

Then, to have your test use that shared kcp server, you add -args --use-default-kcp-server to your go test run:

go test ./test/e2e/apibinding -count 20 -failfast -args --use-default-kcp-server

Community Roles

Reviewers

Reviewers are responsible for reviewing code for correctness and adherence to standards. Oftentimes reviewers will be able to advise on code efficiency and style as it relates to golang or project conventions as well as other considerations that might not be obvious to the contributor.

Approvers

Approvers are responsible for sign-off on the acceptance of the contribution. In essence, approval indicates that the change is desired and good for the project, aligns with code, api, and system conventions, and appears to follow all required process including adequate testing, documentation, follow ups, or notifications to other areas who might be interested or affected by the change.

Approvers are also reviewers.

Management of OWNERS files

If a reviewer or approver no longer wishes to be in their current role it is requested that a PR be opened to update the OWNERS file. OWNERS files may be periodically reviewed and updated based on project activity or feedback to ensure an acceptable contributor experience is maintained.