On (03/10/16 17:44), Victor Tapia wrote:
>Hi,
>
>I just removed the upstart part and left just the systemd notification
>with the fix for the latest master branch. I also fixed the evaluation
>of the return code of sd_notify from the original patch, so this should
>be the "good" one :).
>
>Thanks,
>
>Victor
>
>
>El 22/09/16 a las 12:31, Jakub Hrozek escribió:
>> On Thu, Sep 22, 2016 at 10:07:21AM -0000, Victor Tapia wrote:
>>> Hi,
>>>
>>> I just saw that the ticket 3080 (https://fedorahosted.org/sssd/ticket/3080)
>>> has been closed with a different patch. Is this one landing too?
>>
>> Sorry, that's my fault, I put a wrong URL into patches for #3140. I
>> reopened #3080.
>> _______________________________________________
>> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
>> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>>
>
>From f0b08423e7d215aee49e23a713e53df2c61a239e Mon Sep 17 00:00:00 2001
>From: Victor Tapia <victor.ta...@canonical.com>
>Date: Mon, 3 Oct 2016 17:36:08 +0200
>Subject: [PATCH] Create pidfile after responders started
>
>---
> Makefile.am | 1 +
> src/monitor/monitor.c | 42 ++++++++++++++++++++++++++++++++++++----
> src/sysv/sssd.in | 15 ++++++++++++++
> src/sysv/systemd/sssd.service.in | 3 ++-
> 4 files changed, 56 insertions(+), 5 deletions(-)
>
>diff --git a/Makefile.am b/Makefile.am
>index 1ea8f50..fc25a24 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -1246,6 +1246,7 @@ sssd_LDADD = \
> $(INOTIFY_LIBS) \
> $(LIBNL_LIBS) \
> $(KEYUTILS_LIBS) \
>+ $(SYSTEMD_DAEMON_LIBS) \
> $(SSSD_INTERNAL_LTLIBS)
>
> sssd_nss_SOURCES = \
>diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
>index 84a144e..49624e6 100644
>--- a/src/monitor/monitor.c
>+++ b/src/monitor/monitor.c
>@@ -55,6 +55,10 @@
> #include <keyutils.h>
> #endif
>
>+#ifdef HAVE_SYSTEMD
>+#include <systemd/sd-daemon.h>
>+#endif
>+
> /* terminate the child after this interval by default if it
> * doesn't shutdown on receiving SIGTERM */
> #define MONITOR_DEF_FORCE_TIME 60
>@@ -160,6 +164,7 @@ struct mt_ctx {
> struct netlink_ctx *nlctx;
> const char *conf_path;
> struct sss_sigchild_ctx *sigchld_ctx;
>+ bool pid_file_created;
> bool is_daemon;
> pid_t parent_pid;
>
>@@ -439,7 +444,29 @@ static int mark_service_as_started(struct mt_svc *svc)
> ctx->started_services++;
> }
>
>- if (ctx->started_services == ctx->num_services) {
>+ /* create the pid file if all services are alive */
>+ if (!ctx->pid_file_created && ctx->started_services == ctx->num_services)
>{
>+ DEBUG(SSSDBG_TRACE_FUNC, "All services have successfully started, "
>+ "creating pid file\n");
>+ ret = pidfile(PID_PATH, MONITOR_NAME);
>+ if (ret != EOK) {
>+ DEBUG(SSSDBG_FATAL_FAILURE,
>+ "Error creating pidfile: %s/%s.pid! (%d [%s])\n",
>+ PID_PATH, MONITOR_NAME, ret, strerror(ret));
>+ kill(getpid(), SIGTERM);
>+ }
>+
>+ ctx->pid_file_created = true;
>+
>+#ifdef HAVE_SYSTEMD
>+ DEBUG(SSSDBG_TRACE_FUNC, "Sending startup notification to systemd\n");
>+ ret = sd_notify(0, "READY=1");
>+ if (ret < 0) {
>+ DEBUG(SSSDBG_CRIT_FAILURE, "Error sending notification to systemd"
>+ " %d: %s\n", -ret, strerror(-ret));
>+ }
>+#endif
>+
> /* Initialization is complete, terminate parent process if in daemon
> * mode. Make sure we send the signal to the right process */
> if (ctx->is_daemon) {
>@@ -1495,6 +1522,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx,
> return ENOMEM;
> }
>
>+ ctx->pid_file_created = false;
> talloc_set_destructor((TALLOC_CTX *)ctx, monitor_ctx_destructor);
>
> cdb_file = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
>@@ -2601,9 +2629,6 @@ int main(int argc, const char *argv[])
> return 6;
> }
>
>- /* we want a pid file check */
>- flags |= FLAGS_PID_FILE;
>-
> /* Open before server_setup() does to have logging
> * during configuration checking */
> if (debug_to_file) {
>@@ -2668,6 +2693,15 @@ int main(int argc, const char *argv[])
> }
> }
>
>+ /* Check if the SSSD is already running */
>+ ret = check_file(SSSD_PIDFILE, 0, 0, S_IFREG|0600, 0, NULL, false);
>+ if (ret == EOK) {
>+ DEBUG(SSSDBG_FATAL_FAILURE,
>+ "pidfile exists at %s\n", SSSD_PIDFILE);
>+ ERROR("SSSD is already running\n");
>+ return 2;
>+ }
>+
> /* Parse config file, fail if cannot be done */
> ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR,
> &monitor);
>diff --git a/src/sysv/sssd.in b/src/sysv/sssd.in
>index 5109148..385785e 100644
>--- a/src/sysv/sssd.in
>+++ b/src/sysv/sssd.in
>@@ -40,6 +40,8 @@ SSSD=@sbindir@/sssd
> LOCK_FILE=@localstatedir@/lock/subsys/sssd
> PID_FILE=@localstatedir@/run/sssd.pid
>
>+TIMEOUT=15
>+
> start() {
> [ -x $SSSD ] || exit 5
> echo -n $"Starting $prog: "
>@@ -47,6 +49,19 @@ start() {
> RETVAL=$?
> echo
> [ "$RETVAL" = 0 ] && touch $LOCK_FILE
>+
>+ # Wait for pidfile creation or timeout
>+ sec=0
>+ [ "$RETVAL" = 0 ] && while [ $sec -lt $TIMEOUT -a ! -f $PID_FILE ]
>+ do
>+ sleep 1
>+ sec=$(($sec+1))
>+ done
>+
>+ if [ "$sec" = "$TIMEOUT" ]; then
>+ RETVAL=-1
>+ fi
>+
> return $RETVAL
> }
>
>diff --git a/src/sysv/systemd/sssd.service.in
>b/src/sysv/systemd/sssd.service.in
>index a4f9125..976c9c1 100644
>--- a/src/sysv/systemd/sssd.service.in
>+++ b/src/sysv/systemd/sssd.service.in
>@@ -9,7 +9,8 @@ EnvironmentFile=-@environment_file@
> ExecStart=@sbindir@/sssd -D -f
> # These two should be used with traditional UNIX forking daemons
> # consult systemd.service(5) for more details
>-Type=forking
>+Type=notify
>+NotifyAccess=all
> PIDFile=@localstatedir@/run/sssd.pid
PIDFile is usually not used with "Type=notify"
because behavior of "notify" is similar to "simple"
Therefore it is better to run sssd in interactive mode
and not in daemon mode s/-D/-i/.
This would also allow us to change NotifyAccess from
all -> main. Because that is what code does :-)
To simplify things I am attaching updated patch
with my singoff. I hope you don't mind that I fixed few
coding style issues as well. There might be some comflicts
when applying the patch to 1.13 due to new flag FLAGS_NO_WATCHDOG
in 1.14+
I had to reschedule jobs because I didn't
properly solve conflict with removing FLAGS_PID_FILE
I hope the latest patch will work without issues :-)
I let you know after that.
LS
From ae2cb6525e40b84284a7fd701193770acc10f4bf Mon Sep 17 00:00:00 2001
From: Victor Tapia <victor.ta...@canonical.com>
Date: Mon, 3 Oct 2016 17:36:08 +0200
Subject: [PATCH] Create pidfile after responders started
Signed-off-by: Lukas Slebodnik <lsleb...@redhat.com>
---
Makefile.am | 1 +
src/monitor/monitor.c | 42 +++++++++++++++++++++++++++++++++++++---
src/sysv/sssd.in | 15 ++++++++++++++
src/sysv/systemd/sssd.service.in | 8 +++-----
4 files changed, 58 insertions(+), 8 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index
e037930ff3a4d462ace5db02dad6c3c0d5fbb0d8..5116c846ee9384ffd7e05fce7b4d0d45097345b0
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1268,6 +1268,7 @@ sssd_LDADD = \
$(INOTIFY_LIBS) \
$(LIBNL_LIBS) \
$(KEYUTILS_LIBS) \
+ $(SYSTEMD_DAEMON_LIBS) \
$(SSSD_INTERNAL_LTLIBS)
sssd_nss_SOURCES = \
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index
935febb9544cf2937ff0ad20112dd47855043474..4f9117660ded168c123cd1b15a100cdc511db5f0
100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -55,6 +55,10 @@
#include <keyutils.h>
#endif
+#ifdef HAVE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
+
/* terminate the child after this interval by default if it
* doesn't shutdown on receiving SIGTERM */
#define MONITOR_DEF_FORCE_TIME 60
@@ -160,6 +164,7 @@ struct mt_ctx {
struct netlink_ctx *nlctx;
const char *conf_path;
struct sss_sigchild_ctx *sigchld_ctx;
+ bool pid_file_created;
bool is_daemon;
pid_t parent_pid;
@@ -439,7 +444,30 @@ static int mark_service_as_started(struct mt_svc *svc)
ctx->started_services++;
}
- if (ctx->started_services == ctx->num_services) {
+ /* create the pid file if all services are alive */
+ if (!ctx->pid_file_created && ctx->started_services == ctx->num_services) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "All services have successfully started, creating pid file\n");
+ ret = pidfile(PID_PATH, MONITOR_NAME);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Error creating pidfile: %s/%s.pid! (%d [%s])\n",
+ PID_PATH, MONITOR_NAME, ret, strerror(ret));
+ kill(getpid(), SIGTERM);
+ }
+
+ ctx->pid_file_created = true;
+
+#ifdef HAVE_SYSTEMD
+ DEBUG(SSSDBG_TRACE_FUNC, "Sending startup notification to systemd\n");
+ ret = sd_notify(0, "READY=1");
+ if (ret < 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Error sending notification to systemd %d: %s\n",
+ -ret, strerror(-ret));
+ }
+#endif
+
/* Initialization is complete, terminate parent process if in daemon
* mode. Make sure we send the signal to the right process */
if (ctx->is_daemon) {
@@ -1495,6 +1523,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
+ ctx->pid_file_created = false;
talloc_set_destructor((TALLOC_CTX *)ctx, monitor_ctx_destructor);
cdb_file = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
@@ -2601,8 +2630,6 @@ int main(int argc, const char *argv[])
return 6;
}
- /* we want a pid file check */
- flags |= FLAGS_PID_FILE;
/* the monitor should not run a watchdog on itself */
flags |= FLAGS_NO_WATCHDOG;
@@ -2670,6 +2697,15 @@ int main(int argc, const char *argv[])
}
}
+ /* Check if the SSSD is already running */
+ ret = check_file(SSSD_PIDFILE, 0, 0, S_IFREG|0600, 0, NULL, false);
+ if (ret == EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "pidfile exists at %s\n", SSSD_PIDFILE);
+ ERROR("SSSD is already running\n");
+ return 2;
+ }
+
/* Parse config file, fail if cannot be done */
ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR,
&monitor);
diff --git a/src/sysv/sssd.in b/src/sysv/sssd.in
index
5109148acc444970d4553b1276fb2625782abb57..385785e02b6cba8a6653e12ac4fc1689bca81fbe
100644
--- a/src/sysv/sssd.in
+++ b/src/sysv/sssd.in
@@ -40,6 +40,8 @@ SSSD=@sbindir@/sssd
LOCK_FILE=@localstatedir@/lock/subsys/sssd
PID_FILE=@localstatedir@/run/sssd.pid
+TIMEOUT=15
+
start() {
[ -x $SSSD ] || exit 5
echo -n $"Starting $prog: "
@@ -47,6 +49,19 @@ start() {
RETVAL=$?
echo
[ "$RETVAL" = 0 ] && touch $LOCK_FILE
+
+ # Wait for pidfile creation or timeout
+ sec=0
+ [ "$RETVAL" = 0 ] && while [ $sec -lt $TIMEOUT -a ! -f $PID_FILE ]
+ do
+ sleep 1
+ sec=$(($sec+1))
+ done
+
+ if [ "$sec" = "$TIMEOUT" ]; then
+ RETVAL=-1
+ fi
+
return $RETVAL
}
diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
index
a4f9125b58e72429cc3ac1e679271367ada27f3c..05cfd3705084dbff8b46fb07e736612612c58b70
100644
--- a/src/sysv/systemd/sssd.service.in
+++ b/src/sysv/systemd/sssd.service.in
@@ -6,11 +6,9 @@ Wants=nss-user-lookup.target
[Service]
EnvironmentFile=-@environment_file@
-ExecStart=@sbindir@/sssd -D -f
-# These two should be used with traditional UNIX forking daemons
-# consult systemd.service(5) for more details
-Type=forking
-PIDFile=@localstatedir@/run/sssd.pid
+ExecStart=@sbindir@/sssd -i -f
+Type=notify
+NotifyAccess=main
[Install]
WantedBy=multi-user.target
--
2.9.3
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
