URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
Title: #146: Avoid running two instances of the same service
Action: opened
PR body:
"""
Those two patches together are responsible for avoiding running two "instances"
of the same service.
The commit message of each patch has quite a lot of explanation.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/146/head:pr146
git checkout pr146
From ab32ba8686d1789e89f8a80bb8e974cecb2478cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Sun, 5 Feb 2017 21:38:57 +0100
Subject: [PATCH 1/2] SYSTEMD: Force responders to refuse manual start
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As the responders will either be explicitly started by the monitor or
{dbus,socket}-activated, let's for them to refuse manual start.
By doing this, we can avoid having two instances of the same services
running in the following scenario:
- Admin decides to explicitly add the responders to the services' line;
- For some reason, admin does: systemctl enable sssd-$responder.service;
Although it's not specifically a SSSD matter if the admin does something
wrong in the environment, preventing SSSD to not work properly is
desirable if we can do it from our side.
Resolves:
https://fedorahosted.org/sssd/ticket/3300
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/sysv/systemd/sssd-autofs.service.in | 1 +
src/sysv/systemd/sssd-ifp.service.in | 1 +
src/sysv/systemd/sssd-nss.service.in | 1 +
src/sysv/systemd/sssd-pac.service.in | 1 +
src/sysv/systemd/sssd-pam.service.in | 1 +
src/sysv/systemd/sssd-ssh.service.in | 1 +
src/sysv/systemd/sssd-sudo.service.in | 1 +
7 files changed, 7 insertions(+)
diff --git a/src/sysv/systemd/sssd-autofs.service.in b/src/sysv/systemd/sssd-autofs.service.in
index 0b988f3..32ea6e1 100644
--- a/src/sysv/systemd/sssd-autofs.service.in
+++ b/src/sysv/systemd/sssd-autofs.service.in
@@ -3,6 +3,7 @@ Description=SSSD AutoFS Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-autofs.socket
diff --git a/src/sysv/systemd/sssd-ifp.service.in b/src/sysv/systemd/sssd-ifp.service.in
index 8e7abdb..57d9237 100644
--- a/src/sysv/systemd/sssd-ifp.service.in
+++ b/src/sysv/systemd/sssd-ifp.service.in
@@ -3,6 +3,7 @@ Description=SSSD IFP Service responder
Documentation=man:sssd-ifp(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Service]
Type=dbus
diff --git a/src/sysv/systemd/sssd-nss.service.in b/src/sysv/systemd/sssd-nss.service.in
index 2e6fc79..e2f68bc 100644
--- a/src/sysv/systemd/sssd-nss.service.in
+++ b/src/sysv/systemd/sssd-nss.service.in
@@ -3,6 +3,7 @@ Description=SSSD NSS Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-nss.socket
diff --git a/src/sysv/systemd/sssd-pac.service.in b/src/sysv/systemd/sssd-pac.service.in
index a921c74..ffbfdec 100644
--- a/src/sysv/systemd/sssd-pac.service.in
+++ b/src/sysv/systemd/sssd-pac.service.in
@@ -3,6 +3,7 @@ Description=SSSD PAC Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-pac.socket
diff --git a/src/sysv/systemd/sssd-pam.service.in b/src/sysv/systemd/sssd-pam.service.in
index a7f285c..6dec46f 100644
--- a/src/sysv/systemd/sssd-pam.service.in
+++ b/src/sysv/systemd/sssd-pam.service.in
@@ -3,6 +3,7 @@ Description=SSSD PAM Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-pam.socket sssd-pam-priv.socket
diff --git a/src/sysv/systemd/sssd-ssh.service.in b/src/sysv/systemd/sssd-ssh.service.in
index 339bdaa..6f233b4 100644
--- a/src/sysv/systemd/sssd-ssh.service.in
+++ b/src/sysv/systemd/sssd-ssh.service.in
@@ -3,6 +3,7 @@ Description=SSSD SSH Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-ssh.socket
diff --git a/src/sysv/systemd/sssd-sudo.service.in b/src/sysv/systemd/sssd-sudo.service.in
index 5b736e8..b59bcbc 100644
--- a/src/sysv/systemd/sssd-sudo.service.in
+++ b/src/sysv/systemd/sssd-sudo.service.in
@@ -3,6 +3,7 @@ Description=SSSD Sudo Service responder
Documentation=man:sssd.conf(5) man:sssd-sudo(5)
After=sssd.service
BindsTo=sssd.service
+RefuseManualStart=true
[Install]
Also=sssd-sudo.socket
From cc6f07b4acc515dc58be7c6baa070514ebb0d068 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Sun, 5 Feb 2017 18:42:42 +0100
Subject: [PATCH 2/2] MONITOR: Leave the services' startup to systemd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This patch is the cleanest way that I've been able to find to avoid
having two "instances" of the same service running at the same time (one
started by the monitor and another started by socket-activation).
This situation can happen when booting up a system which has both the
service explicitly configured and also its socket loaded
In order to completely avoid this issue, let's never bypass systemd if
it can be used. In other words, let's add some code that checks whether
the socket is enabled and in case it is, do *not* start the responder
from the monitor and just leave it to systemd instead.
Currently the new code uses sd-bus API, which is not part of the latest
RHEL/CentOS. I could change it to use D-Bus purely, but I do believe
providing a downstream patch doing so is a better option than avoiding
to have not so new code upstream.
As dbus will be needed to communicate with systemd, we also have to add
a "After=dbus.service" in the sssd.service.
As Lukáš Slebdoník made clear, we should *not* be adding more logic to
the monitor and I totally agree with his point. On the other hand, I'm
not able to see neither a better nor a more elegant way to deal with
this situation.
Resolves:
https://fedorahosted.org/sssd/ticket/3300
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/monitor/monitor.c | 118 +++++++++++++++++++++++++++++++++++++++
src/sysv/systemd/sssd.service.in | 1 +
2 files changed, 119 insertions(+)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index b82c6e5..04fdb2a 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -56,7 +56,12 @@
#endif
#ifdef HAVE_SYSTEMD
+#include <systemd/sd-bus.h>
#include <systemd/sd-daemon.h>
+
+#define SYSTEMD_ADDRESS "org.freedesktop.systemd1"
+#define SYSTEMD_PATH "/org/freedesktop/systemd1"
+#define SYSTEMD_MANAGER_IFACE "org.freedesktop.systemd1.Manager"
#endif
/* terminate the child after this interval by default if it
@@ -2203,6 +2208,113 @@ static void missing_resolv_conf(struct tevent_context *ev,
}
}
+#ifdef HAVE_SYSTEMD
+static bool can_use_systemd(sd_bus *bus,
+ const char *svc_name)
+{
+ sd_bus_message *msg = NULL;
+ sd_bus_error error = SD_BUS_ERROR_NULL;
+ char *unit_name = NULL;
+ const char *state = NULL;
+ int r;
+ bool ret;
+
+ if (strcmp(svc_name, "ifp") == 0) {
+ /* As the infopipe esponder is dbus-activatable, as long as SSSD is
+ * ran in a platform where systemd is supported, the monitor can be
+ * bypassed. */
+ ret = true;
+ goto done;
+ }
+
+ unit_name = talloc_asprintf(NULL, "sssd-%s.socket", svc_name);
+ if (unit_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf() failed.\n");
+ ret = false;
+ goto done;
+ }
+
+ r = sd_bus_call_method(bus,
+ SYSTEMD_ADDRESS,
+ SYSTEMD_PATH,
+ SYSTEMD_MANAGER_IFACE,
+ "GetUnitFileState",
+ &error,
+ &msg,
+ "s",
+ unit_name);
+ if (r < 0) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Failed to the systemd unit for the service \"%s\": %s\n",
+ svc_name, error.message);
+ ret = false;
+ goto done;
+ }
+
+ r = sd_bus_message_read(msg, "s", &state);
+ if (r < 0) {
+ r = -r;
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Failed to parse \"ActiveState\"response message from systemd: %s [%d]\n",
+ sss_strerror(r), r);
+ ret = false;
+ goto done;
+ }
+
+ if (strcmp("enabled", state) != 0) {
+ ret = false;
+ goto done;
+ }
+
+ ret = true;
+
+done:
+ talloc_free(unit_name);
+ sd_bus_error_free(&error);
+ sd_bus_message_unref(msg);
+
+ return ret;
+}
+
+static void prefer_systemd_to_monitor(struct mt_ctx *ctx)
+{
+ sd_bus *bus = NULL;
+ char **services_new = NULL;
+ int num_services_new;
+ int ret;
+
+ if (ctx->services == NULL) {
+ return;
+ }
+
+ ret = sd_bus_open_system(&bus);
+ if (ret < 0) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Not able to connect to the system bus: %s [%d].\n",
+ sss_strerror(-ret), -ret);
+ goto done;
+ }
+
+ num_services_new = 0;
+ for (int i = 0; ctx->services[i] != NULL; i++) {
+ if (can_use_systemd(bus, ctx->services[i])) {
+ DEBUG(SSSDBG_TRACE_INTERNAL,
+ "\"%s\" responder's activation will be handled by systemd.\n",
+ ctx->services[i]);
+ continue;
+ }
+ add_string_to_list(ctx, ctx->services[i], &services_new);
+ num_services_new++;
+ }
+
+ ctx->services = services_new;
+ ctx->num_services = num_services_new;
+
+done:
+ sd_bus_unref(bus);
+}
+#endif
+
static int monitor_process_init(struct mt_ctx *ctx,
const char *config_file)
{
@@ -2370,6 +2482,12 @@ static int monitor_process_init(struct mt_ctx *ctx,
}
}
+#ifdef HAVE_SYSTEMD
+ if (ctx->services != NULL) {
+ prefer_systemd_to_monitor(ctx);
+ }
+#endif
+
if (num_providers > 0) {
/* now set the services stratup timeout *
* (responders will be started automatically when all
diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
index 05cfd37..3adca1d 100644
--- a/src/sysv/systemd/sssd.service.in
+++ b/src/sysv/systemd/sssd.service.in
@@ -1,6 +1,7 @@
[Unit]
Description=System Security Services Daemon
# SSSD must be running before we permit user sessions
+After=dbus.service
Before=systemd-user-sessions.service nss-user-lookup.target
Wants=nss-user-lookup.target
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
