Merge pull request #4693 from elliefm/v39/master-ready-file

master ready file
cyrusimap · Nov 17, 2023 · 5539ffb · 5539ffb
2 parents f912397 + 52c4d2f
commit 5539ffb
Show file tree

Hide file tree

Showing 6 changed files with 184 additions and 33 deletions.
diff --git a/cassandane/Cassandane/Cyrus/Master.pm b/cassandane/Cassandane/Cyrus/Master.pm
@@ -40,6 +40,7 @@
 package Cassandane::Cyrus::Master;
 use strict;
 use warnings;
+use File::stat;
 use POSIX qw(getcwd);
 use DateTime;
 
@@ -1320,4 +1321,85 @@ sub test_sighup_reloading_proto
         $self->lemming_census());
 }
 
+sub test_ready_file_new
+{
+    my ($self) = @_;
+
+    my $ready_file = $self->{instance}->get_basedir() . '/conf/master.ready';
+    my $pid_file = $self->{instance}->_pid_file();
+
+    # pid file should not already exist
+    my $pid_sb = stat($pid_file);
+    $self->assert_null($pid_sb);
+
+    # ready file should not already exist
+    my $ready_sb = stat($ready_file);
+    $self->assert_null($ready_sb);
+
+    # start cyrus
+    $self->start();
+
+    # pid file should exist now
+    $pid_sb = stat($pid_file);
+    $self->assert_not_null($pid_sb);
+
+    # ready file should exist soon...
+    timed_wait(sub { $ready_sb = stat($ready_file) },
+               description => "$ready_file to exist");
+    $self->assert_not_null($ready_sb);
+
+    # ready file should be newer than pid file
+    $self->assert_num_gte($pid_sb->mtime, $ready_sb->mtime);
+}
+
+sub test_ready_file_exists
+{
+    my ($self) = @_;
+
+    # force basedir to be computed
+    $self->{instance}->get_basedir();
+
+    # cannot be under basedir because it'll be blown away at startup
+    my $ready_file = "/tmp/cassandane-$$-master.ready";
+    $self->{instance}->{config}->set('master_ready_file', $ready_file);
+
+    # must be after the get_basedir() call above
+    my $pid_file = $self->{instance}->_pid_file();
+
+    system("touch", $ready_file) == 0 or die "touch $ready_file: $?";
+    sleep 3;
+
+    # pid file should not already exist
+    my $pid_sb = stat($pid_file);
+    $self->assert_null($pid_sb);
+
+    # ready file should already exist
+    my $ready_sb = stat($ready_file);
+    $self->assert_not_null($ready_sb);
+    my $orig_mtime = $ready_sb->mtime;
+
+    # start cyrus
+    $self->start();
+
+    # pid file should exist now
+    $pid_sb = stat($pid_file);
+    $self->assert_not_null($pid_sb);
+
+    # ready file should be touched soon
+    timed_wait(sub {
+                    $ready_sb = stat($ready_file);
+                    return 1 if $ready_sb->mtime >= $pid_sb->mtime;
+                    return undef;
+               },
+               description => "$ready_file to be newer than $pid_file");
+    $self->assert_not_null($ready_sb);
+
+    # ready file should be newer than pid file
+    $self->assert_num_gte($pid_sb->mtime, $ready_sb->mtime);
+    $self->assert_num_gt($orig_mtime, $ready_sb->mtime);
+
+    # don't pollute /tmp
+    unlink $ready_file;
+}
+
 1;
