redhat-cop / automation-good-practices Goto Github PK

Please read ansible/ansible-lint#3186 and consider updating the recomandation for prefixing with an example that does not conflict with linter asking for capitalization.

Current example of - name: sub | some task description is not passing linting because it should look like - name: sub | Some task description.

The linter does know about the prefix for roles, but also has a rule that asks for all descriptions/titles to be capitalized, in that particular case, that means that first alpha letter after pipe separator should be capitalized.

var | bool or var | int because variables often come by default as string.

Small quiz, who can guess what happens before trying, and explain why:

---
- name: show an important difference between vars and facts
  hosts: localhost
  gather_facts: false
  become: false

  vars:
    some_var: "{{ lookup('password', '/dev/null') }}"

  tasks:
    - name: define now a fact
      set_fact:
        some_fact: "{{ lookup('password', '/dev/null') }}"
    - name: output now the fact twice
      debug:
        msg: "{{ some_fact }} vs. {{ some_fact }}"
    - name: output again the fact
      debug:
        var: some_fact
    - name: output now the variable also twice
      debug:
        msg: "{{ some_var }} vs. {{ some_var }}"
    - name: output again the variable (uups!)
      debug:
        var: some_var

transfer https://github.com/oasis-roles/meta_standards/ to this new repo and do an initial clean-up, focus on the README.md but the other files might be of interest.

the system roles team recently had a request to be able to run roles when the user has done gather_facts: false in the playbook - what would we recommend? Only document, gather if not yet done, let it fail...

See https://chat.google.com/room/AAAAarh_G-k/LvyYgMPd0-E for details.

I've been pitching this approach for a while and there is now a standard: https://www.conventionalcommits.org/en/v1.0.0/ (it's not as powerful as changelog snippets but probably easier to explain/enforce to your usual admin starting to learn Git/Ansible).

it's easier to apply, and a PR/MR-reviewer/approver can more easily correct/improve the commit message before merging than a file somewhere in the repository.

