[autotest] Require lock reason to lock device
When locking a device, a locking reason now must be provided.
This applies to both adding a new device, and modifying an
existing device from both the web frontend and the atest
command-line tool.
BUG=chromium:336805
DEPLOY=migrate
TEST=Tested adding locked/unlocked devices and locking/unlocking devices
from both the web frontend and using the 'atest host ...' command-line tools.
Change-Id: I3a8cd8891a2999f026dd709ae8a79e2b8cbc251a
Reviewed-on: https://chromium-review.googlesource.com/267595
Tested-by: Matthew Sartori <[email protected]>
Reviewed-by: Dan Shi <[email protected]>
Commit-Queue: Matthew Sartori <[email protected]>
diff --git a/cli/host.py b/cli/host.py
index 53d572d..df82996 100644
--- a/cli/host.py
+++ b/cli/host.py
@@ -63,8 +63,12 @@
self.messages.append('Locked host')
elif options.unlock:
self.data['locked'] = False
+ self.data['lock_reason'] = ''
self.messages.append('Unlocked host')
+ if options.lock and options.lock_reason:
+ self.data['lock_reason'] = options.lock_reason
+
def _cleanup_labels(self, labels, platform=None):
"""Removes the platform label from the overall labels"""
@@ -196,7 +200,7 @@
self.print_list(results, key='hostname')
else:
keys = ['hostname', 'status',
- 'shard', 'locked', 'platform', 'labels']
+ 'shard', 'locked', 'lock_reason', 'platform', 'labels']
super(host_list, self).output(results, keys=keys)
@@ -240,7 +244,7 @@
self.print_fields(stats,
keys=['hostname', 'platform',
'status', 'locked', 'locked_by',
- 'lock_time', 'protection',])
+ 'lock_time', 'lock_reason', 'protection',])
self.print_by_ids(acls, 'ACLs', line_before=True)
labels = self._cleanup_labels(labels)
self.print_by_ids(labels, 'Labels', line_before=True)
@@ -325,6 +329,9 @@
self.parser.add_option('-u', '--unlock',
help='Unlock hosts',
action='store_true')
+ self.parser.add_option('-r', '--lock_reason',
+ help='Reason for locking hosts',
+ default='')
self.parser.add_option('-p', '--protection', type='choice',
help=('Set the protection level on a host. '
'Must be one of: %s' %
@@ -405,6 +412,9 @@
help='Create the hosts as '
'unlocked (default)',
action='store_true')
+ self.parser.add_option('-r', '--lock_reason',
+ help='Reason for locking hosts',
+ default='')
self.parser.add_option('-t', '--platform',
help='Sets the platform label')
self.parser.add_option('-b', '--labels',
@@ -449,8 +459,12 @@
def _execute_add_one_host(self, host):
# Always add the hosts as locked to avoid the host
- # being picked up by the scheduler before it's ACL'ed
+ # being picked up by the scheduler before it's ACL'ed.
+ # We enforce lock reasons for each lock, so we
+ # provide a 'dummy' if we are intending to unlock after.
self.data['locked'] = True
+ if not self.locked:
+ self.data['lock_reason'] = 'Forced lock on device creation'
self.execute_rpc('add_host', hostname=host,
status="Ready", **self.data)
@@ -473,7 +487,8 @@
if not self.locked:
for host in successful_hosts:
- self.execute_rpc('modify_host', id=host, locked=False)
+ self.execute_rpc('modify_host', id=host, locked=False,
+ lock_reason='')
return successful_hosts
diff --git a/cli/host_unittest.py b/cli/host_unittest.py
index a9bdd90..a8c6f98 100755
--- a/cli/host_unittest.py
+++ b/cli/host_unittest.py
@@ -123,6 +123,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [],
u'invalid': False,
u'synch_id': None,
@@ -134,6 +135,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -153,6 +155,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label0', u'label1'],
u'invalid': False,
u'synch_id': None,
@@ -164,6 +167,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -185,6 +189,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -209,6 +214,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -220,6 +226,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -245,6 +252,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -280,6 +288,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -291,6 +300,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'shard': None,
@@ -314,6 +324,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat0'],
u'invalid': False,
u'synch_id': None,
@@ -325,6 +336,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label2', u'plat2'],
u'invalid': False,
u'synch_id': None,
@@ -349,6 +361,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label2', u'label4',
u'plat1'],
u'invalid': False,
@@ -374,6 +387,7 @@
u'shard': None,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label2', u'label4',
u'plat1'],
u'invalid': 0,
@@ -409,6 +423,7 @@
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
u'labels': [u'label2', u'label3', u'plat1'],
+ u'lock_reason': u'',
u'invalid': False,
u'synch_id': None,
u'platform': u'plat1',
@@ -419,6 +434,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -453,6 +469,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -464,6 +481,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -488,6 +506,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -499,6 +518,7 @@
u'locked': True,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -547,6 +567,7 @@
u'hostname': u'host1',
u'locked': True,
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'lock_time': u'2008-07-23 12:54:15',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
@@ -558,6 +579,7 @@
u'hostname': u'host2',
u'locked': True,
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'lock_time': u'2008-07-23 12:54:15',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
@@ -581,6 +603,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label2', u'label3', u'plat1'],
u'invalid': False,
u'synch_id': None,
@@ -592,6 +615,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
u'shard': None,
@@ -616,6 +640,7 @@
u'locked': True,
u'lock_time': u'2008-07-23 12:54:15',
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'protection': 'No protection',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
@@ -631,6 +656,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'protection': u'No protection',
u'labels': [u'label0', u'plat0'],
u'invalid': False,
@@ -694,6 +720,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'protection': u'No protection',
u'labels': [u'label0', u'plat0'],
u'invalid': False,
@@ -745,6 +772,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'protection': u'No protection',
u'labels': [u'label0', u'plat0'],
u'invalid': False,
@@ -794,6 +822,7 @@
u'locked': True,
u'lock_time': u'2008-07-23 12:54:15',
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'protection': 'No protection',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
@@ -806,6 +835,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'protection': u'No protection',
u'labels': [u'label0', u'plat0'],
u'invalid': False,
@@ -866,6 +896,7 @@
u'locked': False,
u'locked_by': 'user0',
u'lock_time': u'2008-07-23 12:54:15',
+ u'lock_reason': u'',
u'protection': u'No protection',
u'labels': [u'label0', u'plat0'],
u'invalid': False,
@@ -880,6 +911,7 @@
u'locked': True,
u'lock_time': u'2008-07-23 12:54:15',
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'protection': 'No protection',
u'labels': [u'label3', u'label4', u'plat1'],
u'invalid': False,
@@ -891,6 +923,7 @@
u'hostname': u'host0',
u'locked': False,
u'locked_by': 'user0',
+ u'lock_reason': u'',
u'protection': 'No protection',
u'lock_time': u'2008-07-23 12:54:15',
u'labels': [u'label0', u'plat0'],
@@ -1264,9 +1297,11 @@
def test_execute_unlock_two_hosts(self):
self.run_cmd(argv=['atest', 'host', 'mod',
'-u', 'host0,host1', '--ignore_site_file'],
- rpcs=[('modify_host', {'id': 'host1', 'locked': False},
+ rpcs=[('modify_host', {'id': 'host1', 'locked': False,
+ 'lock_reason': ''},
True, None),
- ('modify_host', {'id': 'host0', 'locked': False},
+ ('modify_host', {'id': 'host0', 'locked': False,
+ 'lock_reason': ''},
True, None)],
out_words_ok=['Unlocked', 'host0', 'host1'])
@@ -1371,14 +1406,16 @@
True, 5),
('add_host', {'hostname': 'host1',
'status': 'Ready',
- 'locked': True},
+ 'locked': True,
+ 'lock_reason': 'Forced lock on device creation'},
True, 42),
('host_add_labels', {'id': 'host1',
'labels': ['label0']},
True, None),
('add_host', {'hostname': 'host0',
'status': 'Ready',
- 'locked': True},
+ 'locked': True,
+ 'lock_reason': 'Forced lock on device creation'},
True, 42),
('host_add_labels', {'id': 'host0',
'labels': ['label0']},
@@ -1386,9 +1423,11 @@
('acl_group_add_hosts',
{'id': 'acl0', 'hosts': ['host1', 'host0']},
True, None),
- ('modify_host', {'id': 'host1', 'locked': False},
+ ('modify_host', {'id': 'host1', 'locked': False,
+ 'lock_reason': ''},
True, None),
- ('modify_host', {'id': 'host0', 'locked': False},
+ ('modify_host', {'id': 'host0', 'locked': False,
+ 'lock_reason': ''},
True, None)],
out_words_ok=['host0', 'host1'])
@@ -1425,7 +1464,8 @@
True, 5),
('add_host', {'hostname': 'host1',
'status': 'Ready',
- 'locked': True},
+ 'locked': True,
+ 'lock_reason': 'Forced lock on device creation'},
True, 42),
('host_add_labels', {'id': 'host1',
'labels': ['label0', 'label,1',
@@ -1433,7 +1473,8 @@
True, None),
('add_host', {'hostname': 'host0',
'status': 'Ready',
- 'locked': True},
+ 'locked': True,
+ 'lock_reason': 'Forced lock on device creation'},
True, 42),
('host_add_labels', {'id': 'host0',
'labels': ['label0', 'label,1',
@@ -1442,9 +1483,11 @@
('acl_group_add_hosts',
{'id': 'acl0', 'hosts': ['host1', 'host0']},
True, None),
- ('modify_host', {'id': 'host1', 'locked': False},
+ ('modify_host', {'id': 'host1', 'locked': False,
+ 'lock_reason': ''},
True, None),
- ('modify_host', {'id': 'host0', 'locked': False},
+ ('modify_host', {'id': 'host0', 'locked': False,
+ 'lock_reason': ''},
True, None)],
out_words_ok=['host0', 'host1'])
diff --git a/cli/site_host.py b/cli/site_host.py
index a1853c9..c732ca5 100644
--- a/cli/site_host.py
+++ b/cli/site_host.py
@@ -31,7 +31,7 @@
@classmethod
def construct_without_parse(
cls, web_server, hosts, platform=None,
- locked=False, labels=[], acls=[],
+ locked=False, lock_reason='', labels=[], acls=[],
protection=host_protections.Protection.NO_PROTECTION):
"""Construct an site_host_create object and fill in data from args.
@@ -44,6 +44,7 @@
@param hosts: A list of hostnames as strings.
@param platform: A string or None.
@param locked: A boolean.
+ @param lock_reason: A string.
@param labels: A list of labels as strings.
@param acls: A list of acls as strings.
@param protection: An enum defined in host_protections.
@@ -58,6 +59,8 @@
obj.hosts = hosts
obj.platform = platform
obj.locked = locked
+ if locked and lock_reason.strip():
+ obj.data['lock_reason'] = lock_reason.strip()
obj.labels = labels
obj.acls = acls
if protection:
@@ -69,6 +72,8 @@
# Always add the hosts as locked to avoid the host
# being picked up by the scheduler before it's ACL'ed.
self.data['locked'] = True
+ if not self.locked:
+ self.data['lock_reason'] = 'Forced lock on device creation'
self.execute_rpc('add_host', hostname=host,
status="Ready", **self.data)
# If there are labels avaliable for host, use them.
diff --git a/cli/topic_common.py b/cli/topic_common.py
index b191e6c..7762a75 100644
--- a/cli/topic_common.py
+++ b/cli/topic_common.py
@@ -74,6 +74,7 @@
'locked': 'Locked',
'locked_by': 'Locked by',
'lock_time': 'Locked time',
+ 'lock_reason': 'Lock Reason',
'labels': 'Labels',
'description': 'Description',
'hosts': 'Hosts',
