Resolve conflicts with main

Author: minrk

.pre-commit-config.yaml

@@ -42,7 +42,7 @@ repos:
 
   # Autoformat: markdown, yaml
   - repo: https://github.com/pre-commit/mirrors-prettier
-    rev: v3.0.0-alpha.4
+    rev: v3.0.0-alpha.6
     hooks:
       - id: prettier
 

jupyterhub_traefik_proxy/consul.py

@@ -118,9 +118,6 @@ def _start_traefik(self):
                 self.traefik_env.setdefault("CONSUL_HTTP_TOKEN", self.consul_password)
         super()._start_traefik()
 
-    def _stop_traefik(self):
-        super()._stop_traefik()
-
     async def persist_dynamic_config(self):
         data = self.flatten_dict_for_kv(
             self.dynamic_config, prefix=self.kv_traefik_prefix
@@ -307,6 +304,3 @@ async def _kv_get_jupyterhub_prefixed_entries(self):
         )
 
         return routes["Results"]
-
-    async def stop(self):
-        await super().stop()

jupyterhub_traefik_proxy/etcd.py

@@ -137,8 +137,8 @@ def _default_client(self):
             )
         return etcd3.client(**kwargs)
 
-    def _clean_resources(self):
-        super()._clean_resources()
+    def _cleanup(self):
+        super()._cleanup()
         self.etcd.close()
 
     @run_on_executor

jupyterhub_traefik_proxy/fileprovider.py

@@ -66,11 +66,14 @@ def _load_dynamic_config(self):
         except FileNotFoundError:
             dynamic_config = {}
 
-        if not dynamic_config:
-            dynamic_config = {
-                "http": {"services": {}, "routers": {}},
-                "jupyter": {"routers": {}},
-            }
+        # fill in default keys
+        # use setdefault to ensure these are always fully defined
+        # and never _partially_ defined
+        http = dynamic_config.setdefault("http", {})
+        http.setdefault("services", {})
+        http.setdefault("routers", {})
+        jupyter = dynamic_config.setdefault("jupyter", {})
+        jupyter.setdefault("routers", {})
         return dynamic_config
 
     def persist_dynamic_config(self):
@@ -90,14 +93,15 @@ async def _setup_traefik_static_config(self):
         }
         await super()._setup_traefik_static_config()
 
-    def _clean_resources(self):
+    def _cleanup(self):
+        """Cleanup dynamic config file as well"""
+        super()._cleanup()
         try:
-            if self.should_start:
-                os.remove(self.static_config_file)
             os.remove(self.dynamic_config_file)
-        except:
-            self.log.error("Failed to remove traefik's configuration files")
-            raise
+        except Exception as e:
+            self.log.error(
+                f"Failed to remove traefik configuration file {self.dynamic_config_file}: {e}"
+            )
 
     def _get_route_unsafe(self, traefik_routespec):
         service_alias = traefik_utils.generate_alias(traefik_routespec, "service")
@@ -117,32 +121,10 @@ def _get_route_unsafe(self, traefik_routespec):
             result["data"] = jupyter_routers["data"]
 
         if result["data"] is None and result["target"] is None:
-            self.log.info(f"No route for {routespec} found!")
+            self.log.warning(f"No route for {routespec} found!")
             result = None
         return result
 
-    async def start(self):
-        """Start the proxy.
-
-        Will be called during startup if should_start is True.
-
-        **Subclasses must define this method**
-        if the proxy is to be started by the Hub
-        """
-        await super().start()
-        await self._wait_for_static_config()
-
-    async def stop(self):
-        """Stop the proxy.
-
-        Will be called during teardown if should_start is True.
-
-        **Subclasses must define this method**
-        if the proxy is to be started by the Hub
-        """
-        await super().stop()
-        self._clean_resources()
-
     async def add_route(self, routespec, target, data):
         """Add a route to the proxy.
 

jupyterhub_traefik_proxy/kv_proxy.py

@@ -20,7 +20,6 @@
 
 import asyncio
 import json
-import os
 from collections.abc import MutableMapping
 from functools import wraps
 
@@ -205,14 +204,6 @@ async def _kv_get_jupyterhub_prefixed_entries(self):
 
         raise NotImplementedError()
 
-    def _clean_resources(self):
-        try:
-            if self.should_start:
-                os.remove(self.static_config_file)
-        except:
-            self.log.error("Failed to remove traefik's configuration files")
-            raise
-
     async def _setup_traefik_static_config(self):
         self._define_kv_specific_static_config()
         await super()._setup_traefik_static_config()
@@ -221,20 +212,6 @@ async def _setup_traefik_dynamic_config(self):
         await super()._setup_traefik_dynamic_config()
         await self.persist_dynamic_config()
 
-    async def start(self):
-        """Start the proxy.
-        Will be called during startup if should_start is True.
-        """
-        await super().start()
-        await self._wait_for_static_config()
-
-    async def stop(self):
-        """Stop the proxy.
-        Will be called during teardown if should_start is True.
-        """
-        await super().stop()
-        self._clean_resources()
-
     async def add_route(self, routespec, target, data):
         """Add a route to the proxy.
 
