El 01/09/16 a las 14:50, Lukas Slebodnik escribió:
> On (26/08/16 13:16), Victor Tapia wrote:
>> Hi,
>>
>> Sorry for the late response, I've been out for nearly two weeks. I'm
>> attaching a new version of the patch to review.
>>
>>> Does upstart/init script need to be updated as well?
>>> We have some generic in src/sysv/sssd.in
>>
>> Upstart does need to keep track of the pidfile creation. I can't see a
>> service file, so I'll prepare one.
>>
>> Regarding sysv, due to its sequential nature, it's dangerous to include
>> an infinite loop because it could stuck the normal boot of the machine.
>> I included a timeout in the loop to avoid this, please let me know what
>> you think about it.
>>
>>> BTW the patch causes some failures for me on rhel6.
>>> According to logs, the file /var/run/sssd.pid
>>> did not exist in some cases.
>>>
>>> Please also remove changes in src/sysv/systemd/sssd.service.in
>>> in next version. For systemd only I would prefer "Type=notify".
>>
>> Done, I've switched to Type=notify for systemd.
>>
> I ran tests and I do not understand why but some tests on rhel6
> were stucked in weird state. I'm not sure whether it's caused by your patch
> or by wrong expectation in tests. I will also try to test with 1.13.
> 
> But so far it seems to work with systemd.
> 
> LS
> 

Hi Lukas,

here's the last iteration of the patch for 1.14.2, including the upstart
service script.

Thanks,

Victor
>From 53c7ac0cd107a31559b39bce3cd475dbd102d0bc Mon Sep 17 00:00:00 2001
From: Victor Tapia <victor.ta...@canonical.com>
Date: Thu, 8 Sep 2016 12:16:50 +0200
Subject: [PATCH] Create pidfile after responders started

---
 Makefile.am                      | 19 +++++++++++++++++
 configure.ac                     |  4 ++++
 src/conf_macros.m4               | 24 +++++++++++++++++++++-
 src/monitor/monitor.c            | 42 ++++++++++++++++++++++++++++++++++----
 src/sysv/sssd.in                 | 15 ++++++++++++++
 src/sysv/systemd/sssd.service.in |  3 ++-
 src/sysv/upstart/sssd.conf.in    | 44 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 145 insertions(+), 6 deletions(-)
 create mode 100644 src/sysv/upstart/sssd.conf.in

diff --git a/Makefile.am b/Makefile.am
index f89af5a..cc660ff 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -77,6 +77,7 @@ mcpath = @mcpath@
 initdir = @initdir@
 systemdunitdir = @systemdunitdir@
 systemdconfdir = @systemdconfdir@
+upstartservicedir = @upstartservicedir@
 logpath = @logpath@
 pubconfpath = @pubconfpath@
 gpocachepath = @gpocachepath@
@@ -1243,6 +1244,7 @@ sssd_LDADD = \
     $(INOTIFY_LIBS) \
     $(LIBNL_LIBS) \
     $(KEYUTILS_LIBS) \
+    $(SYSTEMD_DAEMON_LIBS) \
     $(SSSD_INTERNAL_LTLIBS)
 
 sssd_nss_SOURCES = \
@@ -3883,6 +3885,7 @@ endif
 init_SCRIPTS =
 systemdunit_DATA =
 systemdconf_DATA =
+upstartservice_DATA =
 if HAVE_SYSTEMD_UNIT
     systemdunit_DATA += \
         src/sysv/systemd/sssd.service \
@@ -3902,11 +3905,16 @@ if HAVE_GENTOO
     init_SCRIPTS += \
         src/sysv/gentoo/sssd
 else
+if HAVE_UPSTART
+    upstartservice_DATA += \
+        src/sysv/upstart/sssd.conf
+else
     init_SCRIPTS += \
         src/sysv/sssd
 endif
 endif
 endif
+endif
 
 
 dist_sssddata_DATA = \
@@ -3927,6 +3935,7 @@ edit_cmd = $(SED) \
         -e 's|@environment_file[@]|$(environment_file)|g' \
         -e 's|@localstatedir[@]|$(localstatedir)|g' \
         -e 's|@libexecdir[@]|$(libexecdir)|g' \
+	-e 's|@sssdconfdir[@]|$(sssdconfdir)|g' \
         -e 's|@prefix[@]|$(prefix)|g'
 
 replace_script = \
@@ -3941,6 +3950,7 @@ EXTRA_DIST += \
     src/sysv/systemd/journal.conf.in \
     src/sysv/systemd/sssd-secrets.socket.in \
     src/sysv/systemd/sssd-secrets.service.in \
+    src/sysv/upstart/sssd.conf.in \
     $(NULL)
 
 src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile
@@ -3959,6 +3969,10 @@ src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in
 	@$(MKDIR_P) src/sysv/systemd/
 	$(replace_script)
 
+src/sysv/upstart/sssd.conf: src/sysv/upstart/sssd.conf.in Makefile
+	@$(MKDIR_P) src/sysv/upstart/
+	$(replace_script)
+
 SSSD_USER_DIRS = \
     $(DESTDIR)$(dbpath) \
     $(DESTDIR)$(keytabdir) \
@@ -4095,8 +4109,12 @@ if HAVE_SYSTEMD_UNIT
 	$(MKDIR_P) $(DESTDIR)$(systemdunitdir)
 	$(MKDIR_P) $(DESTDIR)$(systemdconfdir)
 else
