Dear all,
Managing the flow of email coming from cron(8) can be a challenge,
especially when you manage a lot of machines. A pattern I see in system
administration is that either a ton of logic is put in wrappers/scripts
to sensibly deal with any output - or even worse all output is zapped
with this dreaded pattern:
* * * * * command_goes_here 2>&1 >/dev/null # piloting blind
In the above example when your cron job fails, you will never know about
it. You were depending on cron to backup some files? Screwed! The job
has been failing due to filesystem permission errors for weeks - all
your files are gone.
There are workarounds for cron(8)'s shortcomings: you can either put
logic in your scripts to only output when things went wrong, but then
you'll not know about what didn't go wrong: "do x || echo error x".
Another approach is to use wrappers that buffer all output until it is
clear what the exit code of the underlaying command was, and then print
the output so cron will email. Shims such as cronic [1] or chronic [2]
are popular, but not part of base.
To improve the situation I propose to add a simple crontab(5)
convenience option called "-n" (mnemonic "No mail if run successful").
Note that options already are a thing in vixie cron ("-q" has existed
for decades?), but are not part of POSIX.
With this "no mail if success" option you can do things like:
* * * * * -n cp -rv src/ dest/
With the above example crontab(5) entry you'll only receive a mail from
cron(8) if the cp(1) encountered some kind of error. You'll also have in
that email up until what point cp(1) actually was able to copy files.
The "-n" option also encourages folks to be liberal with adding trace
options to their shell scripts like "set -o errexit -o nounset -o xtrace",
and just focus on making sure the script exits with a sensible exit
code. This way when there is some kind of problem, you can read the
full context from the cron mail and be more productive; and if there is
no problem, you won't receive an email, reducing clutter in your inbox.
Kind regards,
Job
[1]: https://habilis.net/cronic/
[2]: https://joeyh.name/code/moreutils/
usr.sbin/cron/crontab.5 | 17 +++++++-
usr.sbin/cron/do_command.c | 100 ++++++++++++++++++++++++---------------------
usr.sbin/cron/entry.c | 5 +++
usr.sbin/cron/structs.h | 1 +
4 files changed, 75 insertions(+), 48 deletions(-)
diff --git usr.sbin/cron/crontab.5 usr.sbin/cron/crontab.5
index 9c2e651980a..700010faadf 100644
--- usr.sbin/cron/crontab.5
+++ usr.sbin/cron/crontab.5
@@ -193,14 +193,27 @@ will be changed into newline characters, and all data
after the first
.Ql %
will be sent to the command as standard input.
+.Pp
+If the
+.Ar command
+field begins with
+.Ql -n ,
+the execution output will only be mailed if the command exits with a non-zero
+exit code.
+No mail is sent after a successful run.
+The
+.Ql -n
+option is an attempt to cure potentially copious volumes of mail coming from
+.Xr cron 8 .
If the
.Ar command
field begins with
.Ql -q ,
execution will not be logged.
Use whitespace to separate
-.Ql -q
-from the command.
+.Ql -n ,
+.Ql -q ,
+and the command.
.Pp
Commands are executed by
.Xr cron 8
diff --git usr.sbin/cron/do_command.c usr.sbin/cron/do_command.c
index 6a4022fcc9a..5d60bff6421 100644
--- usr.sbin/cron/do_command.c
+++ usr.sbin/cron/do_command.c
@@ -3,6 +3,7 @@
/* Copyright 1988,1990,1993,1994 by Paul Vixie
* Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1997,2000 by Internet Software Consortium, Inc.
+ * Copyright (c) 2018 Job Snijders <[email protected]>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -80,7 +81,6 @@ child_process(entry *e, user *u)
char **p, *input_data, *usernm;
auth_session_t *as;
login_cap_t *lc;
- int children = 0;
extern char **environ;
/* mark ourselves as different to PS command watchers */
@@ -146,7 +146,9 @@ child_process(entry *e, user *u)
/* fork again, this time so we can exec the user's command.
*/
- switch (fork()) {
+
+ pid_t jobpid = -1;
+ switch (jobpid = fork()) {
case -1:
syslog(LOG_ERR, "(CRON) CAN'T FORK (%m)");
_exit(EXIT_FAILURE);
@@ -260,8 +262,6 @@ child_process(entry *e, user *u)
break;
}
- children++;
-
/* middle process, child of original cron, parent of process running
* the user's command.
*/
@@ -283,7 +283,8 @@ child_process(entry *e, user *u)
* we would block here. thus we must fork again.
*/
- if (*input_data && fork() == 0) {
+ pid_t stdinjob = -1;
+ if (*input_data && (stdinjob = fork()) == 0) {
FILE *out = fdopen(stdin_pipe[WRITE_PIPE], "w");
int need_newline = FALSE;
int escaped = FALSE;
@@ -331,8 +332,6 @@ child_process(entry *e, user *u)
*/
close(stdin_pipe[WRITE_PIPE]);
- children++;
-
/*
* read output from the grandchild. it's stderr has been redirected to
* it's stdout, which has been redirected to our pipe. if there is any
@@ -342,15 +341,17 @@ child_process(entry *e, user *u)
(void) signal(SIGPIPE, SIG_IGN);
in = fdopen(stdout_pipe[READ_PIPE], "r");
+
+ char *mailto;
+ FILE *mail = NULL;
+ int status = 0;
+ pid_t mailpid = -1;
+ size_t bytes = 1;
+
if (in != NULL) {
int ch = getc(in);
if (ch != EOF) {
- FILE *mail = NULL;
- char *mailto;
- size_t bytes = 1;
- int status = 0;
- pid_t mailpid;
/* get name of recipient. this is MAILTO if set to a
* valid local username; USER otherwise.
@@ -415,30 +416,6 @@ child_process(entry *e, user *u)
fputc(ch, mail);
}
- /* only close pipe if we opened it -- i.e., we're
- * mailing...
- */
-
- if (mail) {
- /* Note: the pclose will probably see
- * the termination of the grandchild
- * in addition to the mail process, since
- * it (the grandchild) is likely to exit
- * after closing its stdout.
- */
- status = cron_pclose(mail, mailpid);
- }
-
- /* if there was output and we could not mail it,
- * log the facts so the poor user can figure out
- * what's going on.
- */
- if (mail && status) {
- syslog(LOG_NOTICE, "(%s) MAIL (mailed %zu byte"
- "%s of output but got status 0x%04x)", usernm,
- bytes, (bytes == 1) ? "" : "s", status);
- }
-
} /*if data from grandchild*/
fclose(in); /* also closes stdout_pipe[READ_PIPE] */
@@ -446,20 +423,51 @@ child_process(entry *e, user *u)
/* wait for children to die.
*/
- for (; children > 0; children--) {
- int waiter;
- pid_t pid;
-
- while ((pid = wait(&waiter)) < 0 && errno == EINTR)
+ int waiter;
+ if (jobpid > 0) {
+ ;
+ while (waitpid(jobpid, &waiter, 0) < 0 && errno == EINTR)
;
- if (pid < 0) {
- break;
+
+ /* If everything went well, and -n was set, _and_ we have mail,
+ * we won't be mailing... so shoot the messenger!
+ */
+ if (WIFEXITED(waiter) && WEXITSTATUS(waiter) == 0
+ && (e->flags & MAIL_WHEN_ERR) == MAIL_WHEN_ERR
+ && mail) {
+ fflush(mail);
+ kill(mailpid, SIGKILL);
+ (void)fclose(mail);
+ mail = NULL;
}
- /*
- if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
- Debug(DPROC, (", dumped core"))
+
+ /* only close pipe if we opened it -- i.e., we're
+ * mailing...
*/
+ if (mail) {
+ /*
+ * Note: the pclose will probably see the termination
+ * of the grandchild in addition to the mail process,
+ * since it (the grandchild) is likely to exit after
+ * closing its stdout.
+ */
+ status = cron_pclose(mail, mailpid);
+ }
+
+ /* if there was output and we could not mail it,
+ * log the facts so the poor user can figure out
+ * what's going on.
+ */
+ if (mail && status) {
+ syslog(LOG_NOTICE, "(%s) MAIL (mailed %zu byte"
+ "%s of output but got status 0x%04x)", usernm,
+ bytes, (bytes == 1) ? "" : "s", status);
+ }
}
+
+ if (stdinjob > 0)
+ while (waitpid(stdinjob, &waiter, 0) < 0 && errno == EINTR)
+ ;
}
int
diff --git usr.sbin/cron/entry.c usr.sbin/cron/entry.c
index 761f01d3d6f..a9d358a2068 100644
--- usr.sbin/cron/entry.c
+++ usr.sbin/cron/entry.c
@@ -338,6 +338,10 @@ load_entry(FILE *file, void (*error_func)(const char *),
struct passwd *pw,
ch = get_char(file);
while (ch == '-') {
switch (ch = get_char(file)) {
+ case 'n':
+ e->flags |= MAIL_WHEN_ERR;
+ Skip_Nonblanks(ch, file)
+ break;
case 'q':
e->flags |= DONT_LOG;
Skip_Nonblanks(ch, file)
@@ -346,6 +350,7 @@ load_entry(FILE *file, void (*error_func)(const char *),
struct passwd *pw,
ecode = e_option;
goto eof;
}
+
Skip_Blanks(ch, file)
if (ch == EOF || ch == '\n') {
ecode = e_cmd;
diff --git usr.sbin/cron/structs.h usr.sbin/cron/structs.h
index af0d75c3b55..597f65d6614 100644
--- usr.sbin/cron/structs.h
+++ usr.sbin/cron/structs.h
@@ -38,6 +38,7 @@ typedef struct _entry {
#define DOW_STAR 0x08
#define WHEN_REBOOT 0x10
#define DONT_LOG 0x20
+#define MAIL_WHEN_ERR 0x40
} entry;
/* the crontab database will be a list of the
