Cherrypick cl/217729965

Turn on serial log capturing but need to refactor logcat capturing
before turning it on.

Bug: 119614469
Test: acloud create --serial_log_file ./serial.tar.gz
acloud create_cf --{build_id,build_target,branch} ... --serial_log_file ./serial.tar.gz
atest acloud_test
Change-Id: I41463a4b72b0f2f86094c0b7d14879621280baca
diff --git a/create/avd_spec.py b/create/avd_spec.py
index 87d3239..62900ed 100644
--- a/create/avd_spec.py
+++ b/create/avd_spec.py
@@ -89,6 +89,10 @@
         self._hw_property = None
         # Create config instance for android_build_client to query build api.
         self._cfg = config.GetAcloudConfig(args)
+        # Reporting args.
+        self._serial_log_file = None
+        self._logcat_file = None
+
         self._ProcessArgs(args)
 
     def __repr__(self):
@@ -227,6 +231,8 @@
                                constants.INSTANCE_TYPE_REMOTE)
         self._num_of_instances = args.num
         self._kernel_build_id = args.kernel_build_id
+        self._serial_log_file = args.serial_log_file
+        self._logcat_file = args.logcat_file
 
     def _ProcessLocalImageArgs(self, args):
         """Get local image path.
@@ -411,3 +417,13 @@
     def image_download_dir(self, value):
         """Set image download dir."""
         self._image_download_dir = value
+
+    @property
+    def serial_log_file(self):
+        """Return serial log file path."""
+        return self._serial_log_file
+
+    @property
+    def logcat_file(self):
+        """Return logcat file path."""
+        return self._logcat_file
diff --git a/internal/constants.py b/internal/constants.py
index 2825b45..c3d5aa1 100755
--- a/internal/constants.py
+++ b/internal/constants.py
@@ -99,7 +99,8 @@
 CMD_STOP_CVD = "stop_cvd"
 ENV_ANDROID_BUILD_TOP = "ANDROID_BUILD_TOP"
 
-LOCALHOST_ADB_SERIAL = "127.0.0.1:%d"
+LOCALHOST = "127.0.0.1"
+LOCALHOST_ADB_SERIAL = LOCALHOST + ":%d"
 
 SSH_BIN = "ssh"
 ADB_BIN = "adb"
diff --git a/internal/lib/utils.py b/internal/lib/utils.py
index a90d8da..e17794c 100755
--- a/internal/lib/utils.py
+++ b/internal/lib/utils.py
@@ -1,5 +1,3 @@
-#!/usr/bin/env python
-#
 # Copyright 2016 - The Android Open Source Project
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -45,6 +43,10 @@
 
 SSH_KEYGEN_CMD = ["ssh-keygen", "-t", "rsa", "-b", "4096"]
 SSH_KEYGEN_PUB_CMD = ["ssh-keygen", "-y"]
+SSH_ARGS = ["-o", "UserKnownHostsFile=/dev/null",
+            "-o", "StrictHostKeyChecking=no"]
+SSH_CMD = ["ssh"] + SSH_ARGS
+SCP_CMD = ["scp"] + SSH_ARGS
 DEFAULT_RETRY_BACKOFF_FACTOR = 1
 DEFAULT_SLEEP_MULTIPLIER = 0
 
@@ -284,6 +286,39 @@
             tar.add(src, arcname=arcname)
 
 
+def ScpPullFile(src_file, dst_file, host_name, user_name=None,
+                rsa_key_file=None):
+    """Scp pull file from remote.
+
+    Args:
+        src_file: The source file path to be pulled.
+        dst_file: The destiation file path the file is pulled to.
+        host_name: The device host_name or ip to pull file from.
+        user_name: The user_name for scp session.
+        rsa_key_file: The rsa key file.
+    Raises:
+        errors.DeviceConnectionError if scp failed.
+    """
+    scp_cmd_list = SCP_CMD[:]
+    if rsa_key_file:
+        scp_cmd_list.extend(["-i", rsa_key_file])
+    else:
+        logger.warning(
+            "Rsa key file is not specified. "
+            "Will use default rsa key set in user environment")
+    if user_name:
+        scp_cmd_list.append("%s@%s:%s" % (user_name, host_name, src_file))
+    else:
+        scp_cmd_list.append("%s:%s" % (host_name, src_file))
+    scp_cmd_list.append(dst_file)
+    try:
+        subprocess.check_call(scp_cmd_list)
+    except subprocess.CalledProcessError as e:
+        raise errors.DeviceConnectionError(
+            "Failed to pull file %s from %s with '%s': %s" % (
+                src_file, host_name, " ".join(scp_cmd_list), e))
+
+
 def CreateSshKeyPairIfNotExist(private_key_path, public_key_path):
     """Create the ssh key pair if they don't exist.
 