(and you don't have two sources of truth for changes done, but only git log)

Since it's imho a relevant part of Ansible, should we add a section for EE good practices?

Things like: single purpose, versioning, version pinning of deps etc.

in db28838 asciidoctor-pdf is mentioned.
It would be nice if the docs also showed users and/or potential contributors the command to use, just like they show how to generate HTML, or if a Makefile was added.

MR coming in a moment.

pcfe

The document at https://docs.adfinis-sygroup.ch/public/ansible-guide/overview.html no longer exists, and the entire domain is also no longer registered in the DNS.

As I tried to render the complete document as PDF using asciidoctor-pdf after having added a few recommendations, I noticed that they looked not as expected but were identified as "examples":

I'll look into it once we have a bit of content merged, it's probably possible to rename "example" into "recommendation" at least, but I'd like to make them sub-headings so that the table of content shows each recommendation. Let's see if it's possible, it must render under GitHub, HTML and PDF in a sound way, that's the most important.

Current tooling allows you to create - and build - a playbook like: some-test-playbook.yml, but this will not work when used to run that collection playbook:

$> ansible-playbook xyz.abc.some-test-playbook.yml

There was discussion with the ansible inventories and variable precdence in regards to this
https://docs.ansible.com/ansible/latest/user_guide/playbooks_variables.html#understanding-variable-precedence
and combining
https://docs.ansible.com/automation-controller/latest/html/userguide/workflow_templates.html#ug-wf-templates-extravars

Either here or in tower documentation, we should have a good place to go to determine variable precedence for users to refer to.

This is a note for me to work on this issue and add to our best practices when it comes to where to put your extra vars when designing tower jobs/inventories/workflows, etc etc. However anyone is welcome to beat me to it.

currently the docs are fairly behind the current main branch.
Ideally docs are published after a merge into the main branch (kinda like version "latest").

thoughts?

I think we should remove the following rule from the guidelines:

* When using the `template` module, refrain from appending `.j2` to the file name. This alters the syntax highlighting in most editors and will obscure the benefits of highlighting for the particular file type or filename.
  Anything under the `templates` directory of a role is assumed to be treated as a Jinja 2 template, so adding the `.j2` extension is redundant information that is not helpful.

Reasoning:

1. The given explanation that it removes syntax highlighting for the language used in the templated file in favour of Jinja2 syntax highlighting is actually a point in favour of adding the .j2 file name suffix.
   When you're at the stage of writing a template file, you usually already know how the file should end up looking on the file system, so at that point the Jinja2 syntax is more important.
2. it is very easy to define in your editor settings to use, e.g., "html" syntax highlighting for all files ending in .html.j2. It is much harder to automatically enable Jinja2 syntax highlighting for some files ending on .html.

generic variables within or outside of a collection...

See https://chat.google.com/room/AAAAmYHFeFs/4sQz86ZPbEY for the (internal) discussion about the topic

there's no Figure 1. at https://redhat-cop.github.io/automation-good-practices/#_define_which_structure_to_use_for_which_purpose

pretty sure I saw the image rendered there before, but I might mis-remember as I also read the AGP in PDF format.

Since a pic says more than a thousand words;

Recommend to use a .gitignore file to not capture failed playbook runs in source control.

On roles that we don't expect users to run. Should we prefix those role names with '__' like we do with internal variables that aren't exposed
I don't think you can do that in Automation Hub. The system roles team had to use a prefix like "private_…” (and document!). zzz_private_?
I’ve seen “0_prep”, “pre_”, “setup_”

It is my experience that new users prefer a getting started guide to an idex page of docs; as such I'd rather link to https://docs.ansible.com/ansible/latest/user_guide/index.html#getting-started than https://docs.ansible.com/ in the GPA introduction.

MR coming in a short moment.

Hei,

I wonder what the current status of this project is? Am I missing something or didn't change anything here for a while now?

Is there anything specific I can contribute to in order to maybe put some life back into here again?

This issue has been created as a consequence of #67 : it isn't exactly a recommendation, but more a "if you want to support old versions, do this", but we can't recommend supporting old versions, it's a customer/product owner decision resp. requirement unrelated to the actual quality of the code. #35 is another example of such a topic, neither facts nor vars are "better", they just are different in behavior.

Upon looking for input from other Red Hatters on ansible playbook Style Guides,
It was suggested that we use "openshift-ansible Style Guide"
https://github.com/openshift/openshift-ansible/blob/master/docs/style_guide.adoc

The attached guidelines are from a project and some of them could surely be reused (format is Asciidoc even if the extension is txt):

GUIDELINES_AUTOMATION.txt

I disagree with this one, I name my tasks with a non-capital letter, it looks IMHO better in the resulting log.

This said, I'm flabbergasted that I oversaw the rule to have all tasks named, so well catched.

Originally posted by @ericzolf in #64 (comment)

The Example sections are not examples.

Create Examples, that you can read as it should be.

Nothing more, nothing less.

Some people learn better with examples to understand the point you are making.
(My native language is not English, so i rely on examples to make sense about the text)
Understanding the text is one thing, seeing real examples is more valuable.

Thanks

Hi,

I'm not sure, but I think the scope is wrong. It should be "play" and not "role".

Accordingly to the current documentation, Ansible knowns 3 scopes global, play and local. If we create a new variable at block or task level, the variable has the scope of the current play.

Regards,
Carsten

An idea that came up out of my current engagement, a section to feature good practices for developing ansible in an enterprise. This would include sections around the pre-commit checks/pipelines

• yamllint
• ansible-playbook --syntax-check
• ansible-lint
• molecule
• ansible-playbook --check
• sandbox environments
  (see https://www.youtube.com/watch?v=FaXVZ60o8L8&t=1266s)

using development environments

• vs code or atom integrations

and source control strategies.
branches, merge requests, code reviews, etc

Hi,

in the documenation the image "Figure 2. Flow of variable precedences" is not shown. It looks like the file variable_precedences.png is not published, but the first image ansible_structures.svg (Figure 1. Structure of Automation) has been published.

Please publish variable_precedences.png.

Thank you,
Carsten

https://github.com/openshift/openshift-ansible/blob/master/docs/style_guide.adoc and/or https://github.com/openshift/openshift-ansible/blob/master/docs/best_practices_guide.adoc

Image links are broken and need to be resolved.

Anti-pattern:

list_of_hosts_to_create:
  host1: var1, var2, var3
  host2: var1, var2, var3

then loop over the list and (often) create the host against oVirt, vSphere, etc...

Recommended pattern:

inventory/host_vars/{host1,host2}/vm_create.yml (with var1, var2, var3 + variable vm_creation_server)

then:

- hosts: vms_to_be_created
  gather_facts: false  # important or it will fail if the VM doesn't exist yet
  tasks:
    - name: create VM(s)
       create_vm:
          vm_creation_endpoint: "{{ vm_creation_server }}"
          [ ... etc ... ]

Rationale: is parallelized automatically by Ansible, easier to maintain, easier to --limit to certain hosts

As per this ansible lint role, a recommendation to use loop and loop_var - https://ansible-lint.readthedocs.io/en/latest/default_rules/#no-loop-var-prefix

See how much from https://www.ansiblejunky.com/blog/ansible-101-standards/ can be reused

It was a long standing idea, which I've moved now from the CoP minutes to this issue.

Include some guidance around updating all packages

example:

yum:
state: latest
name: "*"
update_only: yes

The file docs/index.html has currently to be generated manually, when required.

The following GitHub actions are required:

• make sure that each PR hasn't broken the asciidoc format
• generate the pages on release version tags vX.Y.Z
• generate the pages to a non-too visible place e.g. docs/index.pre.html on pre-release tags (e.g. vX.Y.Z[ab|rc]N for alpha, beta, release candidate)
• generate the pages to a non-too visible place e.g. docs/index.dev.html on master branch

Overall:

• the tag/branch/commit should be in the generated HTML so that we can track what we see
• we might need to move the content of docs to a specific branch (if this is the decision, a project admin needs to change the configuration of GitHub pages, to take the pages from there instead of master).

Just random notes about recommendations as I go through old documents:

• make delegate_to configurable and localhost only the default (it might not have access to the API, might not have the necessary dependencies, especially true for Tower)
• don't write playbooks against localhost, the host might be remote

Are (parts of) these good practices available as custom rules for ansible-(lint|later), or should they be developed?

The recommendations should make a mention of Tower, because that hat has implications, e.g., on how best to separate inventories and code in Git.

Hey all,

thank you for a wonderful repository of best practices.

I noticed that the "Ansible Guildlines" section (8.3) states:

Use the smallest scope for variables. Facts are global for playbook run, so it is preferable to use other types of variables. Therefore limit (preferably avoid) the use of set_fact. Role variables are exposed to the whole play when the role is applied using roles: or import_role:. A more restricted scope such as task or block variables is preferred.

As far as I can tell this is not correct for import_role (any more). Only when public: true is specified do role arguments bleed into the global scope.

Is this from an older version of ansible and should be changed?

We might want to create a new AAP section for this, but let's first try to get some kind of consensus within the community, so let's start the discussion.