Skip to content

chore(docs): update contributing guidelines (charmkeeper) #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
erinecon wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore(docs): update contributing guidelines (charmkeeper) #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
### Overview
#### What this PR does

<!-- A high level overview of the change -->
#### Why we need it

### Rationale
#### Checklist

<!-- The reason the change is needed -->
- [ ] I followed the [contributing guide](https://github.com/canonical/is-charms-contributing-guide)
- [ ] I added or updated the documentation (if applicable)
- [ ] I updated `docs/changelog.md` with user-relevant changes
- [ ] I added a [change artifact](../docs/release-notes/template/docs/release-notes/template/_change-artifact-template.yaml) for user-relevant changes in `docs/release-notes/artifacts`. If no change artifact is necessary, I tagged the PR with the label `no-release-note`.
- [ ] I used AI to assist with preparing this PR
- [ ] I added or updated tests as needed (unit and integration)
- [ ] **If integration test modules are used:** I updated the workflow configuration
(e.g., in `.github/workflows/integration_tests.yaml`, ensure the `modules` list is correct)
- [ ] **If this is a Grafana dashboard:** I added a screenshot of the dashboard
- [ ] **If this is Terraform:** `terraform fmt` passes and `tflint` reports no errors
- [ ] **If this is Rockcraft:** I updated the version

### Checklist
<!--
You can adjust the checklist to match the project.

If you are an AI assistant (e.g., GitHub Copilot, ChatGPT, Claude), follow these instructions when generating or completing PR descriptions:

- [ ] Changes comply with the project's coding standards and guidelines (see CONTRIBUTING.md and STYLE.md)
- [ ] `CONTRIBUTING.md` has been updated upon changes to the contribution/development process (e.g. changes to the way tests are run)
- [ ] Technical author has been assigned to review the PR in case of documentation changes (usually *.md files)
1. Add a **Test plan** section explaining how the changes were tested, including any manual, unit, integration, or workflow tests.
2. Add a **Review focus** section highlighting tricky parts, important decisions, or areas needing human attention.
3. Identify and document any **potential breaking changes**.
4. Highlight any **new dependencies, APIs, modules, or workflow changes** introduced by this PR.
-->

<!-- Explanation for any unchecked items above -->
224 changes: 103 additions & 121 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
# Contribute
# Contributing

## Overview
This document explains the processes and practices recommended for contributing enhancements to the GitHub runner Operator.

This document explains the processes and practices recommended for contributing enhancements to the codebase.
## Overview

- Generally, before developing enhancements to this code base, you should consider [opening an issue](https://github.com/canonical/github-runner-operator/issues) explaining your use case.
- If you would like to chat with us about your use-cases or proposed implementation, you can reach us at [Canonical Charm Development Matrix public channel](https://matrix.to/#/#charmhub-charmdev:ubuntu.com) or [Discourse](https://discourse.charmhub.io/).
- Generally, before developing enhancements to this charm, you should consider [opening an issue
](https://github.com/canonical/github-runner-operators/issues) explaining your use case.
- If you would like to chat with us about your use-cases or proposed implementation, you can reach
us at [Canonical Matrix public channel](https://matrix.to/#/#charmhub-charmdev:ubuntu.com)
or [Discourse](https://discourse.charmhub.io/).
- Familiarizing yourself with the [Juju documentation](https://documentation.ubuntu.com/juju/3.6/howto/manage-charms/)
will help you a lot when working on new features or bug fixes.
- All enhancements require review before being merged. Code review typically examines
- code quality
- test coverage
- user experience for Juju operators of this charm.
- Once your pull request is approved, we squash and merge your pull request branch onto
the `main` branch. This creates a linear Git commit history.
- For further information on contributing, please refer to our
[Contributing Guide](https://github.com/canonical/is-charms-contributing-guide).

## Code of conduct

When contributing, you must abide by the
[Ubuntu Code of Conduct](https://ubuntu.com/community/ethos/code-of-conduct).

## Changelog

Please ensure that any new feature, fix, or significant change is documented by
adding an entry to the [CHANGELOG.md](docs/changelog.md) file. Use the date of the
contribution as the header for new entries.

To learn more about changelog best practices, visit [Keep a Changelog](https://keepachangelog.com/).

## Submissions

If you want to address an issue or a bug in this project,
Expand All @@ -32,31 +50,46 @@ also, reference the issue or bug number when you submit the changes.

Your changes will be reviewed in due time; if approved, they will be eventually merged.

### Describing pull requests
### AI

To be properly considered, reviewed and merged,
your pull request must provide the following details:
You are free to use any tools you want while preparing your contribution, including
AI, provided that you do so lawfully and ethically.

- **Title**: Summarize the change in a short, descriptive title.
Avoid using AI to complete issues tagged with the "good first issues" label. The
purpose of these issues is to provide newcomers with opportunities to contribute
to our projects and gain coding skills. Using AI to complete these tasks
undermines their purpose.

- **Overview**: Describe the problem that your pull request solves.
Mention any new features, bug fixes or refactoring.
We have created instructions and tools that you can provide AI while preparing your contribution: [`copilot-collections`](https://github.com/canonical/copilot-collections)

- **Rationale**: Explain why the change is needed.
While it isn't necessary to use `copilot-collections` while preparing your
contribution, these files contain details about our quality standards and
practices that will help the AI avoid common pitfalls when interacting with
our projects. By using these tools, you can avoid longer review times and nitpicks.

- **Checklist**: Complete the following items:
If you choose to use AI, please disclose this information to us by indicating
AI usage in the PR description (for instance, marking the checklist item about
AI usage). You don't need to go into explicit details about how and where you used AI.

- The PR is tagged with appropriate label (`urgent`, `trivial`, `senior-review-required`, `documentation`).
Avoid submitting contributions that you don't fully understand.
You are responsible for the entire contribution, including the AI-assisted portions.
You must be willing to engage in discussion and respond to any questions, comments,
or suggestions we may have.

### Signing commits

To improve contribution tracking,
we use the [Canonical contributor license agreement](https://assets.ubuntu.com/v1/ff2478d1-Canonical-HA-CLA-ANY-I_v1.2.pdf)
(CLA) as a legal sign-off, and we require all commits to have verified signatures.

### Canonical contributor agreement
#### Canonical contributor agreement

Canonical welcomes contributions to the GitHub runner Operator. Please check out our
[contributor agreement](https://ubuntu.com/legal/contributors) if you're interested in contributing to the solution.

Canonical welcomes contributions to this repository. Please check out our [contributor agreement](https://ubuntu.com/legal/contributors) if you’re interested in contributing to the solution.
The CLA sign-off is simple line at the
end of the commit message certifying that you wrote it
or have the right to commit it as an open-source contribution.

#### Verified signatures on commits

Expand All @@ -66,138 +99,87 @@ To add signatures on your commits, follow the

## Develop

For any problems with this charm, please [report bugs here](https://github.com/canonical/github-runner-operator/issues).
To make contributions to this charm, you'll need a working
[development setup](https://documentation.ubuntu.com/juju/latest/howto/manage-your-juju-deployment/set-up-your-juju-deployment-local-testing-and-development/).

The code can be downloaded as follows:
The code for this charm can be downloaded as follows:

```shell
git clone https://github.com/canonical/github-runner-operators.git
```

The code structure is as follows

- `internal/`: Internal Go libraries for the applications
- `cmd/`: Entry points for Go applications (planner, webhook-gateway)
- `charms/`: Entry point for charms (planner, webhook-gateway)

### Style

The applications written in this repository are written in Go.
We like to follow idomatic Go practices and community standards when writing Go code.
We have added an instruction file `go.instructions.md` in `.github/instructions.md` that is used by GitHub Copilot to help you write code that follows these practices.
We have added a [Style Guide](./STYLE.md) that you can refer to for more details.


### Test

This project uses standard Go testing tools for unit tests and integration tests.
You can have a look at the GitHub actions workflows in `.github/workflows/` to see how the tests are run in CI.

Run unit tests using:

```shell
go test -race -v ./...
git clone https://github.com/canonical/github-runner-operators
```

Currently, the internal database package is only covered by integration tests. You can run the database integration tests using:
Make sure to install [`uv`](https://docs.astral.sh/uv/). For example, you can install `uv` on Ubuntu using:

```shell
POSTGRESQL_DB_CONNECT_STRING="postgres://postgres:password@localhost:5432/postgres?sslmode=disable" go test -cover -tags=integration -v ./internal/database
```bash
sudo snap install astral-uv --classic
```
It assumes you have access to a Postgres server running reachable at $POSTGRESQL_DB_CONNECT_STRING.
You can use `docker` to run a Postgres server locally:
```shell
docker run -d --rm --name postgres -e POSTGRES_PASSWORD=password -p 5432:5432 postgres:16.11
```

Run `webhook-gateway` integration tests using:

```shell
APP_WEBHOOK_SECRET_VALUE=fake APP_PORT=8080 RABBITMQ_CONNECT_STRING="amqp://guest:guest@localhost:5672/" go test -v -cover -tags=integration -race ./cmd/webhook-gateway/...
```
For other systems, follow the [`uv` installation guide](https://docs.astral.sh/uv/getting-started/installation/).

It assumes you have access to a RabbitMQ server running reachable at $RABBITMQ_CONNECT_STRING.
You can use `docker` to run a RabbitMQ server locally:
Then install `tox` with its extensions, and install a range of Python versions:

```shell
docker run -d --rm --name rabbitmq -p 5672:5672 -p 15672:15672 rabbitmq:4-management
```bash
uv python install
uv tool install tox --with tox-uv
uv tool update-shell
```

Run `planner` integration tests using:
To create a development environment, run:

```shell
APP_PORT=8080 POSTGRESQL_DB_CONNECT_STRING="postgres://postgres:password@localhost:5432/postgres?sslmode=disable" RABBITMQ_CONNECT_STRING="amqp://guest:guest@localhost:5672/" go test -v -cover -tags=integration -race ./cmd/planner/...
```bash
uv sync --all-groups
source .venv/bin/activate
```

It assumes you can reach postgres and rabbitmq servers as described above.

### Test design

We intend to have unit tests for all the logic in the internal packages (located in the `internal/` directory).
Unit tests should test the logic in isolation using mocks/fakes for external dependencies. They should be fast to execute.

Integration tests should test the integration of various components together.
There should be at least an integration test located in the main package of the application they are testing (e.g. in `cmd/webhook-gateway/main_test.go` for the webhook gateway application).

In addition to application integration tests, we also have charm integration tests located in the `charms/tests/integration` directory. These tests should test the charm
deployment and its integration with other charms. These tests are usually slower than application integration tests, and
should not cover application logic tests; those should be covered in the application integration test.
Aim to focus the charm integration tests only on operational aspects. E.g.

- Testing if the charm deploys correctly
- Testing if the charm config options work as expected
- Testing if the charm integrates correctly with other charms

Aim to keep all tests (unit, integration, charm integration)

- Fast to execute
- Reliable
- Deterministic and repeatable
- Easy to set up locally

in order to be able to have fast iterations during development.

### Coverage

We require at least 85% code coverage for all internal packages. New code that lowers the current coverage
should be avoided and discouraged during code reviews.

### Cyclomatic complexity
### Test

We recommend keeping cyclomatic complexity of functions/methods below 10.
Higher complexity leads to code that is harder to read, understand, test and maintain.
There are exceptions where higher complexity is justified (e.g., validation, initialization),
but those should require explicit justification using `nolint` directives.
This project uses `tox` for managing test environments. There are some pre-configured environments
that can be used for linting and formatting code when you're preparing contributions to the charm:

* ``tox``: Executes all of the basic checks and tests (``lint``, ``unit``, ``static``, and ``coverage-report``).
* ``tox -e fmt``: Runs formatting using ``ruff``.
* ``tox -e lint``: Runs a range of static code analysis to check the code.
* ``tox -e static``: Runs other checks such as ``bandit`` for security issues.
* ``tox -e unit``: Runs the unit tests.
* ``tox -e integration``: Runs the integration tests.

### Build the rock and charm

### Charm development
Use [Rockcraft](https://documentation.ubuntu.com/rockcraft/stable/) to create an
OCI image for the GitHub runner app, and then upload the image to a MicroK8s registry,
which stores OCI archives so they can be downloaded and deployed.

The charm uses the [12 factor app pattern](https://canonical-12-factor-app-support.readthedocs-hosted.com/latest/).
In order to build the `charm-name` rock, use the
`build-charm-name-rock.sh` script.
Enable the MicroK8s registry:

The respective charm code is in the `charms/charm-name` directory.
```bash
microk8s enable registry
```

Integration tests for the charm are in the `charms/tests/integration` directory.
The following commands pack the OCI image and push it into
the MicroK8s registry:

Have a look at [this tutorial](https://documentation.ubuntu.com/charmcraft/latest/tutorial/kubernetes-charm-go/)
for a step-by-step guide to develop a Kubernetes charm using Go.
```bash
cd <project_dir>
rockcraft pack
skopeo --insecure-policy copy --dest-tls-verify=false oci-archive:<rock-name>.rock docker://localhost:32000/<app-name>:latest
```

To run the charm integration test, the charm file and rock has to be provided as input.
You would need an LXD and MicroK8s cloud to run the tests. Ensure the `microk8s`
controller is active in your Juju client before running the tests. An
example run command in the root directory is as follows:
Build the charm in this git repository using:

```shell
tox -e webhook-gateway-integration -- --charm-file ./github-runner-webhook-gateway_amd64.charm --webhook-gateway-image localhost:32000/webhook-gateway:0.1
charmcraft pack
```

To add the rock to the MicroK8s registry, use the following command:
### Deploy

```shell
rockcraft.skopeo copy \
--insecure-policy --dest-tls-verify=false \
oci-archive:webhook-gateway_0.1_amd64.rock \
docker://localhost:32000/webhook-gateway:0.1
```bash
# Create a model
juju add-model charm-dev
# Enable DEBUG logging
juju model-config logging-config="<root>=INFO;unit=DEBUG"
# Deploy the charm
juju deploy ./github-runner*.charm
```



9 changes: 9 additions & 0 deletions docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
(changelog)=

# Changelog

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

Each revision is versioned by the date of the revision.
Loading
Loading