elastic / ansible-elasticsearch

Ansible playbook for Elasticsearch
Other
1.59k stars 857 forks source link

Fix idempotency for both supported CentOS versions #747

Closed fourstepper closed 3 years ago

fourstepper commented 3 years ago

Fixes https://github.com/elastic/ansible-elasticsearch/issues/746

elasticmachine commented 3 years ago

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

cla-checker-service[bot] commented 3 years ago

💚 CLA has been signed

bndabbs commented 3 years ago

Thanks for taking the time to dig into this @fourstepper! I'm creating a CentOS 8 machine to test against, as I wasn't able to replicate the issue using CentOS 7.

fourstepper commented 3 years ago

@bndabbs On CentOS 7, the formatting of yum versionlock list is different than on CentOS 8 - I have tested this side by side on the two machines and the method proposed worked well for me on both sides.

Let me know if there is anything that needs editing or anything at all :)

fourstepper commented 3 years ago

Btw, I noticed that you have some CI in place which is not molecule. Have you given molecule a look? I think it's awesome for testing roles and/or entire playbooks (and ansible content in general)

https://github.com/ansible-community/molecule

bndabbs commented 3 years ago

Btw, I noticed that you have some CI in place which is not molecule. Have you given molecule a look? I think it's awesome for testing roles and/or entire playbooks (and ansible content in general)

https://github.com/ansible-community/molecule

I wasn't involved in setting up this CI, so I can't speak to why Molecule wasn't chosen. It's possible @jmlrt may have some better insight there. I am familiar with Molecule though, and agree that it is an awesome tool!

bndabbs commented 3 years ago

jenkins test this please

jmlrt commented 3 years ago

Btw, I noticed that you have some CI in place which is not molecule. Have you given molecule a look? I think it's awesome for testing roles and/or entire playbooks (and ansible content in general) ansible-community/molecule

I wasn't involved in setting up this CI, so I can't speak to why Molecule wasn't chosen. It's possible @jmlrt may have some better insight there. I am familiar with Molecule though, and agree that it is an awesome tool!

Yep, tbh I prefer Molecule too. I wasn't there yet but AFAIK, when this role and the related tests were created in 2015, Molecule didn't exist yet and so Test Kitchen was implemented. Now, despite Molecule being more adapted for Ansible tests, migrating to it is simply not a priority.