Make Ansible default verifier

[webknjaz]

Proposal: Make Ansible default verifier

Author: Sviatoslav Sydorenko <@webknjaz> IRC: webknjaz

Date: 2019-04-26

• Status: New
• Proposal type: core design, consistent UX
• Estimated time to implement: 1h

Motivation

The UX is inconsistent with the Ansible Engine and Galaxy which promise the ability to keep using YAML. "native" way of doing testing should come first which will improve the experience for new users.

Problems

What problems exist that this proposal will solve?

• Inconsistent UX across Ansible-related projects

Solution proposal

• set Ansible to be a default verifier while keeping other options available.

Dependencies (optional)

• Now that #1714 is merged, this is finally possible!

Testing (optional)

N/A

Documentation (optional)

Just a :versionchanged: pointer.

Anything else?

At Ansible Core and Galaxy we try to ensure the consistent user experience so we all agreed that defaults should be "native" to be "closer" to the user expectations and simplicity promises. This will also facilitate the further introduction of Ansible Collections creation, testing and publishing workflows.

[MarkusTeufelberger]

I see some value in having a tool that is NOT Ansible to verify something that was modified by Ansible. Also writing good assertions in Ansible is surprisingly hard and brittle. Lastly the Ansible verifier is very new, I'm not so sure if there are not still some kinks in there (e.g. imho it should fail the test if any job in the verification reports changed).

None of these reasons are good enough to warrant a veto, I'd just be more comfortable with that change for e.g. Molecule around version 1.23, not directly the next release.

[webknjaz]

@MarkusTeufelberger your argument makes sense. The thing is that we don't want to recommend learning a programming language additionally to what Ansible users already know because this makes things inconsistent and more complicated. Modules/plugins authors will develop content in Python though and having playbooks to verify those will also bring in a proper integration testing into the scenario.

[MarkusTeufelberger]

Modules I co-developed are extensively integration tested (https://github.com/ansible/ansible/tree/devel/test/integration), but in those the assertions and state generation are usually quite close together.

The approach here would be to have one playbook to generate state, then one independent(!) playbook to read out that state again and assert on it. Personally, if I see the need to make sure that a certain thing has worked (e.g. setting up a webserver: run a HTTP GET on localhost to make sure it forwards to port 443), I usually just add this as a final task to a role instead of decoupling it completely by having a separate test/assert playbook run sporadically. That's just personal style though.

In general, Ansible is rather bad in testing specifications instead of one single implementation of a specification, since it was built to provide a certain implementation. Independent of this change, I'd love to see one more "standard folder" for roles named tests that are run after a role was applied with a fail if task.failed or task.changed policy. Then we might have a better chance that over time more people learn how to even write good tests and specs for Ansible roles at least.

[decentral1se]

I'm in favour.

We should make a pre-release testing period since it is brand new feature.

[themr0c]

I get splitbrain in this case: one one hand I personally dislike testinfra as default verifier, on the other hand I very much agree with @MarkusTeufelberger that having an independent audit tool is a feature, so replacing testinfra by ansible would be quite a regression. But my preferred candidate, Goss, is not available for Windows testing, and I am not convinced either by the latest candidate in the list, inspec. So I won't be blocking the move to ansible as default verifier, as it is maybe still the least bad option for the time being.

[MarkusTeufelberger]

According to https://www.inspec.io/ it should be "platform agnostic" (whatever that means). There are binaries for Windows here: https://downloads.chef.io/inspec My issue with Inspec last I checked [heh!] it out was more with Ruby than the tool itself...

Ansible and Molecule are not really supported to run on Windows either though, are they?

[themr0c]

You would be surprised, there is a very good support of Windows. My concerns with Inspec are quite the sames as my concerns with Testinfra: the language to describe the tests is quite complex (as opposed to the admirable simplicity of goss which uses YAML by default, and some go specific additions).

[skhoroshavin]

But what's wrong with TestInfra? It integrates neatly with molecule, it allows writing very clear tests (IMO they are more readable than Ansible, and they are easier to write than InSpec, but I think I'm biased with the latter), it allows writing integration tests (spin up several VMs with different roles and run different testinfra tests on them), and it allows running those tests separately.

