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

@@ -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)

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

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

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

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)
 

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))

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)

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)

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

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."