From dde41d0797f0b085099cd5c9ff1e0110d5ae3fbd Mon Sep 17 00:00:00 2001
From: Dean Troyer <dtroyer@gmail.com>
Date: Tue, 9 Dec 2014 17:47:57 -0600
Subject: [PATCH] Deprecate SCREEN_LOGDIR in favor of LOGDIR

This is the first step in the log file cleanup.  If SCREEN_LOGDIR
is still set, symlinks will be created in the old screen log directory
so things like the devstack-gate log collector continues to work.

bp:logging-and-service-names
Change-Id: I3ac796e322a18dbd0b8b2310a08310ca159d7613
---
 doc/source/configuration.rst | 23 ++++++++----------
 functions-common             | 43 +++++++++++++++++++++------------
 lib/dstat                    |  8 ++++--
 stack.sh                     |  7 +++---
 stackrc                      | 47 ++++++++++++++++++++++++++++++++++++
 5 files changed, 94 insertions(+), 34 deletions(-)

diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
index baebe979b0..7dbb5882ee 100644
--- a/doc/source/configuration.rst
+++ b/doc/source/configuration.rst
@@ -200,22 +200,19 @@ Enable Logging
 
         LOG_COLOR=False
 
-Logging the Screen Output
--------------------------
+Logging the Service Output
+--------------------------
 
-    | *Default: ``SCREEN_LOGDIR=""``*
-    |  By default DevStack runs the OpenStack services using ``screen``
-       which is useful for watching log and debug output. However, in
-       automated testing the interactive ``screen`` sessions may not be
-       available after the fact; setting ``SCREEN_LOGDIR`` enables logging
-       of the ``screen`` sessions in the specified directory. There will be
-       one file per ``screen`` session named for the session name and a
-       timestamp.
+    | *Default: ``LOGDIR=""``*
+    |  DevStack will log the stdout output of the services it starts.
+       When using ``screen`` this logs the output in the screen windows
+       to a file.  Without ``screen`` this simply redirects stdout of
+       the service process to a file in ``LOGDIR``.
     |
 
     ::
 
-        SCREEN_LOGDIR=$DEST/logs/screen
+        LOGDIR=$DEST/logs
 
     *Note the use of ``DEST`` to locate the main install directory; this
     is why we suggest setting it in ``local.conf``.*
@@ -413,8 +410,8 @@ Examples
        FIXED_RANGE=10.254.1.0/24
        NETWORK_GATEWAY=10.254.1.1
        LOGDAYS=1
-       LOGFILE=$DEST/logs/stack.sh.log
-       SCREEN_LOGDIR=$DEST/logs/screen
+       LOGDIR=$DEST/logs
+       LOGFILE=$LOGDIR/stack.sh.log
        ADMIN_PASSWORD=quiet
        DATABASE_PASSWORD=$ADMIN_PASSWORD
        RABBIT_PASSWORD=$ADMIN_PASSWORD
