-
Notifications
You must be signed in to change notification settings - Fork 24
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?
Conversation
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.
Please revert the change to the actual management of the config file but leave the check in place. Please also include the error message you're resolving.
reboot: | ||
when: status.reboot_required and reboot | ||
- block: | ||
- name: Ensure SELinux is set to disabled mode |
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.
- 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 comment
The 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 comment
The 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.
The selinux ansible module seems to break fairly easily with Rocky Linux 8 (potentially other RHEL based distros). I believe there is some compatibility issue with python3-libselinux and the ansible module (python misconfigs).
This change checks for the config file and hard sets the disable and targeted values within the file. It assumes that it needs to be reboot ( does not check if anything was changed) and reboots unless the
reboot
variable is set to false.This has been tested on a Slurm build on a base image of Rocky Linux 8.
Error similar to: GitHub Error