diff --git a/changes/next/master-ready-file b/changes/next/master-ready-file
@@ -0,0 +1,28 @@
+Description:
+
+master(8) now touches a ready file to indicate it is "ready for work"
+master(8) now gets its pidfile name from master_pid_file imapd.conf option
+
+
+Config changes:
+
+* master_pid_file
+* master_ready_file
+
+
+Upgrade instructions:
+
+If you have something that monitors syslog looking for master's
+"ready for work" message, you might consider switching to monitoring
+the master.ready file instead, perhaps using Linux inotify.
+
+The master pidfile name is now read from imapd.conf, and defaults
+to ``{configdirectory}/master.pid``.  If you have something that
+looks for this file, you should either update it to look in the new
+default location, or set ``master_pid_file`` in :cyrusman:`imapd.conf(5)`
+to override the default.  The ``-p`` option to :cyrusman:`master(8)`
+can still be used to override it.
+
+GitHub issue:
+
+N/A
diff --git a/configure.ac b/configure.ac
@@ -1595,16 +1595,6 @@ AC_SUBST([GUESSTZ_LIBS])
 AC_SUBST([GUESSTZ_CFLAGS])
 AM_CONDITIONAL([HAVE_GUESSTZ], [test "x$with_guesstz" = xyes])
 
-dnl
-dnl Set pidfile location
-dnl
-AC_ARG_WITH(pidfile,
-        [AS_HELP_STRING([--with-pidfile=DIR], [pidfile in DIR [/var/run/cyrus-master.pid]])],
-        [MASTERPIDFILE="$withval"],
-        [MASTERPIDFILE="/var/run/cyrus-master.pid"])
-MASTERPIDFILE="\"$MASTERPIDFILE\""
-AC_DEFINE_UNQUOTED(MASTER_PIDFILE, $MASTERPIDFILE,[Name of the pidfile for master])
-
 dnl
 dnl see if we're compiling with autocreate support
 dnl

diff --git a/docsrc/imap/reference/manpages/systemcommands/master.rst b/docsrc/imap/reference/manpages/systemcommands/master.rst
@@ -16,7 +16,7 @@ Synopsis
 .. parsed-literal::
 
     **master** [ **-C** *config-file* ] [ **-M** *alternate cyrus.conf* ]
-        [ **-l** *listen queue* ] [ **-p** *pidfile* ]
+        [ **-l** *listen queue* ] [ **-p** *pidfile* ] [ **-r** *ready_file* ]
         [ **-j** *janitor period* ] [ **-d** | **-D** ] [ **-L** *logfile* ]
 
 Description
@@ -64,7 +64,14 @@ Options
 .. option:: -p  pidfile
 
     Use *pidfile* as the pidfile.  If not specified, defaults to
-    ``/var/run/master.pid``
+    ``master_pid_file`` from :cyrusman:`imapd.conf(5)`, which
+    defaults to ``{configdirectory}/master.pid``
+
+.. option:: -r  ready_file
+
+    Use *ready_file* as the ready file.  If not specified, uses
+    ``master_ready_file`` from :cyrusman:`imapd.conf(5)`, which
+    defaults to ``{configdirectory}/master.ready``
 
 .. option:: -d
 

diff --git a/lib/imapoptions b/lib/imapoptions
@@ -1701,6 +1701,15 @@ Blank lines and lines beginning with ``#'' are ignored.
    come up in response to a reconfig+SIGHUP will just be logged and disabled
    like the default behaviour, without causing master to exit. */
 
+{ "master_pid_file", "{configdirectory}/master.pid", STRING, "UNRELEASED" }
+/* The path to a file that master(8) will write its PID to when running
+   as a daemon. */
+
+{ "master_ready_file", "{configdirectory}/master.ready", STRING, "UNRELEASED" }
+/* The path to a file that master(8) will update to indicate that it is
+   ready to accept client connections.  This file will be created if it does
+   not already exist, or truncated if it does.  */
+
 { "maxheaderlines", 1000, INT, "2.3.17" }
 /* Maximum number of lines of header that will be processed into cache
    records.  Default 1000.  If set to zero, it is unlimited.

diff --git a/master/master.c b/master/master.c
@@ -530,7 +530,7 @@ static void service_create(struct service *s, int is_startup)
         res0 = (struct addrinfo *)xzmalloc(sizeof(struct addrinfo));
         res0->ai_flags = AI_PASSIVE;
         res0->ai_family = PF_UNIX;
-        if(!strcmp(s->proto, "tcp")) {
+        if (!strcmp(s->proto, "tcp")) {
             res0->ai_socktype = SOCK_STREAM;
         } else {
             /* udp */