[webknjaz]

Ansible and Molecule are not really supported to run on Windows either though, are they?

Ansible Engine technically has two sides: controller and target. So the controller is only officially supported under UNIX. But targets can be Windows hosts as well (via RDP and SSH connection plugins).

wrt Molecule, there's a generic intention of supporting a consistent UX so Windows support is becoming a priority as well. But I'm not sure whether Molecule CLI itself should run there or just target tested platforms.

But what's wrong with TestInfra?

It's not going anywhere, just shouldn't be kept as a default. Also, it's currently broken for the latest Ansible Engine which may be challenging to fix at this point. Also, for the UX consistency role authors shouldn't have to learn yet another language to write tests from the product design perspective. It's fine if they already know it and choose to use it by themselves. Yet, we shouldn't force them to this.

[decentral1se]

This isn't about replacing or removing testinfra. It's a great tool. This is about changing the default to Ansible for verification as has been well explained in https://github.com/ansible/molecule/issues/2013#issuecomment-487422948.

Something that came to mind that should be extended in this proposal: the "documentation (optional)" part - we should provide some examples and usage examples. Ansible is not a "pure" verification tool but it can do it. We should show some best practices on what modules to use for typical use cases: "is the web server up".

[MarkusTeufelberger]

(With "running on Windows" I meant "supported as control machines - the one running ansible-playbook or molecule test", not "configuring/testing Windows hosts")

[webknjaz]

@MarkusTeufelberger while Windows controller is not supported officially I can imagine that somebody could make it work by using WSL (but it is only my guess). If you feel like you need it, feel free to do the research, implementation proposal and maybe PoC. I'm not sure whether anyone else would have free time for this now.

[gundalow]

We discussed this in the IRC Meeting 2019-05-01, we agreed that

• Documentation needs adding
• Need to be in for a release, get feedback, then we can revisit making this default.

Documentation needed

• Examples on typical use cases?
• Are services up/packages exists/files, dirs perms check/asserts/which modules are recommended
• Details on how to assert, what's a good pattern to use.
• What to assert in an ansible verifier.
• Having a separate verifier outside of the converge playbook is limiting, since you won't have access to facts registered in converge, so it might be good to document ways to get around that problem?

[decentral1se]

Examples of verifying: https://github.com/ansible/ansible/blob/devel/test/integration/targets/stat/tasks/main.yml

[decentral1se]

Further ideas at the latest WG on the documentation issue:

• Link to https://docs.ansible.com/ansible/latest/reference_appendices/test_strategies.html
• Create a "How to use Ansible as a test verifier" in https://molecule.readthedocs.io/en/stable/examples.html
• Link to this examples section from configuration section in https://molecule.readthedocs.io/en/master/configuration.html#id22
• Later on, when 2.8 is out and molecule gets a new release, try to improve the testing_strategies page

[decentral1se]

Does anyone have time to submit a documentation PR for this? We'll be discussing once again an attempt to make a release and this is still required to get that done. Would be nice to hear from people using this in the wild.

[skhoroshavin]

Would be nice to hear from people using this in the wild.

I've decided to give it a try in my pet project and to my surprise I really like it, despite the fact that in the beginning I was kind of opposed to the idea.

[gundalow]

@skhoroshavin that's great feedback. I wonder if we can use your experience to better explain this work for potential future users.

1) Why were you opposed to it initially?

2) What changed your mind?

[tehsmyers]

We'll be discussing once again an attempt to make a release and this is still required to get that done.

I think the ansible verifier might be good enough to include in a release as-is, with the understanding that more documentation is desired before it can become the default verifier.

[skhoroshavin]

@gundalow not taking into account being used to writing tests with testinfra my main concern was verbosity and ease of writing. With testinfra and pytest most things are simple and straightforward, but with ansible many of them can be a major pain, especially when it comes to wrestling with jinja templates. By the way I still think these concerns are valid in some cases. For example, with testinfra writing test that checks that firewall logged unsuccessful connection attempts is as simple as:

