From 554a1620016f22e2ecd15584401523d42aa829e5 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Sun, 21 Feb 2021 07:35:21 -0800 Subject: [PATCH] Use ZooKeeperClient.fromConfig in tests To make the tests more like running the apps, use fromConfig in the tests rather than directly creating a client from a string connection description. Note that since fromConfig requires TLS and the tests don't use it, a temporary argument has been added to that method to allow non-tls connections; it's only used within the test suite. In future changes, we can configure the tests to use TLS connections, remove the argument, and then move client instantiation into the server classes themselves (which will remove additional differences between test and production startup code). Change-Id: Ic45c0e60c464ed8eeb44cb32bab039403f73ec69 --- tests/base.py | 41 ++++++++++++++++++-------------- tests/unit/test_github_driver.py | 2 +- tests/unit/test_gitlab_driver.py | 2 +- tests/unit/test_pagure_driver.py | 4 ++-- tests/unit/test_streaming.py | 9 +++---- tests/unit/test_web.py | 2 +- tests/unit/test_web_urls.py | 2 +- zuul/cmd/web.py | 14 ++++------- zuul/web/__init__.py | 15 ++---------- zuul/zk/__init__.py | 7 ++++-- 10 files changed, 45 insertions(+), 53 deletions(-) diff --git a/tests/base.py b/tests/base.py index 96c61d9b91..fc204a5deb 100644 --- a/tests/base.py +++ b/tests/base.py @@ -3702,7 +3702,7 @@ class ZuulWebFixture(fixtures.Fixture): additional_event_queues, upstream_root: str, rpcclient: RPCClient, poller_events, git_url_with_auth: bool, add_cleanup: Callable[[Callable[[], None]], None], - test_root: str, zk_hosts: str, fake_sql: bool = True, + test_root: str, fake_sql: bool = True, info: Optional[zuul.model.WebInfo] = None): super(ZuulWebFixture, self).__init__() self.gearman_server_port = gearman_server_port @@ -3721,7 +3721,9 @@ class ZuulWebFixture(fixtures.Fixture): self.info = zuul.model.WebInfo.fromConfig(config) else: self.info = info - self.zk_hosts = zk_hosts + self.zk_client = ZooKeeperClient.fromConfig(config, + _require_tls=False) + self.zk_client.connect() self.test_root = test_root def _setUp(self): @@ -3731,8 +3733,7 @@ class ZuulWebFixture(fixtures.Fixture): gear_server='127.0.0.1', gear_port=self.gearman_server_port, info=self.info, connections=self.connections, - zk_hosts=self.zk_hosts, - zk_timeout=10, + zk_client=self.zk_client, command_socket=os.path.join(self.test_root, 'web.socket'), authenticators=self.authenticators) self.web.start() @@ -3958,7 +3959,7 @@ class SymLink(object): class SchedulerTestApp: - def __init__(self, log: Logger, config: ConfigParser, zk_config: str, + def __init__(self, log: Logger, config: ConfigParser, changes: Dict[str, Dict[str, Change]], additional_event_queues, upstream_root: str, rpcclient: RPCClient, poller_events, git_url_with_auth: bool, @@ -3968,7 +3969,6 @@ class SchedulerTestApp: self.log = log self.config = config - self.zk_config = zk_config self.changes = changes self.sched = zuul.scheduler.Scheduler(self.config) @@ -3994,7 +3994,8 @@ class SchedulerTestApp: self.config, self.sched) merge_client = RecordingMergeClient(self.config, self.sched) nodepool = zuul.nodepool.Nodepool(self.sched) - zk_client = ZooKeeperClient(self.zk_config, timeout=30.0) + zk_client = ZooKeeperClient.fromConfig(self.config, + _require_tls=False) zk_client.connect() self.sched.setExecutor(executor_client) @@ -4030,14 +4031,14 @@ class SchedulerTestManager: def __init__(self): self.instances = [] - def create(self, log: Logger, config: ConfigParser, zk_config: str, + def create(self, log: Logger, config: ConfigParser, changes: Dict[str, Dict[str, Change]], additional_event_queues, upstream_root: str, rpcclient: RPCClient, poller_events, git_url_with_auth: bool, source_only: bool, fake_sql: bool, add_cleanup: Callable[[Callable[[], None]], None])\ -> SchedulerTestApp: - app = SchedulerTestApp(log, config, zk_config, changes, + app = SchedulerTestApp(log, config, changes, additional_event_queues, upstream_root, rpcclient, poller_events, git_url_with_auth, source_only, fake_sql, add_cleanup) @@ -4179,9 +4180,6 @@ class ZuulTestCase(BaseTestCase): self.zk_chroot_fixture.zookeeper_port, self.zk_chroot_fixture.zookeeper_chroot) - self.zk_client = ZooKeeperClient(hosts=self.zk_config) - self.zk_client.connect() - if not KEEP_TEMPDIRS: tmp_root = self.useFixture(fixtures.TempDir( rootdir=os.environ.get("ZUUL_TEST_ROOT")) @@ -4262,6 +4260,17 @@ class ZuulTestCase(BaseTestCase): 'gearman', 'ssl_key', os.path.join(FIXTURE_DIR, 'gearman/client.key')) + zk_hosts = '%s:%s%s' % ( + self.zk_chroot_fixture.zookeeper_host, + self.zk_chroot_fixture.zookeeper_port, + self.zk_chroot_fixture.zookeeper_chroot) + self.config.set('zookeeper', 'hosts', zk_hosts) + self.config.set('zookeeper', 'session_timeout', '30') + + self.zk_client = ZooKeeperClient.fromConfig(self.config, + _require_tls=False) + self.zk_client.connect() + self.rpcclient = zuul.rpcclient.RPCClient( self.config.get('gearman', 'server'), self.gearman_server.port, @@ -4302,7 +4311,7 @@ class ZuulTestCase(BaseTestCase): self.scheds = SchedulerTestManager() self.scheds.create( - self.log, self.config, self.zk_config, self.changes, + self.log, self.config, self.changes, self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.source_only, self.fake_sql, self.addCleanup) @@ -4367,7 +4376,7 @@ class ZuulTestCase(BaseTestCase): config = configparser.ConfigParser() config.read(os.path.join(FIXTURE_DIR, config_file)) - sections = ['zuul', 'scheduler', 'executor', 'merger'] + sections = ['zuul', 'scheduler', 'executor', 'merger', 'zookeeper'] for section in sections: if not config.has_section(section): config.add_section(section) @@ -4517,10 +4526,6 @@ class ZuulTestCase(BaseTestCase): def setupZK(self): self.zk_chroot_fixture = self.useFixture( ChrootedKazooFixture(self.id())) - self.zk_config = '%s:%s%s' % ( - self.zk_chroot_fixture.zookeeper_host, - self.zk_chroot_fixture.zookeeper_port, - self.zk_chroot_fixture.zookeeper_chroot) def copyDirToRepo(self, project, source_path): self.init_repo(project) diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 2ef0dc4b75..10a926843d 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1616,7 +1616,7 @@ class TestGithubWebhook(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) host = '127.0.0.1' # Wait until web server is started diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index 54b315c584..4600809c2a 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -38,7 +38,7 @@ class TestGitlabWebhook(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) host = '127.0.0.1' # Wait until web server is started diff --git a/tests/unit/test_pagure_driver.py b/tests/unit/test_pagure_driver.py index e54a9ee8a1..4b01d4724c 100644 --- a/tests/unit/test_pagure_driver.py +++ b/tests/unit/test_pagure_driver.py @@ -1048,7 +1048,7 @@ class TestPagureWebhook(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) host = '127.0.0.1' # Wait until web server is started @@ -1096,7 +1096,7 @@ class TestPagureWebhookWhitelist(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) host = '127.0.0.1' # Wait until web server is started diff --git a/tests/unit/test_streaming.py b/tests/unit/test_streaming.py index e8ef2034e5..67d06e13c9 100644 --- a/tests/unit/test_streaming.py +++ b/tests/unit/test_streaming.py @@ -255,7 +255,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) # Start the finger streamer daemon streamer = zuul.lib.log_streamer.LogStreamer( @@ -334,7 +334,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) # Start the finger streamer daemon streamer = zuul.lib.log_streamer.LogStreamer( @@ -410,7 +410,7 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) # Start the finger streamer daemon streamer = zuul.lib.log_streamer.LogStreamer( @@ -522,7 +522,8 @@ class TestStreaming(tests.base.AnsibleZuulTestCase): logfile = open(ansible_log, 'r') self.addCleanup(logfile.close) - zk_client = ZooKeeperClient(self.zk_config, timeout=30.0) + zk_client = ZooKeeperClient.fromConfig(self.config, + _require_tls=False) zk_client.connect() self.addCleanup(zk_client.disconnect) diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index f06da75051..b6c2e4bd4d 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -58,7 +58,7 @@ class BaseTestWeb(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config, + self.test_root, info=zuul.model.WebInfo.fromConfig( self.zuul_ini_config), fake_sql=self.fake_sql)) diff --git a/tests/unit/test_web_urls.py b/tests/unit/test_web_urls.py index db28fa07a5..1b017602ee 100644 --- a/tests/unit/test_web_urls.py +++ b/tests/unit/test_web_urls.py @@ -31,7 +31,7 @@ class TestWebURLs(ZuulTestCase): self.additional_event_queues, self.upstream_root, self.rpcclient, self.poller_events, self.git_url_with_auth, self.addCleanup, - self.test_root, self.zk_config)) + self.test_root)) def _get(self, port, uri): url = "http://localhost:{}{}".format(port, uri) diff --git a/zuul/cmd/web.py b/zuul/cmd/web.py index bd1b37b9e8..e9d372f1ac 100755 --- a/zuul/cmd/web.py +++ b/zuul/cmd/web.py @@ -24,6 +24,7 @@ import zuul.driver.github import zuul.lib.auth from zuul.lib.config import get_default +from zuul.zk import ZooKeeperClient class WebServer(zuul.cmd.ZuulDaemonApp): @@ -81,16 +82,9 @@ class WebServer(zuul.cmd.ZuulDaemonApp): self.log.exception("Error validating config") sys.exit(1) - params["zk_hosts"] = get_default( - self.config, 'zookeeper', 'hosts', None) - if not params["zk_hosts"]: - raise Exception("The zookeeper hosts config value is required") - params["zk_tls_key"] = get_default(self.config, 'zookeeper', 'tls_key') - params["zk_tls_cert"] = get_default(self.config, - 'zookeeper', 'tls_cert') - params["zk_tls_ca"] = get_default(self.config, 'zookeeper', 'tls_ca') - params["zk_timeout"] = float(get_default(self.config, 'zookeeper', - 'session_timeout', 10.0)) + zk_client = ZooKeeperClient.fromConfig(self.config) + zk_client.connect() + params["zk_client"] = zk_client try: self.web = zuul.web.ZuulWeb(**params) diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 39fa6c9ffb..5019f4330f 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -1211,10 +1211,7 @@ class ZuulWeb(object): gear_server: str, gear_port: int, connections, # ConnectionRegistry, authenticators: AuthenticatorRegistry, - zk_hosts: str, zk_timeout: float = 10.0, - zk_tls_cert: Optional[str] = None, - zk_tls_key: Optional[str] = None, - zk_tls_ca: Optional[str] = None, + zk_client: ZooKeeperClient, ssl_key: str = None, ssl_cert: str = None, ssl_ca: str = None, static_cache_expiry: int = 3600, info: Optional[zuul.model.WebInfo] = None, @@ -1231,15 +1228,7 @@ class ZuulWeb(object): self.rpc = zuul.rpcclient.RPCClient(gear_server, gear_port, ssl_key, ssl_cert, ssl_ca, client_id='Zuul Web Server') - self.zk_client = ZooKeeperClient( - hosts=zk_hosts, - read_only=True, - timeout=zk_timeout, - tls_cert=zk_tls_cert, - tls_key=zk_tls_key, - tls_ca=zk_tls_ca, - ) - self.zk_client.connect() + self.zk_client = zk_client self.connections = connections self.authenticators = authenticators diff --git a/zuul/zk/__init__.py b/zuul/zk/__init__.py index c085d21205..f0388b4570 100644 --- a/zuul/zk/__init__.py +++ b/zuul/zk/__init__.py @@ -144,14 +144,17 @@ class ZooKeeperClient(object): self.client.set_hosts(hosts=hosts) @classmethod - def fromConfig(cls, config: ConfigParser) -> "ZooKeeperClient": + def fromConfig(cls, config: ConfigParser, + _require_tls=True) -> "ZooKeeperClient": + # _require_tls is temporary, only used until we move the tests + # to use TLS. hosts = get_default(config, "zookeeper", "hosts") if not hosts: raise Exception("The zookeeper hosts config value is required") tls_key = get_default(config, "zookeeper", "tls_key") tls_cert = get_default(config, "zookeeper", "tls_cert") tls_ca = get_default(config, "zookeeper", "tls_ca") - if not all([tls_key, tls_cert, tls_ca]): + if _require_tls and not all([tls_key, tls_cert, tls_ca]): raise Exception( "A TLS ZooKeeper connection is required; please supply the " "tls_* zookeeper config values."