Reset lock states of inactive local instances
Acloud keeps the allocation states of local instances in the lock files.
If the instances are terminated due to invalid arguments, bugs, or kill
commands, acloud cannot list the instances. The user can reset the
states of such instances by using --instance-names.
Test: echo -n I > /tmp/acloud_cvd_temp/local-instance-5.lock
acloud-dev delete --instance-names local-instance-5
Bug: 171371739
Change-Id: I9ae9705dbb7f52736c14358d8153dc91eed1ec26
diff --git a/delete/delete.py b/delete/delete.py
index 02260fd..3a7faff 100644
--- a/delete/delete.py
+++ b/delete/delete.py
@@ -51,12 +51,8 @@
instances_to_delete: List of list.Instance() object.
Returns:
- Report instance if there are instances to delete, None otherwise.
+ Report object.
"""
- if not instances_to_delete:
- print("No instances to delete")
- return None
-
delete_report = report.Report(command="delete")
remote_instance_list = []
for instance in instances_to_delete:
@@ -143,6 +139,7 @@
return delete_report
try:
+ ins_lock.SetInUse(False)
instance.Delete()
delete_report.SetStatus(report.Status.SUCCESS)
device_driver.AddDeletionResultToReport(
@@ -153,7 +150,6 @@
delete_report.AddError(str(e))
delete_report.SetStatus(report.Status.FAIL)
finally:
- ins_lock.SetInUse(False)
ins_lock.Unlock()
return delete_report
@@ -179,6 +175,7 @@
return delete_report
try:
+ lock.SetInUse(False)
if instance.adb.EmuCommand("kill") == 0:
delete_report.SetStatus(report.Status.SUCCESS)
device_driver.AddDeletionResultToReport(
@@ -189,12 +186,39 @@
delete_report.AddError("Cannot kill %s." % instance.device_serial)
delete_report.SetStatus(report.Status.FAIL)
finally:
- lock.SetInUse(False)
lock.Unlock()
return delete_report
+def ResetLocalInstanceLockByName(name, delete_report):
+ """Set the lock state of a local instance to be not in use.
+
+ Args:
+ name: The instance name.
+ delete_report: Report object.
+ """
+ ins_lock = list_instances.GetLocalInstanceLockByName(name)
+ if not ins_lock:
+ delete_report.AddError("%s is not a valid local instance name." % name)
+ delete_report.SetStatus(report.Status.FAIL)
+ return
+
+ if not ins_lock.Lock():
+ delete_report.AddError("%s is locked by another process." % name)
+ delete_report.SetStatus(report.Status.FAIL)
+ return
+
+ try:
+ ins_lock.SetInUse(False)
+ delete_report.SetStatus(report.Status.SUCCESS)
+ device_driver.AddDeletionResultToReport(
+ delete_report, [name], failed=[], error_msgs=[],
+ resource_name="instance")
+ finally:
+ ins_lock.Unlock()
+
+
def CleanUpRemoteHost(cfg, remote_host, host_user,
host_ssh_private_key_path=None):
"""Clean up the remote host.
@@ -244,18 +268,22 @@
A Report instance.
"""
delete_report = report.Report(command="delete")
- local_instances = [
- ins for ins in instances if ins.startswith(_LOCAL_INSTANCE_PREFIX)
- ]
- remote_instances = list(set(instances) - set(local_instances))
- if local_instances:
- utils.PrintColorString("Deleting local instances")
- delete_report = DeleteInstances(
- cfg, list_instances.GetLocalInstancesByNames(local_instances))
- if remote_instances:
- delete_report = DeleteRemoteInstances(cfg,
- remote_instances,
- delete_report)
+ local_names = set(name for name in instances if
+ name.startswith(_LOCAL_INSTANCE_PREFIX))
+ remote_names = list(set(instances) - set(local_names))
+ if local_names:
+ active_instances = list_instances.GetLocalInstancesByNames(local_names)
+ inactive_names = local_names.difference(ins.name for ins in
+ active_instances)
+ if active_instances:
+ utils.PrintColorString("Deleting local instances")
+ delete_report = DeleteInstances(cfg, active_instances)
+ if inactive_names:
+ utils.PrintColorString("Unlocking local instances")
+ for name in inactive_names:
+ ResetLocalInstanceLockByName(name, delete_report)
+ if remote_names:
+ delete_report = DeleteRemoteInstances(cfg, remote_names, delete_report)
return delete_report
@@ -293,4 +321,6 @@
# user didn't specify instances in args.
instances = list_instances.ChooseInstancesFromList(instances)
+ if not instances:
+ utils.PrintColorString("No instances to delete")
return DeleteInstances(cfg, instances)
diff --git a/delete/delete_test.py b/delete/delete_test.py
index f8273ca..3d5e8ff 100644
--- a/delete/delete_test.py
+++ b/delete/delete_test.py
@@ -119,17 +119,53 @@
mock_lock.SetInUse.assert_called_once_with(False)
mock_lock.Unlock.assert_called_once()
+ def testResetLocalInstanceLockByName(self):
+ """test ResetLocalInstanceLockByName."""
+ mock_lock = mock.Mock()
+ mock_lock.Lock.return_value = True
+ self.Patch(list_instances, "GetLocalInstanceLockByName",
+ return_value=mock_lock)
+ delete_report = report.Report(command="delete")
+ delete.ResetLocalInstanceLockByName("unittest", delete_report)
+
+ self.assertEqual(delete_report.data, {
+ "deleted": [
+ {
+ "type": "instance",
+ "name": "unittest",
+ },
+ ],
+ })
+ mock_lock.Lock.assert_called_once()
+ mock_lock.SetInUse.assert_called_once_with(False)
+ mock_lock.Unlock.assert_called_once()
+
+ def testResetLocalInstanceLockByNameFailure(self):
+ """test ResetLocalInstanceLockByName with an invalid name."""
+ self.Patch(list_instances, "GetLocalInstanceLockByName",
+ return_value=None)
+ delete_report = report.Report(command="delete")
+ delete.ResetLocalInstanceLockByName("unittest", delete_report)
+
+ self.assertTrue(len(delete_report.errors) > 0)
+ self.assertEqual(delete_report.status, "FAIL")
+
@mock.patch.object(delete, "DeleteInstances", return_value="")
+ @mock.patch.object(delete, "ResetLocalInstanceLockByName")
@mock.patch.object(delete, "DeleteRemoteInstances", return_value="")
def testDeleteInstanceByNames(self, mock_delete_remote_ins,
- mock_delete_local_ins):
+ mock_reset_lock, mock_delete_local_ins):
"""test DeleteInstanceByNames."""
cfg = mock.Mock()
# Test delete local instances.
instances = ["local-instance-1", "local-instance-2"]
- self.Patch(list_instances, "GetLocalInstancesByNames", return_value=[])
+ mock_local_ins = mock.Mock()
+ mock_local_ins.name = "local-instance-1"
+ self.Patch(list_instances, "GetLocalInstancesByNames",
+ return_value=[mock_local_ins])
delete.DeleteInstanceByNames(cfg, instances)
- mock_delete_local_ins.assert_called()
+ mock_delete_local_ins.assert_called_with(cfg, [mock_local_ins])
+ mock_reset_lock.assert_called_with("local-instance-2", mock.ANY)
# Test delete remote instances.
instances = ["ins-id1-cf-x86-phone-userdebug",
diff --git a/list/instance.py b/list/instance.py
index 1733cf1..616711d 100644
--- a/list/instance.py
+++ b/list/instance.py
@@ -595,6 +595,22 @@
self._INSTANCE_NAME_FORMAT % {"id": self._id})
@classmethod
+ def GetIdByName(cls, name):
+ """Get 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 goldfish instance.
+ """
+ match = cls._INSTANCE_NAME_PATTERN.match(name)
+ if match:
+ return int(match.group("id"))
+ return None
+
+ @classmethod
def GetLockById(cls, instance_id):
"""Get LocalInstanceLock by id."""
lock_path = os.path.join(
diff --git a/list/list.py b/list/list.py
index 4598de2..6ccbac1 100644
--- a/list/list.py
+++ b/list/list.py
@@ -275,13 +275,12 @@
return instances_list[0]
-def _FilterInstancesByNames(instances, names, all_match=True):
+def _FilterInstancesByNames(instances, names):
"""Find instances by names.
Args:
instances: Collection of Instance objects.
names: Collection of strings, the names of the instances to search for.
- all_match: Boolean, True to raise error if any instance is missing.
Returns:
List of Instance objects.
@@ -298,24 +297,42 @@
else:
missing_instance_names.append(name)
- if missing_instance_names and all_match:
+ if missing_instance_names:
raise errors.NoInstancesFound("Did not find the following instances: %s" %
" ".join(missing_instance_names))
return found_instances
-def GetLocalInstancesByNames(names, all_match=True):
+def GetLocalInstanceLockByName(name):
+ """Get the lock of a local cuttelfish or goldfish instance.
+
+ Args:
+ name: The instance name.
+
+ Returns:
+ LocalInstanceLock object. None if the name is invalid.
+ """
+ cf_id = instance.GetLocalInstanceIdByName(name)
+ if cf_id is not None:
+ return instance.GetLocalInstanceLock(cf_id)
+
+ gf_id = instance.LocalGoldfishInstance.GetIdByName(name)
+ if gf_id is not None:
+ return instance.LocalGoldfishInstance.GetLockById(gf_id)
+
+ return None
+
+
+def GetLocalInstancesByNames(names):
"""Get local cuttlefish and goldfish instances by names.
+ This method does not raise an error if it cannot find all instances.
+
Args:
names: Collection of instance names.
- all_match: Boolean, True to raise error if any instance is missing.
Returns:
List consisting of LocalInstance and LocalGoldfishInstance objects.
-
- Raises:
- errors.NoInstancesFound: No instances found.
"""
id_cfg_pairs = []
for name in names:
@@ -330,10 +347,11 @@
if cfg_path:
id_cfg_pairs.append((ins_id, cfg_path))
- return _FilterInstancesByNames(
- _GetLocalCuttlefishInstances(id_cfg_pairs) +
- instance.LocalGoldfishInstance.GetExistingInstances(),
- names, all_match)
+ gf_instances = [ins for ins in
+ instance.LocalGoldfishInstance.GetExistingInstances()
+ if ins.name in names]
+
+ return _GetLocalCuttlefishInstances(id_cfg_pairs) + gf_instances
def GetInstancesFromInstanceNames(cfg, instance_names):
@@ -352,8 +370,8 @@
errors.NoInstancesFound: No instances found.
"""
return _FilterInstancesByNames(
- GetLocalInstancesByNames(instance_names, all_match=False) +
- GetRemoteInstances(cfg), instance_names)
+ 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 3fd9857..057bafd 100644
--- a/list/list_test.py
+++ b/list/list_test.py
@@ -99,27 +99,21 @@
return_value="path1")
self.Patch(instance, "GetDefaultCuttlefishConfig",
return_value="path2")
- mock_ins = mock.Mock()
- mock_ins.name = "local-instance-1"
+ mock_cf_ins = mock.Mock()
+ mock_cf_ins.name = "local-instance-1"
mock_get_cf = self.Patch(list_instance,
"_GetLocalCuttlefishInstances",
- return_value=[mock_ins])
+ return_value=[mock_cf_ins])
+ mock_gf_ins = mock.Mock()
+ mock_gf_ins.name = "local-goldfish-instance-1"
self.Patch(instance.LocalGoldfishInstance, "GetExistingInstances",
- return_value=[])
+ return_value=[mock_gf_ins])
- ins_list = list_instance.GetLocalInstancesByNames(["local-instance-1"])
- self.assertEqual(1, len(ins_list))
+ ins_list = list_instance.GetLocalInstancesByNames([
+ mock_cf_ins.name, "local-instance-6", mock_gf_ins.name])
+ self.assertEqual([mock_cf_ins, mock_gf_ins], 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"])
-
- # test get instance without raising error
- ins_list = list_instance.GetLocalInstancesByNames(
- ["local-instance-1", "local-instance-6"], all_match=False)
- self.assertEqual(1, len(ins_list))
-
# pylint: disable=attribute-defined-outside-init
def testFilterInstancesByAdbPort(self):
"""test FilterInstancesByAdbPort."""