diff --git a/internal/lib/utils_test.py b/internal/lib/utils_test.py
index d89cd03..9225ddd 100644
--- a/internal/lib/utils_test.py
+++ b/internal/lib/utils_test.py
@@ -29,6 +29,7 @@
 
 from acloud.internal.lib import driver_test_lib
 from acloud.internal.lib import utils
+from acloud.public import errors
 
 # Tkinter may not be supported so mock it out.
 try:
@@ -337,6 +338,43 @@
         self.assertEqual(expected_value, utils.AddUserGroupsToCmd(command,
                                                                   groups))
 
+    @staticmethod
+    def testScpPullFileSuccess():
+        """Test scp pull file successfully."""
+        subprocess.check_call = mock.MagicMock()
+        utils.ScpPullFile("/tmp/test", "/tmp/test_1.log", "192.168.0.1")
+        subprocess.check_call.assert_called_with(utils.SCP_CMD + [
+            "192.168.0.1:/tmp/test", "/tmp/test_1.log"])
+
+    @staticmethod
+    def testScpPullFileWithUserNameSuccess():
+        """Test scp pull file successfully."""
+        subprocess.check_call = mock.MagicMock()
+        utils.ScpPullFile("/tmp/test", "/tmp/test_1.log", "192.168.0.1",
+                          user_name="abc")
+        subprocess.check_call.assert_called_with(utils.SCP_CMD + [
+            "abc@192.168.0.1:/tmp/test", "/tmp/test_1.log"])
+
+    # pylint: disable=invalid-name
+    @staticmethod
+    def testScpPullFileWithUserNameWithRsaKeySuccess():
+        """Test scp pull file successfully."""
+        subprocess.check_call = mock.MagicMock()
+        utils.ScpPullFile("/tmp/test", "/tmp/test_1.log", "192.168.0.1",
+                          user_name="abc", rsa_key_file="/tmp/my_key")
+        subprocess.check_call.assert_called_with(utils.SCP_CMD + [
+            "-i", "/tmp/my_key", "abc@192.168.0.1:/tmp/test",
+            "/tmp/test_1.log"])
+
+    def testScpPullFileScpFailure(self):
+        """Test scp pull file failure."""
+        subprocess.check_call = mock.MagicMock(
+            side_effect=subprocess.CalledProcessError(123, "fake",
+                                                      "fake error"))
+        self.assertRaises(
+            errors.DeviceConnectionError,
+            utils.ScpPullFile, "/tmp/test", "/tmp/test_1.log", "192.168.0.1")
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/public/actions/common_operations.py b/public/actions/common_operations.py
index b412666..b7df5dd 100644
--- a/public/actions/common_operations.py
+++ b/public/actions/common_operations.py
@@ -23,6 +23,8 @@
 from __future__ import print_function
 import getpass
 import logging
+import os
+import subprocess
 
 from acloud.public import avd
 from acloud.public import errors
@@ -89,6 +91,36 @@
         self._device_factory = device_factory
         self._compute_client = device_factory.GetComputeClient()
 
