On (17/08/16 13:46), Jakub Hrozek wrote:
>On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
>> On (17/08/16 12:27), Jakub Hrozek wrote:
>> >On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
>> >> On (15/08/16 16:05), Jakub Hrozek wrote:
>> >> >From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001
>> >> >From: Jakub Hrozek <[email protected]>
>> >> >Date: Mon, 15 Aug 2016 14:10:23 +0200
>> >> >Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
>> >> >
>> >> >Adds two new files: sssd-secrets.socket and sssd-secrets.service. These
>> >> >can be used to socket-acticate the secrets responder even without
>> >> >explicitly starting it in the sssd config file.
>> >> >
>> >> >The specfile activates the socket after installation which means that
>> >> >the admin would just be able to use the secrets socket and the
>> >> >sssd_secrets responder would be started automatically by systemd.
>> >> >
>> >> >The sssd-secrets responder is started as root, mostly because I didn't
>> >> >think of an easy way to pass the uid/gid to the responders without
>> >> >asking about the sssd user identity in the first place. But nonetheless,
>> >> >the sssd-secrets responder wasn't tested as non-root and at least the
>> >> >initialization should be performed as root for the time being.
>> >> >---
>> >> > Makefile.am                              | 21 +++++++++++++++++++--
>> >> > contrib/sssd.spec.in                     |  5 +++++
>> >> > src/sysv/systemd/sssd-secrets.service.in |  2 ++
>> >> > src/sysv/systemd/sssd-secrets.socket.in  |  5 +++++
>> >> > 4 files changed, 31 insertions(+), 2 deletions(-)
>> >> > create mode 100644 src/sysv/systemd/sssd-secrets.service.in
>> >> > create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
>> >> >
>> >> >diff --git a/Makefile.am b/Makefile.am
>> >> >index 
>> >> >7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d
>> >> > 100644
>> >> >--- a/Makefile.am
>> >> >+++ b/Makefile.am
>> >> >@@ -3848,7 +3848,10 @@ systemdunit_DATA =
>> >> > systemdconf_DATA =
>> >> > if HAVE_SYSTEMD_UNIT
>> >> >     systemdunit_DATA += \
>> >> >-        src/sysv/systemd/sssd.service
>> >> >+        src/sysv/systemd/sssd.service \
>> >> >+        src/sysv/systemd/sssd-secrets.socket \
>> >> >+        src/sysv/systemd/sssd-secrets.service \
>> >> >+        $(NULL)
>> >> > if WITH_JOURNALD
>> >> >     systemdconf_DATA += \
>> >> >         src/sysv/systemd/journal.conf
>> >> >@@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \
>> >> >         -e 's|@sbindir[@]|$(sbindir)|g' \
>> >> >         -e 's|@environment_file[@]|$(environment_file)|g' \
>> >> >         -e 's|@localstatedir[@]|$(localstatedir)|g' \
>> >> >+        -e 's|@libexecdir[@]|$(libexecdir)|g' \
>> >> >         -e 's|@prefix[@]|$(prefix)|g'
>> >> > 
>> >> > replace_script = \
>> >> >@@ -3897,7 +3901,10 @@ replace_script = \
>> >> > 
>> >> > EXTRA_DIST += \
>> >> >     src/sysv/systemd/sssd.service.in \
>> >> >-    src/sysv/systemd/journal.conf.in
>> >> >+    src/sysv/systemd/journal.conf.in \
>> >> >+    src/sysv/systemd/sssd-secrets.socket.in \
>> >> >+    src/sysv/systemd/sssd-secrets.service.in \
>> >> >+    $(NULL)
>> >> > 
>> >> > src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile
>> >> >         @$(MKDIR_P) src/sysv/systemd/
>> >> >@@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: 
>> >> >src/sysv/systemd/journal.conf.in Makefile
>> >> >         @$(MKDIR_P) src/sysv/systemd/
>> >> >         $(replace_script)
>> >> > 
>> >> >+src/sysv/systemd/sssd-secrets.socket: 
>> >> >src/sysv/systemd/sssd-secrets.socket.in Makefile
>> >> >+        @$(MKDIR_P) src/sysv/systemd/
>> >> >+        $(replace_script)
>> >> >+
>> >> >+src/sysv/systemd/sssd-secrets.service: 
>> >> >src/sysv/systemd/sssd-secrets.service.in Makefile
>> >> >+        @$(MKDIR_P) src/sysv/systemd/
>> >> >+        $(replace_script)
>> >> >+
>> >> > SSSD_USER_DIRS = \
>> >> >     $(DESTDIR)$(dbpath) \
>> >> >     $(DESTDIR)$(keytabdir) \
>> >> >@@ -4122,6 +4137,8 @@ endif
>> >> >         done;
>> >> >         rm -Rf ldb_mod_test_dir
>> >> >         rm -f $(builddir)/src/sysv/systemd/sssd.service
>> >> >+        rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
>> >> >+        rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service
>> >> >         rm -f $(builddir)/src/sysv/systemd/journal.conf
>> >> > 
>> >> > CLEANFILES += *.X */*.X */*/*.X
>> >> >diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>> >> >index 
>> >> >a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632
>> >> > 100644
>> >> >--- a/contrib/sssd.spec.in
>> >> >+++ b/contrib/sssd.spec.in
>> >> >@@ -746,6 +746,8 @@ done
>> >> > %{_sbindir}/sssd
>> >> > %if (0%{?use_systemd} == 1)
>> >> > %{_unitdir}/sssd.service
>> >> >+%{_unitdir}/sssd-secrets.socket
>> >> >+%{_unitdir}/sssd-secrets.service
>> >> > %else
>> >> > %{_initrddir}/%{name}
>> >> > %endif
>> >> >@@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g 
>> >> >sssd -d / -s /sbin/nologin -c "Us
>> >> > # systemd
>> >> > %post common
>> >> > %systemd_post sssd.service
>> >> >+%systemd_post sssd-secrets.socket
>> >> > 
>> >> > %preun common
>> >> > %systemd_preun sssd.service
>> >> >+%systemd_preun sssd-secrets.socket
>> >> > 
>> >> > %postun common
>> >> > %systemd_postun_with_restart sssd.service
>> >> >+%systemd_postun_with_restart sssd-secrets.socket
>> >> >
>> >> systemd_post and systemd_preun should be used just with *.socket
>> >> Because you do not want to directly enable/disable socket activated 
>> >> service
>> >> 
>> >> systemd_postun_with_restart is an alias for "systemctl try-restart"
>> >> for package upgrade. So we shoudl try to restart sssd-secrets.service
>> >
>> >I agree about not starting, but are you sure about not *enabling*? I can
>> >test that, but I thought that if the service wasn't enabled, then
>> >systemd wouldn't start it.
>> >
>> The latest patch do not enable sssd-secrets.service and that's fine.
>> and other socket activated services are not enabled by default.
>> So we needn't change systemd_post, systemd_preun.
>> 
>> [root@host ~]# systemctl status rpcbind.socket rpcbind.service
>> ● rpcbind.socket - RPCbind Server Activation Socket
>>    Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor 
>> preset: disabled)
>>    Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 
>> days ago
>>    Listen: /var/run/rpcbind.sock (Stream)
>> 
>> Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server 
>> Activation Socket.
>> 
>> ● rpcbind.service - RPC bind service
>>    Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor 
>> preset: disabled)
>>    Active: inactive (dead)
>> 
>> 
>> 
>> 
>> [root@host ~]# systemctl status cockpit.socket cockpit.service
>> ● cockpit.socket - Cockpit Web Service Socket
>>    Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor 
>> preset: enabled)
>>    Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago
>>      Docs: man:cockpit-ws(8)
>>    Listen: [::]:9090 (Stream)
>> 
>> Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web 
>> Service Socket.
>> 
>> ● cockpit.service - Cockpit Web Service
>>    Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor 
>> preset: disabled)
>>    Active: inactive (dead)
>>      Docs: man:cockpit-ws(8)
>> 
>> 
>> 
>> [root@host ~]# systemctl status dbus.socket dbus.service
>> ● dbus.socket - D-Bus System Message Bus Socket
>>    Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor 
>> preset: disabled)
>>    Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 
>> days ago
>>    Listen: /run/dbus/system_bus_socket (Stream)
>> 
>> Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System 
>> Message Bus Socket.
>> 
>> ● dbus.service - D-Bus System Message Bus
>>    Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor 
>> preset: disabled)
>>    Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 
>> days ago
>>      Docs: man:dbus-daemon(1)
>>  Main PID: 1102 (dbus-daemon)
>>     Tasks: 2 (limit: 4915)
>>    CGroup: /system.slice/dbus.service
>>            └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork 
>> --nopidfile --systemd-activation
>> 
>> >> 
>> >> > %else
>> >> > # sysv
>> >> >diff --git a/src/sysv/systemd/sssd-secrets.service.in 
>> >> >b/src/sysv/systemd/sssd-secrets.service.in
>> >> >new file mode 100644
>> >> >index 
>> >> >0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9
>> >> >--- /dev/null
>> >> >+++ b/src/sysv/systemd/sssd-secrets.service.in
>> >> >@@ -0,0 +1,2 @@
>> >> >+[Service]
>> >> >+ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 
>> >> >--debug-to-files
>> >> 
>> >> I checked another socket activated service (cups, pcscd)
>> >> and they have "Also=$name.socket" in Install section for service
>> >> But on the other hand other socket activated services(cockpit, dbus,
>> >> systemd-journald) does not have it. So I'm not sure whether we need it.
>> >
>> >Hmm, Also= seems to imply that if the service is enabled, the socket
>> >should be enabled as well, which IMO makes sense. So I will add it.
>> >
>> 
>> >> 
>> >> 
>> >> Anyway, It would be good to add "[Unit]" section to both files + 
>> >> Description
>> >> and later also Documentation=man:$name($number)
>> >
>> >Do we need Unit? I thought that if it's not set, then systemd would use
>> >the same name as the socket for the service. But I /can/ add it if you
>> >prefer.
>> >
>> It might work without it but it would be good to have human readable
>> description and later also hint for man page.
>> 
>> e.g.
>> [root@host ~]# systemctl cat cockpit.socket
>> # /usr/lib/systemd/system/cockpit.socket
>> [Unit]
>> Description=Cockpit Web Service Socket
>> Documentation=man:cockpit-ws(8)
>> 
>> [Socket]
>> ListenStream=9090
>> 
>> [Install]
>> WantedBy=sockets.target
>> 
>> 
>> 
>> [root@host ~]# systemctl cat cockpit.service
>> # /usr/lib/systemd/system/cockpit.service
>> [Unit]
>> Description=Cockpit Web Service
>> Documentation=man:cockpit-ws(8)
>> Requires=cockpit.socket
>> 
>> [Service]
>> ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root 
>> --group=cockpit-ws --selinux-type=etc_t
>> ExecStart=/usr/libexec/cockpit-ws
>> PermissionsStartOnly=true
>> User=cockpit-ws
>> Group=cockpit-ws
>> 
>> >I'm still working on the manpage..
>> It was just a note that you should not forget :-)
>
>See the new patch:
>
># systemctl disable sssd-secrets.service
>Removed symlink
>/etc/systemd/system/sockets.target.wants/sssd-secrets.socket.
># systemctl enable sssd-secrets.service
>Created symlink from 
>/etc/systemd/system/sockets.target.wants/sssd-secrets.socket to 
>/usr/lib/systemd/system/sssd-secrets.socket.
>
>I think that's the Also= directive at work
>
OK

>The Description now shows with "systemctl show sssd-secrets.service" and
>.socket.

systemd_postun_with_restart is an alias for "systemctl try-restart"
for package upgrade. So we should try to restart sssd-secrets.service
and not just sssd-secrets.socket. Otherwise, old sssd-sevice will be running
after upgrade.

LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to