Allow dry-run with our role #128
Conversation
hush-hush
commented
Apr 25, 2018
•
hush-hush
commented
- Fix the role to be compatible with ansible dry-run
- Fix downgrading from agent 5 to 6 on centos
- Improve our travis tests
hush-hush
force-pushed
the
maxime/fix-dry-run
branch
3 times, most recently
from
49ef603
to
d7f0805
Compare
|
@hush-hush |
|
@chaosoldier I'm waiting for a review from @olivielpeau. We're currently focusing on releasing the next version of the agent (6.2.0). We will merge and release the ansible role as soon as possible, |
|
@hush-hush
I understood! |
There was a problem hiding this comment.
One general comment: according to https://docs.ansible.com/ansible/devel/user_guide/playbooks_checkmode.html#information-about-check-mode-in-variables, the variable ansible_check_mode is only available on versions >=2.1 of ansible. What happens on earlier versions of ansible?
Other than that and the inline nits, LGTM! The tests are very nice 👌
README.md
Outdated
| @@ -41,7 +41,7 @@ To install agent5, you need to: | |||
| - either set `datadog_agent_version` to an existing agent5 version | |||
| (recommended) or leave it empty to always install the latest version (`5.*`). | |||
|
|
|||
| To downgrade from agent6 to agent5, you need to: | |||
| To downgrade from agent6 to agent5, you need to (**on centos this will only work with ansible 2.4+**): | |||
There was a problem hiding this comment.
you should also update the description of the datadog_agent_allow_downgrade variable in the Role Variables section of the readme
tasks/pkg-debian.yml
Outdated
| @@ -1,6 +1,7 @@ | |||
| --- | |||
| - name: Install apt-transport-https | |||
| apt: name=apt-transport-https state=present | |||
| when: not ansible_check_mode | |||
There was a problem hiding this comment.
why is this needed? Shouldn't the package be always in the apt repo?
There was a problem hiding this comment.
if no ansible job using apt was ever run, --check will fail because python-apt is not install.
fatal: [127.0.0.1]: FAILED! => {"changed": false, "msg": "python-apt must be installed to use check mode. If run normally this module can auto-install it."}
Adding the check allow users to check their playbook without installing python-apt.
| @@ -18,18 +19,21 @@ | |||
| with_items: | |||
| - "deb http://apt.datadoghq.com/ stable main" | |||
| - "deb http://apt.datadoghq.com/ stable 6" | |||
| when: not ansible_check_mode | |||
|
|
||
| - name: Ensure Datadog repository is up-to-date | ||
| apt_repository: | ||
| repo: "{{ datadog_apt_repo }}" | ||
| state: "{% if datadog_agent5 %}absent{% else %}present{% endif %}" | ||
| update_cache: yes | ||
| when: not ansible_check_mode |
|
@olivielpeau ansible 2.1 entered end of life last year (2017-06-01). I think we can no longer support it ? |
- Testing dry-run - Testing installing agent 5 and 6 - Testing downgrading to agent 5 - Test the role with ansible 2.2 and 2.4
hush-hush
force-pushed
the
maxime/fix-dry-run
branch
from
d7f0805
to
88dd977
Compare
There was a problem hiding this comment.
LGTM!
I'm fine with dropping support of <2.1 ansible versions, but let's not forget to update the declared minimum supported version of ansible here: https://github.com/DataDog/ansible-datadog/blob/2.0.3/meta/main.yml#L6 :)
This allow us to use the latest feature from ansible like dry_run (ie: 'ansible_check_mode').
