Fix zuul-web sql connections

A recent change to scope sql connections to tenants was incorrect
because it assumed zuul-web would have access to a tenant's layout,
but that is only in the zuul-scheduler's memory.  Instead, this change
causes zuul-web to perform an RPC call to the scheduler to ask which
connection to use before performing sql queries.

This slipped through testing because the zuul web test fixture used
the same connection registry object as the scheduler, and therefore
*was* able to access data which normally would not be available to it.
This updates the zuul web test fixture to use a separate connection
registry.

Also, because zuul-web doesn't need any other kinds of connections at
the moment, add a facility to allow it to only load sql connections,
so that it doesn't do anything with the gerrit or github drivers.

Change-Id: Iac6d8c6960f4a31eb2f78511664b704758e41687
This commit is contained in:
James E. Blair 2018-06-04 13:55:59 -07:00
parent dc01c8f206
commit ac1bb06c45
11 changed files with 61 additions and 16 deletions

View File

@ -62,6 +62,8 @@ import tests.fakegithub
import zuul.driver.gerrit.gerritsource as gerritsource import zuul.driver.gerrit.gerritsource as gerritsource
import zuul.driver.gerrit.gerritconnection as gerritconnection import zuul.driver.gerrit.gerritconnection as gerritconnection
import zuul.driver.github.githubconnection as githubconnection import zuul.driver.github.githubconnection as githubconnection
import zuul.driver.github
import zuul.driver.sql
import zuul.scheduler import zuul.scheduler
import zuul.executor.server import zuul.executor.server
import zuul.executor.client import zuul.executor.client
@ -1904,10 +1906,14 @@ class WebProxyFixture(fixtures.Fixture):
class ZuulWebFixture(fixtures.Fixture): class ZuulWebFixture(fixtures.Fixture):
def __init__(self, gearman_server_port, connections, info=None): def __init__(self, gearman_server_port, config, info=None):
super(ZuulWebFixture, self).__init__() super(ZuulWebFixture, self).__init__()
self.gearman_server_port = gearman_server_port self.gearman_server_port = gearman_server_port
self.connections = connections self.connections = zuul.lib.connections.ConnectionRegistry()
self.connections.configure(
config,
include_drivers=[zuul.driver.sql.SQLDriver,
zuul.driver.github.GithubDriver])
if info is None: if info is None:
self.info = zuul.model.WebInfo() self.info = zuul.model.WebInfo()
else: else:
@ -1921,7 +1927,7 @@ class ZuulWebFixture(fixtures.Fixture):
info=self.info, info=self.info,
connections=self.connections) connections=self.connections)
self.web.start() self.web.start()
self.addCleanup(self.web.stop) self.addCleanup(self.stop)
self.host = 'localhost' self.host = 'localhost'
# Wait until web server is started # Wait until web server is started
@ -1933,6 +1939,10 @@ class ZuulWebFixture(fixtures.Fixture):
except ConnectionRefusedError: except ConnectionRefusedError:
pass pass
def stop(self):
self.web.stop()
self.connections.stop()
class MySQLSchemaFixture(fixtures.Fixture): class MySQLSchemaFixture(fixtures.Fixture):
def setUp(self): def setUp(self):

View File

@ -827,7 +827,7 @@ class TestGithubWebhook(ZuulTestCase):
# Start the web server # Start the web server
self.web = self.useFixture( self.web = self.useFixture(
ZuulWebFixture(self.gearman_server.port, ZuulWebFixture(self.gearman_server.port,
self.connections)) self.config))
host = '127.0.0.1' host = '127.0.0.1'
# Wait until web server is started # Wait until web server is started

View File

