Reland "Use dev_server to determine the url of the devserver."
Condition for deploying required a scheduler restart. Original
change was fine.
This reverts commit 5ca9d1b7a16b80d71acdbf01bd5b8019b13e3165.
BUG=chromium-os:26451
TEST=Unittests + local run with prod devservers
Change-Id: I9acd6bce50a8919f85ca3ed65420ad235e8501e7
Reviewed-on: https://gerrit.chromium.org/gerrit/28855
Reviewed-by: Chris Sosa <sosa@chromium.org>
Tested-by: Chris Sosa <sosa@chromium.org>
diff --git a/client/common_lib/cros/dev_server.py b/client/common_lib/cros/dev_server.py
index 7f31a3b..8a9833b 100644
--- a/client/common_lib/cros/dev_server.py
+++ b/client/common_lib/cros/dev_server.py
@@ -78,11 +78,16 @@
pass
+
+# TODO(sosa): Make this class use sub-classes of a common Server class rather
+# than if/else cases with crash server/devserver.
class DevServer(object):
"""Helper class for interacting with the Dev Server via http."""
+
_CRASH_SERVER_RPC_CALLS = set(['stage_debug', 'symbolicate_dump'])
+
def __init__(self, dev_host=None, crash_host=None):
"""Constructor.
@@ -100,13 +105,35 @@
else:
self._crash_servers = _get_crash_server_list()
+
@staticmethod
def create(dev_host=None, crash_host=None):
"""Wraps the constructor. Purely for mocking purposes."""
return DevServer(dev_host, crash_host)
+ @staticmethod
+ def _server_for_hashing_value(servers, hashing_value):
+ """Returns the server in servers to use for the given hashing_value.
+ Args:
+ @param servers: List of servers
+ @param hashing_value: The value that determines which server to use.
+ """
+ return servers[hash(hashing_value) % len(servers)]
+
+
+ @classmethod
+ def devserver_url_for_build(cls, build):
+ """Returns the devserver url which contains the build.
+
+ Args:
+ @param build: The build name i.e. builder/version that is being tested.
+ """
+ return cls._server_for_hashing_value(_get_dev_server_list(), build)
+
+
def _servers_for(self, method):
+ """Return the list of servers to use for the given method."""
if method in DevServer._CRASH_SERVER_RPC_CALLS:
return self._crash_servers
else:
@@ -129,12 +156,13 @@
# value to give us an index of the devserver to use. The hashing value
# must be the same for RPC's that should go to the same devserver.
server_pool = self._servers_for(method)
- devserver = server_pool[hash(hashing_value) % len(server_pool)]
+ server = self._server_for_hashing_value(server_pool, hashing_value)
argstr = '&'.join(map(lambda x: "%s=%s" % x, kwargs.iteritems()))
- return "%(host)s/%(method)s?%(args)s" % {'host': devserver,
+ return "%(host)s/%(method)s?%(args)s" % {'host': server,
'method': method,
'args': argstr}
+
def _build_all_calls(self, method, **kwargs):
"""Builds a list of URLs that makes RPC calls on all devservers.
diff --git a/client/common_lib/cros/dev_server_unittest.py b/client/common_lib/cros/dev_server_unittest.py
index c324ea6..baf66bb 100644
--- a/client/common_lib/cros/dev_server_unittest.py
+++ b/client/common_lib/cros/dev_server_unittest.py
@@ -14,6 +14,7 @@
import unittest
import urllib2
+from autotest_lib.client.common_lib import global_config
from autotest_lib.client.common_lib.cros import dev_server
@@ -25,6 +26,8 @@
_HOST = 'http://nothing'
_CRASH_HOST = 'http://nothing-crashed'
+ _CONFIG = global_config.global_config
+
def setUp(self):
super(DevServerTest, self).setUp()
@@ -253,6 +256,21 @@
self.assertEquals(build_string2, build)
+ def testDevserverUrlWithManyDevservers(self):
+ """Should be able to return different urls with multiple devservers."""
+ host0_expected = 'http://host0:8082'
+ host1_expected = 'http://host1:8099'
+ self._CONFIG.override_config_value(
+ 'CROS',
+ 'dev_server',
+ '%s,%s' % (host0_expected, host1_expected))
+ host0 = dev_server.DevServer.devserver_url_for_build(0)
+ host1 = dev_server.DevServer.devserver_url_for_build(1)
+
+ self.assertEqual(host0, host0_expected)
+ self.assertEqual(host1, host1_expected)
+
+
def testCrashesAreSetToTheCrashServer(self):
"""Should send symbolicate dump rpc calls to crash_server."""
hv = 'iliketacos'
diff --git a/global_config.ini b/global_config.ini
index d0741c9..adb1be8 100644
--- a/global_config.ini
+++ b/global_config.ini
@@ -132,11 +132,9 @@
dev_server: http://172.22.50.2:8082
crash_server: http://172.22.50.2:8082
sharding_factor: 1
-# The below should be %(dev_server)s/update/%%(name)s so that we'd fill in
-# the above value for dev_server when this config is parsed, but leave the
-# name field to be populated later. Sadly, that doesn't parse.
-image_url_pattern: %(dev_server)s/update/%%s
-package_url_pattern: %(dev_server)s/static/archive/%%s/autotest/packages
+
+image_url_pattern: %s/update/%s
+package_url_pattern: %s/static/archive/%s/autotest/packages
log_url_pattern: http://%s/tko/retrieve_logs.cgi?job=/results/%s/
# Username and password for connecting to remote power control switches of
# the "Sentry Switched CDU" type
diff --git a/server/cros/dynamic_suite.py b/server/cros/dynamic_suite.py
index fa4a7fa..86e935d 100644
--- a/server/cros/dynamic_suite.py
+++ b/server/cros/dynamic_suite.py
@@ -277,7 +277,6 @@
debug=False)
manager = host_lock_manager.HostLockManager(afe=afe)
reimager = Reimager(job.autodir, afe, tko, results_dir=job.resultdir)
-
try:
if skip_reimage or reimager.attempt(build, board, pool,
job.record_entry, check_hosts,
@@ -368,6 +367,12 @@
return CONFIG.get_config_value('CROS', 'package_url_pattern', type=str)
+def get_package_url(build):
+ """Returns the package url for the given build."""
+ devserver_url = dev_server.DevServer.devserver_url_for_build(build)
+ return _package_url_pattern() % (devserver_url, build)
+
+
def skip_reimage(g):
return g.get('SKIP_IMAGE')
@@ -493,7 +498,7 @@
return job_status.gather_per_host_results(self._afe,
self._tko,
[canary_job],
- REIMAGE_JOB_NAME+'-')
+ REIMAGE_JOB_NAME + '-')
def _ensure_enough_hosts(self, board, pool, num):
@@ -601,8 +606,10 @@
@param num_machines: how many devices to reimage.
@return a frontend.Job object for the reimaging job we scheduled.
"""
+ image_url = _image_url_pattern() % (
+ dev_server.DevServer.devserver_url_for_build(build), build)
control_file = inject_vars(
- {'image_url': _image_url_pattern() % build, 'image_name': build},
+ dict(image_url=image_url, image_name=build),
self._cf_getter.get_control_file_contents_by_name('autoupdate'))
job_deps = []
if pool:
diff --git a/server/cros/dynamic_suite_unittest.py b/server/cros/dynamic_suite_unittest.py
index 5efd76f..476ac51 100755
--- a/server/cros/dynamic_suite_unittest.py
+++ b/server/cros/dynamic_suite_unittest.py
@@ -16,6 +16,7 @@
from autotest_lib.client.common_lib import base_job, control_data, error
from autotest_lib.client.common_lib import global_config
+from autotest_lib.client.common_lib.cros import dev_server
from autotest_lib.frontend.afe.json_rpc import proxy
from autotest_lib.server.cros import control_file_getter, dynamic_suite
from autotest_lib.server.cros import host_lock_manager, job_status
@@ -89,7 +90,7 @@
def testVetRequiredReimageAndRunArgs(self):
"""Should verify only that required args are present and correct."""
- build, board, name, job, _, _, _, _,_ = \
+ build, board, name, job, _, _, _, _, _ = \
dynamic_suite._vet_reimage_and_run_args(**self._DARGS)
self.assertEquals(build, self._DARGS['build'])
self.assertEquals(board, self._DARGS['board'])
@@ -165,7 +166,8 @@
@var _BOARD: fake board to reimage
"""
- _URL = 'http://nothing/%s'
+ _DEVSERVER_URL = 'http://nothing:8082'
+ _URL = '%s/%s'
_BUILD = 'build'
_NUM = 4
_BOARD = 'board'
@@ -232,12 +234,12 @@
"""Should inject dict of varibles into provided strings."""
def find_all_in(d, s):
"""Returns true if all key-value pairs in |d| are printed in |s|."""
- for k,v in d.iteritems():
+ for k, v in d.iteritems():
if isinstance(v, str):
- if "%s='%s'\n" % (k,v) not in s:
+ if "%s='%s'\n" % (k, v) not in s:
return False
else:
- if "%s=%r\n" % (k,v) not in s:
+ if "%s=%r\n" % (k, v) not in s:
return False
return True
@@ -252,13 +254,17 @@
cf_getter = self.mox.CreateMock(control_file_getter.ControlFileGetter)
cf_getter.get_control_file_contents_by_name('autoupdate').AndReturn('')
self.reimager._cf_getter = cf_getter
-
+ self._CONFIG.override_config_value('CROS',
+ 'dev_server',
+ self._DEVSERVER_URL)
self._CONFIG.override_config_value('CROS',
'image_url_pattern',
self._URL)
self.afe.create_job(
- control_file=mox.And(mox.StrContains(self._BUILD),
- mox.StrContains(self._URL % self._BUILD)),
+ control_file=mox.And(
+ mox.StrContains(self._BUILD),
+ mox.StrContains(self._URL % (self._DEVSERVER_URL,
+ self._BUILD))),
name=mox.StrContains(self._BUILD),
control_type='Server',
meta_hosts=[self._BOARD] * self._NUM,
@@ -268,6 +274,18 @@
self.reimager._schedule_reimage_job(self._BUILD, self._BOARD, None,
self._NUM)
+ def testPackageUrl(self):
+ """Should be able to get the package_url for any build."""
+ self._CONFIG.override_config_value('CROS',
+ 'dev_server',
+ self._DEVSERVER_URL)
+ self._CONFIG.override_config_value('CROS',
+ 'package_url_pattern',
+ self._URL)
+ self.mox.ReplayAll()
+ package_url = dynamic_suite.get_package_url(self._BUILD)
+ self.assertEqual(package_url, self._URL % (self._DEVSERVER_URL,
+ self._BUILD))
def expect_attempt(self, canary_job, statuses, ex=None, check_hosts=True):
"""Sets up |self.reimager| to expect an attempt() that returns |success|
@@ -293,7 +311,6 @@
self.mox.StubOutWithMock(job_status, 'gather_per_host_results')
self.mox.StubOutWithMock(job_status, 'record_and_report_results')
-
self.reimager._ensure_version_label(mox.StrContains(self._BUILD))
self.reimager._schedule_reimage_job(self._BUILD,
self._BOARD,
@@ -318,7 +335,7 @@
statuses)
if statuses:
- ret_val = reduce(lambda v,s: v and s.is_good(),
+ ret_val = reduce(lambda v, s: v and s.is_good(),
statuses.values(), True)
job_status.record_and_report_results(
statuses.values(), mox.IgnoreArg()).AndReturn(ret_val)
diff --git a/server/site_tests/autoupdate/control b/server/site_tests/autoupdate/control
index 374a4ce..0f36977 100644
--- a/server/site_tests/autoupdate/control
+++ b/server/site_tests/autoupdate/control
@@ -16,7 +16,7 @@
Assumes that a label including the image name has already been created.
@param image_url: the devserver URL at which the image to install is served.
-@param image_name: the name of the image, used to label the device
+@param image_name: the name of the image, used to label the device a.k.a. build
"""
from autotest_lib.server.cros import frontend_wrappers
@@ -26,7 +26,7 @@
if 'image_name' in locals():
from autotest_lib.server.cros import dynamic_suite
vers = dynamic_suite.VERSION_PREFIX
- repo_url = dynamic_suite._package_url_pattern() % image_name
+ repo_url = dynamic_suite.get_package_url(build=image_name)
AFE = frontend_wrappers.RetryingAFE(timeout_min=30, delay_sec=10, debug=False)