Get rid of systemV #9078
Get rid of systemV #9078
Conversation
eduardoj
added
Packages 📦
|
All the tests done passed, confirming that the services involved were succesfully reloaded:
Removing the "DO NOT MERGE" label. The pull request is ready to review. |
eduardoj
removed
the
DO NOT MERGE ⚠️
| @@ -188,7 +188,6 @@ BuildRequires: obs-server-macros | |||
|
|
|||
| %if 0%{?suse_version:1} | |||
| BuildRequires: fdupes | |||
| PreReq: %insserv_prereq permissions pwdutils | |||
There was a problem hiding this comment.
actually, yes "permissions" can go as well, as "%run_permissions" call was removed in commit d0c101d from 2013 ...
but then the cleanup needs to be completed by removing
%verifyscript -n obs-server
%verify_permissions
as well ...
and "pwdutils" should be kept, but replaced by "shadow" as it's still used here:
%triggerin -n obs-server -- docker
usermod -a -G docker obsservicerun
so this should IMHO be:
-PreReq: %insserv_prereq permissions pwdutils
+Requires(pre): shadow
There was a problem hiding this comment.
Thanks! %verify* lines removed, and suggested patch applied.
| @@ -276,7 +269,6 @@ Summary: The Open Build Service -- base configuration files | |||
| Group: Productivity/Networking/Web/Utilities | |||
| %if 0%{?suse_version} | |||
| Requires(pre): shadow | |||
| PreReq: %fillup_prereq | |||
There was a problem hiding this comment.
no, this is still needed for the %fillup_only call,
should probably be replaced by
Requires(pre): %fillup_prereq
dist/obsworker
Outdated
| check_vmcp || rc_status -v | ||
| create_initrd $OBS_VM_KERNEL $OBS_VM_INITRD || rc_status -v | ||
| check_vmcp | ||
| create_initrd $OBS_VM_KERNEL $OBS_VM_INITRD |
There was a problem hiding this comment.
you're loosing the state here, should probably just be
check_vmcp || exit 1
create_initrd .... || exit 1
or something else to fail if intermediate steps have failed
There was a problem hiding this comment.
Done. I substituted these lines as you proposed and added a comment in the description of the affected commit.
Please, re-review it again.
eduardoj
force-pushed
the
move_obsworker_to_systemd
branch
2 times, most recently
from
55fbbfd
to
09e81f1
Compare
This script is called from the systemd obsworker.service file. It doesn't need to call systemV methods. This is part of removing systemV from the codebase. Also substitute a couple of `rc_status` calls with `exit 1`, to make sure that an error in the previous step gets cached.
This script is called from the systemd obsstoragesetup.service file. It doesn't need to call systemV methods. This is part of removing systemV from the codebase.
This script is called from the systemd obsscheduler.service file. It doesn't need to call systemV methods. This is part of removing systemV from the codebase.
This file defined three methods used in systemV scripts that are not used any more, and no longer needed.
As all the calls to systemV methods are removed, the insserv-compat package is no longer needed.
eduardoj
force-pushed
the
move_obsworker_to_systemd
branch
from
09e81f1
to
1c9bd10
Compare
|
"OBS Package Build" tests failed after merging the "Update rack" pull request. I only rebased with master. Waiting for the tests to pass to merge the pull request. |
As systemd is being used to manage OBS services, systemV method calls are no longer needed. Removing systemV code and compatibility packages (as insserv-compat) makes it more clear OBS services handling.
Labelling as "DO NOT MERGE" untill the changes in the service scripts are tested. The pull request was created to receive feedback about removing systemV from the OBS codebase.