Remove version_lock option from the role
tgadiev opened this issue · 12 comments
Do we really need version_lock parameter to be included in this role? This looks to be excessive since not directly related to role scenario. It breaks idempotency in many cases. I'd suggest to remove this option from the role. Version lock can be easily done by an additional playbook task after role if necessary. It can be included in README examples if needed.
Then a version should also be optional to have apt installing the current latest. Without the version_lock you would otherwise get downgrade errors for example when the system using unattended upgrades.
@MartinVerges, yes you're right. I'll try to raise a PR accordingly.
Hi @tgadiev,
As mentionned by @MartinVerges, version_lock is what allows to select the version to install. but also when beats upgrade will happen, as we don't recommend letting beats upgrade without any control (despite it's not as much critical as Elasticsearch upgrade).
As for version_lock breaking indempotency, our current Kitchen tests scenario are verifying indempotency and didn't detect any issue. Can you give us more details?
Hi @jmlrt,
Maybe I'm missing some points, but what's the reason for specific version lock to be additionally set by this role? If the exact beats_version is specified then no other versions are installed until beats_version is changed. Upgrade will not happen if you don't change specified beats_version parameter. What kind of additional control are we talking about?
Regarding idempotency. Current ansible code uses enforced version lock delete before install and optional version lock set after package is installed. These steps are not shown as changed due to changed_when: false settings. But such approach is not a best practice since the real systems state does not match the status of the tasks shown. If you remove that kind of tricky option and try to make the real status of changed flag the idempotency will fail since package lock option will be changed on each run despite of the actual system state.
Upgrade will not happen if you don't change specified beats_version parameter
Any yum upgrade or apt upgrade command run on the system could upgrade beats to latest version if we don't lock the package. Running this ansible role again after some "external" package upgrade would fail because Ansible would try to downgrade the package, requiring to either update beats_version or force downgrade with es_allow_downgrades=true.
Ok, if we're talking about system-wide commands like yum upgrade - yes, it can change the version of the installed beat. But why should we handle that with this role? That's completely different approaches - set up system by configuration tools like Ansible and perform manual tasks like yum upgrade. If you're using Ansible as the main tool for system configuration - you shouldn't perform any manual configuration until it's absolutely necessary and you know what you're doing. Thus, I don't see any benefits of including such extra configuration steps in this role. IMHO, the role has to be simple and supportable to be easily reused. If you really need to lock the version of the installed package to prevent unexpected version change with commands like yum upgrade - you can do it by a simple additional task in a playbook.
As creators of Beats and this Ansible role, our goal is to provide a good experience to Beats users which are deploying Beats via Ansible, whatever they are using Ansible for their whole configuration management or only for deploying this role, and whatever external process there are using for managing their system upgrade.
Despite I also prefer keep things simple and avoid unneeded complexity when possible, IMHO, having these 3 simple tasks in the role are worst the case if they avoid users opening issue because a yum upgrade broke their deployments and Beats experience.
This role is already using version lock system so as elastic/ansible-elasticsearch, and we won't remove it to maintain consistency between the 2 roles and avoid a breaking changes for the current users which are already using that version lock feature.
Finally I still don't see any issue about removing changed_when: false in this role and why this approach is against best practices as the final state isn't changed at the end of the role.
Ok, we have an option to handle the version lock by the role tasks to prevent unexpected changes on manual upgrades. But we get other drawbacks this way. If we set version lock manually and assume that our beats version is locked, this role will break this state since it removes version lock during each play. And you won't even notice that from ansible output due to change_when: false. Role actually removes the lock but states that nothing is changed. Why should we prevent one strange case (manual upgrade after ansible deployment) but create other possible version conflicts?
@jmlrt how about the case described above? Is it ok to have the code that removes version lock without any notice?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This issue has been automatically closed because it has not had recent activity since being marked as stale.