@ -233,7 +233,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase):
# Start the web server # Start the web server
web = self.useFixture( web = self.useFixture(
ZuulWebFixture(self.gearman_server.port, ZuulWebFixture(self.gearman_server.port,
self.connections)) self.config))
# Start the finger streamer daemon # Start the finger streamer daemon
streamer = zuul.lib.log_streamer.LogStreamer( streamer = zuul.lib.log_streamer.LogStreamer(
@ -315,7 +315,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase):
# Start the web server # Start the web server
web = self.useFixture( web = self.useFixture(
ZuulWebFixture(self.gearman_server.port, ZuulWebFixture(self.gearman_server.port,
self.connections)) self.config))
# Start the finger streamer daemon # Start the finger streamer daemon
streamer = zuul.lib.log_streamer.LogStreamer( streamer = zuul.lib.log_streamer.LogStreamer(

View File

@ -53,7 +53,7 @@ class BaseTestWeb(ZuulTestCase):
self.web = self.useFixture( self.web = self.useFixture(
ZuulWebFixture( ZuulWebFixture(
self.gearman_server.port, self.gearman_server.port,
self.connections, self.config,
info=zuul.model.WebInfo.fromConfig(self.zuul_ini_config))) info=zuul.model.WebInfo.fromConfig(self.zuul_ini_config)))
self.executor_server.hold_jobs_in_build = True self.executor_server.hold_jobs_in_build = True

View File

@ -30,7 +30,7 @@ class TestWebURLs(ZuulTestCase):
super(TestWebURLs, self).setUp() super(TestWebURLs, self).setUp()
self.web = self.useFixture( self.web = self.useFixture(
ZuulWebFixture(self.gearman_server.port, ZuulWebFixture(self.gearman_server.port,
self.connections)) self.config))
def _get(self, port, uri): def _get(self, port, uri):
url = "http://localhost:{}{}".format(port, uri) url = "http://localhost:{}{}".format(port, uri)

View File

@ -150,9 +150,9 @@ class ZuulApp(object):
logging_config = logconfig.ServerLoggingConfig(server=section) logging_config = logconfig.ServerLoggingConfig(server=section)
logging_config.apply() logging_config.apply()
def configure_connections(self, source_only=False): def configure_connections(self, source_only=False, include_drivers=None):
self.connections = zuul.lib.connections.ConnectionRegistry() self.connections = zuul.lib.connections.ConnectionRegistry()
self.connections.configure(self.config, source_only) self.connections.configure(self.config, source_only, include_drivers)
class ZuulDaemonApp(ZuulApp, metaclass=abc.ABCMeta): class ZuulDaemonApp(ZuulApp, metaclass=abc.ABCMeta):

View File

@ -20,6 +20,8 @@ import sys
import zuul.cmd import zuul.cmd
import zuul.model import zuul.model
import zuul.web import zuul.web
import zuul.driver.sql
import zuul.driver.github
from zuul.lib.config import get_default from zuul.lib.config import get_default
@ -87,7 +89,9 @@ class WebServer(zuul.cmd.ZuulDaemonApp):
self.setup_logging('web', 'log_config') self.setup_logging('web', 'log_config')
self.log = logging.getLogger("zuul.WebServer") self.log = logging.getLogger("zuul.WebServer")
self.configure_connections() self.configure_connections(
include_drivers=[zuul.driver.sql.SQLDriver,
zuul.driver.github.GithubDriver])
try: try:
self._run() self._run()

View File

@ -508,8 +508,12 @@ class GithubConnection(BaseConnection):
self.gearman_worker.start() self.gearman_worker.start()
def onStop(self): def onStop(self):
self.gearman_worker.stop() # TODO(jeblair): remove this check which is here only so that
self._stop_event_connector() # zuul-web can call connections.stop to shut down the sql
# connection.
if hasattr(self, 'gearman_worker'):
self.gearman_worker.stop()
self._stop_event_connector()
def _start_event_connector(self): def _start_event_connector(self):
self.github_event_connector = GithubEventConnector(self) self.github_event_connector = GithubEventConnector(self)

View File

@ -80,7 +80,7 @@ class ConnectionRegistry(object):
for driver in self.drivers.values(): for driver in self.drivers.values():
driver.stop() driver.stop()
def configure(self, config, source_only=False): def configure(self, config, source_only=False, include_drivers=None):
# Register connections from the config # Register connections from the config
connections = OrderedDict() connections = OrderedDict()
@ -109,6 +109,16 @@ class ConnectionRegistry(object):
if source_only and not isinstance(driver, SourceInterface): if source_only and not isinstance(driver, SourceInterface):
continue continue
# Zuul web needs only the SQL driver, accomodate that here:
if include_drivers is not None:
found = False
for driver_class in include_drivers:
if isinstance(driver, driver_class):
found = True
break
if not found:
continue
connection = driver.getConnection(con_name, con_config) connection = driver.getConnection(con_name, con_config)
connections[con_name] = connection connections[con_name] = connection

View File

@ -58,6 +58,7 @@ class RPCListener(object):
self.worker.registerFunction("zuul:get_running_jobs") self.worker.registerFunction("zuul:get_running_jobs")
self.worker.registerFunction("zuul:get_job_log_stream_address") self.worker.registerFunction("zuul:get_job_log_stream_address")
self.worker.registerFunction("zuul:tenant_list") self.worker.registerFunction("zuul:tenant_list")
self.worker.registerFunction("zuul:tenant_sql_connection")
self.worker.registerFunction("zuul:status_get") self.worker.registerFunction("zuul:status_get")
self.worker.registerFunction("zuul:job_list") self.worker.registerFunction("zuul:job_list")
self.worker.registerFunction("zuul:key_get") self.worker.registerFunction("zuul:key_get")
@ -320,6 +321,16 @@ class RPCListener(object):
'queue': queue_size}) 'queue': queue_size})
job.sendWorkComplete(json.dumps(output)) job.sendWorkComplete(json.dumps(output))
def handle_tenant_sql_connection(self, job):
args = json.loads(job.arguments)
sql_driver = self.sched.connections.drivers['sql']
conn = sql_driver.tenant_connections.get(args['tenant'])
if conn:
name = conn.connection_name
else:
name = ''
job.sendWorkComplete(json.dumps(name))
def handle_status_get(self, job): def handle_status_get(self, job):
args = json.loads(job.arguments) args = json.loads(job.arguments)
output = self.sched.formatStatusJSON(args.get("tenant")) output = self.sched.formatStatusJSON(args.get("tenant"))

View File

@ -241,9 +241,15 @@ class ZuulWebAPI(object):
uuid=None, job_name=None, voting=None, node_name=None, uuid=None, job_name=None, voting=None, node_name=None,
result=None, limit=50, skip=0): result=None, limit=50, skip=0):
sql_driver = self.zuulweb.connections.drivers['sql'] sql_driver = self.zuulweb.connections.drivers['sql']
connection = sql_driver.tenant_connections.get(tenant)
if not connection: # Ask the scheduler which sql connection to use for this tenant
job = self.rpc.submitJob('zuul:tenant_sql_connection',
{'tenant': tenant})
connection_name = json.loads(job.data[0])
if not connection_name:
raise Exception("Unable to find connection for tenant %s" % tenant) raise Exception("Unable to find connection for tenant %s" % tenant)
connection = self.zuulweb.connections.connections[connection_name]
args = { args = {
'buildset_filters': {'tenant': [tenant]}, 'buildset_filters': {'tenant': [tenant]},