Remove the ability for host status to be modified via the frontend.
This interferes with the monitor_db scheduler as it owns the field.
Signed-off-by: Gregory Smith <gps@google.com>
git-svn-id: http://test.kernel.org/svn/autotest/trunk@3584 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/cli/host.py b/cli/host.py
index 94c2016..34130db 100755
--- a/cli/host.py
+++ b/cli/host.py
@@ -299,13 +299,13 @@
print '-'*5
print 'Hostname: %s' % host
self.print_table(jobs, keys_header=['job_id',
- 'job_owner',
- 'job_name',
- 'status'])
+ 'job_owner',
+ 'job_name',
+ 'status'])
class host_mod(host):
- """atest host mod --lock|--unlock|--ready
+ """atest host mod --lock|--unlock|--protection
--mlist <file>|<hosts>"""
usage_action = 'mod'
@@ -314,9 +314,6 @@
self.data = {}
self.messages = []
super(host_mod, self).__init__()
- self.parser.add_option('-y', '--ready',
- help='Mark this host ready',
- action='store_true')
self.parser.add_option('-l', '--lock',
help='Lock hosts',
action='store_true')
@@ -337,12 +334,9 @@
self._parse_lock_options(options)
- if options.ready:
- self.data['status'] = 'Ready'
- self.messages.append('Set status to Ready for host')
-
if options.protection:
self.data['protection'] = options.protection
+ self.messages.append('Protection set to "%s"' % options.protection)
if len(self.data) == 0:
self.invalid_syntax('No modification requested')
diff --git a/cli/host_unittest.py b/cli/host_unittest.py
index e64477f..f6436c2 100755
--- a/cli/host_unittest.py
+++ b/cli/host_unittest.py
@@ -1231,26 +1231,34 @@
err_words_ok=['Host', 'matching', 'query', 'host1'])
- def test_execute_ready_hosts(self):
+ def test_execute_protection_hosts(self):
mfile = cli_mock.create_file('host0\nhost1,host2\nhost3 host4')
- self.run_cmd(argv=['atest', 'host', 'mod', '--ready',
+ self.run_cmd(argv=['atest', 'host', 'mod', '--protection',
+ 'Do not repair',
'host5' ,'--mlist', mfile.name, 'host1', 'host6',
'--ignore_site_file'],
- rpcs=[('modify_host', {'id': 'host6', 'status': 'Ready'},
+ rpcs=[('modify_host', {'id': 'host6',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host5', 'status': 'Ready'},
+ ('modify_host', {'id': 'host5',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host4', 'status': 'Ready'},
+ ('modify_host', {'id': 'host4',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host3', 'status': 'Ready'},
+ ('modify_host', {'id': 'host3',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host2', 'status': 'Ready'},
+ ('modify_host', {'id': 'host2',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host1', 'status': 'Ready'},
+ ('modify_host', {'id': 'host1',
+ 'protection': 'Do not repair'},
True, None),
- ('modify_host', {'id': 'host0', 'status': 'Ready'},
+ ('modify_host', {'id': 'host0',
+ 'protection': 'Do not repair'},
True, None)],
- out_words_ok=['Ready', 'host0', 'host1', 'host2',
+ out_words_ok=['Do not repair', 'host0', 'host1', 'host2',
'host3', 'host4', 'host5', 'host6'])
mfile.clean()
diff --git a/frontend/afe/doctests/001_rpc_test.txt b/frontend/afe/doctests/001_rpc_test.txt
index f640025..064be38 100644
--- a/frontend/afe/doctests/001_rpc_test.txt
+++ b/frontend/afe/doctests/001_rpc_test.txt
@@ -88,7 +88,6 @@
# hosts...
>>> rpc_interface.add_host(hostname='ipaj1', locked=True)
1
->>> rpc_interface.modify_host('ipaj1', status='Hello')
>>> data = rpc_interface.get_hosts()
# delete the lock_time field, since that can't be reliably checked
@@ -97,7 +96,7 @@
... 'hostname': 'ipaj1',
... 'locked': 1,
... 'synch_id': None,
-... 'status': 'Hello',
+... 'status': 'Ready',
... 'labels': [],
... 'atomic_group': None,
... 'acls': ['Everyone'],
@@ -108,7 +107,17 @@
... 'locked_by': 'debug_user',
... 'dirty': True}]
True
->>> rpc_interface.delete_host('ipaj1')
+>>> rpc_interface.modify_host('ipaj1', status='Hello')
+Traceback (most recent call last):
+ValidationError: {'status': 'Host status can not be modified by the frontend.'}
+>>> rpc_interface.modify_host('ipaj1', hostname='ipaj1000')
+>>> rpc_interface.modify_hosts(
+... host_filter_data={'hostname': 'ipaj1000'},
+... update_data={'locked': False})
+>>> data = rpc_interface.get_hosts()
+>>> bool(data[0]['locked'])
+False
+>>> rpc_interface.delete_host('ipaj1000')
>>> rpc_interface.get_hosts() == []
True
diff --git a/frontend/afe/rpc_interface.py b/frontend/afe/rpc_interface.py
index bfd0810..67da52d 100644
--- a/frontend/afe/rpc_interface.py
+++ b/frontend/afe/rpc_interface.py
@@ -114,14 +114,16 @@
def modify_host(id, **data):
+ rpc_utils.check_modify_host(data)
models.Host.smart_get(id).update_object(data)
def modify_hosts(host_filter_data, update_data):
"""
- @param host_filter_data filters out which hosts to modify
- @param update_data dictionary with the changes to make to the hosts
+ @param host_filter_data: Filters out which hosts to modify.
+ @param update_data: A dictionary with the changes to make to the hosts.
"""
+ rpc_utils.check_modify_host(update_data)
hosts = models.Host.query_objects(host_filter_data)
for host in hosts:
host.update_object(update_data)
diff --git a/frontend/afe/rpc_utils.py b/frontend/afe/rpc_utils.py
index 2b0fa83..e87553a 100644
--- a/frontend/afe/rpc_utils.py
+++ b/frontend/afe/rpc_utils.py
@@ -319,6 +319,21 @@
(atomic_group.name,)})
+def check_modify_host(update_data):
+ """
+ Sanity check modify_host* requests.
+
+ @param update_data: A dictionary with the changes to make to a host
+ or hosts.
+ """
+ # Only the scheduler (monitor_db) is allowed to modify Host status.
+ # Otherwise race conditions happen as a hosts state is changed out from
+ # beneath tasks being run on a host.
+ if 'status' in update_data:
+ raise model_logic.ValidationError({
+ 'status': 'Host status can not be modified by the frontend.'})
+
+
def get_motd():
dirname = os.path.dirname(__file__)
filename = os.path.join(dirname, "..", "..", "motd.txt")