Merge pull request #184 from minrk/rm-autodiscover

Make traefik_entrypoint name explicit

Author: minrk

jupyterhub_traefik_proxy/proxy.py

@@ -21,6 +21,7 @@
 import asyncio
 import json
 import os
+import ssl
 from os.path import abspath
 from subprocess import Popen, TimeoutExpired
 from urllib.parse import urlparse, urlunparse
@@ -105,6 +106,14 @@ def __init__(self, **kwargs):
             if trait.metadata.get("deprecated_in"):
                 self.observe(self._deprecated_trait, name)
         super().__init__(**kwargs)
+        # not calling .start, make sure we load our initial dynamic config
+        # and that our static config matches what we expect
+        # can't await here because we're not async,
+        # so await in our Proxy methods
+        if not self.should_start:
+            self._start_future = asyncio.ensure_future(self._start_external())
+        else:
+            self._start_future = None
 
     static_config = Dict()
     dynamic_config = Dict()
@@ -319,6 +328,9 @@ async def _check_traefik_static_conf_ready():
             """Check if traefik loaded its static configuration yet"""
             try:
                 await self._traefik_api_request("/api/overview")
+                await self._traefik_api_request(
+                    f"/api/entrypoints/{self.traefik_entrypoint}"
+                )
             except ConnectionRefusedError:
                 self.log.debug(
                     f"Connection Refused waiting for traefik at {self.traefik_api_url}. It's probably starting up..."
@@ -331,13 +343,24 @@ async def _check_traefik_static_conf_ready():
                     )
                     return False
                 if e.code == 404:
-                    self.log.debug(
-                        f"traefik api at {e.response.request.url} overview not ready yet"
-                    )
+                    if "/entrypoints/" in e.response.request.url:
+                        self.log.warning(
+                            f"c.{self.__class__.__name__}.traefik_entrypoint={self.traefik_entrypoint!r} not found in traefik. Is it correct?"
+                        )
+                    else:
+                        self.log.debug(
+                            f"traefik api at {e.response.request.url} overview not ready yet"
+                        )
                     return False
                 # unexpected
                 self.log.error(f"Error checking for traefik static configuration {e}")
                 return False
+            except (OSError, ssl.SSLError) as e:
+                # Can occur if SSL isn't set up yet
+                self.log.warning(
+                    f"SSL Error checking for traefik static configuration: {e}"
+                )
+                return False
 
             return True
 
@@ -459,6 +482,15 @@ async def start(self):
         self._start_traefik()
         await self._wait_for_static_config()
 
+    async def _start_external(self):
+        """Startup function called when `not self.should_start`
+
+        Ensures dynamic config is setup and static config is loaded
+        """
+        await self._setup_traefik_dynamic_config()
+        await self._wait_for_static_config()
+        self._start_future = None
+
     async def stop(self):
         """Stop the proxy.
 
@@ -557,6 +589,8 @@ async def add_route(self, routespec, target, data):
         The proxy implementation should also have a way to associate the fact that a
         route came from JupyterHub.
         """
+        if self._start_future and not self._start_future.done():
+            await self._start_future
         routespec = self.validate_routespec(routespec)
 
         traefik_config, jupyterhub_config = self._dynamic_config_for_route(
@@ -613,6 +647,11 @@ async def _get_jupyterhub_dynamic_config(self):
         """
         raise NotImplementedError()
 
+    async def check_routes(self, *args, **kwargs):
+        if self._start_future and not self._start_future.done():
+            await self._start_future
+        return await super().check_routes(*args, **kwargs)
+
     async def get_all_routes(self):
         """Fetch and return all the routes associated by JupyterHub from the
         proxy.
@@ -628,6 +667,9 @@ async def get_all_routes(self):
             'data': the attached data dict for this route (as specified in add_route)
           }
         """
+        if self._start_future and not self._start_future.done():
+            await self._start_future
+
         jupyterhub_config = await self._get_jupyterhub_dynamic_config()
 
         all_routes = {}

tests/config_files/dynamic_config/dynamic_conf.toml

@@ -7,7 +7,7 @@ level = "debug"
 providersThrottleDuration = "0s"
 
 [providers.file]
-directory = "./tests/config_files/dynamic_config"
+directory = "/tmp/jupyterhub-traefik-proxy-test"
 watch = true
 
 [entryPoints.websecure]

tests/config_files/traefik.toml

@@ -11,6 +11,7 @@
 from tempfile import TemporaryDirectory
 
 import pytest
+import utils
 from consul.aio import Consul
 from jupyterhub.utils import exponential_backoff
 from traitlets.log import get_logger
@@ -78,6 +79,17 @@ def pytest_configure(config):
     config.addinivalue_line("markers", "slow: marks tests as slow.")
 
 
+@pytest.fixture
+def dynamic_config_dir():
+    # matches traefik.toml
+    path = Path("/tmp/jupyterhub-traefik-proxy-test")
+    if path.exists():
+        shutil.rmtree(path)
+    path.mkdir()
+    yield path
+    shutil.rmtree(path)
+
+
 @pytest.fixture
 async def no_auth_consul_proxy(launch_consul):
     """
