Skip to content

Commit 5d96003

Browse files
AkrogAkrog
committed
Docs: Contributor guide
This patch adds some documentations to help new contributors get started. It is not complete, since it doesn't explain how to work with merged and unmerged dependencies with other operators and lib-common, but it should serve as a good start.
1 parent de54439 commit 5d96003

File tree

7 files changed

+616
-0
lines changed

7 files changed

+616
-0
lines changed

‎CONTRIBUTING.md‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# Contributing to the cinder operator
2+
3+
Thank you for taking the time to contribute!
4+
5+
The following is a set of guidelines for contributing to the [cinder-operator
6+
hosted in GitHub](https://github.com/openstack-k8s-operators/cinder-operator).
7+
Feel free to propose changes to this document or any other in a pull request.
8+
9+
## What should I know before I get started?
10+
11+
The cinder-operator is not a large open source project, but it's part of the
12+
[OpenStack podification effort](https://github.com/openstack-k8s-operators)
13+
which is of some size.
14+
15+
There are some requirements to deploy a working Cinder control plane within
16+
OpenShift using the cinder-operator. For simplicity's sake all the
17+
documentation will assume that the
18+
[openstack-operator](https://github.com/openstack-k8s-operators/openstack-operator)
19+
will be used to deploy necessary services, though this doesn't mean that there
20+
are no other ways to do it.
21+
22+
Before working on your first code contribution it would be a good idea to
23+
complete the [getting started](README.md#getting-started) section first to get
24+
familiar with the deploying of the services and to get a feeling of the
25+
behavior of a working OpenStack deployment.
26+
27+
Please refer to the [OpenStack documentation](https://docs.openstack.org) to
28+
learn about OpenStack itself.
29+
30+
### Design decisions
31+
32+
There is some [global podification
33+
documentation](https://github.com/openstack-k8s-operators/docs) relevant for
34+
all the operators, but there are also some global design decisions that have
35+
not been spelled out yet anywhere else, so they are included in the
36+
[cinder-operator design decisions document](docs/dev/design-decisions.md).
37+
38+
## How to contribute?
39+
40+
This project is [using pull
41+
request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)
42+
to submit changes, but it is not currently using the [issues
43+
section](https://github.com/openstack-k8s-operators/cinder-operator/issues) or
44+
any other GitHub feature to track the project's bugs or features and instead it
45+
is expected that whoever finds an issue will fix it or will find someone else
46+
to work on it.
47+
48+
### Testing changes
49+
50+
Developers submitting PRs are expected to have run the code and manually
51+
verified the functionality before submitting the PR, with the exception
52+
of documentation only and CI only PRs.
53+
54+
There are multiple ways to test code changes, but in general terms they can be
55+
split into 2 categories: running things locally or in a container in the
56+
OpenShift cluster.
57+
58+
Running things locally is considerably faster but it doesn't use the real ACLs
59+
(RBACs) the cinder-operator would use on a normal deployment. On the other hand
60+
running things in OpenShift is slower (because we have to build a new
61+
container) but runs as close as a real deployment as we can.
62+
63+
Each of the approaches has its own advantages and disadvantages, so different
64+
variants of both approaches will be covered for readers to choose the one they
65+
prefer.
66+
67+
The following list of articles assumes that podified OpenStack has been
68+
deployed using the openstack-operator as described in the [Getting started
69+
section](README.md#getting-started):
70+
71+
- [Run operator locally](docs/dev/local.md)
72+
- [Run operator in OpenShift using a custom image](docs/dev/custom-image.md)
73+
74+
### Pull Requests
75+
76+
While the pull request flow is used for submitting new issues and for
77+
reviewing, the git repository should **always** be the source of truth, so all
78+
decisions made through the reviewing and design phases should be properly
79+
reflected in the final code, code comments, documentation, and git commit
80+
message that is merged. It should not be necessary to go to a PR in GitHub and
81+
go through the comments to know what a commit is meant to do or why it is doing
82+
it.
83+
84+
*One-liner* commit messages should be avoided like the plague. Please refer to
85+
the [commit messages guide line](#commit-messages) for good practices on commit
86+
messages.
87+
88+
The cinder-operator project will not squash all pull request commits into a
89+
single commit but will look to preserve all submitted individual commits
90+
instead using a merge strategy instead. This means that we can have both
91+
single commit and as multi-commit PRs, and both have their places. It's all
92+
about how and when to split changes.
93+
94+
#### Structural split of changes
95+
96+
The general rule for how to split code changes into commits is that we should
97+
aim to have a single "logical change" per commit.
98+
99+
As the [OpenStack Git Commit Good
100+
Practice](https://wiki.openstack.org/wiki/GitCommitMessages) explains, there
101+
are multiple reasons why this rule is important:
102+
103+
- The smaller the amount of code being changed in a commit, the quicker &
104+
easier it is to review & identify potential flaws in that specific code
105+
change.
106+
- If a change is found to be flawed later, it may be necessary to revert the
107+
broken commit. This is much easier to do if there are not other unrelated
108+
code changes entangled with the original commit.
109+
- When troubleshooting problems using Git's bisect capability, small well
110+
defined changes will aid in isolating exactly where the code problem was
111+
introduced.
112+
- When browsing history using Git annotate/blame, small well defined changes
113+
also aid in isolating exactly where & why a piece of code came from.
114+
115+
In the [OpenStack Git Commit Good Practice
116+
page](https://wiki.openstack.org/wiki/GitCommitMessages) we can also find two
117+
great sections explaining [things to avoid when creating
118+
commits](https://wiki.openstack.org/wiki/GitCommitMessages#Things_to_avoid_when_creating_commits)
119+
and [examples of bad
120+
practice](https://wiki.openstack.org/wiki/GitCommitMessages#Examples_of_bad_practice)
121+
that contributors to this repository should take into consideration.
122+
123+
#### PR Images
124+
125+
Once a PR merges it will trigger an image rebuild and publish, so how can we
126+
tell when the new image is ready?
127+
128+
For that we'll go to the [project's
129+
actions](https://github.com/openstack-k8s-operators/cinder-operator/actions)
130+
and in there [we select the `Cinder Operator image builder`
131+
job](https://github.com/openstack-k8s-operators/cinder-operator/actions/workflows/build-cinder-operator.yaml)
132+
where we'll see the job that is building the new operator container image.
133+
134+
In there how the job is building 3 images: Operator, Bundle, and Index, and if
135+
you go into each one of them and expand the `Push **** To quay.io` step you can
136+
find the actual image location.
137+
138+
For example:
139+
140+
```
141+
Successfully pushed "cinder-operator-index:9f3d1ec26ba8939710f146f3e6a1d81f5077be8a" to "quay.io/***/cinder-operator-index:9f3d1ec26ba8939710f146f3e6a1d81f5077be8a"
142+
```
143+
144+
## Style Guides
145+
146+
### Go Style Guide
147+
148+
While the project has not formally defined a Go Style Guide for the project it
149+
uses [Gofmt](https://pkg.go.dev/cmd/gofmt) for automated code formatting.
150+
151+
This tool is automatically called when building the cinder-operator binary
152+
using the `Makefile` or when it is manually invoked with:
153+
154+
```sh
155+
make fmt
156+
```
157+
158+
Pull Requests are expected to have passed the formatting tool before committing
159+
the code and submitting the PR.
160+
161+
### Commit Messages
162+
163+
As mentioned before, commit messages are a very important part of a pull
164+
request, and their contents must be carefully crafted.
165+
166+
Commit messages should:
167+
168+
- Provide a brief description of the change in the first line.
169+
- Insert a single blank line after the first line.
170+
- Provide a detailed description of the change in the following lines, breaking
171+
paragraphs where needed.
172+
- Lines should be wrapped at 72 characters.
173+
174+
Once again the OpenStack documentation goes over the [important things to
175+
consider when writing the commit
176+
message](https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages),
177+
so please take a good look. The short version is:
178+
179+
- Do not assume the reviewer understands what the original problem was.
180+
- Do not assume the reviewer has access to external web services/site.
181+
- Do not assume the code is self-evident/self-documenting.
182+
- Describe why a change is being made.
183+
- Read the commit message to see if it hints at improved code structure.
184+
- The first commit line is the most important.
185+
- Describe any limitations of the current code.
186+
- Do not assume the reviewer has knowledge of the tests executed on the change
187+
- Do not include patch set-specific comments.
188+
189+
## Helpful scripts
190+
191+
Under the `hack` directory we'll find scripts that can help us in the
192+
development of the cinder-operator as well as some tips and tricks.
193+
194+
Refer to its `README.md` file for additional information.

‎README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,9 @@ targets.
233233
More information about the Makefile can be found via the [Kubebuilder
234234
Documentation]( https://book.kubebuilder.io/introduction.html).
235235

236+
For developer specific documentation please refer to the [Contributing
237+
Guide](CONTRIBUTING.md).
238+
236239
# LICENSE
237240

238241
Copyright 2022.

‎docs/dev/assumptions.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Assumptions
2+
3+
The different articles describing how to run custom cinder-operator code make
4+
some assumptions, please adjust the steps in those cases where your system
5+
doesn't match them:
6+
7+
- You have an OpenShift cluster running and you have logged in it with `oc`, so
8+
there are valid credentials in the system.
9+
10+
- OpenStack operator repositories that are referenced are located directly in a
11+
directory within the home directory following the GitHub repository name. In
12+
the cinder-operator case this will be `~/cinder-operator`.
13+
14+
- The `install_yamls` repository is in `~/install_yamls`.
15+
16+
- Local repositories will have 2 remotes defined `origin` for our fork and
17+
`upstream` for the `openstack-k8s-operators` repository.

‎docs/dev/custom-image.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Running the operator in OpenShift
2+
3+
**NOTE**: This article [makes some assumptions](assumptions.md), make sure they
4+
are correct or adapt the steps accordingly.
5+
6+
This development model is the closest to the *real thing* because the
7+
cinder-operator will be running in OpenShift in a pod and using the RBACs we
8+
have defined, but since we have to build the container and upload it to a
9+
registry, and then OpenShift needs to download it, it will be considerably
10+
slower than just running [the operator locally](local.md).
11+
12+
Before we go and build the container image we should decide what container
13+
image registry we want to use, because we can use a public registry,such as
14+
quay.io, a private registry, or even run a *toy* registry locally.
15+
16+
Running a *toy* registry may require running a couple more commands the first
17+
time, but it will prove to be much faster, since image pushing and pulling will
18+
not go through your internet connection.
19+
20+
To make things easier we [have a simplified explanation of how to run a local
21+
toy registry](local-registry.md) in the IP address (`192.168.130.1`) that CRC
22+
assigns the host when deployint the VM.
23+
24+
The next of the document assumes the *toy* registry is being used.
25+
26+
### Preparation
27+
28+
This article assumes we have followed the [Getting
29+
Started](../../README.md#getting-started) section successfully so we'll not
30+
only have a cinder-operator pod running, but also the different cinder
31+
services.
32+
33+
Unlike when we are [running the operator locally](local.md) here we don't need
34+
to manually stop the existing operator, we can leave it running. And we won't
35+
stop the Cinder services either to illustrate another way of doing things,
36+
though we could do it here if we want, just like we did when [running the
37+
operator locally](local.md#preparation).
38+
39+
### Image build
40+
41+
Before continuing make sure you are in the `cinder-operator` directory where
42+
the changes to test are.
43+
44+
Now we will build the container image for the cinder-operator:
45+
46+
```sh
47+
export IMG=192.168.130.1:5000/cinder-operator:latest
48+
49+
make docker-build
50+
```
51+
52+
This command takes a while to execute, but once it completes we'll have the new
53+
cinder-operator container image in our local registry (don't mistake this for
54+
the *toy* registry) and we can confirm the presence of the image with `podman
55+
images`.
56+
57+
Now it's time to push it to our *toy* registry:
58+
59+
```sh
60+
export VERIFY_TLS=false
61+
62+
make docker-push
63+
```
64+
65+
At this point OpenShift will be able to pull the new image container from this
66+
*toy* registry instead of having to access a registry from Internet.
67+
68+
### Run
69+
70+
Before we try to run the new operator we may have to generate and install the
71+
new CRDs and RBACs if they have changed. This can be easily done by running
72+
`make install`.
73+
74+
As mentioned before we haven't stopped the cinder-operator that is running in
75+
OpenShift, so what we are going to do is replace the running operator to make
76+
it use the one we just built.
77+
78+
We start by searching for the name of the `ClusterServiceVersion` of the
79+
OpenStack operator and editing its current CR:
80+
81+
```sh
82+
CSV=`oc get -l operators.coreos.com/openstack-operator.openstack= -o custom-columns=CSV:.metadata.name --no-headers csv`
83+
84+
oc edit csv $CSV
85+
```
86+
87+
Now we search for the first instance of `name:
88+
cinder-operator-controller-manager`, and within its `spec.containers` sections
89+
look for the `image:` definition where the cinder-container image location is
90+
defined.
91+
92+
Now we change the location to `192.168.130.1:5000/cinder-operator:latest`, save
93+
and exit the editor.
94+
95+
At this point OpenShift should detect the change and try to reconcile the CSV,
96+
this will terminate the existing cinder-operator pod that is running and start
97+
a new one with the new image. We can confirm it using `oc describe pod` with
98+
the name of the new cinder-operator pod.
99+
100+
**NOTE**: If the new image is not used we may need to force a faster respose by
101+
changing the cinder-operator replicas to 0 in the CSV, save and exit, wait
102+
until the pod is terminated and then change it back to 1.
103+
104+
Since we didn't remove the Cinder services that were running, once the new
105+
cinder-operator is running it should reconcile the existing `Cinder` CR,
106+
modifying existing `StatefulSet` and `Deployment` manifests used for the Cinder
107+
services according to the new code.
108+
109+
We may also want to uninstall all cinder services and see how our newly
110+
deployed operator deploys them from scratch. To do that we'll need to edit the
111+
`OpenStackControlPlane` CR that was used to deploy OpenStack and currently
112+
exists in the OpenShift cluster.
113+
114+
```sh
115+
oc edit OpenStackControlPlane openstack
116+
```
117+
118+
Now we search for the `cinder:` section and remove it or set the `replication`
119+
value of the 4 services in its `template:` section to `0`, then save and exit
120+
the editor.
121+
122+
This will make the openstack-operator notice the change and modify the `Cinder`
123+
CR, which in turn will be detected by the cinder-operator triggering the
124+
termination of the cinder services in order during the reconciliation.
125+
126+
Once the cinder service pods are gone we can add the `cinder` section back or
127+
set the `replicas` back to `1` in the `OpenStackControlPlane` CR to trigger the
128+
deployment of the Cinder services. The easiest way to do so is to apply the
129+
original manifest we used to deploy OpenStack in the first place:
130+
131+
```sh
132+
oc apply -f hack/dev/openstack.yaml
133+
```
134+
135+
**NOTE**: The Cinder DB is not deleted when uninstalling Cinder services, so
136+
Cinder DB migrations will run faster this time (they won't do anything).
137+
138+
To see what the cinder-operator is doing we'll need to do `oc logs -f` for the
139+
cinder-operator.
140+
141+
### Final notes
142+
143+
If we need to make changes to the operator we'll need to go through the `make
144+
install` and `make docker-build docker-push` cycle again to create a new image.
145+
146+
Making OpenShift use the new image we just built should be easy enough, we just
147+
need to delete the cinder-operator pod, since the `imagePullPolicy` policy of
148+
the pod is `Always`, so it checks that the source image is up to date every
149+
time it runs the pod.

‎docs/dev/design-decisions.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Design decisions
2+
3+
We have agreed on a very basic set of principles we would like to follow during
4+
the development of the OpenStack Operators:
5+
6+
- OpenShift is the only intended Container Orchestration system we aim to
7+
support, and code can depend on OpenShift only available features.
8+
9+
- Should use configuration snippets for the system administrators (or the
10+
meta-operator) to provide the service specific configuration. This will
11+
reduce the domain specific knowledge required, making it easy for anyone that
12+
knows how to configure the specific service to use the operator.
13+
14+
- Should aim to follow the Kubernetes [Operator pattern](
15+
https://kubernetes.io/docs/concepts/extend-kubernetes/operator/).
16+
17+
- Must use [Controllers](
18+
https://kubernetes.io/docs/concepts/architecture/controller/) which provide
19+
a reconcile function responsible for synchronizing resources until the
20+
desired state is reached on the cluster.
21+
22+
- Intended to be deployed via OLM [Operator Lifecycle Manager](
23+
https://github.com/operator-framework/operator-lifecycle-manager).

0 commit comments

Comments
 (0)