@@ -275,10 +252,10 @@ async def add_route(self, routespec, target, data):
             )
 
         if status:
-            self.log.info(
+            self.log.debug(
                 "Added service %s with the alias %s.", target, route_keys.service_alias
             )
-            self.log.info(
+            self.log.debug(
                 "Added router %s for service %s with the following routing rule %s.",
                 route_keys.router_alias,
                 route_keys.service_alias,
@@ -309,7 +286,7 @@ async def delete_route(self, routespec):
                 jupyterhub_routespec, route_keys
             )
         if status:
-            self.log.info("Routespec %s was deleted.", routespec)
+            self.log.debug("Routespec %s was deleted.", routespec)
         else:
             self.log.error(
                 "Couldn't delete route %s. Response: %s", routespec, response

jupyterhub_traefik_proxy/proxy.py

@@ -288,7 +288,7 @@ async def _check_for_traefik_service(self, routespec, kind):
         return True
 
     async def _wait_for_route(self, routespec):
-        self.log.info(f"Waiting for {routespec} to register with traefik")
+        self.log.debug("Waiting for %s to register with traefik", routespec)
 
         async def _check_traefik_dynamic_conf_ready():
             """Check if traefik loaded its dynamic configuration yet"""
@@ -326,17 +326,26 @@ async def _wait_for_static_config(self):
         async def _check_traefik_static_conf_ready():
             """Check if traefik loaded its static configuration yet"""
             try:
-                resp = await self._traefik_api_request("/api/overview")
-            except Exception:
-                self.log.exception("Error checking for traefik static configuration")
-                return False
-
-            if resp.code != 200:
-                self.log.error(
-                    "Unexpected response code %s checking for traefik static configuration",
-                    resp.code,
+                await self._traefik_api_request("/api/overview")
+            except ConnectionRefusedError:
+                self.log.debug(
+                    f"Connection Refused waiting for traefik at {self.traefik_api_url}. It's probably starting up..."
                 )
                 return False
+            except HTTPClientError as e:
+                if e.code == 599:
+                    self.log.debug(
+                        f"Connection error waiting for traefik at {self.traefik_api_url}. It's probably starting up..."
+                    )
+                    return False
+                if e.code == 404:
+                    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
 
             return True
 
@@ -347,7 +356,7 @@ async def _check_traefik_static_conf_ready():
         )
 
     def _stop_traefik(self):
-        self.log.info("Cleaning up proxy[%i]...", self.traefik_process.pid)
+        self.log.info("Cleaning up traefik proxy [pid=%i]...", self.traefik_process.pid)
         self.traefik_process.terminate()
         try:
             self.traefik_process.communicate(timeout=10)
@@ -363,6 +372,7 @@ def _start_traefik(self):
                 "Configuration mode not supported \n.\
                 The proxy can only be configured through fileprovider, etcd and consul"
             )
+
         env = os.environ.copy()
         env.update(self.traefik_env)
         try:
@@ -419,7 +429,7 @@ async def _setup_traefik_static_config(self):
             raise
 
     async def _setup_traefik_dynamic_config(self):
-        self.log.info("Setting up traefik's dynamic config...")
+        self.log.debug("Setting up traefik's dynamic config...")
         self._generate_htpassword()
         api_url = urlparse(self.traefik_api_url)
         api_path = api_url.path if api_url.path else '/api'
@@ -478,6 +488,7 @@ async def start(self):
         await self._setup_traefik_static_config()
         await self._setup_traefik_dynamic_config()
         self._start_traefik()
+        await self._wait_for_static_config()
 
     async def stop(self):
         """Stop the proxy.
@@ -488,6 +499,20 @@ async def stop(self):
         if the proxy is to be started by the Hub
         """
         self._stop_traefik()
+        self._cleanup()
+
+    def _cleanup(self):
+        """Cleanup after stop
+
+        Extend if there's more to cleanup than the static config file
+        """
+        if self.should_start:
+            try:
+                os.remove(self.static_config_file)
+            except Exception as e:
+                self.log.error(
+                    f"Failed to remove traefik config file {self.static_config_file}: {e}"
+                )
 
     async def add_route(self, routespec, target, data):
         """Add a route to the proxy.