From 93aea48c3840faab4745e362c36d3d3891e023d3 Mon Sep 17 00:00:00 2001 From: Cory Miller <13227161+cory-miller@users.noreply.github.com> Date: Fri, 21 Oct 2022 09:56:42 -0400 Subject: [PATCH] Revamp the contribution guide (#1917) * Revamp the contribution guide * Fix link * Update CONTRIBUTING.md Co-authored-by: Ava Stancu * Update CONTRIBUTING.md Co-authored-by: Ava Stancu * add guidance on image tool requests Co-authored-by: Ava Stancu --- .github/ISSUE_TEMPLATE/feature_request.md | 20 +- CONTRIBUTING.md | 212 ++++++++++++++-------- adrs/0000-TEMPLATE.md | 18 ++ 3 files changed, 161 insertions(+), 89 deletions(-) create mode 100644 adrs/0000-TEMPLATE.md diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 0e3a58f1..2cb07329 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -1,19 +1,21 @@ --- name: Feature request about: Suggest an idea for this project +labels: enhancement title: '' assignees: '' - --- -**Is your feature request related to a problem? Please describe.** -A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] +### What would you like added? -**Describe the solution you'd like** -A clear and concise description of what you want to happen. +*A clear and concise description of what you want to happen.* -**Describe alternatives you've considered** -A clear and concise description of any alternative solutions or features you've considered. +Note: Feature requests to integrate vendor specific cloud tools (e.g. `awscli`, `gcloud-sdk`, `azure-cli`) will likely be rejected as the Runner image aims to be vendor agnostic. -**Additional context** -Add any other context or screenshots about the feature request here. +### Why is this needed? + +*A clear and concise description of any alternative solutions or features you've considered.* + +### Additional context + +*Add any other context or screenshots about the feature request here.* diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e968d2f7..7a48c8ff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,85 +1,53 @@ -## Contributing +# Contribution Guide -### Testing Controller Built from a Pull Request +- [Contribution Guide](#contribution-guide) + - [Welcome](#welcome) + - [Before contributing code](#before-contributing-code) + - [How to Contribute a Patch](#how-to-contribute-a-patch) + - [Developing the Controller](#developing-the-controller) + - [Developing the Runners](#developing-the-runners) + - [Tests](#tests) + - [Running Ginkgo Tests](#running-ginkgo-tests) + - [Running End to End Tests](#running-end-to-end-tests) + - [Rerunning a failed test](#rerunning-a-failed-test) + - [Testing in a non-kind cluster](#testing-in-a-non-kind-cluster) + - [Code conventions](#code-conventions) + - [Opening the Pull Request](#opening-the-pull-request) + - [Helm Version Changes](#helm-version-changes) + - [Testing Controller Built from a Pull Request](#testing-controller-built-from-a-pull-request) -We always appreciate your help in testing open pull requests by deploying custom builds of actions-runner-controller onto your own environment, so that we are extra sure we didn't break anything. +## Welcome -It is especially true when the pull request is about GitHub Enterprise, both GHEC and GHES, as [maintainers don't have GitHub Enterprise environments for testing](docs/detailed-docs.md#github-enterprise-support). +This document is the single source of truth for how to contribute to the code base. +Feel free to browse the [open issues](https://github.com/actions-runner-controller/actions-runner-controller/issues) or file a new one, all feedback is welcome! +By reading this guide, we hope to give you all of the information you need to be able to pick up issues, contribute new features, and get your work +reviewed and merged. -The process would look like the below: +## Before contributing code -- Clone this repository locally -- Checkout the branch. If you use the `gh` command, run `gh pr checkout $PR_NUMBER` -- Run `NAME=$DOCKER_USER/actions-runner-controller VERSION=canary make docker-build docker-push` for a custom container image build -- Update your actions-runner-controller's controller-manager deployment to use the new image, `$DOCKER_USER/actions-runner-controller:canary` +We welcome code patches, but to make sure things are well coordinated you should discuss any significant change before starting the work. +The maintainers ask that you signal your intention to contribute to the project using the issue tracker. +If there is an existing issue that you want to work on, please let us know so we can get it assigned to you. +If you noticed a bug or want to add a new feature, there are issue templates you can fill out. -Please also note that you need to replace `$DOCKER_USER` with your own DockerHub account name. +When filing a feature request, the maintainers will review the change and give you a decision on whether we are willing to accept the feature into the project. +For significantly large and/or complex features, we may request that you write up an architectural decision record ([ADR](https://github.blog/2020-08-13-why-write-adrs/)) detailing the change. +Please use the [template](/adrs/0000-TEMPLATE.md) as guidance. -### How to Contribute a Patch + -Depending on what you are patching depends on how you should go about it. Below are some guides on how to test patches locally as well as develop the controller and runners. +## How to Contribute a Patch -When submitting a PR for a change please provide evidence that your change works as we still need to work on improving the CI of the project. Some resources are provided for helping achieve this, see this guide for details. +Depending on what you are patching depends on how you should go about it. +Below are some guides on how to test patches locally as well as develop the controller and runners. -#### Running an End to End Test +When submitting a PR for a change please provide evidence that your change works as we still need to work on improving the CI of the project. +Some resources are provided for helping achieve this, see this guide for details. -> **Notes for Ubuntu 20.04+ users** -> -> If you're using Ubuntu 20.04 or greater, you might have installed `docker` with `snap`. -> -> If you want to stick with `snap`-provided `docker`, do not forget to set `TMPDIR` to -> somewhere under `$HOME`. -> Otherwise `kind load docker-image` fail while running `docker save`. -> See https://kind.sigs.k8s.io/docs/user/known-issues/#docker-installed-with-snap for more information. - -To test your local changes against both PAT and App based authentication please run the `acceptance` make target with the authentication configuration details provided: - -```shell -# This sets `VERSION` envvar to some appropriate value -. hack/make-env.sh - -DOCKER_USER=*** \ - GITHUB_TOKEN=*** \ - APP_ID=*** \ - PRIVATE_KEY_FILE_PATH=path/to/pem/file \ - INSTALLATION_ID=*** \ - make acceptance -``` - -**Rerunning a failed test** - -When one of tests run by `make acceptance` failed, you'd probably like to rerun only the failed one. - -It can be done by `make acceptance/run` and by setting the combination of `ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm|kubectl` and `ACCEPTANCE_TEST_SECRET_TYPE=token|app` values that failed (note, you just need to set the corresponding authentication configuration in this circumstance) - -In the example below, we rerun the test for the combination `ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm ACCEPTANCE_TEST_SECRET_TYPE=token` only: - -```shell -DOCKER_USER=*** \ - GITHUB_TOKEN=*** \ - ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm - ACCEPTANCE_TEST_SECRET_TYPE=token \ - make acceptance/run -``` - -**Testing in a non-kind cluster** - -If you prefer to test in a non-kind cluster, you can instead run: - -```shell -KUBECONFIG=path/to/kubeconfig \ - DOCKER_USER=*** \ - GITHUB_TOKEN=*** \ - APP_ID=*** \ - PRIVATE_KEY_FILE_PATH=path/to/pem/file \ - INSTALLATION_ID=*** \ - ACCEPTANCE_TEST_SECRET_TYPE=token \ - make docker-build acceptance/setup \ - acceptance/deploy \ - acceptance/tests -``` - -#### Developing the Controller +### Developing the Controller Rerunning the whole acceptance test suite from scratch on every little change to the controller, the runner, and the chart would be counter-productive. @@ -119,13 +87,14 @@ NAME=$DOCKER_USER/actions-runner make \ (kubectl get po -ojsonpath={.items[*].metadata.name} | xargs -n1 kubectl delete po) ``` -#### Developing the Runners +### Developing the Runners -**Tests** +#### Tests -A set of example pipelines (./acceptance/pipelines) are provided in this repository which you can use to validate your runners are working as expected. When raising a PR please run the relevant suites to prove your change hasn't broken anything. +A set of example pipelines (./acceptance/pipelines) are provided in this repository which you can use to validate your runners are working as expected. +When raising a PR please run the relevant suites to prove your change hasn't broken anything. -**Running Ginkgo Tests** +#### Running Ginkgo Tests You can run the integration test suite that is written in Ginkgo with: @@ -135,7 +104,8 @@ make test-with-deps This will firstly install a few binaries required to setup the integration test environment and then runs `go test` to start the Ginkgo test. -If you don't want to use `make`, like when you're running tests from your IDE, install required binaries to `/usr/local/kubebuilder/bin`. That's the directory in which controller-runtime's `envtest` framework locates the binaries. +If you don't want to use `make`, like when you're running tests from your IDE, install required binaries to `/usr/local/kubebuilder/bin`. +That's the directory in which controller-runtime's `envtest` framework locates the binaries. ```shell sudo mkdir -p /usr/local/kubebuilder/bin @@ -152,10 +122,92 @@ GINKGO_FOCUS='[It] should create a new Runner resource from the specified templa go test -v -run TestAPIs github.com/actions-runner-controller/actions-runner-controller/controllers ``` -#### Helm Version Bumps +### Running End to End Tests -In general we ask you not to bump the version in your PR, the maintainers in general manage the publishing of a new chart. +> **Notes for Ubuntu 20.04+ users** +> +> If you're using Ubuntu 20.04 or greater, you might have installed `docker` with `snap`. +> +> If you want to stick with `snap`-provided `docker`, do not forget to set `TMPDIR` to somewhere under `$HOME`. +> Otherwise `kind load docker-image` fail while running `docker save`. +> See https://kind.sigs.k8s.io/docs/user/known-issues/#docker-installed-with-snap for more information. -#### Running linters locally +To test your local changes against both PAT and App based authentication please run the `acceptance` make target with the authentication configuration details provided: -To run the `golangci-lint` tool locally, we recommend you use `make lint` to run the tool using a Docker container matching the CI version. \ No newline at end of file +```shell +# This sets `VERSION` envvar to some appropriate value +. hack/make-env.sh + +DOCKER_USER=*** \ + GITHUB_TOKEN=*** \ + APP_ID=*** \ + PRIVATE_KEY_FILE_PATH=path/to/pem/file \ + INSTALLATION_ID=*** \ + make acceptance +``` + +#### Rerunning a failed test + +When one of tests run by `make acceptance` failed, you'd probably like to rerun only the failed one. + +It can be done by `make acceptance/run` and by setting the combination of `ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm|kubectl` and `ACCEPTANCE_TEST_SECRET_TYPE=token|app` values that failed (note, you just need to set the corresponding authentication configuration in this circumstance) + +In the example below, we rerun the test for the combination `ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm ACCEPTANCE_TEST_SECRET_TYPE=token` only: + +```shell +DOCKER_USER=*** \ + GITHUB_TOKEN=*** \ + ACCEPTANCE_TEST_DEPLOYMENT_TOOL=helm \ + ACCEPTANCE_TEST_SECRET_TYPE=token \ + make acceptance/run +``` + +#### Testing in a non-kind cluster + +If you prefer to test in a non-kind cluster, you can instead run: + +```shell +KUBECONFIG=path/to/kubeconfig \ + DOCKER_USER=*** \ + GITHUB_TOKEN=*** \ + APP_ID=*** \ + PRIVATE_KEY_FILE_PATH=path/to/pem/file \ + INSTALLATION_ID=*** \ + ACCEPTANCE_TEST_SECRET_TYPE=token \ + make docker-build acceptance/setup \ + acceptance/deploy \ + acceptance/tests +``` + +### Code conventions + +Before shipping your PR, please check the following items to make sure CI passes. + +- Run `go mod tidy` if you made changes to dependencies. +- Format the code using `gofmt` +- Run the `golangci-lint` tool locally. + - We recommend you use `make lint` to run the tool using a Docker container matching the CI version. + +### Opening the Pull Request + +Send PR, add issue number to description + +## Helm Version Changes + +In general we ask you not to bump the version in your PR. +The maintainers will manage releases and publishing new charts. + +## Testing Controller Built from a Pull Request + +We always appreciate your help in testing open pull requests by deploying custom builds of actions-runner-controller onto your own environment, so that we are extra sure we didn't break anything. + +It is especially true when the pull request is about GitHub Enterprise, both GHEC and GHES, as [maintainers don't have GitHub Enterprise environments for testing](docs/detailed-docs.md#github-enterprise-support). + +The process would look like the below: + +- Clone this repository locally +- Checkout the branch. If you use the `gh` command, run `gh pr checkout $PR_NUMBER` +- Run `NAME=$DOCKER_USER/actions-runner-controller VERSION=canary make docker-build docker-push` for a custom container image build +- Update your actions-runner-controller's controller-manager deployment to use the new image, `$DOCKER_USER/actions-runner-controller:canary` + +Please also note that you need to replace `$DOCKER_USER` with your own DockerHub account name. \ No newline at end of file diff --git a/adrs/0000-TEMPLATE.md b/adrs/0000-TEMPLATE.md new file mode 100644 index 00000000..8f15d030 --- /dev/null +++ b/adrs/0000-TEMPLATE.md @@ -0,0 +1,18 @@ +# Title + + + +**Status**: (Proposed|Accepted|Rejected|Superceded|Deprecated) + +## Context + +*What is the issue or background knowledge necessary for future readers +to understand why this ADR was written?* + +## Decision + +**What** is the change being proposed? / **How** will it be implemented?* + +## Consequences + +*What becomes easier or more difficult to do because of this change?*