Add cloudtail start/stop in recipe_modules/goma

scripts/slave/recipe_modules/goma/resources/cloudtail_utils.py

+#!/usr/bin/env python
+# Copyright (c) 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import argparse
+import os
+import signal
+import subprocess
+import sys
+
+from common import goma_utils

[ukai, 2016/08/15 06:54:00]

goma_utils in common? it is in slave?

[tikuta, 2016/08/16 02:16:56]

Done.

+
+
+def start_cloudtail(args, f):
+  """Write process id of started cloudtail to file object f"""
+
+  proc = subprocess.Popen([args.cloudtail_path,
+                    'tail',
+                    '--log-id', args.log_id,
+                    '--path', goma_utils.GetLatestGomaCompilerProxyInfo()],

[Yoshisato Yanagisawa, 2016/08/15 07:19:30]

I want to know the design choice why --log-id is passed as an argument but
GetLatestGomaCompilerProxyInfo is not?

Since I thought the id goma_compiler_proxy is only valid for goma's
compiler_proxy.INFO, I thought either of three designs.
1. also pass compiler_proxy info as command argument.
2. won't pass either of --log-id or compiler_proxy.INFO filename.
3. pass --log-id via command line argument, and set --path
goma_utils.GetLatestGomaCompilerProxyInfo only if the id goma_compiler_proxy is
set.

I prefer 3rd case but in 3rd case, it might be better to print pid to JSON file
instead of stdout?  To run multiple cloudtail at once?

[tikuta, 2016/08/16 02:16:56]

Thank you for comment.
But I'm not sure why JSON is better than raw_io.
It is possible to run multiple cloudtail if we use raw_io?

[Yoshisato Yanagisawa, 2016/08/16 02:50:10]

As we talked offline,
it should be better for cloudtail script uses constant value goma_compiler_proxy
because we only upload compiler_proxy.INFO here.

[tikuta, 2016/08/16 02:56:21]

Done.

+  )
+
+  f.write(str(proc.pid))
+
+
+def main():
+  parser = argparse.ArgumentParser(description='cloudtail utility for goma recipe module.')

[ukai, 2016/08/15 06:54:00]

long line?

[tikuta, 2016/08/16 02:16:56]

Done.

+
+  parser.add_argument('--cloudtail-path',
+                      help='path of cloudtail binary')
+  parser.add_argument('--log-id',

[Yoshisato Yanagisawa, 2016/08/16 02:50:10]

i.e. I suggest to remove this command line argument.

[tikuta, 2016/08/16 02:56:21]

Done.

+                      help='id of log')
+  parser.add_argument('--start-cloudtail', action='store_true',
+                      help='start cloudtail if this option is enabled')
+  parser.add_argument('--killed-pid',
+                      help='pid that is killed')

[Yoshisato Yanagisawa, 2016/08/15 07:19:30]

Is it allowed to set both start-cloudtail and killed-pid?

[tikuta, 2016/08/16 02:16:56]

I wrote assertion and added a message to help.

+
+  args = parser.parse_args()
+
+  if args.start_cloudtail:
+    start_cloudtail(args, sys.stdout)
+
+  if args.killed_pid is not None:
+    os.kill(int(args.killed_pid), signal.SIGTERM)

[Yoshisato Yanagisawa, 2016/08/15 07:19:30]

Don't you wait the program termination?
(also SIGKILL if not stopped in certain duration?)

[tikuta, 2016/08/16 02:16:56]

Done.

+
+
+if '__main__' == __name__:
+  sys.exit(main())