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 <jhro...@redhat.com>
> >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.

> 
> > %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.

I'm still working on the manpage..
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to