-
Notifications
You must be signed in to change notification settings - Fork 741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove execshield sysctl-parameter on rhel7 #119
Conversation
Travis fails because of the errors that are fixed in #138. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. Note that I have never used RHEL7. One improvement could be implemented which will make maintenance easier in the long run.
tasks/main.yml
Outdated
- '{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml' | ||
- '{{ ansible_distribution }}.yml' | ||
- '{{ ansible_os_family }}-{{ ansible_distribution_major_version }}.yml' | ||
- '{{ ansible_os_family }}.yml' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe include {{ ansible_os_family }}.yml
always a first task and then have a second task which does the with_first_found
? This way redundancy could be decreased.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand on this? I don't understand how this decreased redundancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this:
- name: Set OS family dependent variables
include_vars: '{{ ansible_os_family }}.yml'
- name: Set OS dependent variables
include_vars: '{{ item }}'
with_first_found:
- '{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml'
- '{{ ansible_distribution }}.yml
- '{{ ansible_os_family }}-{{ ansible_distribution_major_version }}.yml'
Then {{ ansible_os_family }}.yml
could contain all the basics and other files could be kept at a minimum.
vars/RedHat.yml
Outdated
owner: root | ||
group: root | ||
mode: '0644' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Basically \n EOF
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, well. I don't know.
use package instead of yum so the operation works on Fedora
use package instead of yum so the operation works on Fedora
remove execshield sysctl-parameter on rhel7
We're having code-duplicates here, but I still think its the cleanest solution, as I don't want to include more conditionals to check for than necessary.
Fixes #118