snapshot/scheduler: work without SELinux userspace and stop misreporting init errors - #4821
Draft
ThalesBarretto wants to merge 1 commit into
Draft
ThalesBarretto wants to merge 1 commit into
ThalesBarretto wants to merge 1 commit into
Conversation
…ing init errors
`snap_scheduler.py init` fails on every host that has no SELinux userspace
(Debian and its derivatives ship none by default) and reports it as
"Another snap_scheduler command is running. Please try again after some
time." - a message that names a condition which does not exist. Three
defects combine to produce this:
1. get_selinux_status() returns the raw stdout of `getenforce`, which
under Python 3 is bytes. set_cronjob_user_share() then compares it
with the str "Disabled", so the early return for a host where SELinux
is disabled is never taken and the code always proceeds to query the
boolean. get_bool_val() has the same bytes-vs-str problem with the
`getsebool` output it returns.
2. get_bool_val() and the `setsebool` call in set_cronjob_user_share()
spawn their tools without guarding against the tool being absent.
Where SELinux userspace is not installed Popen raises
FileNotFoundError (an OSError) from inside initialise_scheduler().
get_selinux_status() does guard `getenforce`, but turns its absence
into a hard failure (-1, "Failed to get selinux status") instead of
the obvious reading that there is no SELinux to configure.
3. main() wraps both the flock() acquisition and perform_operation() in
one `except IOError:` handler that prints the "another command is
running" message. Under Python 3 IOError is OSError, so every OSError
raised by the operation itself - the missing tool above, but equally
an unwritable /etc/cron.d or a failing shutil.move() - is swallowed
and misreported, and the lock fd is leaked with the lock still held.
On CentOS/RHEL, where the test suite runs, policycoreutils is present, so
`getsebool -a` exists and the comparison bug is masked by the code simply
taking the longer path through the boolean. tests/basic/volume-snap-
scheduler.t therefore passes there and fails everywhere else: subtests
9-14 ("snap_scheduler.py init/enable/disable/list", "check_status_
scheduler") go red with the misreport above. strace shows the failing
syscall: execve of "getsebool" -> ENOENT, right after `getenforce`
printed "Disabled".
Fix all three:
- decode the tool output (universal_newlines=True) so the status and the
boolean compare as str;
- treat a missing getenforce/getsebool/setsebool as "SELinux not
present": get_selinux_status() returns "Disabled", get_bool_val()
returns -1 (no boolean, which set_cronjob_user_share() already maps to
"nothing to do"), a missing setsebool is logged and skipped;
- in main(), map only the flock() failure to ANOTHER_TRANSACTION_IN_
PROGRESS; an error raised by the operation is logged with the action
name, printed as "Failed: <error>" and returned as INTERNAL_ERROR, and
the lock is always released and the fd closed.
get_bool_val() also checks that the grep'ed line has the three fields it
indexes, instead of raising IndexError on unexpected output.
With this change `snap_scheduler.py init` succeeds on a host without
SELinux userspace and tests/basic/volume-snap-scheduler.t passes there
(16/16); behaviour on SELinux hosts is unchanged apart from the boolean
now being compared correctly.
Fixes: gluster#4820
Signed-off-by: Thales Antunes de Oliveira Barretto <thales.barretto.git@gmail.com>
ThalesBarretto
marked this pull request as draft
September 22, 2026 10:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
snapshot/scheduler: work without SELinux userspace and stop misreporting init errors
snap_scheduler.py initfails on every host without SELinux userspace and says "Another snap_scheduler command isrunning" while nothing else runs. Three defects in
extras/snap_scheduler/snap_scheduler.pycombine: thegetenforceoutput is compared as bytes against the str
"Disabled"(never equal under Python 3, so the SELinux-disabled earlyreturn is dead), the absence of
getsebool/setseboolis not handled (FileNotFoundErrorfrom inside the operation),and
main()'sexcept IOError:aroundperform_operation()catches everyOSErrorof the operation and labels it as aheld lock, leaking the lock fd. CentOS masks it because
getseboolis present there. Details in #4820.The change (
extras/snap_scheduler/snap_scheduler.py, +55/-23)get_selinux_status()/get_bool_val()/ thesetseboolcall: spawn withuniversal_newlines=Trueso the valuescompare as str; a missing
getenforcenow yields"Disabled", a missinggetseboolyields-1(no boolean, whichset_cronjob_user_share()already treats as nothing to do), a missingsetseboolis logged and skipped.get_bool_val()also checks the grep'ed line has the three fields it indexes.main(): acquire the lock in its owntry; only that failure means "another command is running".perform_operation()runs in a separate
trywhoseOSErroris logged with the action name, printed asFailed: <error>and returned asINTERNAL_ERROR; afinallyreleases the lock and closes the fd on every path.Behaviour on SELinux hosts is unchanged apart from the boolean now being compared correctly (
"off"is finallyrecognised, so the
setsebool -P cron_system_cronjob_use_shares onpath is reachable as intended).Test
No new test:
tests/basic/volume-snap-scheduler.talready coversinit/enable/disable/list; on a host withoutSELinux userspace it fails subtests 9-14 before this change and passes 16/16 after it. On CentOS it passes before and
after.
Relation to other changes
Independent of the
errno-NameError fix in the same file (the twoexcept OSErrorhandlers aroundos.makedirs()inmain()), which touches different lines; both apply in either order.Fixes: #4820