search

jaytaylor / ansible-kafka

Ansible Kafka role
https://galaxy.ansible.com/jaytaylor/kafka/
Other
67 stars 52 forks source link

run register tasks also in ansible check mode (requires ansible v2.2) #21

Closed lhoss closed 6 years ago

jaytaylor commented 7 years ago

@lhoss So am I understanding this right - This will break older ansibles, e.g. 1.9.x series?

What is the benefit of adding this?

lhoss commented 7 years ago

@lhoss So am I understanding this right - This will break older ansibles, e.g. 1.9.x series?

indeed, because that check_mode feature was added only in v2.2: Ref: http://docs.ansible.com/ansible/playbooks_checkmode.html#enabling-or-disabling-check-mode-for-tasks In this case the old feature always_run: yes would work, but it's deprecated (so we prefer to use the future_proof one, in our roles instead of caring to support outdated ansible), see also: https://github.com/willthames/ansible-lint/issues/220

Benefit: You can test/run the full role with --check, a very valuable way to test a live/prod deployment (like seeing the config changes with --diff) without really deploying changes.

jaytaylor commented 7 years ago

@lhoss Can the .travis.yml file be updated to reflect what we'll support? Can't really consider merging this until the build is passing.

lhoss commented 7 years ago

@lhoss Can the .travis.yml file be updated to reflect what we'll support? Can't really consider merging this until the build is passing.

indeed, should run (most) tests with a recent 2.2.x ansible, for ex as in https://github.com/asaladin/ansible-kafka/commit/c869b3e56966a7b4051f072678e398d1490cfee0 ps: this fork might contain a number of interesting improvements

lhoss commented 6 years ago

@jaytaylor sry for not getting back earlier .. so are the travis tests still not requiring a more recent ansible ? ( I can help check howto adapt the tests) We are now at stable v2.4.x, and I'ld say it's not a good idea to use anymore v2.2.x So we could use check_mode were useful!

jaytaylor commented 6 years ago

@lhoss I'm open to deprecating support for old versions of Ansible. Just open a PR and we can work to get it merged.

lhoss commented 6 years ago

closing this PR, as a --check deployments now work (except the final kafka port test) thanks to recent improvements like: