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

cmd/traffic_manager/traffic_manager.cc

@@ -27,6 +27,7 @@
 #include "ts/ink_sock.h"
 #include "ts/ink_args.h"
 #include "ts/ink_syslog.h"
+#include <vector>
 
 #include "WebMgmtUtils.h"
 #include "WebOverview.h"
@@ -65,6 +66,9 @@
 #define FD_THROTTLE_HEADROOM (128 + 64) // TODO: consolidate with THROTTLE_FD_HEADROOM
 #define DIAGS_LOG_FILENAME "manager.log"
 
+// Used to pass traffic_server options to the RPC API
+extern std::function<void(std::vector<char*>&)> proxy_options_callback;  // declared in CoreAPI.cc
+
 // These globals are still referenced directly by management API.
 LocalManager *lmgmt = NULL;
 FileManager *configFiles;
@@ -445,6 +449,8 @@ main(int argc, const char **argv)
   int binding_version      = 0;
   BindingInstance *binding = NULL;
 
+  std::vector<char*> callback_proxy_options;
+
   ArgumentDescription argument_descriptions[] = {
     {"proxyOff", '-', "Disable proxy", "F", &proxy_off, NULL, NULL},
     {"aconfPort", '-', "Autoconf port", "I", &aconf_port_arg, "MGMT_ACONF_PORT", NULL},
@@ -619,6 +625,9 @@ main(int argc, const char **argv)
   // 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) {
+    callback_proxy_options.push_back(ats_strdup(TM_OPT_BIND_STDOUT));
+    callback_proxy_options.push_back(ats_strdup(bind_stdout));
+
     size_t l = strlen(lmgmt->proxy_options);
     size_t n = 3                            /* " --" */
                + sizeof(TM_OPT_BIND_STDOUT) /* nul accounted for here */
@@ -629,6 +638,9 @@ main(int argc, const char **argv)
   }
 
   if (*bind_stderr != 0) {
+    callback_proxy_options.push_back(ats_strdup(TM_OPT_BIND_STDERR));
+    callback_proxy_options.push_back(ats_strdup(bind_stderr));
+
     size_t l = strlen(lmgmt->proxy_options);
     size_t n = 3                            /* space dash dash */
                + sizeof(TM_OPT_BIND_STDERR) /* nul accounted for here */
@@ -638,6 +650,16 @@ main(int argc, const char **argv)
     snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDERR, bind_stderr);
   }
 
+  // If there exist proxy options we want CoreAPI to have, we set the callback
+  // to a lambda that will pass the options
+  if (callback_proxy_options.size() > 0) {
+    proxy_options_callback = [&callback_proxy_options](std::vector<char*> &opts) {
+      for (auto it = callback_proxy_options.begin(); it < callback_proxy_options.end(); ++it) {
+        opts.push_back(ats_strdup(*it));
+      }
+    };
+  }
+
   if (proxy_port) {
     HttpProxyPort::loadValue(lmgmt->m_proxy_ports, proxy_port);
   }
@@ -844,6 +866,11 @@ main(int argc, const char **argv)
     }
   }
 
+  // Delete the proxy options we saved for CoreAPI.
+  // This probably isn't needed b/c the process dies here...
+  for (auto it = callback_proxy_options.begin(); it < callback_proxy_options.end(); ++it)
+    free(*it);
+
   if (binding) {
     metrics_binding_destroy(*binding);
     delete binding;

mgmt/api/CoreAPI.cc

@@ -41,6 +41,7 @@
 #include "ts/Diags.h"
 #include "ts/ink_hash_table.h"
 #include "ExpandingArray.h"
+#include <vector>
 //#include "I_AccCrypto.h"
 
 #include "CoreAPI.h"
@@ -53,6 +54,9 @@
 // global variable
 CallbackTable *local_event_callbacks;
 
+// Used to get any proxy options that traffic_manager might want to tack on
+std::function<void(std::vector<char*>&)> proxy_options_callback;
+
 extern FileManager *configFiles; // global in traffic_manager
 
 /*-------------------------------------------------------------------------
@@ -184,12 +188,27 @@ 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);
+    // If we've been provided an options callback, then we call the callback and
+    // put all the extra options we receive into tsArgs
+    //
+    // proxy_options_callback is given to us by traffic_manager
+    if (proxy_options_callback) {
+      std::vector<char*> options;
+      proxy_options_callback(options);
+
+      for (auto it = options.begin(); it < options.end(); ++it) {
+        ink_strlcat(tsArgs, " ", sizeof(tsArgs));
+        ink_strlcat(tsArgs, static_cast<const char*>(*it), sizeof(tsArgs));
+        free(*it);
+      }
     }
 
+    // We always want to delete the previous run's proxy_options
+    // because we to repopulate with the most up to date options each TS restart
+    ats_free(lmgmt->proxy_options);
+    lmgmt->proxy_options = ats_strdup(tsArgs);
+    mgmt_log("[ProxyStateSet] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
+
     lmgmt->run_proxy = true;
     lmgmt->listenForProxy();