ufw_log = host.file('/var/log/ufw.log').content_string.splitlines()
for line in ufw_log:
   assert '[UFW BLOCK]' in line
   assert f'SRC={auditor_ip}' in line
   assert f'DST={instance_ip}' in line
   assert f'DPT=53' in line

However with ansible this turns into

    - name: Read firewall log
      slurp:
        src: /var/log/ufw.log
      register: ufw_log
      become: yes

    - name: Parse firewall log
      set_fact:
        ufw_log: |
          {% set ufw_log = ufw_log.content | b64decode %}
          {{ ufw_log.splitlines() }}

    - name: Ensure failed connection attempt was logged
      assert:
        that:
          - ufw_log | length > 0
          - "'[UFW BLOCK]' in item"
          - "'SRC={{ auditor_ip }}' in item"
          - "'DST={{ instance_ip }}' in item"
          - "'DPT=53' in item"
      loop: "{{ ufw_log }}"

Much more verbose and cryptic, isn't it?

However, it turned out that ansible verifier has its own strengths, especially in multi-node integration tests:

• it's straightforward to restrict groups of test tasks to particular groups of hosts without splitting test into multiple files, and it even allows to switch hosts in the middle using delegate_to if needed (for example, try connecting to filtered port from auditor node, then check logs from instance under test, then run more stuff from auditor)
• it's straightforward to access host and group variables. With testinfra you need to do something like host.ansible.get_variables()['foo']['bar'], but with ansible its just foo.bar. It doesn't look like a big deal until you write a bunch of tests with testinfra and then try to rewrite them in ansible - it feels so much more natural

Also, not all tests are as verbose and painful in ansible as above mentioned example. Some are very neat, in particular following example reads very well in my opinion (and was easy to write), and I'm not sure whether it would look better with testinfra:

- name: Verify
  hosts: all
  vars:
    client_addr: "{{ hostvars.client.ansible_facts[net_dev].ipv4.address }}"
    router_addr: "{{ hostvars.router.ansible_facts[net_dev].ipv4.address }}"
  tasks:
    - name: Ensure IP addresses are assigned correctly
      assert:
        that:
          - router_addr == hostvars.router.lan_addr
          - client_addr >= hostvars.router.lan_min_addr
          - client_addr <= hostvars.router.lan_max_addr
      run_once: yes

    - name: Ensure instances can access each other
      wait_for:
        host: "{{ item }}"
        port: 22
      loop:
        - "{{ router_addr }}"
        - "{{ client_addr }}"

FInally, when following TDD switching back and forth between ansible and testinfra forces you to repeatedly switch your mind between two languages. It's not a major issue, but in the long run it's still tangible in my opinion.

[ssbarnea]

Note to everyone involved in this proposal: please use up/down buttons on the ticket description to vote if you support making ansible verifier default instead of testinfra. That is not about dropping support but only about which verifier is the default one.

[MarkusTeufelberger]

My :-1: is conditional on making it default right away, I'd be more convinced after 2-3 releases with the verifier in so it is more clear if it is actively used and useful.

[tehsmyers]

My :-1: is similarly conditioned. I'm in favor of reconsidering whether or not to make ansible the default verifier once it's been in molecule for a while.

[tima]

I will third the motion 👎 -- the ansible verifier is a great piece of work and I think should eventually be the default (testinfra requires python skills that most ansible users do not have) but it more throughly tested and refined before that switch is made. I think there is a lot that can be done to make testing easier to do and less verbose than what is necessary today. (See the useful feedback from @skhoroshavin as an example of what needs work.)

To be clear, I'm not saying never just not now. I think this issue should remain open and revisited at a later date.

[decentral1se]

So, until another time then folks :tada:

[gundalow]

For those following at home, a PR has been raised (thanks @ssbarnea) for doing this Makes ansible default, verifier.