Merge "Do not check all CF configs if instance names are specified"
diff --git a/delete/delete.py b/delete/delete.py
index 055fb4b..02260fd 100644
--- a/delete/delete.py
+++ b/delete/delete.py
@@ -250,8 +250,8 @@
remote_instances = list(set(instances) - set(local_instances))
if local_instances:
utils.PrintColorString("Deleting local instances")
- delete_report = DeleteInstances(cfg, list_instances.FilterInstancesByNames(
- list_instances.GetLocalInstances(), local_instances))
+ delete_report = DeleteInstances(
+ cfg, list_instances.GetLocalInstancesByNames(local_instances))
if remote_instances:
delete_report = DeleteRemoteInstances(cfg,
remote_instances,
diff --git a/delete/delete_test.py b/delete/delete_test.py
index cf17905..f8273ca 100644
--- a/delete/delete_test.py
+++ b/delete/delete_test.py
@@ -127,8 +127,7 @@
cfg = mock.Mock()
# Test delete local instances.
instances = ["local-instance-1", "local-instance-2"]
- self.Patch(list_instances, "FilterInstancesByNames", return_value="")
- self.Patch(list_instances, "GetLocalInstances", return_value=[])
+ self.Patch(list_instances, "GetLocalInstancesByNames", return_value=[])
delete.DeleteInstanceByNames(cfg, instances)
mock_delete_local_ins.assert_called()
diff --git a/internal/constants.py b/internal/constants.py
index b507734..0d412a0 100755
--- a/internal/constants.py
+++ b/internal/constants.py
@@ -165,7 +165,6 @@
INS_KEY_IS_LOCAL = "remote"
INS_KEY_ZONE = "zone"
INS_STATUS_RUNNING = "RUNNING"
-LOCAL_INS_NAME = "local-instance"
ENV_CUTTLEFISH_CONFIG_FILE = "CUTTLEFISH_CONFIG_FILE"
ENV_CUTTLEFISH_INSTANCE = "CUTTLEFISH_INSTANCE"
ENV_CVD_HOME = "HOME"
diff --git a/list/instance.py b/list/instance.py
index 2cc51ca..10063c4 100644
--- a/list/instance.py
+++ b/list/instance.py
@@ -50,6 +50,8 @@
_ACLOUD_CVD_TEMP = os.path.join(tempfile.gettempdir(), "acloud_cvd_temp")
_CVD_RUNTIME_FOLDER_NAME = "cuttlefish_runtime"
_CVD_STATUS_BIN = "cvd_status"
+_LOCAL_INSTANCE_NAME_FORMAT = "local-instance-%(id)d"
+_LOCAL_INSTANCE_NAME_PATTERN = re.compile(r"^local-instance-(?P<id>\d+)$")
_MSG_UNABLE_TO_CALCULATE = "Unable to calculate"
_NO_ANDROID_ENV = "android source not available"
_RE_GROUP_ADB = "local_adb_port"
@@ -77,8 +79,11 @@
Return:
String, path of cf runtime config.
"""
- return os.path.join(os.path.expanduser("~"), _CVD_RUNTIME_FOLDER_NAME,
- constants.CUTTLEFISH_CONFIG_FILE)
+ cfg_path = os.path.join(os.path.expanduser("~"), _CVD_RUNTIME_FOLDER_NAME,
+ constants.CUTTLEFISH_CONFIG_FILE)
+ if os.path.isfile(cfg_path):
+ return cfg_path
+ return None
def GetLocalInstanceName(local_instance_id):
@@ -90,7 +95,23 @@
Return:
String, the instance name.
"""
- return "%s-%d" % (constants.LOCAL_INS_NAME, local_instance_id)
+ return _LOCAL_INSTANCE_NAME_FORMAT % {"id": local_instance_id}
+
+
+def GetLocalInstanceIdByName(name):
+ """Get local cuttlefish instance id by name.
+
+ Args:
+ name: String of instance name.
+
+ Return:
+ The instance id as an integer if the name is in valid format.
+ None if the name does not represent a local cuttlefish instance.
+ """
+ match = _LOCAL_INSTANCE_NAME_PATTERN.match(name)
+ if match:
+ return int(match.group("id"))
+ return None
def GetLocalInstanceConfig(local_instance_id):
@@ -110,27 +131,27 @@
def GetAllLocalInstanceConfigs():
- """Get the list of instance config.
+ """Get all cuttlefish runtime configs from the known locations.
Return:
- List of instance config path.
+ List of tuples. Each tuple consists of an instance id and a config
+ path.
"""
- cfg_list = []
+ id_cfg_pairs = []
# Check if any instance config is under home folder.
cfg_path = GetDefaultCuttlefishConfig()
- if os.path.isfile(cfg_path):
- cfg_list.append(cfg_path)
+ if cfg_path:
+ id_cfg_pairs.append((1, cfg_path))
# Check if any instance config is under acloud cvd temp folder.
if os.path.exists(_ACLOUD_CVD_TEMP):
for ins_name in os.listdir(_ACLOUD_CVD_TEMP):
- cfg_path = os.path.join(_ACLOUD_CVD_TEMP,
- ins_name,
- _CVD_RUNTIME_FOLDER_NAME,
- constants.CUTTLEFISH_CONFIG_FILE)
- if os.path.isfile(cfg_path):
- cfg_list.append(cfg_path)
- return cfg_list
+ ins_id = GetLocalInstanceIdByName(ins_name)
+ if ins_id is not None:
+ cfg_path = GetLocalInstanceConfig(ins_id)
+ if os.path.isfile(cfg_path):
+ id_cfg_pairs.append((ins_id, cfg_path))
+ return id_cfg_pairs
def GetLocalInstanceHomeDir(local_instance_id):
diff --git a/list/instance_test.py b/list/instance_test.py
index f51cb57..f5d2575 100644
--- a/list/instance_test.py
+++ b/list/instance_test.py
@@ -18,7 +18,6 @@
import collections
import datetime
import subprocess
-
import unittest
from six import b
@@ -77,12 +76,12 @@
return_value=cf_config)
local_instance = instance.LocalInstance(cf_config)
- self.assertEqual(constants.LOCAL_INS_NAME + "-2", local_instance.name)
+ self.assertEqual("local-instance-2", local_instance.name)
self.assertEqual(True, local_instance.islocal)
self.assertEqual("1080x1920 (480)", local_instance.display)
expected_full_name = ("device serial: 127.0.0.1:%s (%s) elapsed time: %s"
% ("6521",
- constants.LOCAL_INS_NAME + "-2",
+ "local-instance-2",
"None"))
self.assertEqual(expected_full_name, local_instance.fullname)
self.assertEqual(6521, local_instance.adb_port)
diff --git a/list/list.py b/list/list.py
index c60bcb6..42487f9 100644
--- a/list/list.py
+++ b/list/list.py
@@ -127,28 +127,44 @@
return _SortInstancesForDisplay(_ProcessInstances(all_instances))
-def _GetLocalCuttlefishInstances():
+def _GetLocalCuttlefishInstances(id_cfg_pairs):
"""Look for local cuttelfish instances.
Gather local instances information from cuttlefish runtime config.
+ Args:
+ id_cfg_pairs: List of tuples. Each tuple consists of an instance id and
+ a config path.
+
Returns:
instance_list: List of local instances.
"""
local_instance_list = []
- for cf_runtime_config_path in instance.GetAllLocalInstanceConfigs():
- ins = instance.LocalInstance(cf_runtime_config_path)
- if ins.CvdStatus():
- local_instance_list.append(ins)
- else:
- logger.info("cvd runtime config found but instance is not active:%s"
- , cf_runtime_config_path)
+ for ins_id, cfg_path in id_cfg_pairs:
+ ins_lock = instance.GetLocalInstanceLock(ins_id)
+ if not ins_lock.Lock():
+ logger.warning("Cuttlefish Instance %d is locked by another "
+ "process.", ins_id)
+ continue
+ try:
+ if not os.path.isfile(cfg_path):
+ continue
+ ins = instance.LocalInstance(cfg_path)
+ if ins.CvdStatus():
+ local_instance_list.append(ins)
+ else:
+ logger.info("Cvd runtime config is found at %s but instance "
+ "%d is not active.", cfg_path, ins_id)
+ finally:
+ ins_lock.Unlock()
return local_instance_list
def GetActiveCVD(local_instance_id):
"""Check if the local AVD with specific instance id is running
+ This function does not lock the instance.
+
Args:
local_instance_id: Integer of instance id.
@@ -161,7 +177,7 @@
if ins.CvdStatus():
return ins
cfg_path = instance.GetDefaultCuttlefishConfig()
- if local_instance_id == 1 and os.path.isfile(cfg_path):
+ if local_instance_id == 1 and cfg_path:
ins = instance.LocalInstance(cfg_path)
if ins.CvdStatus():
return ins
@@ -178,7 +194,8 @@
if not utils.IsSupportedPlatform():
return []
- return (_GetLocalCuttlefishInstances() +
+ id_cfg_pairs = instance.GetAllLocalInstanceConfigs()
+ return (_GetLocalCuttlefishInstances(id_cfg_pairs) +
instance.LocalGoldfishInstance.GetExistingInstances())
@@ -258,7 +275,7 @@
return instances_list[0]
-def FilterInstancesByNames(instances, names):
+def _FilterInstancesByNames(instances, names):
"""Find instances by names.
Args:
@@ -286,6 +303,37 @@
return found_instances
+def GetLocalInstancesByNames(names):
+ """Get local cuttlefish and goldfish instances by names.
+
+ Args:
+ names: Collection of instance names.
+
+ Returns:
+ List consisting of LocalInstance and LocalGoldfishInstance objects.
+
+ Raises:
+ errors.NoInstancesFound: No instances found.
+ """
+ id_cfg_pairs = []
+ for name in names:
+ ins_id = instance.GetLocalInstanceIdByName(name)
+ if ins_id is None:
+ continue
+ cfg_path = instance.GetLocalInstanceConfig(ins_id)
+ if cfg_path:
+ id_cfg_pairs.append((ins_id, cfg_path))
+ if ins_id == 1:
+ cfg_path = instance.GetDefaultCuttlefishConfig()
+ if cfg_path:
+ id_cfg_pairs.append((ins_id, cfg_path))
+
+ return _FilterInstancesByNames(
+ _GetLocalCuttlefishInstances(id_cfg_pairs) +
+ instance.LocalGoldfishInstance.GetExistingInstances(),
+ names)
+
+
def GetInstancesFromInstanceNames(cfg, instance_names):
"""Get instances from instance names.
@@ -301,7 +349,9 @@
Raises:
errors.NoInstancesFound: No instances found.
"""
- return FilterInstancesByNames(GetInstances(cfg), instance_names)
+ return _FilterInstancesByNames(
+ GetLocalInstancesByNames(instance_names) + GetRemoteInstances(cfg),
+ instance_names)
def FilterInstancesByAdbPort(instances, adb_port):
diff --git a/list/list_test.py b/list/list_test.py
index a4b466c..c6d04bd 100644
--- a/list/list_test.py
+++ b/list/list_test.py
@@ -25,7 +25,7 @@
from acloud.list import instance
-class InstanceObject(object):
+class InstanceObject:
"""Mock to store data of instance."""
def __init__(self, name):
@@ -44,8 +44,10 @@
alive_instance2 = InstanceObject("alive_instance2")
alive_local_instance = InstanceObject("alive_local_instance")
- instance_alive = [alive_instance1, alive_instance2, alive_local_instance]
- self.Patch(list_instance, "GetInstances", return_value=instance_alive)
+ self.Patch(list_instance, "GetLocalInstancesByNames",
+ return_value=[alive_local_instance])
+ self.Patch(list_instance, "GetRemoteInstances",
+ return_value=[alive_instance1, alive_instance2])
instances_list = list_instance.GetInstancesFromInstanceNames(cfg, instance_names)
instances_name_in_list = [instance_object.name for instance_object in instances_list]
self.assertEqual(instances_name_in_list.sort(), instance_names.sort())
@@ -58,7 +60,7 @@
# test get instance from instance name error with invalid input.
instance_names = ["miss2_local_instance", "alive_instance1"]
miss_instance_names = ["miss2_local_instance"]
- self.assertRaisesRegexp(
+ self.assertRaisesRegex(
errors.NoInstancesFound,
"Did not find the following instances: %s" % ' '.join(miss_instance_names),
list_instance.GetInstancesFromInstanceNames,
@@ -88,6 +90,31 @@
expected_instance = "cf_instance2"
self.assertEqual(list_instance.ChooseOneRemoteInstance(cfg), expected_instance)
+ def testGetLocalInstancesByNames(self):
+ """test GetLocalInstancesByNames."""
+ self.Patch(
+ instance, "GetLocalInstanceIdByName",
+ side_effect=lambda name: 1 if name == "local-instance-1" else None)
+ self.Patch(instance, "GetLocalInstanceConfig",
+ return_value="path1")
+ self.Patch(instance, "GetDefaultCuttlefishConfig",
+ return_value="path2")
+ mock_ins = mock.Mock()
+ mock_ins.name = "local-instance-1"
+ mock_get_cf = self.Patch(list_instance,
+ "_GetLocalCuttlefishInstances",
+ return_value=[mock_ins])
+ self.Patch(instance.LocalGoldfishInstance, "GetExistingInstances",
+ return_value=[])
+
+ ins_list = list_instance.GetLocalInstancesByNames(["local-instance-1"])
+ self.assertEqual(1, len(ins_list))
+ mock_get_cf.assert_called_with([(1, "path1"), (1, "path2")])
+
+ with self.assertRaises(errors.NoInstancesFound):
+ ins_list = list_instance.GetLocalInstancesByNames(
+ ["local-instance-1", "local-instance-6"])
+
# pylint: disable=attribute-defined-outside-init
def testFilterInstancesByAdbPort(self):
"""test FilterInstancesByAdbPort."""
@@ -107,22 +134,42 @@
def testGetLocalCuttlefishInstances(self):
"""test _GetLocalCuttlefishInstances."""
# Test getting two instance case
- self.Patch(instance, "GetAllLocalInstanceConfigs",
- return_value=["fake_path1", "fake_path2"])
- self.Patch(instance, "GetLocalInstanceRuntimeDir")
+ id_cfg_pairs = [(1, "fake_path1"), (2, "fake_path2")]
+ mock_isfile = self.Patch(list_instance.os.path, "isfile",
+ return_value=True)
+
+ mock_lock = mock.Mock()
+ mock_lock.Lock.return_value = True
+ self.Patch(instance, "GetLocalInstanceLock", return_value=mock_lock)
local_ins = mock.MagicMock()
local_ins.CvdStatus.return_value = True
self.Patch(instance, "LocalInstance", return_value=local_ins)
- ins_list = list_instance._GetLocalCuttlefishInstances()
+ ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
self.assertEqual(2, len(ins_list))
+ mock_isfile.assert_called()
+ local_ins.CvdStatus.assert_called()
+ self.assertEqual(2, mock_lock.Lock.call_count)
+ self.assertEqual(2, mock_lock.Unlock.call_count)
- local_ins = mock.MagicMock()
- local_ins.CvdStatus.return_value = False
- self.Patch(instance, "LocalInstance", return_value=local_ins)
- ins_list = list_instance._GetLocalCuttlefishInstances()
+ local_ins.CvdStatus.reset_mock()
+ mock_lock.Lock.reset_mock()
+ mock_lock.Lock.return_value = False
+ mock_lock.Unlock.reset_mock()
+ ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
self.assertEqual(0, len(ins_list))
+ local_ins.CvdStatus.assert_not_called()
+ self.assertEqual(2, mock_lock.Lock.call_count)
+ mock_lock.Unlock.assert_not_called()
+
+ mock_lock.Lock.reset_mock()
+ mock_lock.Lock.return_value = True
+ local_ins.CvdStatus.return_value = False
+ ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
+ self.assertEqual(0, len(ins_list))
+ self.assertEqual(2, mock_lock.Lock.call_count)
+ self.assertEqual(2, mock_lock.Unlock.call_count)
# pylint: disable=no-member
def testPrintInstancesDetails(self):