Fix provisioning on Python 3 / Amazon Linux 2

[moleskin-smile]

Fix provisioning on Python 3 / Amazon Linux 2.

This (c1db357) is the essence of this PR.

Don't use python.version.major >= 3 condition to select dnf instead
of yum, because it breaks Python 3 on Amazon Linux 2 setups, where
yum is still used. Use ansible_pkg_mgr fact directly instead.

Remove an explicit fail that's too defensive. Improve the explicit "fail early" task to take into account all the variables.

Vagrantfile to reproduce the problem:

Vagrant.configure("2") do |config|
  config.vm.define "amazonlinux" do |c|
    c.vm.box = "bento/amazonlinux-2"
  end

  # existence of Python 3 is enough to fail this role
  config.vm.provision "shell", inline: "yum install -y python3 python3-pip"

  config.vm.provision "ansible" do |ansible|
    ansible.playbook = "test_7_full.yml"
  end
end

Fixes: #406.

Add manual testing for Centos 8 and Amazon Linux 2

(cb9d25b)

Add manual testing on Centos 8 and Amazon Linux 2

1. Extend manual tests setup to include more operating systems – modern
   RedHat-style one (Centos 8) that uses dnf, legacy RedHat-style one
   which uses yum (Amazon Linux 2). Keep Ubuntu as a default one.
2. Run Ansible on provisioned machines automatically. By default all
   playbooks are run. Now there is no need to change current working
   directory to interact with Ansible and Vagrant. There's also no need
   to modify inventory file if ports change. There're also no problems
   with SSH known hosts anymore.
3. Update readme.md.
4. Git ignore .vagrant.

[bkabrda]

Hey 👋 thanks for sending the PR. Overall it looks great, but I would like to keep the condition Fail early if Python 3 is used on CentOS / RHEL < 8. If this is a problem for you on Amazon Linux (which I'm assuming it is), could you add another part to the condition, something like ansible_facts.distribution != 'Amazon'? That should make it work for Amazon Linux, but also keep the condition working for CentOS/RHEL. Thanks!

[moleskin-smile]

Hey! Thanks a lot for looking at this.

Regarding the Fail early condition – could you please explain what is the point of failing here? Python 3 can still be used on Centos 7 without problems. In my opinion using python.version.major >= 3 condition to determine package manager is not correct regardless of Linux distribution.

The following setup works with my fixes, but breaks if we restore Fail early or use dnf based on Python version:

❯ head -5 test_7_full.yml

- hosts: all
  roles:
    - { role: ../../ansible-datadog, become: yes }  # On Ansible < 1.9, use `sudo: yes` instead of `become: yes`
  vars:
    ansible_python_interpreter: /usr/bin/python3

❯ cat Vagrantfile

Vagrant.configure("2") do |config|
  config.vm.define "centos" do |c|
    c.vm.box = "centos/7"
  end

  config.vm.provision "shell", inline: "yum install -y python3 python3-pip"

  config.vm.provision "ansible" do |ansible|
    ansible.playbook = "test_7_full.yml"
  end
end

[cswatt]

looks good for docs!

[bkabrda]

Hey 👋 are you using Ansible 4 or newer? Ansible 4 (which uses ansible-core >= 2.11.0) implemented respawn_module [1] (which gets used by yum module) which will respawn the python process running on the host under Python 2 and thus your play would pass even without the condition, while it would fail on ansible < 4.

Because we still support Ansible >= 2.6, we want to keep this condition present (and I'll be more than happy to drop it once we only support Ansible >= 4).

Does this make sense? If so, could you please re-add the condition and also add the Amazon clause to it, so it would work for you? Thanks!

[1] ansible/ansible@4c5ce5a

[moleskin-smile]

Thanks, that explains a lot! So I've re-added the Fail early task, but altered it the way that's more specific. I think this way it won't break real setups but still would be helpful directing users towards possible solutions.

So the new logic to fail is the following:

  when: (not datadog_ignore_old_centos_python3_error)
        and (ansible_version.full is version("2.11", operator="lt", strict=True))
        and (ansible_pkg_mgr == "yum")
        and (ansible_facts.python.version.major | int >= 3)

I've tested this manually with test_7_full.yml playbook and:

1. Ansible 2.8.20 + Python 3 + Centos/7 – early fail
2. Ansible 2.8.20 + Python 3 + Amazonlinux – early fail
3. Ansible 2.8.20 + Python 3 + Centos/8 – pass
4. Ansible 2.8.20 + default Python + Centos/7 – pass
5. Ansible 2.8.20 + default Python + Amazonlinux – pass
6. Ansible 2.8.20 + default Python + Centos/8 – pass
7. Ansible core 2.12.1 + Python 3 + Centos/7 – pass
8. Ansible core 2.12.1 + Python 3 + Amazonlinux – pass
9. Ansible core 2.12.1 + Python 3 + Centos/8 – pass
10. Ansible core 2.12.1 + default Python + Centos/7 – pass
11. Ansible core 2.12.1 + default Python + Amazonlinux – pass
12. Ansible core 2.12.1 + default Python + Centos/8 – pass

BTW, can we also test with Ansible 2.11+ on CircleCI?

[bkabrda]

Looks great, merging.

BTW I'm working on adding ansible 2.11 tests in CI and will hopefully have them set up later this week. I tested this PR locally with ansible 2.11 and it works fine.

[daroga0002]

@bkabrda when is planned release of module with this feature?

[bkabrda]

@daroga0002 I'd like to do a new release this week. I'm currently doing pre-release testing, so we'll see if I hit any problems or not. I'll update this issue later today with the progress.

[bkabrda]

@daroga0002 I've just released version 4.14.0 with this feature: https://github.com/DataDog/ansible-datadog/releases/tag/4.14.0 Enjoy!

[daroga0002]

Thank you