@@ -181,9 +193,9 @@ def traitlets_log():
 
 # There must be a way to parameterise this to run on both yaml and toml files?
 @pytest.fixture
-async def file_proxy_toml():
+async def file_proxy_toml(dynamic_config_dir):
     """Fixture returning a configured TraefikFileProviderProxy"""
-    dynamic_config_file = os.path.join(config_files, "dynamic_config", "rules.toml")
+    dynamic_config_file = str(dynamic_config_dir / "rules.toml")
     static_config_file = "traefik.toml"
     proxy = _file_proxy(
         dynamic_config_file, static_config_file=static_config_file, should_start=True
@@ -194,8 +206,8 @@ async def file_proxy_toml():
 
 
 @pytest.fixture
-async def file_proxy_yaml():
-    dynamic_config_file = os.path.join(config_files, "dynamic_config", "rules.yaml")
+async def file_proxy_yaml(dynamic_config_dir):
+    dynamic_config_file = str(dynamic_config_dir / "rules.yaml")
     static_config_file = "traefik.yaml"
     proxy = _file_proxy(
         dynamic_config_file, static_config_file=static_config_file, should_start=True
@@ -218,16 +230,16 @@ def _file_proxy(dynamic_config_file, **kwargs):
 
 
 @pytest.fixture
-async def external_file_proxy_yaml(launch_traefik_file):
-    dynamic_config_file = os.path.join(config_files, "dynamic_config", "rules.yaml")
+async def external_file_proxy_yaml(launch_traefik_file, dynamic_config_dir):
+    dynamic_config_file = str(dynamic_config_dir / "rules.yaml")
     proxy = _file_proxy(dynamic_config_file, should_start=False)
     yield proxy
     os.remove(dynamic_config_file)
 
 
 @pytest.fixture
-async def external_file_proxy_toml(launch_traefik_file):
-    dynamic_config_file = os.path.join(config_files, "dynamic_config", "rules.toml")
+async def external_file_proxy_toml(launch_traefik_file, dynamic_config_dir):
+    dynamic_config_file = str(dynamic_config_dir / "rules.toml")
     proxy = _file_proxy(dynamic_config_file, should_start=False)
     yield proxy
     os.remove(dynamic_config_file)
@@ -277,6 +289,36 @@ async def auth_external_etcd_proxy(
     proxy.etcd.close()
 
 
+@pytest.fixture(
+    params=[
+        "no_auth_consul_proxy",
+        "auth_consul_proxy",
+        "no_auth_etcd_proxy",
+        "auth_etcd_proxy",
+        "file_proxy_toml",
+        "file_proxy_yaml",
+        "external_consul_proxy",
+        "auth_external_consul_proxy",
+        "external_etcd_proxy",
+        "auth_external_etcd_proxy",
+        "external_file_proxy_toml",
+        "external_file_proxy_yaml",
+    ]
+)
+def proxy(request):
+    """Parameterized fixture to run all the tests with every proxy implementation"""
+    proxy = request.getfixturevalue(request.param)
+    # wait for public endpoint to be reachable
+    asyncio.run(
+        exponential_backoff(
+            utils.check_host_up_http,
+            f"Proxy public url {proxy.public_url} cannot be reached",
+            url=proxy.public_url,
+        )
+    )
+    return proxy
+
+
 #########################################################################
 # Fixtures for launching traefik, with each backend and with or without #
 # authentication                                                        #

tests/conftest.py

@@ -138,36 +138,6 @@ async def _launch_backends(n=1):
         proc.wait()
 
 
-@pytest.fixture(
-    params=[
-        "no_auth_consul_proxy",
-        "auth_consul_proxy",
-        "no_auth_etcd_proxy",
-        "auth_etcd_proxy",
-        "file_proxy_toml",
-        "file_proxy_yaml",
-        "external_consul_proxy",
-        "auth_external_consul_proxy",
-        "external_etcd_proxy",
-        "auth_external_etcd_proxy",
-        "external_file_proxy_toml",
-        "external_file_proxy_yaml",
-    ]
-)
-def proxy(request):
-    """Parameterized fixture to run all the tests with every proxy implementation"""
-    proxy = request.getfixturevalue(request.param)
-    # wait for public endpoint to be reachable
-    asyncio.run(
-        exponential_backoff(
-            utils.check_host_up_http,
-            f"Proxy public url {proxy.public_url} cannot be reached",
-            url=proxy.public_url,
-        )
-    )
-    return proxy
-
-
 async def wait_for_services(urls):
     # Wait until traefik and the backend are ready
     await exponential_backoff(

tests/test_proxy.py

@@ -7,20 +7,6 @@
 pytestmark = pytest.mark.asyncio
 
 
-@pytest.fixture(
-    params=[
-        "file_proxy_toml",
-        "file_proxy_yaml",
-        "no_auth_etcd_proxy",
-        "auth_etcd_proxy",
-        "no_auth_consul_proxy",
-        "auth_consul_proxy",
-    ]
-)
-def proxy(request):
-    return request.getfixturevalue(request.param)
-
-
 @pytest.mark.parametrize(
     "username, password, expected_rc",
     [("api_admin", "admin", 200), ("api_admin", "1234", 401), ("", "", 401)],