Support instance directory outside of acloud temporary directory
This commit adds --local-instance-dir that links the instance directory
to the specified directory. When acloud deletes and restarts the
instance, it preserves the directory. The infrastructure can safely
collect the log files in it.
Test: acloud-dev create --local-instance --local-image \
--local-instance-dir ./temp
Bug: 160942364
Change-Id: I4097cdd4ff924df539ed2c1073663a1240472a57
diff --git a/create/avd_spec.py b/create/avd_spec.py
index b6a42ea..189fcd2 100644
--- a/create/avd_spec.py
+++ b/create/avd_spec.py
@@ -112,6 +112,7 @@
self._instance_type = None
self._local_image_dir = None
self._local_image_artifact = None
+ self._local_instance_dir = None
self._local_system_image_dir = None
self._local_tool_dirs = None
self._image_download_dir = None
@@ -301,6 +302,7 @@
self._host_user = args.host_user
self._host_ssh_private_key_path = args.host_ssh_private_key_path
self._local_instance_id = args.local_instance
+ self._local_instance_dir = args.local_instance_dir
self._local_tool_dirs = args.local_tool
self._num_of_instances = args.num
self._num_avds_per_instance = args.num_avds_per_instance
@@ -707,6 +709,11 @@
return self._local_image_artifact
@property
+ def local_instance_dir(self):
+ """Return local instance directory."""
+ return self._local_instance_dir
+
+ @property
def local_system_image_dir(self):
"""Return local system image dir."""
return self._local_system_image_dir
diff --git a/create/create_args.py b/create/create_args.py
index 4258334..f54c1a2 100644
--- a/create/create_args.py
+++ b/create/create_args.py
@@ -216,6 +216,11 @@
"local gpu support.")
# Hide following args for users, it is only used in infra.
parser.add_argument(
+ "--local-instance-dir",
+ dest="local_instance_dir",
+ required=False,
+ help=argparse.SUPPRESS)
+ parser.add_argument(
"--num-avds-per-instance",
type=int,
dest="num_avds_per_instance",
@@ -510,6 +515,10 @@
raise errors.CheckPathError(
"Specified path doesn't exist: %s" % args.local_image)
+ if args.local_instance_dir and not os.path.exists(args.local_instance_dir):
+ raise errors.CheckPathError(
+ "Specified path doesn't exist: %s" % args.local_instance_dir)
+
# TODO(b/133211308): Support TYPE_CF.
if args.local_system_image != "" and args.avd_type != constants.TYPE_GF:
raise errors.UnsupportedCreateArgs("%s instance does not support "
diff --git a/create/create_common.py b/create/create_common.py
index 2846e31..f0d6fd8 100644
--- a/create/create_common.py
+++ b/create/create_common.py
@@ -17,6 +17,7 @@
import logging
import os
+import shutil
from acloud import errors
from acloud.internal import constants
@@ -118,3 +119,28 @@
logger.debug("Deleted temporary file %s", temp_file)
except OSError as e:
logger.error("Failed to delete temporary file: %s", str(e))
+
+
+def PrepareLocalInstanceDir(instance_dir, avd_spec):
+ """Create a directory for a local cuttlefish or goldfish instance.
+
+ If avd_spec has the local instance directory, this method creates a
+ symbolic link from instance_dir to the directory. Otherwise, it creates an
+ empty directory at instance_dir.
+
+ Args:
+ instance_dir: The absolute path to the default instance directory.
+ avd_spec: AVDSpec object that provides the instance directory.
+ """
+ if os.path.islink(instance_dir):
+ os.remove(instance_dir)
+ else:
+ shutil.rmtree(instance_dir, ignore_errors=True)
+
+ if avd_spec.local_instance_dir:
+ abs_instance_dir = os.path.abspath(avd_spec.local_instance_dir)
+ if instance_dir != abs_instance_dir:
+ os.symlink(abs_instance_dir, instance_dir)
+ return
+ if not os.path.exists(instance_dir):
+ os.makedirs(instance_dir)
diff --git a/create/create_common_test.py b/create/create_common_test.py
index adfd0c2..bcd8209 100644
--- a/create/create_common_test.py
+++ b/create/create_common_test.py
@@ -14,6 +14,8 @@
"""Tests for create_common."""
import os
+import shutil
+import tempfile
import unittest
import mock
@@ -26,7 +28,7 @@
from acloud.internal.lib import utils
-class FakeZipFile(object):
+class FakeZipFile:
"""Fake implementation of ZipFile()"""
# pylint: disable=invalid-name,unused-argument,no-self-use
@@ -146,6 +148,25 @@
"%s/%s" % (extract_path, checkfile2))
self.assertEqual(mock_decompress.call_count, 0)
+ def testPrepareLocalInstanceDir(self):
+ """test PrepareLocalInstanceDir."""
+ temp_dir = tempfile.mkdtemp()
+ try:
+ cvd_home_dir = os.path.join(temp_dir, "local-instance-1")
+ mock_avd_spec = mock.Mock(local_instance_dir=None)
+ create_common.PrepareLocalInstanceDir(cvd_home_dir, mock_avd_spec)
+ self.assertTrue(os.path.isdir(cvd_home_dir) and
+ not os.path.islink(cvd_home_dir))
+
+ link_target_dir = os.path.join(temp_dir, "cvd_home")
+ os.mkdir(link_target_dir)
+ mock_avd_spec.local_instance_dir = link_target_dir
+ create_common.PrepareLocalInstanceDir(cvd_home_dir, mock_avd_spec)
+ self.assertTrue(os.path.islink(cvd_home_dir) and
+ os.path.samefile(cvd_home_dir, link_target_dir))
+ finally:
+ shutil.rmtree(temp_dir)
+
if __name__ == "__main__":
unittest.main()
diff --git a/create/goldfish_local_image_local_instance.py b/create/goldfish_local_image_local_instance.py
index c066b3c..3cb3af7 100644
--- a/create/goldfish_local_image_local_instance.py
+++ b/create/goldfish_local_image_local_instance.py
@@ -45,6 +45,7 @@
from acloud import errors
from acloud.create import base_avd_create
+from acloud.create import create_common
from acloud.internal import constants
from acloud.internal.lib import ota_tools
from acloud.internal.lib import utils
@@ -254,8 +255,7 @@
self._CopyBuildProp(image_dir)
instance_dir = ins.instance_dir
- shutil.rmtree(instance_dir, ignore_errors=True)
- os.makedirs(instance_dir)
+ create_common.PrepareLocalInstanceDir(instance_dir, avd_spec)
extra_args = self._ConvertAvdSpecToArgs(avd_spec, instance_dir)
@@ -455,6 +455,8 @@
if avd_spec.local_system_image_dir:
mixed_image_dir = os.path.join(instance_dir, "mixed_images")
+ if os.path.exists(mixed_image_dir):
+ shutil.rmtree(mixed_image_dir)
os.mkdir(mixed_image_dir)
image_dir = os.path.abspath(avd_spec.local_image_dir)
diff --git a/create/goldfish_local_image_local_instance_test.py b/create/goldfish_local_image_local_instance_test.py
index 54b5944..2248d2d 100644
--- a/create/goldfish_local_image_local_instance_test.py
+++ b/create/goldfish_local_image_local_instance_test.py
@@ -141,6 +141,7 @@
gpu=None,
autoconnect=True,
local_instance_id=1,
+ local_instance_dir=None,
local_image_dir=self._image_dir,
local_system_image_dir=None,
local_tool_dirs=[])
@@ -183,11 +184,15 @@
self._CreateEmptyFile(os.path.join(self._image_dir, "system.img"))
self._CreateEmptyFile(os.path.join(self._image_dir, "build.prop"))
+ instance_dir = os.path.join(self._temp_dir, "local_instance_dir")
+ os.mkdir(instance_dir)
+
mock_avd_spec = mock.Mock(flavor="phone",
boot_timeout_secs=None,
gpu=None,
autoconnect=True,
local_instance_id=2,
+ local_instance_dir=instance_dir,
local_image_dir=self._image_dir,
local_system_image_dir=None,
local_tool_dirs=[self._tool_dir])
@@ -206,7 +211,8 @@
mock_instance.assert_called_once_with(2, avd_flavor="phone")
- self.assertTrue(os.path.isdir(self._instance_dir))
+ self.assertTrue(os.path.isdir(self._instance_dir) and
+ os.path.islink(self._instance_dir))
mock_popen.assert_called_once()
self.assertEqual(mock_popen.call_args[0][0],
@@ -236,6 +242,7 @@
gpu=None,
autoconnect=True,
local_instance_id=2,
+ local_instance_dir=None,
local_image_dir=self._image_dir,
local_system_image_dir=None,
local_tool_dirs=[self._tool_dir])
@@ -269,6 +276,7 @@
gpu=None,
autoconnect=True,
local_instance_id=0,
+ local_instance_dir=None,
local_image_dir=self._image_dir,
local_system_image_dir=None,
local_tool_dirs=[self._tool_dir])
@@ -318,6 +326,7 @@
gpu="auto",
autoconnect=False,
local_instance_id=3,
+ local_instance_dir=None,
local_image_dir=self._image_dir,
local_system_image_dir="/unit/test",
local_tool_dirs=[])
diff --git a/create/local_image_local_instance.py b/create/local_image_local_instance.py
index c5b6d33..196b456 100644
--- a/create/local_image_local_instance.py
+++ b/create/local_image_local_instance.py
@@ -22,7 +22,8 @@
- HOME: To specify the temporary folder of launch_cvd.
- CUTTLEFISH_INSTANCE: To specify the instance id.
Acloud user must either set ANDROID_HOST_OUT or run acloud with --local-tool.
-Acloud sets the other 2 variables for each local instance.
+The user can optionally specify the folder by --local-instance-dir and the
+instance id by --local-instance.
The adb port and vnc port of local instance will be decided according to
instance id. The rule of adb port will be '6520 + [instance id] - 1' and the vnc
@@ -36,13 +37,13 @@
import logging
import os
-import shutil
import subprocess
import threading
import sys
from acloud import errors
from acloud.create import base_avd_create
+from acloud.create import create_common
from acloud.internal import constants
from acloud.internal.lib import utils
from acloud.internal.lib.adb_tools import AdbTools
@@ -160,13 +161,17 @@
if avd_spec.connect_webrtc:
utils.ReleasePort(constants.WEBRTC_LOCAL_PORT)
+ cvd_home_dir = instance.GetLocalInstanceHomeDir(local_instance_id)
+ create_common.PrepareLocalInstanceDir(cvd_home_dir, avd_spec)
+ runtime_dir = instance.GetLocalInstanceRuntimeDir(local_instance_id)
+
launch_cvd_path = os.path.join(host_bins_path, "bin",
constants.CMD_LAUNCH_CVD)
cmd = self.PrepareLaunchCVDCmd(launch_cvd_path,
avd_spec.hw_property,
avd_spec.connect_adb,
local_image_path,
- local_instance_id,
+ runtime_dir,
avd_spec.connect_webrtc,
avd_spec.gpu)
@@ -174,13 +179,15 @@
instance_name = instance.GetLocalInstanceName(local_instance_id)
try:
self._LaunchCvd(cmd, local_instance_id, host_bins_path,
- (avd_spec.boot_timeout_secs or
- constants.DEFAULT_CF_BOOT_TIMEOUT))
+ cvd_home_dir, (avd_spec.boot_timeout_secs or
+ constants.DEFAULT_CF_BOOT_TIMEOUT))
except errors.LaunchCVDFail as launch_error:
+ err_msg = ("Cannot create cuttlefish instance: %s\n"
+ "For more detail: %s/launcher.log" %
+ (launch_error, runtime_dir))
result_report.SetStatus(report.Status.BOOT_FAIL)
result_report.AddDeviceBootFailure(
- instance_name, constants.LOCALHOST, None, None,
- error=str(launch_error))
+ instance_name, constants.LOCALHOST, None, None, error=err_msg)
return result_report
active_ins = list_instance.GetActiveCVD(local_instance_id)
@@ -240,7 +247,7 @@
@staticmethod
def PrepareLaunchCVDCmd(launch_cvd_path, hw_property, connect_adb,
- system_image_dir, local_instance_id, connect_webrtc,
+ system_image_dir, runtime_dir, connect_webrtc,
gpu):
"""Prepare launch_cvd command.
@@ -252,7 +259,7 @@
hw_property: dict object of hw property.
system_image_dir: String of local images path.
connect_adb: Boolean flag that enables adb_connector.
- local_instance_id: Integer of instance id.
+ runtime_dir: String of runtime directory path.
connect_webrtc: Boolean of connect_webrtc.
gpu: String of gpu name, the gpu name of local instance should be
"default" if gpu is enabled.
@@ -260,12 +267,11 @@
Returns:
String, launch_cvd cmd.
"""
- instance_dir = instance.GetLocalInstanceRuntimeDir(local_instance_id)
launch_cvd_w_args = launch_cvd_path + _CMD_LAUNCH_CVD_ARGS % (
hw_property["cpu"], hw_property["x_res"], hw_property["y_res"],
hw_property["dpi"], hw_property["memory"],
("true" if connect_adb else "false"), system_image_dir,
- instance_dir)
+ runtime_dir)
if constants.HW_ALIAS_DISK in hw_property:
launch_cvd_w_args = (launch_cvd_w_args + _CMD_LAUNCH_CVD_DISK_ARGS %
hw_property[constants.HW_ALIAS_DISK])
@@ -303,7 +309,8 @@
@staticmethod
@utils.TimeExecute(function_description="Waiting for AVD(s) to boot up")
- def _LaunchCvd(cmd, local_instance_id, host_bins_path, timeout=None):
+ def _LaunchCvd(cmd, local_instance_id, host_bins_path, cvd_home_dir,
+ timeout=None):
"""Execute Launch CVD.
Kick off the launch_cvd command and log the output.
@@ -312,18 +319,12 @@
cmd: String, launch_cvd command.
local_instance_id: Integer of instance id.
host_bins_path: String of host package directory.
+ cvd_home_dir: String, the home directory for the instance.
timeout: Integer, the number of seconds to wait for the AVD to boot up.
Raises:
- errors.LaunchCVDFail when any CalledProcessError.
+ errors.LaunchCVDFail if launch_cvd times out or returns non-zero.
"""
- # Delete the cvd home/runtime temp if exist. The runtime folder is
- # under the cvd home dir, so we only delete them from home dir.
- cvd_home_dir = instance.GetLocalInstanceHomeDir(local_instance_id)
- cvd_runtime_dir = instance.GetLocalInstanceRuntimeDir(local_instance_id)
- shutil.rmtree(cvd_home_dir, ignore_errors=True)
- os.makedirs(cvd_runtime_dir)
-
cvd_env = os.environ.copy()
# launch_cvd assumes host bins are in $ANDROID_HOST_OUT.
cvd_env[constants.ENV_ANDROID_HOST_OUT] = host_bins_path
@@ -341,6 +342,5 @@
timer.cancel()
if process.returncode == 0:
return
- raise errors.LaunchCVDFail(
- "Can't launch cuttlefish AVD. Return code:%s. \nFor more detail: "
- "%s/launcher.log" % (str(process.returncode), cvd_runtime_dir))
+ raise errors.LaunchCVDFail("launch_cvd returned %s" %
+ process.returncode)
diff --git a/create/local_image_local_instance_test.py b/create/local_image_local_instance_test.py
index e9107b7..ad48893 100644
--- a/create/local_image_local_instance_test.py
+++ b/create/local_image_local_instance_test.py
@@ -15,8 +15,6 @@
# limitations under the License.
"""Tests for LocalImageLocalInstance."""
-import os
-import shutil
import subprocess
import unittest
import mock
@@ -121,11 +119,16 @@
mock_lock.Unlock.assert_called_once()
@mock.patch("acloud.create.local_image_local_instance.utils")
+ @mock.patch("acloud.create.local_image_local_instance.create_common")
@mock.patch.object(local_image_local_instance.LocalImageLocalInstance,
"_LaunchCvd")
@mock.patch.object(local_image_local_instance.LocalImageLocalInstance,
"PrepareLaunchCVDCmd")
- def testCreateInstance(self, _mock_prepare, mock_launch_cvd, _mock_utils):
+ @mock.patch.object(instance, "GetLocalInstanceRuntimeDir")
+ @mock.patch.object(instance, "GetLocalInstanceHomeDir")
+ def testCreateInstance(self, _mock_home_dir, _mock_runtime_dir,
+ _mock_prepare_cmd, mock_launch_cvd,
+ _mock_create_common, _mock_utils):
"""Test the report returned by _CreateInstance."""
self.Patch(instance, "GetLocalInstanceName",
return_value="local-instance-1")
@@ -148,14 +151,14 @@
self._EXPECTED_DEVICES_IN_REPORT)
# Failure
- mock_launch_cvd.side_effect = errors.LaunchCVDFail("timeout")
+ mock_launch_cvd.side_effect = errors.LaunchCVDFail("unit test")
report = self.local_image_local_instance._CreateInstance(
1, "/image/path", "/host/bin/path", mock_avd_spec, no_prompts=True)
self.assertEqual(report.data.get("devices_failing_boot"),
self._EXPECTED_DEVICES_IN_FAILED_REPORT)
- self.assertEqual(report.errors, ["timeout"])
+ self.assertIn("unit test", report.errors[0])
# pylint: disable=protected-access
@mock.patch("acloud.create.local_image_local_instance.os.path.isfile")
@@ -184,13 +187,10 @@
[cvd_host_dir])
self.assertEqual(path, cvd_host_dir)
- # pylint: disable=protected-access
- @mock.patch.object(instance, "GetLocalInstanceRuntimeDir")
@mock.patch.object(utils, "CheckUserInGroups")
- def testPrepareLaunchCVDCmd(self, mock_usergroups, mock_cvd_dir):
+ def testPrepareLaunchCVDCmd(self, mock_usergroups):
"""test PrepareLaunchCVDCmd."""
mock_usergroups.return_value = False
- mock_cvd_dir.return_value = "fake_cvd_dir"
hw_property = {"cpu": "fake", "x_res": "fake", "y_res": "fake",
"dpi":"fake", "memory": "fake", "disk": "fake"}
constants.LIST_CF_USER_GROUPS = ["group1", "group2"]
@@ -245,22 +245,20 @@
local_instance_id = 3
launch_cvd_cmd = "launch_cvd"
host_bins_path = "host_bins_path"
+ cvd_home_dir = "fake_home"
cvd_env = {}
- cvd_env[constants.ENV_CVD_HOME] = "fake_home"
+ cvd_env[constants.ENV_CVD_HOME] = cvd_home_dir
cvd_env[constants.ENV_CUTTLEFISH_INSTANCE] = str(local_instance_id)
cvd_env[constants.ENV_ANDROID_HOST_OUT] = host_bins_path
process = mock.MagicMock()
process.wait.return_value = True
process.returncode = 0
self.Patch(subprocess, "Popen", return_value=process)
- self.Patch(instance, "GetLocalInstanceHomeDir",
- return_value="fake_home")
- self.Patch(os, "makedirs")
- self.Patch(shutil, "rmtree")
self.local_image_local_instance._LaunchCvd(launch_cvd_cmd,
local_instance_id,
- host_bins_path)
+ host_bins_path,
+ cvd_home_dir)
# pylint: disable=no-member
subprocess.Popen.assert_called_once_with(launch_cvd_cmd,
shell=True,
diff --git a/list/instance.py b/list/instance.py
index 10063c4..1733cf1 100644
--- a/list/instance.py
+++ b/list/instance.py
@@ -149,7 +149,7 @@
ins_id = GetLocalInstanceIdByName(ins_name)
if ins_id is not None:
cfg_path = GetLocalInstanceConfig(ins_id)
- if os.path.isfile(cfg_path):
+ if cfg_path:
id_cfg_pairs.append((ins_id, cfg_path))
return id_cfg_pairs
