From b3a2ba638e809aad1588f214654a42d2af401fd0 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Fri, 4 Nov 2016 13:19:03 +0100 Subject: [PATCH] NO_ARGS: White labelling - Don't mention options that don't exist in Usage. - Don't include a log prefix when NO_ARGS is set. - Turn up the default verbosity to FATAL when NO_ARGS is set. - Expose verbosity via an ENV var for debugging. --- ci/run_build.sh | 40 +++++++++++++++++------- src/tini.c | 69 ++++++++++++++++++++++++++++++++--------- test/run_inner_tests.py | 10 ++++-- 3 files changed, 91 insertions(+), 28 deletions(-) diff --git a/ci/run_build.sh b/ci/run_build.sh index 543aeea..bb87d79 100755 --- a/ci/run_build.sh +++ b/ci/run_build.sh @@ -71,8 +71,11 @@ if [[ -n "${ARCH_NATIVE:=}" ]]; then # Smoke tests (actual tests need Docker to run; they don't run within the CI environment) for tini in "${BUILD_DIR}/tini" "${BUILD_DIR}/tini-static"; do + echo "Smoke test for ${tini}" + "$tini" --version + echo "Testing ${tini} --version" - "$tini" --version | grep "tini version" + "$tini" --version | grep -q "tini version" if [[ -n "${NO_ARGS:-}" ]]; then echo "Testing $tini with: true" @@ -83,6 +86,18 @@ if [[ -n "${ARCH_NATIVE:=}" ]]; then exit 1 fi + echo "Testing ${tini} without arguments exits with 1" + ! "$tini" 2>/dev/null + + echo "Testing ${tini} shows help message" + { + ! "$tini" 2>&1 + } | grep -q "supervision of a valid init process" + + ! { + ! "$tini" 2>&1 + } | grep -q "more verbose" + # We try running binaries named after flags (both valid and invalid # flags) and test that they run. for flag in h s x; do @@ -94,15 +109,15 @@ if [[ -n "${ARCH_NATIVE:=}" ]]; then echo "Testing $tini can run binary --version if args are given" cp "$(which true)" "${BIN_TEST_DIR}/--version" - if "$tini" "--version" --foo | grep "tini version"; then + if "$tini" "--version" --foo | grep -q "tini version"; then exit 1 fi else - echo "Smoke test for $tini" + echo "Testing ${tini} -h" "${tini}" -h echo "Testing $tini for license" - "${tini}" -l | grep -i "mit license" + "${tini}" -l | grep -q -i "mit license" echo "Testing $tini with: true" "${tini}" -vvv true @@ -111,14 +126,17 @@ if [[ -n "${ARCH_NATIVE:=}" ]]; then if "${tini}" -vvv false; then exit 1 fi + fi - # Test stdin / stdout are handed over to child - echo "Testing pipe" - echo "exit 0" | "${tini}" -vvv sh - if [[ ! "$?" -eq "0" ]]; then - echo "Pipe test failed" - exit 1 - fi + echo "Testing ${tini} supports TINI_VERBOSITY" + TINI_VERBOSITY=3 "$tini" true 2>&1 | grep -q 'Received SIGCHLD' + + # Test stdin / stdout are handed over to child + echo "Testing ${tini} does not break pipes" + echo "exit 0" | "${tini}" sh + if [[ ! "$?" -eq "0" ]]; then + echo "Pipe test failed" + exit 1 fi echo "Checking hardening on $tini" diff --git a/src/tini.c b/src/tini.c index fa86254..b84bb05 100644 --- a/src/tini.c +++ b/src/tini.c @@ -17,11 +17,21 @@ #include "tiniConfig.h" #include "tiniLicense.h" +#if TINI_NO_ARGS +#define PRINT_FATAL(...) fprintf(stderr, __VA_ARGS__); fprintf(stderr, "\n"); +#define PRINT_WARNING(...) if (verbosity > 0) { fprintf(stderr, __VA_ARGS__); fprintf(stderr, "\n"); } +#define PRINT_INFO(...) if (verbosity > 1) { fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } +#define PRINT_DEBUG(...) if (verbosity > 2) { fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } +#define PRINT_TRACE(...) if (verbosity > 3) { fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } +#define DEFAULT_VERBOSITY 0 +#else #define PRINT_FATAL(...) fprintf(stderr, "[FATAL tini (%i)] ", getpid()); fprintf(stderr, __VA_ARGS__); fprintf(stderr, "\n"); #define PRINT_WARNING(...) if (verbosity > 0) { fprintf(stderr, "[WARN tini (%i)] ", getpid()); fprintf(stderr, __VA_ARGS__); fprintf(stderr, "\n"); } #define PRINT_INFO(...) if (verbosity > 1) { fprintf(stdout, "[INFO tini (%i)] ", getpid()); fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } #define PRINT_DEBUG(...) if (verbosity > 2) { fprintf(stdout, "[DEBUG tini (%i)] ", getpid()); fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } #define PRINT_TRACE(...) if (verbosity > 3) { fprintf(stdout, "[TRACE tini (%i)] ", getpid()); fprintf(stdout, __VA_ARGS__); fprintf(stdout, "\n"); } +#define DEFAULT_VERBOSITY 1 +#endif #define ARRAY_LEN(x) (sizeof(x) / sizeof(x[0])) @@ -31,6 +41,7 @@ typedef struct { struct sigaction* const sigttou_action_ptr; } signal_configuration_t; +static unsigned int verbosity = DEFAULT_VERBOSITY; #ifdef PR_SET_CHILD_SUBREAPER #define HAS_SUBREAPER 1 @@ -41,13 +52,14 @@ typedef struct { #define OPT_STRING "hvgl" #endif +#define VERBOSITY_ENV_VAR "TINI_VERBOSITY" + #define TINI_VERSION_STRING "tini version " TINI_VERSION TINI_GIT #if HAS_SUBREAPER static unsigned int subreaper = 0; #endif -static unsigned int verbosity = 1; static unsigned int kill_process_group = 0; static struct timespec ts = { .tv_sec = 1, .tv_nsec = 0 }; @@ -56,13 +68,16 @@ static const char reaper_warning[] = "Tini is not running as PID 1 " #if HAS_SUBREAPER "and isn't registered as a child subreaper" #endif - ".\n\ - Zombie processes will not be re-parented to Tini, so zombie reaping won't work.\n\ - To fix the problem, " +".\n\ +Zombie processes will not be re-parented to Tini, so zombie reaping won't work.\n\ +To fix the problem, " #if HAS_SUBREAPER - "use -s or set the environment variable " SUBREAPER_ENV_VAR " to register Tini as a child subreaper, or " +#ifndef TINI_NO_ARGS +"use the -s option " #endif - "run Tini as PID 1."; +"or set the environment variable " SUBREAPER_ENV_VAR " to register Tini as a child subreaper, or " +#endif +"run Tini as PID 1."; int restore_signals(const signal_configuration_t* const sigconf_ptr) { if (sigprocmask(SIG_SETMASK, sigconf_ptr->sigmask_ptr, NULL)) { @@ -86,7 +101,7 @@ int restore_signals(const signal_configuration_t* const sigconf_ptr) { int isolate_child() { // Put the child into a new process group. if (setpgid(0, 0) < 0) { - PRINT_FATAL("setpgid failed: '%s'", strerror(errno)); + PRINT_FATAL("setpgid failed: %s", strerror(errno)); return 1; } @@ -102,7 +117,7 @@ int isolate_child() { if (errno == ENOTTY) { PRINT_DEBUG("tcsetpgrp failed: no tty (ok to proceed)") } else { - PRINT_FATAL("tcsetpgrp failed: '%s'", strerror(errno)); + PRINT_FATAL("tcsetpgrp failed: %s", strerror(errno)); return 1; } } @@ -118,7 +133,7 @@ int spawn(const signal_configuration_t* const sigconf_ptr, char* const argv[], i pid = fork(); if (pid < 0) { - PRINT_FATAL("Fork failed: '%s'", strerror(errno)); + PRINT_FATAL("fork failed: %s", strerror(errno)); return 1; } else if (pid == 0) { @@ -145,8 +160,20 @@ int spawn(const signal_configuration_t* const sigconf_ptr, char* const argv[], i void print_usage(char* const name, FILE* const file) { fprintf(file, "%s (%s)\n", basename(name), TINI_VERSION_STRING); - fprintf(file, "Usage: %s [OPTIONS] PROGRAM -- [ARGS]\n\n", basename(name)); + +#if TINI_NO_ARGS + fprintf(file, "Usage: %s PROGRAM [ARGS] | --version\n\n", basename(name)); +#else + fprintf(file, "Usage: %s [OPTIONS] PROGRAM -- [ARGS] | --version\n\n", basename(name)); +#endif fprintf(file, "Execute a program under the supervision of a valid init process (%s)\n\n", basename(name)); + + fprintf(file, "Command line options:\n\n"); + + fprintf(file, " --version: Show version and exit.\n"); + +#if TINI_NO_ARGS +#else fprintf(file, " -h: Show this help message and exit.\n"); #if HAS_SUBREAPER fprintf(file, " -s: Register as a process subreaper (requires Linux >= 3.4).\n"); @@ -154,6 +181,16 @@ void print_usage(char* const name, FILE* const file) { fprintf(file, " -v: Generate more verbose output. Repeat up to 3 times.\n"); fprintf(file, " -g: Send signals to the child's process group.\n"); fprintf(file, " -l: Show license and exit.\n"); +#endif + + fprintf(file, "\n"); + + fprintf(file, "Environment variables:\n\n"); +#if HAS_SUBREAPER + fprintf(file, " %s: Register as a process subreaper (requires Linux >= 3.4)\n", SUBREAPER_ENV_VAR); +#endif + fprintf(file, " %s: Set the verbosity level (default: %d)\n", VERBOSITY_ENV_VAR, DEFAULT_VERBOSITY); + fprintf(file, "\n"); } @@ -172,9 +209,7 @@ int parse_args(const int argc, char* const argv[], char* (**child_args_ptr_ptr)[ return 1; } -#if TINI_NO_ARGS - *parse_fail_exitcode_ptr = 0; -#else +#ifndef TINI_NO_ARGS int c; while ((c = getopt(argc, argv, OPT_STRING)) != -1) { switch (c) { @@ -237,6 +272,12 @@ int parse_env() { subreaper++; } #endif + + char* env_verbosity = getenv(VERBOSITY_ENV_VAR); + if (env_verbosity != NULL) { + verbosity = atoi(env_verbosity); + } + return 0; } @@ -468,7 +509,7 @@ int main(int argc, char *argv[]) { reaper_check(); /* Go on */ - if (spawn(&child_sigconf, *child_args_ptr, &child_pid)) { + if(spawn(&child_sigconf, *child_args_ptr, &child_pid)) { return 1; } free(child_args_ptr); diff --git a/test/run_inner_tests.py b/test/run_inner_tests.py index 3cbc66b..b65d153 100755 --- a/test/run_inner_tests.py +++ b/test/run_inner_tests.py @@ -80,10 +80,14 @@ def main(): p.send_signal(signal.SIGUSR1) busy_wait(lambda: p.poll() is not None, 10) - - # Run failing test + # Run failing test. Force verbosity to 1 so we see the subreaper warning + # regardless of whether NO_ARGS is set. print "Running zombie reaping failure test (Tini should warn)" - p = subprocess.Popen([tini, os.path.join(src, "test", "reaping", "stage_1.py")], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + p = subprocess.Popen( + [tini, os.path.join(src, "test", "reaping", "stage_1.py")], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env={'TINI_VERBOSITY': '1'} + ) out, err = p.communicate() assert "zombie reaping won't work" in err, "No warning message was output!" ret = p.wait()