On Wed, Aug 17, 2016 at 04:25:47PM +0200, Lukas Slebodnik wrote:
> On (17/08/16 15:03), Jakub Hrozek wrote:
> >On Wed, Aug 17, 2016 at 02:30:15PM +0200, Lukas Slebodnik wrote:
> >> 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 <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.
> >> >> >
> >> >> 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.
> >
> >OK, the attached patch also adds "%systemd_postun_with_restart
> >sssd-secrets.service"
>
> ACK++
>
> http://sssd-ci.duckdns.org/logs/job/51/87/summary.html
* master:
b3a22ee1d91aa4ed1544475be16ec2b7cf886180
942b4ce6e60e88e4e31600655fad8980f3986f68
733100a12138a701d0ae7ef5af2b04b08e225033
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
