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(a)lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-leave(a)lists.fedorahosted.org
>
From f0b08423e7d215aee49e23a713e53df2c61a239e Mon Sep 17 00:00:00
2001
From: Victor Tapia <victor.tapia(a)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(a)/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(a)/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