@@ -725,7 +725,7 @@ static void service_create(struct service *s, int is_startup)
         nsocket++;
     }
     if (res0) {
-        if(res0_is_local)
+        if (res0_is_local)
             free(res0);
         else
             freeaddrinfo(res0);
@@ -1274,7 +1274,7 @@ static void spawn_schedule(struct timeval now)
     while (a && a != schedule) {
         /* if a->exec is NULL, we just used the event to wake up,
          * so we actually don't need to exec anything at the moment */
-        if(a->exec) {
+        if (a->exec) {
             get_executable(path, sizeof(path), a->exec);
             switch (p = fork()) {
             case -1:
@@ -1328,7 +1328,7 @@ static void spawn_schedule(struct timeval now)
         /* reschedule as needed */
         b = a->next;
         if (a->period) {
-            if(a->periodic) {
+            if (a->periodic) {
                 a->mark = now;
                 a->mark.tv_sec += a->period;
             } else {
@@ -2238,8 +2238,8 @@ static void add_service(const char *name, struct entry *e, void *rock)
     int reconfig = 0;
     int i, j;
 
-    if(babysit && prefork == 0) prefork = 1;
-    if(babysit && maxforkrate == 0) maxforkrate = 10; /* reasonable safety */
+    if (babysit && prefork == 0) prefork = 1;
+    if (babysit && maxforkrate == 0) maxforkrate = 10; /* reasonable safety */
 
     if (!strcmp(cmd,"") || !strcmp(listen,"")) {
         char buf[256];
@@ -2856,12 +2856,39 @@ static void check_undermanned(struct service *s, int si, int wdi)
     }
 }
 
+static void master_ready(const char *ready_file)
+{
+    int fd;
+
+    syslog(LOG_DEBUG, "ready for work");
+
+    if (!ready_file) ready_file = config_getstring(IMAPOPT_MASTER_READY_FILE);
+    if (!ready_file) return;
+
+    if (cyrus_mkdir(ready_file, 0755 /* ignored */)) return;
+
+    fd = creat(ready_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+    if (fd >= 0) {
+        fsync(fd);
+        close(fd);
+    }
+    else {
+        xsyslog(LOG_ERR, "unable to create ready file",
+                         "fname=<%s>",
+                         ready_file);
+    }
+
+    /* we did our best */
+    errno = 0;
+}
+
 int main(int argc, char **argv)
 {
     static const char lock_suffix[] = ".lock";
 
-    const char *pidfile = MASTER_PIDFILE;
+    const char *pidfile = NULL;
     char *pidfile_lock = NULL;
+    const char *ready_file = NULL;
 
     int startup_pipe[2] = { -1, -1 };
     int pidlock_fd = -1;
@@ -2881,7 +2908,7 @@ int main(int argc, char **argv)
 
     p = getenv("CYRUS_VERBOSE");
     if (p) verbose = atoi(p) + 1;
-    while ((opt = getopt(argc, argv, "C:L:M:p:l:Ddj:vV")) != EOF) {
+    while ((opt = getopt(argc, argv, "C:L:M:p:r:l:Ddj:vV")) != EOF) {
         switch (opt) {
         case 'C': /* alt imapd.conf file */
             alt_config = optarg;
@@ -2897,6 +2924,10 @@ int main(int argc, char **argv)
             /* Set the pidfile name */
             pidfile = optarg;
             break;
+        case 'r':
+            /* Set the ready file name */
+            ready_file = optarg;
+            break;
         case 'd':
             /* Daemon Mode */
             daemon_mode = 1;
@@ -2912,7 +2943,7 @@ int main(int argc, char **argv)
         case 'j':
             /* Janitor frequency */
             janitor_frequency = atoi(optarg);
-            if(janitor_frequency < 1)
+            if (janitor_frequency < 1)
                 fatal("The janitor period must be at least 1 second", EX_CONFIG);
             break;
         case 'v':
@@ -2952,6 +2983,9 @@ int main(int argc, char **argv)
         }
     }
 
+    if (!pidfile) pidfile = config_getstring(IMAPOPT_MASTER_PID_FILE);
+    if (!pidfile) fatal("couldn't determine pidfile name", EX_CONFIG);
+
     /* Pidfile Algorithm in Daemon Mode.  This is a little subtle because
      * we want to ensure that we can report an error to our parent if the
      * child fails to lock the pidfile.
@@ -2967,25 +3001,25 @@ int main(int argc, char **argv)
      * [A] xunlink pidfile.lock and exit(code read from pipe)
      *
      */
-    if(daemon_mode) {
+    if (daemon_mode) {
         /* Daemonize */
         pid_t pid = -1;
 
         pidfile_lock = strconcat(pidfile, lock_suffix, (char *)NULL);
 
         pidlock_fd = open(pidfile_lock, O_CREAT|O_TRUNC|O_RDWR, 0644);
-        if(pidlock_fd == -1) {
+        if (pidlock_fd == -1) {
             syslog(LOG_ERR, "can't open pidfile lock: %s (%m)", pidfile_lock);
             exit(EX_OSERR);
         } else {
-            if(lock_nonblocking(pidlock_fd, pidfile)) {
+            if (lock_nonblocking(pidlock_fd, pidfile)) {
                 syslog(LOG_ERR, "can't get exclusive lock on %s",
                        pidfile_lock);
                 exit(EX_TEMPFAIL);
             }
         }
 
-        if(pipe(startup_pipe) == -1) {
+        if (pipe(startup_pipe) == -1) {
             syslog(LOG_ERR, "can't create startup pipe (%m)");
             exit(EX_OSERR);
         }
@@ -3016,7 +3050,7 @@ int main(int argc, char **argv)
             int exit_code;
 
             /* Parent, wait for child */
-            if(read(startup_pipe[0], &exit_code, sizeof(exit_code)) == -1) {
+            if (read(startup_pipe[0], &exit_code, sizeof(exit_code)) == -1) {
                 syslog(LOG_ERR, "could not read from startup_pipe (%m)");
                 xunlink(pidfile_lock);
                 exit(EX_OSERR);
@@ -3049,7 +3083,7 @@ int main(int argc, char **argv)
 
     /* Write out the pidfile */
     pidfd = open(pidfile, O_CREAT|O_RDWR, 0644);
-    if(pidfd == -1) {
+    if (pidfd == -1) {
         int exit_result = EX_OSERR;
 
         syslog(LOG_ERR, "can't open pidfile: %m");
@@ -3063,7 +3097,7 @@ int main(int argc, char **argv)
     } else {
         char buf[100];
 
-        if(lock_nonblocking(pidfd, pidfile)) {
+        if (lock_nonblocking(pidfd, pidfile)) {
             int exit_result = EX_OSERR;
 
             /* Tell our parent that we failed. */
@@ -3092,9 +3126,10 @@ int main(int argc, char **argv)
 
             /* Write PID */
             snprintf(buf, sizeof(buf), "%lu\n", (unsigned long int)getpid());
-            if(lseek(pidfd, 0, SEEK_SET) == -1 ||
-               ftruncate(pidfd, 0) == -1 ||
-               write(pidfd, buf, strlen(buf)) == -1) {
+            if (lseek(pidfd, 0, SEEK_SET) == -1 ||
+                ftruncate(pidfd, 0) == -1 ||
+                write(pidfd, buf, strlen(buf)) == -1)
+            {
                 int exit_result = EX_OSERR;
 
                 syslog(LOG_ERR, "unable to write to pidfile: %m");
@@ -3111,7 +3146,7 @@ int main(int argc, char **argv)
         }
     }
 
-    if(daemon_mode) {
+    if (daemon_mode) {
         int exit_result = 0;
 
         /* success! */
@@ -3170,7 +3205,7 @@ int main(int argc, char **argv)
     }
 
     /* ok, we're going to start spawning like mad now */
-    syslog(LOG_DEBUG, "ready for work");
+    master_ready(ready_file); /* ready for work */
 
     for (;;) {
         int i, maxfd, ready_fds, total_children = 0;