+if HAVE_UPSTART
+	$(MKDIR_P) $(DESTDIR)$(upstartservicedir)
+else
 	$(MKDIR_P) $(DESTDIR)$(initdir)
 endif
+endif
 
 if SSSD_USER
 	-chgrp $(SSSD_USER) $(DESTDIR)$(sssdlibexecdir)/ldap_child
@@ -4177,6 +4195,7 @@ endif
 	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
+	rm -f $(builddir)/src/sysv/upstart/sssd.conf
 
 CLEANFILES += *.X */*.X */*/*.X
 
diff --git a/configure.ac b/configure.ac
index 8760a85..a751cca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -221,11 +221,15 @@ if test x$initscript = xsystemd; then
     WITH_SYSTEMD_UNIT_DIR
     WITH_SYSTEMD_CONF_DIR
 else
+if test x$initscript = xupstart; then
+    WITH_UPSTART_SERVICE_DIR
+else
     AC_CHECK_PROG(HAVE_SERVICE, service, yes, no)
     AS_IF([test x$HAVE_SERVICE = xyes],
           [AC_DEFINE(HAVE_SERVICE, 1,
                      [Whether the service command is available])])
 fi
+fi
 
 PKG_CHECK_MODULES([DBUS],[dbus-1])
 dnl if test -n "`$PKG_CONFIG --modversion dbus-1 | grep '^0\.'`" ; then
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index bc295c5..8544775 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -120,7 +120,7 @@ AC_DEFUN([WITH_MCACHE_PATH],
 AC_DEFUN([WITH_INITSCRIPT],
   [ AC_ARG_WITH([initscript],
                 [AC_HELP_STRING([--with-initscript=INITSCRIPT_TYPE],
-                                [Type of your init script (sysv|systemd). [sysv]]
+                                [Type of your init script (sysv|systemd|upstart). [sysv]]
                                )
                 ]
                )
@@ -130,6 +130,7 @@ AC_DEFUN([WITH_INITSCRIPT],
   fi
 
   if test x"$with_initscript" = xsysv || \
+     test x"$with_initscript" = xupstart || \
      test x"$with_initscript" = xsystemd; then
         initscript=$with_initscript
   else
@@ -137,6 +138,7 @@ AC_DEFUN([WITH_INITSCRIPT],
   fi
 
   AM_CONDITIONAL([HAVE_SYSV], [test x"$initscript" = xsysv])
+  AM_CONDITIONAL([HAVE_UPSTART], [test x"$initscript" = xupstart])
   AM_CONDITIONAL([HAVE_SYSTEMD_UNIT], [test x"$initscript" = xsystemd])
   AC_MSG_NOTICE([Will use init script type: $initscript])
   ])
@@ -191,6 +193,26 @@ AC_DEFUN([WITH_INIT_DIR],
     AC_SUBST(initdir)
   ])
 
+dnl A macro to configure the directory to install the upstart service files to
+AC_DEFUN([WITH_UPSTART_SERVICE_DIR],
+  [ AC_ARG_WITH([upstartservicedir],
+                [ AC_HELP_STRING([--with-upstartservicedir=DIR],
+                                 [Directory for systemd service files [Auto]]
+                                ),
+                ],
+               )
+  if test x"$with_upstartservicedir" != x; then
+    upstartservicedir=$with_upstartservicedir
+  else
+    upstartservicedir="${sysconfdir}/init"
+    if test x"$upstartservicedir" = x; then
+      AC_MSG_ERROR([Could not detect upstart service directory])
+    fi
+  fi
+  AC_SUBST(upstartservicedir)
+  ])
+
+
 dnl A macro to configure the directory to install the systemd unit files to
 AC_DEFUN([WITH_SYSTEMD_UNIT_DIR],
   [ AC_ARG_WITH([systemdunitdir],
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 1f89c5a..b6682cf 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 != EOK) {
+	    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);
@@ -2579,9 +2607,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) {
@@ -2646,6 +2671,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
 
 [Install]
diff --git a/src/sysv/upstart/sssd.conf.in b/src/sysv/upstart/sssd.conf.in
new file mode 100644
index 0000000..4a7444e
--- /dev/null
+++ b/src/sysv/upstart/sssd.conf.in
@@ -0,0 +1,44 @@
+# sssd - System Security Services Daemon
+#
+# Provides a set of daemons to manage access to remote directories
+# and authentication mechanisms. It provides an NSS and PAM
+# interface toward the system and a pluggable backend system to
+# connect to multiple different account sources. It is also the
+# basis to provide client auditing and policy services for projects
+# like FreeIPA.
+
+description	"System Security Services Daemon"
+
+start on (filesystem and net-device-up and starting autofs)
+stop on runlevel [06]
+
+respawn
+
+pre-start script
+	test -f @sssdconfdir@/sssd.conf || { stop; exit 0; }
+end script
+
+script
+	if [ -f @environment_file@ ]; then
+	. @environment_file@
+	fi
+
+        exec 0>&1
+
+	exec @sbindir@/sssd $DAEMON_OPTS
+end script
+
+post-start script
+        # Wait until the responders are active to transition to started.
+        TIMEOUT=30
+	sec=0
+	while [ $sec -lt $TIMEOUT -a ! -f @localstatedir@/run/sssd.pid ]
+	do
+		sleep 1
+		sec=$(($sec+1))
+	done
+
+	if [ "$sec" -eq "$TIMEOUT" ]; then
+		stop
+	fi
+end script
-- 
2.7.4

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to