Coding Guide

This document contains a coding guideline specifically written for this repository. For general information please refer to the Yaook Development Process Documentation.

pre-commit-hooks

This repository contains pre-commit hooks to validate the linting stage of our CI (except ansible-lint) before committing. To use this, install pre-commit (if you use Nix flakes, it is automatically installed for you) and then run pre-commit install to enable the hooks in the repo (if you use direnv, they are automatically enabled for you).

Creation of release notes

The changelog/releasenotes is a place, where a user can see, what has changed. It’s a first reference where to look when e.g. something no longer works. So the important information which needs to be given here is what has changed. Not why or how. These are informations which can be found in the history. As a developer try to keep it short (see keepachangelog) and provide further information in the related issue/MR.

Attention

No direct editing of the CHANGELOG-file!

We use towncrier for the management of our release notes. Therefore developers must adhere to some conventions.

For every MR you need to place a file called <merge-request-ID>.<type>[.<whatever-you-want>] into /docs/_releasenotes. The content of the file is the actual release note and is in reStructuredText format.

The merge-request-ID will automatically be added/corrected for you. So if you don’t know the merge-request-ID in advance, just type anything (except +, which will not be replaced and mark a note with no link to a MR) instead of the ID. Please provide the file in your last commit as the pipeline will git commit --amend and git push --force the corrected filename back to your branch. Don’t forget to git pull --rebase=true afterwards, if you make new changes.

Note

When you are working in a fork the file won’t be changed, but the pipeline will fail. Please edit the file manually.

Note

Sometimes the pipeline fails with RuntimeError: No releasenote file added. Make sure to provide a file with your MR. If you provided a note it’s likely that you have to rebase to origin/devel to make the pipeline pass.

Currently we use the following types:

type	description
`BREAKING`	anything which requires manual intervention of users or changes the behavior of end resources or workload
`feature`	new feature introduced
`change`	old functionality changed/updated
`fix`	any bugfixes
`removal`	soon-to-be-removed and removed features (note our policy on deprecation)
`docs`	any changes in the documentation
`security`	security patches
`chore`	updating grunt tasks etc, no production code change, non-user-facing-changes e.g. configuration changes (like .gitignore) private methods update dependencies refactoring
`misc`	everything not needing a description and not of interest to users (even if there is content it’s not written in the releasenotes-file)

Note

For breaking changes please provide detailed information on what needs to be done as an operator. Either in the releasenote itself or linking inside the note to some other source (e.g. parts in our docs, ..)

Nothing to report in the releasenotes

leave your file empty, this will just leave a link to the corresponding MR.

Really nothing to add to releasenotes

if you just correct a typo or something which really no user cares, name your file +.misc.<random>, this will not provide an entry in the releasenotes (and no link to a MR)

So the following file-names would be valid examples:

123.feature
12ads3.feature.addedsomethingdifferent
12.docs.rst
+.misc.jkdfskjhsfd2
something.chore.rst
99.BREAKING.rst
100.BREAKING.idestroyedeverything.rst
käsekuchen.fix.istlecker

The content in the file can be formated using rst-syntax. Headlines are not allowed. For further informations see the towncrier docu.

Disruption

disruption

A disruption is defined as a loss of state or data or loss of availability.

disruptive

Disruptive code is code which may under certain circumstances cause a disruption.

Ansible code MUST be written so that it is non-disruptive by default. It is only allowed to execute disruptive actions if and only if the _allow_disruption variable evaluates to true.

Examples

(Non-exhaustive) examples of disruptive actions:

Restarting docker (for example via a docker upgrade)
Draining a worker or master node
Killing a pod
Rebooting a worker or master node with an OSD on it

Examples of non-disruptive actions:

Rebooting a gateway node if at least one other gateway node is up
Updating a (non-customer) Deployment via Kubernetes

Ansible Styleguide

New-style module syntax

Correct

- name: Upgrade all packages
  dnf:
    name:
    - '*'
    state: latest

Incorrect

- name: Upgrade all packages
  dnf: name=* state=latest

Rationale

The first version is easier to scan. It also supports the use of Jinja2 templates without having to worry about quotation and spaces.

Command module usage

Correct

- name: Get node info
  command:
  args:
    argv:
    - kubectl
    - describe
    - node
    - "{{ inventory_hostname }}"

Also correct

- name: Get node info
  command:
  args:
    argv: ["kubectl", "describe", "node", "{{ inventory_hostname }}"]

Not correct

- name: Get node info
  command: "kubectl describe node {{ inventory_hostname }}"

Rationale

Spaces and possibly quotes in the hostname would lead to issues.

Shell module usage

Correct

- name: Load shared public key
  shell: "wg pubkey > {{ wg_local_pub_path | quote }} < {{ wg_local_priv_path | quote }}"

Not correct

- name: Load shared public key
  shell: "cat {{ wg_local_priv_path }} | wg pubkey > {{ wg_local_pub_path | quote }}"

Partially better

- name: Load shared public key
  shell: "set -o pipefail && cat {{ wg_local_priv_path }} | wg pubkey > {{ wg_local_pub_path | quote }}"

Rationale