+    def _CollectAdbLogcats(self, output_dir):
+        """Collect Adb logcats.
+
+        Args:
+            output_dir: String, the output file directory to store adb logcats.
+
+        Returns:
+            The file information dictionary with file path and file name.
+        """
+        file_dict = {}
+        for device in self._devices:
+            if not device.adb_port:
+                # If device adb tunnel is not established, do not do adb logcat
+                continue
+            file_name = "%s_adb_logcat.log" % device.instance_name
+            full_file_path = os.path.join(output_dir, file_name)
+            logger.info("Get adb %s:%s logcat for instance %s",
+                        constants.LOCALHOST, device.adb_port,
+                        device.instance_name)
+            try:
+                subprocess.check_call(
+                    ["adb -s %s:%s logcat -b all -d > %s" % (
+                        constants.LOCALHOST, device.adb_port, full_file_path)],
+                    shell=True)
+                file_dict[full_file_path] = file_name
+            except subprocess.CalledProcessError:
+                logging.error("Failed to get adb logcat for %s for instance %s",
+                              device.serial_number, device.instance_name)
+        return file_dict
+
     def CreateDevices(self, num):
         """Creates |num| devices for given build_target and build_id.
 
@@ -120,6 +152,82 @@
                 failures[device.instance_name] = e
         return failures
 
+    def PullLogs(self, source_files, output_dir, user=None, ssh_rsa_path=None):
+        """Tar logs from GCE instance into output_dir.
+
+        Args:
+            source_files: List of file names to be pulled.
+            output_dir: String. The output file dirtory
+            user: String, the ssh username to access GCE
+            ssh_rsa_path: String, the ssh rsa key path to access GCE
+
+        Returns:
+            The file dictionary with file_path and file_name
+        """
+
+        file_dict = {}
+        for device in self._devices:
+            if isinstance(source_files, basestring):
+                source_files = [source_files]
+            for source_file in source_files:
+                file_name = "%s_%s" % (device.instance_name,
+                                       os.path.basename(source_file))
+                dst_file = os.path.join(output_dir, file_name)
+                logger.info("Pull %s for instance %s with user %s to %s",
+                            source_file, device.instance_name, user, dst_file)
+                try:
+                    utils.ScpPullFile(source_file, dst_file, device.ip,
+                                      user_name=user, rsa_key_file=ssh_rsa_path)
+                    file_dict[dst_file] = file_name
+                except errors.DeviceConnectionError as e:
+                    logger.warning("Failed to pull %s from instance %s: %s",
+                                   source_file, device.instance_name, e)
+        return file_dict
+
+    def CollectSerialPortLogs(self, output_file,
+                              port=constants.DEFAULT_SERIAL_PORT):
+        """Tar the instance serial logs into specified output_file.
+
+        Args:
+            output_file: String, the output tar file path
+            port: The serial port number to be collected
+        """
+        # For emulator, the serial log is the virtual host serial log.
+        # For GCE AVD device, the serial log is the AVD device serial log.
+        with utils.TempDir() as tempdir:
+            src_dict = {}
+            for device in self._devices:
+                logger.info("Store instance %s serial port %s output to %s",
+                            device.instance_name, port, output_file)
+                serial_log = self._compute_client.GetSerialPortOutput(
+                    instance=device.instance_name, port=port)
+                file_name = "%s_serial_%s.log" % (device.instance_name, port)
+                file_path = os.path.join(tempdir, file_name)
+                src_dict[file_path] = file_name
+                with open(file_path, "w") as f:
+                    f.write(serial_log.encode("utf-8"))
+            utils.MakeTarFile(src_dict, output_file)
+
+    def CollectLogcats(self, output_file, ssh_user, ssh_rsa_path):
+        """Tar the instances' logcat and other logs into specified output_file.
+
+        Args:
+            output_file: String, the output tar file path
+            ssh_user: The ssh user name
+            ssh_rsa_path: The ssh rsa key path
+        """
+        with utils.TempDir() as tempdir:
+            file_dict = {}
+            if getattr(self._device_factory, "LOG_FILES", None):
+                file_dict = self.PullLogs(
+                    self._device_factory.LOG_FILES, tempdir, user=ssh_user,
+                    ssh_rsa_path=ssh_rsa_path)
+            # If the device is auto-connected, get adb logcat
+            for file_path, file_name in self._CollectAdbLogcats(
+                    tempdir).items():
+                file_dict[file_path] = file_name
+            utils.MakeTarFile(file_dict, output_file)
+
     @property
     def devices(self):
         """Returns a list of devices in the pool.
@@ -129,10 +237,11 @@
         """
         return self._devices
 
-
+# TODO: Delete unused-argument when b/119614469 is resolved.
+# pylint: disable=unused-argument
 # pylint: disable=too-many-locals
 def CreateDevices(command, cfg, device_factory, num, report_internal_ip=False,
-                  autoconnect=False):
+                  autoconnect=False, serial_log_file=None, logcat_file=None):
     """Create a set of devices using the given factory.
 
     Main jobs in create devices.
@@ -146,6 +255,9 @@
         num: The number of devices to create.
         report_internal_ip: Boolean to report the internal ip instead of
                             external ip.
+        serial_log_file: String, the file path to tar the serial logs.
+        logcat_file: String, the file path to tar the logcats.
+        autoconnect: Boolean, whether to auto connect to device.
 
     Raises:
         errors: Create instance fail.
@@ -163,6 +275,16 @@
             reporter.SetStatus(report.Status.BOOT_FAIL)
         else:
             reporter.SetStatus(report.Status.SUCCESS)
