-
Notifications
You must be signed in to change notification settings - Fork 22
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
Updating selinux role to get around selinux module incompatibilities #225
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,14 +14,26 @@ | |
# limitations under the License. | ||
|
||
- block: | ||
- name: Disable SELinux | ||
selinux: | ||
configfile: '{{selinux_conf}}' | ||
state: disabled | ||
policy: targeted | ||
register: status | ||
- name: Check SELinux | ||
stat: | ||
path: "{{ selinux_conf }}" | ||
register: stat_result | ||
|
||
- name: Reboot | ||
reboot: | ||
when: status.reboot_required and reboot | ||
- block: | ||
- name: Ensure SELinux is set to disabled mode | ||
ansible.builtin.lineinfile: | ||
path: "{{ selinux_conf }}" | ||
regexp: '^SELINUX=' | ||
line: SELINUX=disabled | ||
|
||
- name: Ensure SELinux is set to targeted mode | ||
ansible.builtin.lineinfile: | ||
path: "{{ selinux_conf }}" | ||
regexp: '^SELINUXTYPE=' | ||
line: SELINUXTYPE=targeted | ||
|
||
- name: Reboot | ||
reboot: | ||
when: reboot | ||
when: stat_result.stat.exists | ||
when: ansible_os_family == "RedHat" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In principle, we want to figure out how to disable selinux on Debian and Ubuntu but (A) it's not typically installed (B) it uses a different configuration path (to my level of investigation). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was already present in the task. This can be removed as another PR down the road. |
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.
Don't change the implementation away from ansible.posix.selinux. This accurately reports whether the changes you're making to the configuration file require a reboot. selinux is integrated into the kernel and it's not sufficient merely to change the file.
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.
relatedly: use the fully qualified name (ansible.posix.selinux)
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.
I didn't change the module name so if the PR is not merged, it will remain not fully qualified.
The issue here is that the ansible.posix.selinux module does not work with certain OSs e.g. Rocky Linux 8, especially with our need for a more modern version (>3.6) of python when building Slurm.
My assumption is that if we always reboot (unless
reboot: false
which overrode the selinux module regardless) we won't have this issue. A reboot during an image build should not be significant burden, especially when there's an expectation that it can happen some percentage of the time anyway.