Using pipes in the shell module can lead to silent failures without set -o pipefail
Variables should be properly escaped. A ‘;’ or a ‘&&’ in, e.g., the path can lead to funny things. Especially critial if the content of the variable can be influenced from the outside.
The use of cat here is redundant

Use `to_json` in templates when writing YAML or JSON

Correct:

{
   "do_create": {{ some_variable | to_json }}
}

Incorrect:

{
   "do_create": {{ some_variable }}
}

Also incorrect:

{
   "do_create": "{{ some_variable }}"
}

Rationale

If some_variable contains data which can be interpreted as different data type in YAML (such as no or true or 00:01) or quotes which would break the JSON string, unexpected effects or syntax errors can occur. to_json will properly encode the data.

Terraform Styleguide

Use jsonencode in templates when writing YAML

Correct:

subnet_id: ${jsonencode(some_subnet_id)}

Incorrect:

subnet_id: ${some_subnet_id}

Also incorrect:

subnet_id: "${some_subnet_id}"

Rationale

If some_subnet_id contains data which can be interpreted as different data type in YAML (such as no or true or 00:01), unexpected effects can occur. jsonencode() will wrap the some_subnet_id in quotes and also take care of any necessary escaping.

If you are responsible for the creation of releases

How to trigger a release:

Go to rdm and start a pipeline setting YAOOK_K8S_CI_RELEASE to true.
After a few minutes there should be a new release-prepare/v$Major.$Minor.$Patch-branch.
The pipeline is triggered like described in the policy
Make sure the pipeline did pass sucessfully and especially the changelog is rendered correctly, otherwise correct it directly on the branch, this will start a new pipeline.
If you for whatever reason don’t need the predefined timeperiod before the release candidate will become a release, manually start the delayed merge-to-release-branch-job.

What not to do

Don’t change anything on the release-prepare/v$Major.$Minor.$Patch branch, after it was merged to the corresponding
release/v$Major.$Minor-branch. If you see an error or something which needs to be fixed, do it before the merge-to-release-branch-job has started or on devel for the next release.

How to create hotfixes

The general and mandatory outline is described in our policy. As an aid we give an example for a full hotfix process here.

Create a branch of the merge-base of the latest release release/v$Major.$Minor and devel into hotfix/base/$name and create the fix.
```
$ sha=$(git merge-base devel release/v$Major.$Minor)
$ git checkout -b hotfix/base/$name $sha
```

For all releases needing the hotfix, do:

Create a branch of the corresponding release/v$Major.$Minor-branch named hotfix/v$Major.$Minor/$name and cherry pick hotfix/base/$name into it.
```
$ git checkout -b hotfix/v$Major.$Minor/$name release/v$Major.$Minor
$ git cherry-pick $COMMIT_SHA_OF_HOTFIX
```
- Place your relasenote inside docs/_releasenotes/hotfix.
- Create a MR to release/v$Major.$Minor. We will update the version-number in version accordingly and create the changelog using towncrier.
```
$ git push --set-upstream origin hotfix/v$Major.$Minor/$name \
   -o merge_request.create \
   -o merge_request.target=release/v$Major.$Minor \
   -o merge_request.title="Hotfix: $name" \
   -o merge_request.label="hotfix"
```
- Please make sure the version number is correct (it’s a fix for the latest release) and the changelog looks sane before merging.
- Merge the MR.
If you updated the latest release, also update devel by:
- Create a MR from the hotfix/$latest-release-branch to devel as well.
- Please make sure the version number is correct (it’s a fix for the latest release) and the changelog looks sane before merging.
- Merge the MR.

In the end check that the release is tagged and a gitlab-release created. If not, do it manually.

Note

If a hotfix is only relevant for an older version, then create a MR from a branch hotfix/v$Major.$Minor/$name against the corresponding release/v$Major.$Minor-branch directly and skip the other steps.

Special case: There is an open `release-prepare`-branch around

As described in our policy, we have to look at the pipeline. If we can stop the pipeline, do:

Create the hotfix/base/$name-branch as merge-base of the release-prepare/v$Major.$Minor.$Patch and devel branch and create the fix.
Merge the branch into the release-prepare/v$Major.$Minor.$Patch-branch and start the pipeline for the branch again (should happen automatically).
For all older versions needing the hotfix proceed like described above. (Have in mind that the release-prepare/v$Major.$Minor.$Patch-branch could also be a fix-release and merge to the last release/v$Major.$Minor-branch)

Important

Don’t create a hotfix/devel/$name branch merging back to devel as the hotfix will be merged via the release-prepare/v$Major.$Minor.$Patch-branch!

hotfixing-strategy-for-open-release-prepare-branch

Coding Guide

pre-commit-hooks

Creation of release notes

Disruption

Examples

Ansible Styleguide

New-style module syntax

Command module usage

Shell module usage

Use to_json in templates when writing YAML or JSON

Terraform Styleguide

Use jsonencode in templates when writing YAML

If you are responsible for the creation of releases

How to create hotfixes

Special case: There is an open release-prepare-branch around

Use `to_json` in templates when writing YAML or JSON

Special case: There is an open `release-prepare`-branch around