diff --git a/functions-common b/functions-common
index 44d5671236..8d54473a99 100644
--- a/functions-common
+++ b/functions-common
@@ -319,8 +319,8 @@ function err {
     set +o xtrace
     local msg="[ERROR] ${BASH_SOURCE[2]}:$1 $2"
     echo $msg 1>&2;
-    if [[ -n ${SCREEN_LOGDIR} ]]; then
-        echo $msg >> "${SCREEN_LOGDIR}/error.log"
+    if [[ -n ${LOGDIR} ]]; then
+        echo $msg >> "${LOGDIR}/error.log"
     fi
     $xtrace
     return $exitcode
@@ -372,8 +372,8 @@ function warn {
     set +o xtrace
     local msg="[WARNING] ${BASH_SOURCE[2]}:$1 $2"
     echo $msg 1>&2;
-    if [[ -n ${SCREEN_LOGDIR} ]]; then
-        echo $msg >> "${SCREEN_LOGDIR}/error.log"
+    if [[ -n ${LOGDIR} ]]; then
+        echo $msg >> "${LOGDIR}/error.log"
     fi
     $xtrace
     return $exitcode
@@ -1261,8 +1261,8 @@ function zypper_install {
 # _run_process() is designed to be backgrounded by run_process() to simulate a
 # fork.  It includes the dirty work of closing extra filehandles and preparing log
 # files to produce the same logs as screen_it().  The log filename is derived
-# from the service name and global-and-now-misnamed ``SCREEN_LOGDIR``
-# Uses globals ``CURRENT_LOG_TIME``, ``SCREEN_LOGDIR``, ``SCREEN_NAME``, ``SERVICE_DIR``
+# from the service name.
+# Uses globals ``CURRENT_LOG_TIME``, ``LOGDIR``, ``SCREEN_LOGDIR``, ``SCREEN_NAME``, ``SERVICE_DIR``
 # If an optional group is provided sg will be used to set the group of
 # the command.
 # _run_process service "command-line" [group]
@@ -1277,9 +1277,14 @@ function _run_process {
     exec 3>&-
     exec 6>&-
 
-    if [[ -n ${SCREEN_LOGDIR} ]]; then
-        exec 1>&${SCREEN_LOGDIR}/screen-${service}.log.${CURRENT_LOG_TIME} 2>&1
-        ln -sf ${SCREEN_LOGDIR}/screen-${service}.log.${CURRENT_LOG_TIME} ${SCREEN_LOGDIR}/screen-${service}.log
+    local real_logfile="${LOGDIR}/${service}.log.${CURRENT_LOG_TIME}"
+    if [[ -n ${LOGDIR} ]]; then
+        exec 1>&"$real_logfile" 2>&1
+        ln -sf "$real_logfile" ${LOGDIR}/${service}.log
+        if [[ -n ${SCREEN_LOGDIR} ]]; then
+            # Drop the backward-compat symlink
+            ln -sf "$real_logfile" ${SCREEN_LOGDIR}/screen-${service}.log
+        fi
 
         # TODO(dtroyer): Hack to get stdout from the Python interpreter for the logs.
         export PYTHONUNBUFFERED=1
@@ -1343,7 +1348,7 @@ function run_process {
 }
 
 # Helper to launch a process in a named screen
-# Uses globals ``CURRENT_LOG_TIME``, ``SCREEN_NAME``, ``SCREEN_LOGDIR``,
+# Uses globals ``CURRENT_LOG_TIME``, ```LOGDIR``, ``SCREEN_LOGDIR``, `SCREEN_NAME``,
 # ``SERVICE_DIR``, ``USE_SCREEN``
 # screen_process name "command-line" [group]
 # Run a command in a shell in a screen window, if an optional group
@@ -1362,10 +1367,18 @@ function screen_process {
 
     screen -S $SCREEN_NAME -X screen -t $name
 
-    if [[ -n ${SCREEN_LOGDIR} ]]; then
-        screen -S $SCREEN_NAME -p $name -X logfile ${SCREEN_LOGDIR}/screen-${name}.log.${CURRENT_LOG_TIME}
+    local real_logfile="${LOGDIR}/${name}.log.${CURRENT_LOG_TIME}"
+    echo "LOGDIR: $LOGDIR"
+    echo "SCREEN_LOGDIR: $SCREEN_LOGDIR"
+    echo "log: $real_logfile"
+    if [[ -n ${LOGDIR} ]]; then
+        screen -S $SCREEN_NAME -p $name -X logfile "$real_logfile"
         screen -S $SCREEN_NAME -p $name -X log on
-        ln -sf ${SCREEN_LOGDIR}/screen-${name}.log.${CURRENT_LOG_TIME} ${SCREEN_LOGDIR}/screen-${name}.log
+        ln -sf "$real_logfile" ${LOGDIR}/${name}.log
+        if [[ -n ${SCREEN_LOGDIR} ]]; then
+            # Drop the backward-compat symlink
+            ln -sf "$real_logfile" ${SCREEN_LOGDIR}/screen-${1}.log
+        fi
     fi
 
     # sleep to allow bash to be ready to be send the command - we are
@@ -1410,8 +1423,8 @@ function screen_rc {
         echo "screen -t $1 bash" >> $SCREENRC
         echo "stuff \"$2$NL\"" >> $SCREENRC
 
-        if [[ -n ${SCREEN_LOGDIR} ]]; then
-            echo "logfile ${SCREEN_LOGDIR}/screen-${1}.log.${CURRENT_LOG_TIME}" >>$SCREENRC
+        if [[ -n ${LOGDIR} ]]; then
+            echo "logfile ${LOGDIR}/${1}.log.${CURRENT_LOG_TIME}" >>$SCREENRC
             echo "log on" >>$SCREENRC
         fi
     fi
diff --git a/lib/dstat b/lib/dstat
index a17125fc92..d1db469aa6 100644
--- a/lib/dstat
+++ b/lib/dstat
@@ -27,8 +27,12 @@ DSTAT_FILE=${DSTAT_FILE:-"dstat.log"}
 function start_dstat {
     # A better kind of sysstat, with the top process per time slice
     DSTAT_OPTS="-tcmndrylpg --top-cpu-adv --top-io-adv"
-    if [[ -n ${SCREEN_LOGDIR} ]]; then
-        screen_it dstat "cd $TOP_DIR; dstat $DSTAT_OPTS | tee $SCREEN_LOGDIR/$DSTAT_FILE"
+    if [[ -n ${LOGDIR} ]]; then
+        screen_it dstat "cd $TOP_DIR; dstat $DSTAT_OPTS | tee $LOGDIR/$DSTAT_FILE"
+        if [[ -n ${SCREEN_LOGDIR} ]]; then
+            # Drop the backward-compat symlink
+            ln -sf $LOGDIR/$DSTAT_FILE ${SCREEN_LOGDIR}/$DSTAT_FILE
+        fi
     else
         screen_it dstat "dstat $DSTAT_OPTS"
     fi
diff --git a/stack.sh b/stack.sh
index aeaaf6072c..558b07eb07 100755
--- a/stack.sh
+++ b/stack.sh
@@ -373,10 +373,8 @@ fi
 # Append '.xxxxxxxx' to the given name to maintain history
 # where 'xxxxxxxx' is a representation of the date the file was created
 TIMESTAMP_FORMAT=${TIMESTAMP_FORMAT:-"%F-%H%M%S"}
-if [[ -n "$LOGFILE" || -n "$SCREEN_LOGDIR" ]]; then
-    LOGDAYS=${LOGDAYS:-7}
-    CURRENT_LOG_TIME=$(date "+$TIMESTAMP_FORMAT")
-fi
+LOGDAYS=${LOGDAYS:-7}
+CURRENT_LOG_TIME=$(date "+$TIMESTAMP_FORMAT")
 
 if [[ -n "$LOGFILE" ]]; then
     # Clean up old log files.  Append '.*' to the user-specified
@@ -428,6 +426,7 @@ fi
 # ``screen-$SERVICE_NAME-$TIMESTAMP.log`` in that dir and have a link
 # ``screen-$SERVICE_NAME.log`` to the latest log file.
 # Logs are kept for as long specified in ``LOGDAYS``.
+# This is deprecated....logs go in ``LOGDIR``, only symlinks will be here now.
 if [[ -n "$SCREEN_LOGDIR" ]]; then
 
     # We make sure the directory is created.
diff --git a/stackrc b/stackrc
index 2dd56e8104..ff821401d7 100644
--- a/stackrc
+++ b/stackrc
@@ -705,6 +705,53 @@ USE_SSL=$(trueorfalse False USE_SSL)
 
 # Following entries need to be last items in file
 
+# Compatibility bits required by other callers like Grenade
+
+# Old way was using SCREEN_LOGDIR to locate those logs and LOGFILE for the stack.sh trace log.
+# LOGFILE       SCREEN_LOGDIR       output
+# not set       not set             no log files
+# set           not set             stack.sh log to LOGFILE
+# not set       set                 screen logs to SCREEN_LOGDIR
+# set           set                 stack.sh log to LOGFILE, screen logs to SCREEN_LOGDIR
+
+# New way is LOGDIR for all logs and LOGFILE for stack.sh trace log, but if not fully-qualified will be in LOGDIR
+# LOGFILE       LOGDIR              output
+# not set       not set             (new) set LOGDIR from default
+# set           not set             stack.sh log to LOGFILE, (new) set LOGDIR from LOGFILE
+# not set       set                 screen logs to LOGDIR
+# set           set                 stack.sh log to LOGFILE, screen logs to LOGDIR
+
+# For compat, if SCREEN_LOGDIR is set, it will be used to create back-compat symlinks to the LOGDIR
+# symlinks to SCREEN_LOGDIR (compat)
+
+
+# Set up new logging defaults
+if [[ -z "${LOGDIR:-}" ]]; then
+    default_logdir=$DEST/logs
+    if [[ -z "${LOGFILE:-}" ]]; then
+        # Nothing is set, we need a default
+        LOGDIR="$default_logdir"
+    else
+        # Set default LOGDIR
+        LOGDIR="${LOGFILE%/*}"
+        logfile="${LOGFILE##*/}"
+        if [[ -z "$LOGDIR" || "$LOGDIR" == "$logfile" ]]; then
+            # LOGFILE had no path, set a default
+            LOGDIR="$default_logdir"
+        fi
+
+        # Check for duplication
+        if [[ "${SCREEN_LOGDIR:-}" == "${LOGDIR}" ]]; then
+            # We don't need the symlinks since it's the same directory
+            unset SCREEN_LOGDIR
+        fi
+    fi
+    unset default_logdir logfile
+fi
+
+# LOGDIR is always set at this point so it is not useful as a 'enable' for service logs
+# SCREEN_LOGDIR may be set, it is useful to enable the compat symlinks
+
 # Local variables:
 # mode: shell-script
 # End: