Fix versionlock centos 8 #797
Conversation
|
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? |
1 similar comment
|
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? |
|
❌ Author of the following commits did not sign a Contributor Agreement: Please, read and sign the above mentioned agreement if you want to contribute to this project |
|
I see "CLA Expected - Waiting for status to be reported" - unsure if any further action is needed on my end after signing the CLA - please let me know if this is the case |
| @@ -2,13 +2,13 @@ | |||
| - name: RedHat - install yum-version-lock | |||
There was a problem hiding this comment.
yum-version-lock should also be replaced here with {{ versionlock_plugin_package }}.
| @@ -2,13 +2,13 @@ | |||
| - name: RedHat - install yum-version-lock | |||
| become: yes | |||
| yum: | |||
There was a problem hiding this comment.
Should this then better be package to be more generic?
vars/RedHat.yml
Outdated
| @@ -2,3 +2,5 @@ | |||
| java: "{{ es_java | default('java-1.8.0-openjdk.x86_64') }}" | |||
| default_file: "/etc/sysconfig/elasticsearch" | |||
| es_home: "/usr/share/elasticsearch" | |||
| package_manager: "{% if ansible_os_family == 'RedHat' and ansible_distribution_major_version >= '8' %}dnf{% else %}yum{% endif %}" | |||
There was a problem hiding this comment.
Better use ansible_distribution_major_version | int >= 8 or ansible_distribution_version is version('8', '>=') because for higher version numbers this comparison is not returning the expected result, e.g. '10' >= '8' evaluates to false.
vars/RedHat.yml
Outdated
| @@ -2,3 +2,5 @@ | |||
| java: "{{ es_java | default('java-1.8.0-openjdk.x86_64') }}" | |||
| default_file: "/etc/sysconfig/elasticsearch" | |||
| es_home: "/usr/share/elasticsearch" | |||
| package_manager: "{% if ansible_os_family == 'RedHat' and ansible_distribution_major_version >= '8' %}dnf{% else %}yum{% endif %}" | |||
There was a problem hiding this comment.
ansible_os_family should not be required here because this file is only included in tasks/main.yml using ansible_os_family to derive the filename.
There was a problem hiding this comment.
Agreed. I primarily didn't want to diverge too far from existing statements in e.g. Debian.yml which has the same unnecessary check. But I'll remove it here as it's been discussed (I'm leaving it in Debian.yml for now though as I haven't tested there)
There was a problem hiding this comment.
Actually I've just removed all that and gone with ansible_facts.pkg_mgr to simplify things.
|
Note: I am not a maintainer. |
|
This PR 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. |
|
Please keep it open. |
|
Also I def signd the contributor agreement, about 3 times. Not sure why it thinks I haven't - possible issue with the commit workflow? |
|
This PR 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 resolves a corner case where a RHEL/CentOS 8 system does not have the
yumcompatibility package installed - everywhere else, the role is using theyumansible module which detects and usesdnfin recent ansible versions, so the only concern is when:yuminstalled and onlydnfis available to the shell modulees_version_lockis set to true, invoking theelasticsearch-RedHat-version-lock.ymlplaybook.