diff options
| -rw-r--r-- | docs/best_practices_guide.adoc | 73 |
1 files changed, 69 insertions, 4 deletions
diff --git a/docs/best_practices_guide.adoc b/docs/best_practices_guide.adoc index 301c6ccda..841f6e72c 100644 --- a/docs/best_practices_guide.adoc +++ b/docs/best_practices_guide.adoc @@ -19,7 +19,6 @@ This guide complies with https://www.ietf.org/rfc/rfc2119.txt[RFC2119]. | All pull requests MUST pass the build bot *before* they are merged. |=== - The purpose of this rule is to avoid cases where the build bot will fail pull requests for code modified in a previous pull request. The tooling is flexible enough that exceptions can be made so that the tool the build bot is running will ignore certain areas or certain checks, but the build bot itself must pass for the pull request to be merged. @@ -50,6 +49,7 @@ Instead, http://docs.pylint.org/faq.html#is-it-possible-to-locally-disable-a-par .Exceptions: 1. When PyLint fails because of a dependency that can't be installed on the build bot 1. When PyLint fails because of including a module that is outside of control (like Ansible) +1. When PyLint fails, but the code makes more sense the way it is formatted (stylistic exception). For this exception, the description of the PyLint disable MUST state why the code is more clear, AND the person reviewing the PR will decide if they agree or not. The reviewer may reject the PR if they disagree with the reason for the disable. ''' [cols="2v,v"] @@ -78,9 +78,71 @@ metadata[line] = results.pop() == Ansible -=== Roles +=== Yaml Files (Playbooks, Roles, Vars, etc) + +''' +[cols="2v,v"] +|=== +| **Rule** +| Ansible files SHOULD NOT use JSON (use pure YAML instead). +|=== + +YAML is a superset of JSON, which means that Ansible allows JSON syntax to be interspersed. Even though YAML (and by extension Ansible) allows for this, JSON SHOULD NOT be used. + +.Reasons: +* Ansible is able to give clearer error messages when the files are pure YAML +* YAML reads nicer (preference held by several team members) +* YAML makes for nicer diffs as YAML tends to be multi-line, whereas JSON tends to be more concise + +.Exceptions: +* Ansible static inventory files are INI files. To pass in variables for specific hosts, Ansible allows for these variables to be put inside of the static inventory files. These variables can be in JSON format, but can't be in YAML format. This is an acceptable use of JSON, as YAML is not allowed in this case. + +Every effort should be made to keep our Ansible YAML files in pure YAML. + +=== Defensive Programming + .Context -* http://docs.ansible.com/playbooks_best_practices.html#directory-layout[Ansible Suggested Directory Layout] +* http://docs.ansible.com/fail_module.html[Ansible Fail Module] + +''' +[cols="2v,v"] +|=== +| **Rule** +| Ansible playbooks MUST begin with checks for any variables that they require. +|=== + +If an Ansible playbook requires certain variables to be set, it's best to check for these up front before any other actions have been performed. In this way, the user knows exactly what needs to be passed into the playbook. + +.Example: +[source,yaml] +---- +--- +- hosts: localhost + gather_facts: no + tasks: + - fail: msg="This playbook requires g_environment to be set and non empty" + when: g_environment is not defined or g_environment == '' +---- + +''' +[cols="2v,v"] +|=== +| **Rule** +| Ansible roles tasks/main.yml file MUST begin with checks for any variables that they require. +|=== + +If an Ansible role requires certain variables to be set, it's best to check for these up front before any other actions have been performed. In this way, the user knows exactly what needs to be passed into the role. + +.Example: +[source,yaml] +---- +--- +# tasks/main.yml +- fail: msg="This role requires arl_environment to be set and non empty" + when: arl_environment is not defined or arl_environment == '' +---- + +=== Roles ''' [cols="2v,v"] @@ -89,6 +151,9 @@ metadata[line] = results.pop() | The Ansible roles directory MUST maintain a flat structure. |=== +.Context +* http://docs.ansible.com/playbooks_best_practices.html#directory-layout[Ansible Suggested Directory Layout] + .The purpose of this rule is to: * Comply with the upstream best practices * Make it familiar for new contributors @@ -101,7 +166,7 @@ metadata[line] = results.pop() | Ansible Roles SHOULD be named like technology_component[_subcomponent]. |=== -For clarity, it is suggested to follow a pattern when naming roles. It is important to note that this is a recommendation for role naming, and follows the pattern used by upstream. +For consistency, role names SHOULD follow the above naming pattern. It is important to note that this is a recommendation for role naming, and follows the pattern used by upstream. Many times the `technology` portion of the pattern will line up with a package name. It is advised that whenever possible, the package name should be used. |