+
+        # Collect logs
+        if serial_log_file:
+            device_pool.CollectSerialPortLogs(
+                serial_log_file, port=constants.DEFAULT_SERIAL_PORT)
+        # TODO(b/119614469): Refactor CollectLogcats into a utils lib and
+        #                    turn it on inside the reporting loop.
+        # if logcat_file:
+        #     device_pool.CollectLogcats(logcat_file, ssh_user, ssh_rsa_path)
+
         # Write result to report.
         for device in device_pool.devices:
             ip = (device.ip.internal if report_internal_ip
diff --git a/public/actions/create_cuttlefish_action.py b/public/actions/create_cuttlefish_action.py
index e86c34b..fec4c2d 100644
--- a/public/actions/create_cuttlefish_action.py
+++ b/public/actions/create_cuttlefish_action.py
@@ -46,6 +46,9 @@
 
     RELEASE_BRANCH_SUFFIX = "-release"
     RELEASE_BRANCH_PATH_GLOB_PATTERN = "*-%s"
+    LOG_FILES = ["/home/vsoc-01/cuttlefish_runtime/kernel.log",
+                 "/home/vsoc-01/cuttlefish_runtime/logcat",
+                 "/home/vsoc-01/cuttlefish_runtime/cuttlefish_config.json"]
 
     def __init__(self, cfg, build_target, build_id, kernel_build_id=None,
                  avd_spec=None):
@@ -130,8 +133,8 @@
         build_id: String, Build id, e.g. "2263051", "P2804227"
         kernel_build_id: String, Kernel build id.
         num: Integer, Number of devices to create.
-        serial_log_file: String, A path to a file where serial output should
-                        be saved to.
+        serial_log_file: String, A path to a tar file where serial output should
+                         be saved to.
         logcat_file: String, A path to a file where logcat logs should be saved.
         autoconnect: Boolean, Create ssh tunnel(s) and adb connect after device creation.
         report_internal_ip: Boolean to report the internal ip instead of
@@ -140,9 +143,6 @@
     Returns:
         A Report instance.
     """
-    # TODO: Implement copying files from the instance, including
-    # the serial log (kernel log), and logcat log files.
-    # TODO: Implement autoconnect.
     if avd_spec:
         cfg = avd_spec.cfg
         build_target = avd_spec.remote_image[constants.BUILD_TARGET]
@@ -151,6 +151,8 @@
         num = avd_spec.num
         autoconnect = avd_spec.autoconnect
         report_internal_ip = avd_spec.report_internal_ip
+        serial_log_file = avd_spec.serial_log_file
+        logcat_file = avd_spec.logcat_file
     logger.info(
         "Creating a cuttlefish device in project %s, build_target: %s, "
         "build_id: %s, num: %s, serial_log_file: %s, logcat_file: %s, "
@@ -160,4 +162,5 @@
     device_factory = CuttlefishDeviceFactory(cfg, build_target, build_id,
                                              kernel_build_id, avd_spec)
     return common_operations.CreateDevices("create_cf", cfg, device_factory,
-                                           num, report_internal_ip, autoconnect)
+                                           num, report_internal_ip, autoconnect,
+                                           serial_log_file, logcat_file)
diff --git a/public/actions/create_goldfish_action.py b/public/actions/create_goldfish_action.py
index 8aaa249..b47f52b 100644
--- a/public/actions/create_goldfish_action.py
+++ b/public/actions/create_goldfish_action.py
@@ -52,6 +52,10 @@
         _branch: String, android branch name, e.g. git_master
         _emulator_branch: String, emulator branch name, e.g. "aosp-emu-master-dev"
     """
+    LOG_FILES = ["/home/vsoc-01/emulator.log",
+                 "/home/vsoc-01/log/logcat.log",
+                 "/home/vsoc-01/log/adb.log",
+                 "/var/log/daemon.log"]
 
     def __init__(self,
                  cfg,
@@ -228,9 +232,6 @@
     if build_id is None:
         raise errors.CommandArgError("Emulator system image build id not found "
                                      "in %s" % _SYSIMAGE_INFO_FILENAME)
-    # TODO: Implement copying files from the instance, including
-    # the serial log (kernel log), and logcat log files.
-    # TODO: Implement autoconnect.
     logger.info(
         "Creating a goldfish device in project %s, build_target: %s, "
         "build_id: %s, emulator_bid: %s, GPU: %s, num: %s, "
@@ -243,4 +244,5 @@
                                            emulator_build_id, gpu)
 
     return common_operations.CreateDevices("create_gf", cfg, device_factory,
-                                           num, report_internal_ip)
+                                           num, report_internal_ip,
+                                           serial_log_file, logcat_file)
diff --git a/public/errors.py b/public/errors.py
index eb003f1..9c13193 100755
--- a/public/errors.py
+++ b/public/errors.py
@@ -85,6 +85,10 @@
     """When there is no subnetwork for the GCE."""
 
 
+class DeviceConnectionError(DriverError):
+    """To catch device connection errors."""
+
+
 class DeviceBootTimeoutError(DeviceBootError):
     """Raised when an AVD defice failed to boot within timeout."""