Skip to content
This repository was archived by the owner on Feb 7, 2023. It is now read-only.

Commit 2b16eca

Browse files
author
Micah Abbott
authored
docs: howto contribute to the repo (#163)
* docs: howto contribute to the repo It was requested that additional documentation be made available about how to start contributing to the repo. I've created a new `docs` directory that currently holds a single file that goes into some of the details of the repo and how to create roles/tests. * fixup! docs: howto contribute to the repo * fixup! docs: howto contribute to the repo
1 parent dbb94aa commit 2b16eca

File tree

2 files changed

+343
-1
lines changed

2 files changed

+343
-1
lines changed

‎CONTRIBUTING.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ requested changes to your outstanding PR. It is useful to preserve the
2727
history of the changes to a PR during the review process. Please don't
2828
force-push a new commit on your PR unless absolutely necessary. The use
2929
of `git commit --fixup` is encouraged as it can make squashing commits
30-
easier.
30+
easier.
31+
32+
For more details about contributing to the repo, please see [How To Contribute](/docs/howto_contribute.md).
3133

3234
### Reviewing Patches
3335

‎docs/howto_contribute.md‎

Lines changed: 340 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,340 @@
1+
### Before We Get Started...
2+
3+
This document assumes that you have some basic familiarity with the following
4+
topics:
5+
6+
- general `git` usage
7+
- submitting and reviewing pull requests (PRs) on GitHub
8+
- general Ansible usage (tasks, roles, playbooks)
9+
- have previously read though [CONTRIBUTING.md](https://github.com/projectatomic/atomic-host-tests/blob/master/CONTRIBUTING.md)
10+
11+
The intention is for the repo to not have any exotic requirements and be
12+
relatively easy to use and to contribute to. If you feel this is not the
13+
case, feel free to open an [issue](https://github.com/projectatomic/atomic-host-tests/issues/new) to discuss how it can be improved.
14+
15+
### A Brief Introduction to the Repo
16+
17+
At the top-level of the repository, we have five directories. Of these five
18+
directories, the `roles` and `tests` directories are where the majority of
19+
the development happens.
20+
21+
The `callback_plugins` directory is where we have a single plugin that
22+
provides improved output formatting from the Ansible playbooks. It also
23+
provides some functionality that allows the tests to collect the journal
24+
from a system after a failure.
25+
26+
The `common` directory contains pieces of Ansible playbooks/roles that did
27+
not fit in with the other roles and tests that exist, but are still used
28+
in parts of the repository.
29+
30+
The `docs` directory contains documentation important to using the repo
31+
and developing for the repo.
32+
33+
The `roles` directory is the traditional `roles` directory that is often
34+
used in Ansible playbooks. We have placed the directory at the top-level
35+
of the repo, so that we can re-use the roles across multiple tests.
36+
37+
The `tests` directory contains playbooks which execute tests using a mix
38+
of roles and tasks.
39+
40+
### Setting up your Environment/Tools
41+
42+
As stated in the opening of this doc, the repo does not have any exotic
43+
requirements and should be easy to use and develop for.
44+
45+
You are free to use any code editor that you are most familiar with, as long
46+
as it does not cause any breakage in the roles/tests or distrupts the overall
47+
style of the repo.
48+
49+
That being said, `vim` users would probably be interested in the the
50+
[Ansible plugin](https://github.com/pearofducks/ansible-vim) which enables syntax highlighting.
51+
This `vim` plugin is also why many of the `.yml` files in the repo have the
52+
following comment at the head of file:
53+
54+
`# vim: set ft=ansible:`
55+
56+
### Writing Your First Role
57+
58+
The main goal when writing a new role for the repo is to create something
59+
that is generic enough that can be used by other users. This means that
60+
the role should have a limited scope in terms of what is intends to do and
61+
is not unique to your particular test.
62+
63+
As an example, there is a role defined below that verifies the `docker build`
64+
operation is successful.
65+
66+
67+
```yaml
68+
---
69+
- name: Fail if required variables are not set
70+
fail:
71+
msg: "The required variables are not set."
72+
when: image_name is not defined or
73+
build_path is not defined or
74+
dockerfile_name is not defined
75+
76+
- name: Build Docker image
77+
command: "docker build -t {{ image_name }} -f {{ build_path }}/{{ dockerfile_name }} {{ build_path }}"
78+
79+
- name: Verify Docker image was built
80+
command: docker images
81+
register: docker_images
82+
failed_when: "'{{ image_name }}' not in docker_images.stdout"
83+
```
84+
85+
This role is very flexible as it allows users the ability to define the
86+
location of the Dockerfile, the name of the Dockerfile, and the name of the
87+
image being built. Other roles or tasks will need to be used to land the
88+
Dockerfile on the host, which allows this role to stay simple in its
89+
definition.
90+
91+
### Writing Your First Test
92+
93+
Now you have a new role created, let's put it to use in a new test playbook.
94+
95+
#### Directory Setup
96+
97+
Firstly, you will have to create a new sub-directory under `tests` and create
98+
symlinks to the top-level `callback_plugins` and `roles` directories. This
99+
allows the test to reference the available roles and have its output nicely
100+
formatted by the callback plugin. In this example, we are going to pretend we
101+
are creating a test playbook for the `docker build` command.
102+
103+
```bash
104+
$ mkdir -p tests/docker-build/
105+
$ ln -s ../../callback_plugins/ tests/docker-build/callback_plugins
106+
$ ln -s ../../roles/ tests/docker-build/roles
107+
```
108+
109+
Additional directories such as `files`, `templates` or `vars` may be required
110+
for your test. Please reference the [Ansible documentation](http://docs.ansible.com/ansible/playbooks_best_practices.html#content-organization) about
111+
content organization about when and how to use these directories.
112+
113+
During this phase, it would be wise to setup any RHEL subscription data under
114+
`/roles/redhat_subscription/files`. The `redhat_subscription` role defaults
115+
to looking for subscription data in the `/roles/redhat_subscription/files/subscription_data.csv`
116+
file. You can use the sample data in [subscription_data.csv.sample](/roles/redhat_subscription/files/subscription_data.csv.sample) to see how
117+
the file is structured.
118+
119+
#### Playbook Structure + System State
120+
121+
Now we can start developing the actual playbook. When running an Ansible
122+
playbook, the operations are mostly executed in serial, so there is a sense
123+
of state of the host under test. This means you can structure your playbook
124+
to assume certain conditions of the host during the execution of the playbook.
125+
126+
A simple playbook will have a list of tasks or roles defined in a single file
127+
and that is all that will be required. Sometimes, it is desirable to create
128+
multiple playbooks in a single file for the purposes of providing delineation
129+
between certain features being tested. This can be seen in [tests/admin-unlock](/tests/admin-unlock/main.yml)
130+
where each set of features is separated into their own playbooks.
131+
132+
For our example, we will use the simple case of a single playbook in a single
133+
file.
134+
135+
#### Playbook Sections
136+
137+
All of the playbooks should start with a `name`, `hosts`, and `become`
138+
declaration.
139+
140+
The `name` is up to the test developer, but should be somewhat descriptive
141+
of what the test is doing.
142+
143+
For the `host` value, we recommend the following:
144+
145+
`- hosts: "{{ testnodes | default('all') }}"`
146+
147+
This allows users to supply simple inventory data to `ansible-playbook`, but
148+
retains the flexibility to support multi-node tests in the future.
149+
(Almost all of the tests in the repo are single-node tests.)
150+
151+
Since most of our test operations require root privileges, the `become: yes`
152+
declaration ensures that we have those privileges.
153+
154+
The playbook should be tagged via a `tag` value. This allows the test
155+
executor the ability to skip the entire playbook, if they were to try to run
156+
multiple playbooks via a script or other framework. Typically, this value
157+
is the name of the test being run.
158+
159+
If the playbook requires additional variables to be defined, they can be
160+
specified via the `vars_files` or `vars` declaration.
161+
162+
The bulk of your test will then be constructed using the various roles that
163+
are available in the `roles` directory. Remember to tag each role with its
164+
name to allow users running your test to skip roles if they choose to. (If
165+
you are re-using the roles multiple times in the same test, you should tag
166+
each role with a unique value.)
167+
168+
#### Example Playbook
169+
170+
In the example playbook, we are going to copy a couple of Dockerfiles to the
171+
host and the use our `docker_build` role to build Docker images with those
172+
Dockerfiles. Afterwards, we will remove the Docker images that have been
173+
built.
174+
175+
Before we start to write out any YAML, it is helpful to start by documenting
176+
your test in a README.md file. In addtion to providing useful information to
177+
users running the tests, it can also help to provide a general structure to
178+
the test you want to write.
179+
180+
We require new tests to minimally include a README.md that covers which kind
181+
of testing the playbook will perform, any kind of prerequisites for running
182+
the playbook, and an example invocation of the playbook. You can reference
183+
the [README.md](/tests/improved-sanity-test/README.md) from the `improved-sanity-test`
184+
as an example.
185+
186+
Since we are going to be copying multiple Dockerfiles, we will need to create
187+
a `files` directory to hold them. See below for the directory structure and
188+
some simple example Dockerfiles.
189+
190+
```bash
191+
$ mkdir -p tests/docker-build/files/
192+
$ cat tests/docker-build/files/centos-httpd-Dockerfile
193+
FROM registry.centos.org/centos/centos:7
194+
RUN yum -y install httpd
195+
196+
$ cat tests/docker-build/files/fedora-httpd-Dockerfile
197+
FROM registry.fedoraproject.org/fedora:25
198+
RUN dnf -y install httpd
199+
```
200+
201+
With the files in place, we can edit our playbook at `tests/docker-build/main.yml`.
202+
While it is not required to use the name `main.yml`, it is the standard file name
203+
we have used thusfar.
204+
205+
```yaml
206+
---
207+
- name: Docker Build
208+
hosts: all
209+
become: yes
210+
211+
tags:
212+
- docker_build
213+
214+
pre_tasks:
215+
- name: Make temp directory to hold Dockerfiles
216+
command: mktemp -d
217+
register: temp_dir
218+
219+
- name: Copy Dockerfiles to temp directory
220+
synchronize:
221+
src: files/
222+
dest: "{{ temp_dir.stdout }}/"
223+
recursive: yes
224+
225+
roles:
226+
- role: ansible_version_check
227+
avc_major: "2"
228+
avc_minor: "2"
229+
tags:
230+
- ansible_version_check
231+
232+
- role: docker_build
233+
build_path: "{{ temp_dir.stdout }}"
234+
dockerfile_name: "centos-httpd-Dockerfile"
235+
image_name: "centos-httpd"
236+
tags:
237+
- centos_docker_build
238+
239+
- role: docker_build
240+
build_path: "{{ temp_dir.stdout }}"
241+
dockerfile_name: "fedora-httpd-Dockerfile"
242+
image_name: "fedora-httpd"
243+
tags:
244+
- fedora_docker_build
245+
246+
post_tasks:
247+
- name: Remove all docker images
248+
shell: docker rmi -f $(docker images -qa)
249+
```
250+
251+
Here we've made a temporary directory, copied the Dockerfiles to the directory,
252+
built the images using the `docker_build` role from the earlier example, and
253+
then cleaned up after the test.
254+
255+
Note, we used the `ansible_version_check` role to verify that the version of
256+
Ansible being used to execute the playbook is what we currently support. This
257+
is suggested for all playbooks.
258+
259+
Each role is tagged in the same way the whole playbook is tagged - to enable
260+
users to skip certain roles during test execution.
261+
262+
Additionally, it is good practice to clean up any artifacts from the test
263+
which may interfere with other tests that are run afterwards.
264+
265+
### Testing Your Playbook
266+
267+
After you have finished with your playbook, it is important to test that it can
268+
successfully run against the Atomic Host variant of CentOS, Fedora, and RHEL.
269+
If one of those platforms does not run successfully, you'll need to make the
270+
necessary adjustments before submitting a pull request with your changes.
271+
272+
### Following Established Style
273+
274+
The [CONTRIBUTING.md](https://github.com/projectatomic/atomic-host-tests/blob/master/CONTRIBUTING.md) doc points out that the members of the repo try to follow the best
275+
practices and style guidelines that are used by the [openshift-ansible](https://github.com/openshift/openshift-ansible) project.
276+
The are noted here again:
277+
278+
- [OpenShift Ansible Best Practices](https://github.com/openshift/openshift-ansible/blob/master/docs/best_practices_guide.adoc#ansible)
279+
- [OpenShift Ansible Style Guide](https://github.com/openshift/openshift-ansible/blob/master/docs/style_guide.adoc#ansible)
280+
281+
You should familiarize yourself with these practices and rules, and try to
282+
follow them as you work on your own changes.
283+
284+
The enforcement of these best practices and rules is not stringent, but be
285+
aware that changes may be requested to comply where it makes sense.
286+
287+
Generally, the YAML structure used in the repo favors the 'multi-line' approach
288+
when writing out tasks or roles:
289+
290+
```yaml
291+
- name: my task
292+
module_name:
293+
arg1: "val1"
294+
arg2: "val2"
295+
when: something == "foobar"
296+
register: output_var
297+
```
298+
299+
If you need to run a command that has multiple arguments, you can break them
300+
across multiple lines:
301+
302+
```yaml
303+
- name: long command
304+
command: >
305+
long_foobar -v
306+
--arg1 val1
307+
--arg2 val2
308+
--arg3 val3
309+
some_file_name
310+
```
311+
312+
Finally, on the subject of tabs vs. spaces, the repo uses spaces throughout.
313+
314+
### Asking for Feedback
315+
316+
There are two ways to solicit feedback about your work. First, you can open
317+
an issue in the repo to discuss how you would like to test a certain feature
318+
or to discuss some changes you would like to make to a repo/test. This allows
319+
other contributors to weigh in on the proposed changes and provide some
320+
feedback on how to best accomplish your goals.
321+
322+
If you feel your work is ready to be reviewed, you can submit your changes
323+
as a pull request and wait for feedback from the contributors.
324+
325+
#### PR Submission Checklist
326+
327+
If your pull request makes any changes to the roles or tests of the repo, the
328+
following checklist may be useful to follow in order to minimize any extra
329+
changes that may be requested:
330+
331+
- [ ] role/test has successfully run against the following platforms
332+
- [ ] Fedora 25 Atomic Host
333+
- [ ] CentOS 7 Atomic Host
334+
- [ ] [CentOS AH Continuous](https://wiki.centos.org/SpecialInterestGroup/Atomic/Devel) or [Fedora AH Continuous](https://pagure.io/fedora-atomic-host-continuous)
335+
- [ ] RHEL 7 Atomic Host (if available)
336+
- [ ] best effort was made to make changes meet [best practices](https://github.com/openshift/openshift-ansible/blob/master/docs/best_practices_guide.adoc#ansible) and [style guidelines](https://github.com/openshift/openshift-ansible/blob/master/docs/style_guide.adoc#ansible)
337+
- [ ] commit message clearly explains changes and rationale for changes
338+
339+
The members of this repo strive to be friendly and helpful with contributors,
340+
so do not be afraid to ask for help or guidance at any point during the process.

0 commit comments

Comments
 (0)