Fix version determination

[jardleex]

Hello @mrlesmithjr

just found a minor issue in the version upgrade enhancement which get's fixed here. The shell module needs to be used instead command as pipe's are used in it. Documentation

I also added a comment on the consul_update option which recommends to use --fork to keep the impact for the consul cluster low.

Best

Jard

[jardleex]

I just found an issue with the shell replacement. The return code is always 0 of the piped shell commands. This can cause other errors in the play as the return code gets checked on other tasks.

I'm working on a solution right now.

[jardleex]

Okay I made it more robust with checking if the binary is present so the role does not directly depend on the return code of the shell command.

What do you think?

[mrlesmithjr]

I think we should recommend using serial: in the playbook using this role rather than using --forks when doing upgrades and etc. Or maybe even just recommend this for an upgrade playbook.

---
- hosts: consul
  # Define number of hosts to execute at a time
  serial: 3
  roles:
    - role: ansible-consul

Thoughts?

[jardleex]

Recommending serial would be an option too. I choose --forks as I excpect an upgrade run to be a relative rare task. Altering a playbook for only one run which is maybe even under source control ain't looked so attractive to me then just recoomending a onetime command line switch. Maybe offer both way's as comment next to the consul_upgrade switch? So the user can decide what he likes more.

I defenitly would not recommend it as default as it would slow down the whole role exectuion drasticly. When I'm thinking of on of our locations with 300+ nodes, it would take like ages.

[mrlesmithjr]

Good point. I say we go with --forks as an option as well. I completely agree that it is a real pain to modify a playbook for a single task in version control.

[jardleex]

How's about this? Would put this into defaults/main.yml and README.md

...
consul_version: '0.8.1'

# Defines if role upgrades consul binary on servers to consul_version
# Downgrades are not considered
# Recommendation: When set to true and after raising `consul_version`, consider using `--forks=1` on next Ansible run to keep impact on Consul cluster low on Upgrade.
consul_upgrade: false

consul_wan_group: 'consul_wan'

[mrlesmithjr]

I like that

[jardleex]

Is there anything else you'd like to have adjusted or can this get merged?

Thanks!

[mrlesmithjr]

Was just waiting for you to say you were done ;)

[jardleex]

All done here. Can get merged :).

[mrlesmithjr]

Done! Thanks man