TS-4399 TS-4400 Management API messes up proxy options

cmd/traffic_manager/traffic_manager.cc

@@ -37,6 +37,7 @@
 #include "FileManager.h"
 #include "ts/I_Layout.h"
 #include "ts/I_Version.h"
+#include "ts/TextBuffer.h"
 #include "DiagsConfig.h"
 #include "HTTP.h"
 #include "CoreAPI.h"
@@ -611,32 +612,23 @@ main(int argc, const char **argv)
 
   /* Update cmd line overrides/environmental overrides/etc */
   if (tsArgs) { /* Passed command line args for proxy */
-    ats_free(lmgmt->proxy_options);
-    lmgmt->proxy_options = tsArgs;
-    mgmt_log("[main] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
+    lmgmt->proxy_options = ats_strdup(tsArgs);
   }
 
-  // we must pass in bind_stdout and bind_stderr values to TS
-  // we do it so TS is able to create BaseLogFiles for each value
-  if (*bind_stdout != 0) {
-    size_t l = strlen(lmgmt->proxy_options);
-    size_t n = 3                            /* " --" */
-               + sizeof(TM_OPT_BIND_STDOUT) /* nul accounted for here */
-               + 1                          /* space */
-               + strlen(bind_stdout);
-    lmgmt->proxy_options = static_cast<char *>(ats_realloc(lmgmt->proxy_options, n + l));
-    snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDOUT, bind_stdout);
+  // TS needs to be started up with the same outputlog bindings each time,
+  // so we append the outputlog location to the persistent proxy options
+  //
+  // TS needs them to be able to create BaseLogFiles for each value
+  textBuffer args(1024);
+  if (*bind_stdout) {
+    const char *space = args.empty() ? "" : " ";
+    args.format("%s%s %s", space, "--" TM_OPT_BIND_STDOUT, bind_stdout);
   }
-
-  if (*bind_stderr != 0) {
-    size_t l = strlen(lmgmt->proxy_options);
-    size_t n = 3                            /* space dash dash */
-               + sizeof(TM_OPT_BIND_STDERR) /* nul accounted for here */
-               + 1                          /* space */
-               + strlen(bind_stderr);
-    lmgmt->proxy_options = static_cast<char *>(ats_realloc(lmgmt->proxy_options, n + l));
-    snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDERR, bind_stderr);
+  if (*bind_stderr) {
+    const char *space = args.empty() ? "" : " ";
+    args.format("%s%s %s", space, "--" TM_OPT_BIND_STDERR, bind_stderr);
   }
+  lmgmt->proxy_options = args.release();
 
   if (proxy_port) {
     HttpProxyPort::loadValue(lmgmt->m_proxy_ports, proxy_port);
@@ -813,12 +805,14 @@ main(int argc, const char **argv)
       } else {
         sleep_time = 1;
       }
-      if (lmgmt->startProxy()) {
+      lmgmt->coreapi_sleep = false;
+      if (ProxyStateSet(TS_PROXY_ON, TS_CACHE_CLEAR_NONE) == TS_ERR_OKAY) {
         just_started = 0;
         sleep_time   = 0;
       } else {
         just_started++;
       }
+      lmgmt->coreapi_sleep = true;
     } else { /* Give the proxy a chance to fire up */
       just_started++;
     }

mgmt/LocalManager.cc

@@ -189,6 +189,7 @@ LocalManager::LocalManager(bool proxy_on) : BaseManager(), run_proxy(proxy_on),
   proxy_launch_outstanding  = false;
   mgmt_shutdown_outstanding = MGMT_PENDING_NONE;
   proxy_running             = 0;
+  coreapi_sleep             = true;
 
   RecRegisterStatInt(RECT_NODE, "proxy.node.proxy_running", 0, RECP_NON_PERSISTENT);
 
@@ -239,8 +240,8 @@ LocalManager::LocalManager(bool proxy_on) : BaseManager(), run_proxy(proxy_on),
   process_server_timeout_msecs = REC_readInteger("proxy.config.lm.pserver_timeout_msecs", &found);
   proxy_name                   = REC_readString("proxy.config.proxy_name", &found);
   proxy_binary                 = REC_readString("proxy.config.proxy_binary", &found);
-  proxy_options                = REC_readString("proxy.config.proxy_binary_opts", &found);
   env_prep                     = REC_readString("proxy.config.env_prep", &found);
+  proxy_options                = NULL;
 
   // Calculate proxy_binary from the absolute bin_path
   absolute_proxy_binary = Layout::relative_to(bindir, proxy_binary);
@@ -831,9 +832,13 @@ LocalManager::processEventQueue()
 /*
  * startProxy()
  *   Function fires up a proxy process.
+ *
+ * Args:
+ *   onetime_options: one time options that traffic_server should be started with (ie
+ *                    these options do not persist across reboots)
  */
 bool
-LocalManager::startProxy()
+LocalManager::startProxy(const char *onetime_options)
 {
   if (proxy_launch_outstanding) {
     return false;
@@ -902,8 +907,13 @@ LocalManager::startProxy()
     Vec<char> real_proxy_options;
 
     real_proxy_options.append(proxy_options, strlen(proxy_options));
+    if (onetime_options && *onetime_options) {
+      real_proxy_options.append(" ", strlen(" "));
+      real_proxy_options.append(onetime_options, strlen(onetime_options));
+    }
 
-    if (!strstr(proxy_options, MGMT_OPT)) { // Make sure we're starting the proxy in mgmt mode
+    // Make sure we're starting the proxy in mgmt mode
+    if (!strstr(proxy_options, MGMT_OPT) && !strstr(onetime_options, MGMT_OPT)) {
       real_proxy_options.append(" ", strlen(" "));
       real_proxy_options.append(MGMT_OPT, sizeof(MGMT_OPT) - 1);
     }

mgmt/LocalManager.h

@@ -74,7 +74,7 @@ class LocalManager : public BaseManager
   void signalAlarm(int alarm_id, const char *desc = NULL, const char *ip = NULL);
 
   void processEventQueue();
-  bool startProxy();
+  bool startProxy(const char *onetime_options);
   void listenForProxy();
   void bindProxyPort(HttpProxyPort &);
   void closeProxyPorts();
@@ -90,6 +90,7 @@ class LocalManager : public BaseManager
   bool processRunning();
   bool clusterOk();
 
+  volatile bool coreapi_sleep;
   volatile bool run_proxy;
   volatile time_t manager_started_at;
   volatile time_t proxy_started_at;
@@ -108,7 +109,7 @@ class LocalManager : public BaseManager
   char *absolute_proxy_binary;
   char *proxy_name;
   char *proxy_binary;
-  char *proxy_options;
+  char *proxy_options; // These options should persist across proxy reboots
   char *env_prep;
 
   int process_server_sockfd;

mgmt/api/CoreAPI.cc

@@ -184,21 +184,19 @@ ProxyStateSet(TSProxyStateT state, TSCacheClearT clear)
       ink_strlcat(tsArgs, " -k", sizeof(tsArgs));
     }
 
-    if (strlen(tsArgs) > 0) { /* Passed command line args for proxy */
-      ats_free(lmgmt->proxy_options);
-      lmgmt->proxy_options = ats_strdup(tsArgs);
-      mgmt_log("[ProxyStateSet] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
-    }
+    mgmt_log("[ProxyStateSet] Traffic Server Args: '%s %s'\n", lmgmt->proxy_options ? lmgmt->proxy_options : "", tsArgs);
 
     lmgmt->run_proxy = true;
     lmgmt->listenForProxy();
+    lmgmt->startProxy(tsArgs);
 
-    do {
-      mgmt_sleep_sec(1);
-    } while (i++ < 20 && (lmgmt->proxy_running == 0));
+    if (lmgmt->coreapi_sleep) {
+      do {
+        mgmt_sleep_sec(1);
+      } while (i++ < 20 && (lmgmt->proxy_running == 0));
 
-    if (!lmgmt->processRunning()) {
-      goto Lerror;
+      if (!lmgmt->processRunning())
+        goto Lerror;
     }
 
     break;