How To Contribute

Clone and Provision Environment

  1. Make sure you have a GitHub account

  2. Clone the cilium repository into your GOPATH.

    1. mkdir -p $GOPATH/src/github.com/cilium
    2. cd $GOPATH/src/github.com/cilium
    3. git clone https://github.com/cilium/cilium.git
    4. cd cilium

  3. Set up your Development Setup.

  4. Check the GitHub issues for good tasks to get started.

Submitting a pull request

Contributions must be submitted in the form of pull requests against the github repository at: https://github.com/cilium/cilium.

  1. Fork the Cilium repository to your own personal GitHub space or request access to a Cilium developer account on Slack
  2. Push your changes to the topic branch in your fork of the repository.
  3. Submit a pull request on https://github.com/cilium/cilium.

Before hitting the submit button, please make sure that the following requirements have been met:

  1. Each commit compiles and is functional on its own to allow for bisecting of commits.

  2. All code is covered by unit and/or runtime tests where feasible.

  3. All changes have been tested and checked for regressions by running the existing testsuite against your changes. See the End-To-End Testing Framework section for additional details.

  4. All commits contain a well written commit description including a title, description and a Fixes: #XXX line if the commit addresses a particular GitHub issue. Note that the GitHub issue will be automatically closed when the commit is merged.

    1. apipanic: Log stack at debug level
    3. Previously, it was difficult to debug issues when the API panicked
    4. because only a single line like the following was printed:
    6. level=warning msg="Cilium API handler panicked" client=@ method=GET
    7. panic_message="write unix /var/run/cilium/cilium.sock->@: write: broken
    8. pipe"
    10. This patch logs the stack at this point at debug level so that it can at
    11. least be determined in developer environments.
    13. Fixes: #4191
    15. Signed-off-by: Joe Stringer <joe@cilium.io>

  5. If any of the commits fixes a particular commit already in the tree, that commit is referenced in the commit message of the bugfix. This ensures that whoever performs a backport will pull in all required fixes:

    1. daemon: use endpoint RLock in HandleEndpoint
    3. Fixes: a804c7c7dd9a ("daemon: wait for endpoint to be in ready state if specified via EndpointChangeRequest")
    5. Signed-off-by: André Martins <andre@cilium.io>

  6. If you change CLI arguments of any binaries in this repo, the CI will reject your PR if you don’t also update the command reference docs. To do so, make sure to run the postcheck make target.

    1. $ make postcheck
    2. $ git add Documentation/cmdref
    3. $ git commit

  7. All commits are signed off. See the section Coordination.

  8. Document any user-facing or breaking changes in Documentation/install/upgrade.rst.

  9. (optional) Pick the appropriate milestone for which this PR is being targeted, e.g. 1.6, 1.7. This is in particular important in the time frame between the feature freeze and final release date.

  10. If you have permissions to do so, pick the right release-note label. These labels will be used to generate the release notes which will primarily be read by users.

    LabelsWhen to set
    release-note/bugThis is a non-trivial bugfix and is a user-facing bug
    release-note/majorThis is a major feature addition, e.g. Add MongoDB support
    release-note/minorThis is a minor feature addition, e.g. Add support for a Kubernetes version
    release-note/miscThis is a not user-facing change , e.g. Refactor endpoint package, a bug fix of a non-released feature
    release-note/ciThis is a CI feature of bug fix.

  11. Verify the release note text. If not explicitly changed, the title of the PR will be used for the release notes. If you want to change this, you can add a special section to the description of the PR. These release notes are primarily going to be read by users so it is important that release notes for bugs, major and minor features do not contain internal details of Cilium functionality which sometimes are irrelevant for users.

    Example of a bad release note

    1. ```release-note
    2. Fix concurrent access in k8s watchers structures
    3. ```

    Example of a good release note

    1. ```release-note
    2. Fix panic when Cilium received an invalid Cilium Network Policy from Kubernetes
    3. ```

    Note

    If multiple lines are provided, then the first line serves as the high level bullet point item and any additional line will be added as a sub item to the first line.

  12. If you have permissions, pick the right labels for your PR:

    LabelsWhen to set
    kind/bugThis is a bugfix worth mentioning in the release notes
    kind/enhancementThis enhances existing functionality in Cilium
    kind/featureThis is a feature
    release-blocker/X.YThis PR should block the next X.Y release
    needs-backport/X.YPR needs to be backported to these stable releases
    backport/X.YThis is backport PR, may only be set as part of Backporting process
    upgrade-impactThe code changes have a potential upgrade impact
    area/* (Optional)Code area this PR covers

  13. Open a draft pull request. GitHub provides the ability to create a Pull Request in “draft” mode. On the “New Pull Request” page, below the pull request description box there is a button for creating the pull request. Click the arrow and choose “Create draft pull request”. If your PR is still a work in progress, please select this mode. You will still be able to run the CI against it. Once the PR is ready for review you can click in “Ready for review” button at the bottom of the page” and reviewers will start reviewing. When you are actively changing your PR, set it back to draft PR mode to signal that reviewers do not need to spend time reviewing the PR right now. When it is ready for review again, mark it as such.

https://i1.wp.com/user-images.githubusercontent.com/3477155/52671177-5d0e0100-2ee8-11e9-8645-bdd923b7d93b.gif

Getting a pull request merged

  1. As you submit the pull request as described in the section Submitting a pull request. One of the reviewers will start a CI run by replying with a comment test-me-please as described in Triggering Pull-Request Builds With Jenkins. If you are a core team member, you may trigger the CI run yourself.

    1. Hound: basic golang/lint static code analyzer. You need to make the puppy happy.

    2. CI / Jenkins: Will run a series of tests:

      1. Unit tests
      2. Single node runtime tests
      3. Multi node Kubernetes tests

      If a CI test fails which seems unrelated to your PR, it may be a flaky test. Follow the process described in CI Failure Triage.

  2. As part of the submission, GitHub will have requested a review from the respective code owners according to the CODEOWNERS file in the repository.

    1. Address any feedback received from the reviewers
    2. You can push individual commits to address feedback and then rebase your branch at the end before merging.

  3. Owners of the repository will automatically adjust the labels on the pull request to track its state and progress towards merging.

  4. Once the PR has been reviewed and the CI tests have passed, the PR will be merged by one of the repository owners. In case this does not happen, ping us on Slack.

  5. If reviewers have requested changes and those changes have been addressed, re-request a review for the reviewers that have requested changes. Otherwise, those reviewers will not be notified and your PR will not receive any reviews. If the PR is considerably large (e.g. with more than 200 lines changed and/or more than 6 commits) create new commit for each review. This will make the review process smoother as GitHub has limitations that prevents reviewers from only seeing the new changes added since the last time they have reviewed a PR. Once all reviews are addressed those commits should be squashed against the commit that introduced those changes. This can be easily accomplished by the usage of git rebase -i origin/master and in that windows, move these new commits below the commit that introduced the changes and replace the work pick with fixup. In the following example, commit d2cb02265 will be meld into 9c62e62d8 and commit 146829b59 will be meld into 9400fed20.

    1. pick 9c62e62d8 docs: updating contribution guide process
    2. fixup d2cb02265 joe + paul + chris changes
    3. pick 9400fed20 docs: fixing typo
    4. fixup 146829b59 Quetin and Maciej reviews

    Once this is done you can perform push force into your branch and request for your PR to be merged.

Pull requests review process for committers

  1. Every committer in the committers team belongs to one or more other teams in the Cilium organization if you would like to be added or removed from any team, please contact any of the maintainers.

  2. Once a PR is open, GitHub will automatically pick which teams should review the PR using the CODEOWNERS file. Each committer can see the PRs they need to review by filtering by reviews requested. A good filter is provided in this link so make sure to bookmark it. If a PR review is assigned to one or more committers, they are expected to review that PR. Once they have performed their review, they should remove themselves from the list of assignees.

  3. Belonging to a team does not mean that a committer should know every single line of code the team is maintaining. For this reason it is recommended that once you have reviewed a PR, if you feel that another pair of eyes is needed, you should re-request a review from the appropriate team. In the example below, the committer belonging to the CI team is re-requesting a review for other team members to review the PR. This allows other team members belonging to the CI team to see the PR as part of the PRs that require review in the filter provided above

    ../../../_images/re-request-review.png

  4. When all review objectives for all CODEOWNERS are met, all required CI tests have passed and a proper release label as been set, you may set the ready-to-merge label to indicate that all criteria have been met. Maintainer’s little helper might set this label automatically if the previous requirements were met.

    LabelsWhen to set
    ready-to-mergePR is ready to be merged

Weekly duties

Some members of the committers team will have rotational duties that change every week. The following steps describe how to perform those duties. Please submit changes to these steps if you have found a better way to perform each duty.

Pull request review process for Janitors team

Note

These instructions assume that whoever is reviewing is a member of the Cilium GitHub organization or has the status of a contributor. This is required to obtain the privileges to modify GitHub labels on the pull request.

Dedicated expectation time for each member of Janitors team: Follow the next steps 1 to 2 times per day. Works best if done first thing in the working day.

  1. Review all PRs requesting for review in for you;

  2. If this PR was opened by a non-committer (e.g. external contributor) please assign yourself to that PR and make sure to keep track the PR gets reviewed and merged. This may extend beyond your assigned week for Janitor duty.

    If the contributor is a Cilium committer, then they are responsible for getting the PR in a ready to be merged state by adding the ready-to-merge label, once all reviews have been addressed and CI checks are successful, so that the janitor can merge it (see below).

    If this PR is a backport PR (e.g. with the label kind/backport) and no-one else has reviewed the PR, review the changes as a sanity check. If any individual commits deviate from the original patch, request review from the original author to validate that the backport was correctly applied.

  3. Review overall correctness of the PR according to the rules specified in the section Submitting a pull request.

    Set the labels accordingly, a bot called maintainer’s little helper might automatically help you with this.

    LabelsWhen to set
    dont-merge/needs-sign-offSome commits are not signed off
    needs-rebasePR is outdated and needs to be rebased

  4. Validate that bugfixes are marked with kind/bug and validate whether the assessment of backport requirements as requested by the submitter conforms to the Backport Criteria.

    LabelsWhen to set
    needs-backport/X.YPR needs to be backported to these stable releases

  5. If the PR is subject to backport, validate that the PR does not mix bugfix and refactoring of code as it will heavily complicate the backport process. Demand for the PR to be split.

  6. Validate the release-note/* label and check the PR title for release note suitability. Put yourself into the perspective of a future release notes reader with lack of context and ensure the title is precise but brief.

    LabelsWhen to set
    dont-merge/needs-release-noteDo NOT merge PR, needs a release note
    release-note/bugThis is a non-trivial bugfix and is a user-facing bug
    release-note/majorThis is a major feature addition, e.g. Add MongoDB support
    release-note/minorThis is a minor feature addition, e.g. Add support for a Kubernetes version
    release-note/miscThis is a not user-facing change , e.g. Refactor endpoint package, a bug fix of a non-released feature
    release-note/ciThis is a CI feature of bug fix.

  7. Check for upgrade compatibility impact and if in doubt, set the label upgrade-impact and discuss in the Slack channel or in the weekly meeting.

    LabelsWhen to set
    upgrade-impactThe code changes have a potential upgrade impact

  8. When all review objectives for all CODEOWNERS are met, all CI tests have passed, and all reviewers have approved the requested changes, merge the PR by clicking in the “Rebase and merge” button.

  9. Merge PRs with the ready-to-merge label set here

  10. If the PR is a backport PR, update the labels of cherry-picked PRs with the command included at the end of the original post. For example:

    1. $ for pr in 12589 12568; do contrib/backporting/set-labels.py $pr done 1.8; done

Triage issues for Triage team

Dedicated expectation time for each member of Triage team: 15/30 minutes per day. Works best if done first thing in the working day.

  1. Committers belonging to the Triage team should make sure that:

    1. Issues opened by community users are tracked down:

      1. Add the label kind/community-report;
      2. If feasible, try to reproduce the issue described;
      3. Assign a member that is responsible for that code section to that GitHub issue;
      4. If it is a relevant bug to the rest of the committers, bring the issue up in the weekly meeting. For example:
        • The issue may impact an upcoming release; or
        • The resolution is unclear and assistance is needed to make progress; or
        • The issue needs additional attention from core contributors to confirm the resolution is the right path.

    2. Issues recently commented are not left out unanswered:

      1. If there is someone already assigned to that GitHub issue and that committer hasn’t provided an answer to that user for a while, ping that committer directly on Slack;
      2. If the issue cannot be solved, bring the issue up in the weekly meeting.

Backporting PR for Backport team

Dedicated expectation time for each member of Backport team: 60 minutes per week depending on releases that need to be performed at the moment.

Even if the next release is not imminently planned, it is still important to perform backports to keep the process smooth and to catch potential regressions in stable branches as soon as possible. If backports are delayed, this can also delay releases which is important to avoid especially if there are security-sensitive bug fixes that require an immediate release.

In addition, when a backport PR is open, the person opening it is responsible to drive it to completion, even if it stretches after the assigned week of backporting hat. If this is not feasible (e.g. PTO), you are responsible to initiate handover of the PR to the next week’s backporters.

If you can’t backport a PR due technical constraints feel free to contact the original author of that PR directly so they can backport the PR themselves.

Follow the Backporting process guide to know how to perform this task.

Coordination

In general, coordinating in the #launchpad Slack channel with the other hat owner for the week is encouraged. It can reduce your workload and it will avoid backporting conflicts such as opening a PR with the same backports. Such discussions will typically revolve around which branches to tackle and which day of the week.

An example interaction in #launchpad:

  1. Starting backport round for v1.7 and v1.8 now
  2. cc @other-hat-wearer

The other hat owner can then handle v1.9 and v1.10 backports the next day, for example.

If there are many backports to be done, then splitting up the rounds can be beneficial. Typically, backporters opt to start a round in the beginning of the week and then another near the end of the week.

By the start / end of the week, if there are other backport PRs that haven’t been merged, then please coordinate with the previous / next backporter to check what the status is and establish who will work on getting the backports into the tree (for instance by investigating CI failures and addressing review feedback). There’s leeway to negotiate depending on who has time available.

Developer’s Certificate of Origin

To improve tracking of who did what, we’ve introduced a “sign-off” procedure.

The sign-off is a simple line at the end of the explanation for the commit, which certifies that you wrote it or otherwise have the right to pass it on as open-source work. The rules are pretty simple: if you can certify the below:

  1. Developer Certificate of Origin
  2. Version 1.1
  4. Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
  5. 1 Letterman Drive
  6. Suite D4700
  7. San Francisco, CA, 94129
  9. Everyone is permitted to copy and distribute verbatim copies of this
  10. license document, but changing it is not allowed.
  13. Developer's Certificate of Origin 1.1
  15. By making a contribution to this project, I certify that:
  17. (a) The contribution was created in whole or in part by me and I
  18.     have the right to submit it under the open source license
  19.     indicated in the file; or
  21. (b) The contribution is based upon previous work that, to the best
  22.     of my knowledge, is covered under an appropriate open source
  23.     license and I have the right under that license to submit that
  24.     work with modifications, whether created in whole or in part
  25.     by me, under the same open source license (unless I am
  26.     permitted to submit under a different license), as indicated
  27.     in the file; or
  29. (c) The contribution was provided directly to me by some other
  30.     person who certified (a), (b) or (c) and I have not modified
  31.     it.
  33. (d) I understand and agree that this project and the contribution
  34.     are public and that a record of the contribution (including all
  35.     personal information I submit with it, including my sign-off) is
  36.     maintained indefinitely and may be redistributed consistent with
  37.     this project or the open source license(s) involved.

then you just add a line saying:

  1. Signed-off-by: Random J Developer <random@developer.example.org>

Use your real name (sorry, no pseudonyms or anonymous contributions.)