search

devops-coop / ansible-haproxy

Installs and configure HAProxy
Apache License 2.0
96 stars 97 forks source link

Role is not idempotent #68

Closed onitake closed 7 years ago

onitake commented 7 years ago

While check mode works as expected and displays a proper diff between old and new configs, executing the role will always rewrite the configuration directories.

This does not actually break things, but seems like poor design.

Is there a particular reason why there is a removal task before writing the configuration files?

If there is concern about leftovers, why not just leave out those that will be written?

Also, it would be better to use the file module instead of running find, I think.

benwebber commented 7 years ago

The directories are emptied first so that assemble does not collect additional files.

This part of the code is quite old and will probably be replaced very shortly with a simpler template.

onitake commented 7 years ago

Thanks for the great fix!

It's a bit unfortunate that standard Ansible distributions don't include jmespath (which is required for json_query) by default, but json_query is so useful that it should be a dependency IMHO.

benwebber commented 7 years ago

Ah, shoot. My local Ansible distribution includes that dependency, so I didn't notice it.

We can work around this by writing a custom filter plugin (#77). I'll take a stab at that shortly.