From 9160ea9b05819e2538d9fdaf73b3e719e73f1b94 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 26 Jan 2022 15:23:26 -0700
Subject: [PATCH 01/56] Change kubernetes to kubernetes_asyncio in imports

---
 kubespawner/clients.py                |  4 +-
 kubespawner/objects.py                | 80 +++++++++++++--------------
 kubespawner/proxy.py                  |  2 +-
 kubespawner/reflector.py              |  4 +-
 kubespawner/spawner.py                |  2 +-
 setup.py                              |  2 +-
 tests/conftest.py                     | 27 ++++-----
 tests/test_multi_namespace_spawner.py | 12 ++--
 tests/test_objects.py                 |  2 +-
 tests/test_spawner.py                 | 10 ++--
 tests/test_utils.py                   | 10 ++--
 11 files changed, 78 insertions(+), 77 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index cd0dc6a6..4f7a9d68 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -6,8 +6,8 @@
 import weakref
 from unittest.mock import Mock
 
-import kubernetes.client
-from kubernetes.client import api_client
+import kubernetes_asyncio.client
+from kubernetes_asyncio.client import api_client
 
 # FIXME: remove when instantiating a kubernetes client
 # doesn't create N-CPUs threads unconditionally.
diff --git a/kubespawner/objects.py b/kubespawner/objects.py
index 800f1c27..e5b7d6ee 100644
--- a/kubespawner/objects.py
+++ b/kubespawner/objects.py
@@ -9,47 +9,47 @@
 import re
 from urllib.parse import urlparse
 
-from kubernetes.client.models import V1Affinity
-from kubernetes.client.models import V1Container
-from kubernetes.client.models import V1ContainerPort
-from kubernetes.client.models import V1EndpointAddress
-from kubernetes.client.models import V1Endpoints
-from kubernetes.client.models import V1EndpointSubset
-from kubernetes.client.models import V1EnvVar
-from kubernetes.client.models import V1LabelSelector
-from kubernetes.client.models import V1Lifecycle
-from kubernetes.client.models import V1LocalObjectReference
-from kubernetes.client.models import V1Namespace
-from kubernetes.client.models import V1NodeAffinity
-from kubernetes.client.models import V1NodeSelector
-from kubernetes.client.models import V1NodeSelectorRequirement
-from kubernetes.client.models import V1NodeSelectorTerm
-from kubernetes.client.models import V1ObjectMeta
-from kubernetes.client.models import V1OwnerReference
-from kubernetes.client.models import V1PersistentVolumeClaim
-from kubernetes.client.models import V1PersistentVolumeClaimSpec
-from kubernetes.client.models import V1Pod
-from kubernetes.client.models import V1PodAffinity
-from kubernetes.client.models import V1PodAffinityTerm
-from kubernetes.client.models import V1PodAntiAffinity
-from kubernetes.client.models import V1PodSecurityContext
-from kubernetes.client.models import V1PodSpec
-from kubernetes.client.models import V1PreferredSchedulingTerm
-from kubernetes.client.models import V1ResourceRequirements
-from kubernetes.client.models import V1Secret
-from kubernetes.client.models import V1SecurityContext
-from kubernetes.client.models import V1Service
-from kubernetes.client.models import V1ServicePort
-from kubernetes.client.models import V1ServiceSpec
-from kubernetes.client.models import V1Toleration
-from kubernetes.client.models import V1Volume
-from kubernetes.client.models import V1VolumeMount
-from kubernetes.client.models import V1WeightedPodAffinityTerm
+from kubernetes_asyncio.client.models import V1Affinity
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1ContainerPort
+from kubernetes_asyncio.client.models import V1EndpointAddress
+from kubernetes_asyncio.client.models import V1Endpoints
+from kubernetes_asyncio.client.models import V1EndpointSubset
+from kubernetes_asyncio.client.models import V1EnvVar
+from kubernetes_asyncio.client.models import V1LabelSelector
+from kubernetes_asyncio.client.models import V1Lifecycle
+from kubernetes_asyncio.client.models import V1LocalObjectReference
+from kubernetes_asyncio.client.models import V1Namespace
+from kubernetes_asyncio.client.models import V1NodeAffinity
+from kubernetes_asyncio.client.models import V1NodeSelector
+from kubernetes_asyncio.client.models import V1NodeSelectorRequirement
+from kubernetes_asyncio.client.models import V1NodeSelectorTerm
+from kubernetes_asyncio.client.models import V1ObjectMeta
+from kubernetes_asyncio.client.models import V1OwnerReference
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaimSpec
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1PodAffinity
+from kubernetes_asyncio.client.models import V1PodAffinityTerm
+from kubernetes_asyncio.client.models import V1PodAntiAffinity
+from kubernetes_asyncio.client.models import V1PodSecurityContext
+from kubernetes_asyncio.client.models import V1PodSpec
+from kubernetes_asyncio.client.models import V1PreferredSchedulingTerm
+from kubernetes_asyncio.client.models import V1ResourceRequirements
+from kubernetes_asyncio.client.models import V1Secret
+from kubernetes_asyncio.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import V1Service
+from kubernetes_asyncio.client.models import V1ServicePort
+from kubernetes_asyncio.client.models import V1ServiceSpec
+from kubernetes_asyncio.client.models import V1Toleration
+from kubernetes_asyncio.client.models import V1Volume
+from kubernetes_asyncio.client.models import V1VolumeMount
+from kubernetes_asyncio.client.models import V1WeightedPodAffinityTerm
 
 try:
-    from kubernetes.client.models import CoreV1EndpointPort
+    from kubernetes_asyncio.client.models import CoreV1EndpointPort
 except ImportError:
-    from kubernetes.client.models import V1EndpointPort as CoreV1EndpointPort
+    from kubernetes_asyncio.client.models import V1EndpointPort as CoreV1EndpointPort
 
 from kubespawner.utils import get_k8s_model
 from kubespawner.utils import update_k8s_model
@@ -734,7 +734,7 @@ def make_ingress(name, routespec, target, labels, data):
     # to keep compatibility with older K8S versions
 
     try:
-        from kubernetes.client.models import (
+        from kubernetes_asyncio.client.models import (
             ExtensionsV1beta1Ingress,
             ExtensionsV1beta1IngressSpec,
             ExtensionsV1beta1IngressRule,
@@ -743,7 +743,7 @@ def make_ingress(name, routespec, target, labels, data):
             ExtensionsV1beta1IngressBackend,
         )
     except ImportError:
-        from kubernetes.client.models import (
+        from kubernetes_asyncio.client.models import (
             V1beta1Ingress as ExtensionsV1beta1Ingress,
             V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
             V1beta1IngressRule as ExtensionsV1beta1IngressRule,
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index b52e8253..38acb7d2 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -7,7 +7,7 @@
 import kubernetes.config
 from jupyterhub.proxy import Proxy
 from jupyterhub.utils import exponential_backoff
-from kubernetes import client
+from kubernetes_asyncio import client
 from tornado import gen
 from tornado.concurrent import run_on_executor
 from traitlets import Unicode
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index ac0c61d3..dddd99ce 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -6,8 +6,8 @@
 from concurrent.futures import Future
 from functools import partial
 
-from kubernetes import config
-from kubernetes import watch
+from kubernetes_asyncio import config
+from kubernetes_asyncio import watch
 from traitlets import Any
 from traitlets import Bool
 from traitlets import Dict
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 6c9e9d26..815377dd 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -17,7 +17,7 @@
 from urllib.parse import urlparse
 
 import escapism
-import kubernetes.config
+import kubernetes_asyncio.config
 from jinja2 import BaseLoader
 from jinja2 import Environment
 from jupyterhub.spawner import Spawner
diff --git a/setup.py b/setup.py
index 0c304b00..c8068345 100644
--- a/setup.py
+++ b/setup.py
@@ -20,7 +20,7 @@
         'python-slugify',
         'jupyterhub>=0.8',
         'jinja2',
-        'kubernetes>=10.1.0',
+        'kubernetes_asyncio>=10.1.0',
         'urllib3',
         'pyYAML',
     ],
diff --git a/tests/conftest.py b/tests/conftest.py
index d70b8d09..3a852a63 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -11,22 +11,23 @@
 from functools import partial
 from threading import Thread
 
-import kubernetes
+import kubernetes_asyncio
 import pytest
 from jupyterhub.app import JupyterHub
 from jupyterhub.objects import Hub
-from kubernetes.client import V1ConfigMap
-from kubernetes.client import V1Namespace
-from kubernetes.client import V1Pod
-from kubernetes.client import V1PodSpec
-from kubernetes.client import V1Secret
-from kubernetes.client import V1Service
-from kubernetes.client import V1ServicePort
-from kubernetes.client import V1ServiceSpec
-from kubernetes.client.rest import ApiException
-from kubernetes.config import load_kube_config
-from kubernetes.stream import stream
-from kubernetes.watch import Watch
+from kubernetes_asyncio.client import V1ConfigMap
+from kubernetes_asyncio.client import V1Namespace
+from kubernetes_asyncio.client import V1Pod
+from kubernetes_asyncio.client import V1PodSpec
+from kubernetes_asyncio.client import V1Secret
+from kubernetes_asyncio.client import V1Service
+from kubernetes_asyncio.client import V1ServicePort
+from kubernetes_asyncio.client import V1ServiceSpec
+from kubernetes_asyncio.client.rest import ApiException
+from kubernetes_asyncio.config import load_kube_config
+#from kubernetes.stream import stream
+from kubernetes_asyncio.stream import stream
+from kubernetes_asyncio.watch import Watch
 from traitlets.config import Config
 
 from kubespawner.clients import shared_client
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index f9742455..47c77c74 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -7,12 +7,12 @@
 from jupyterhub.objects import Hub
 from jupyterhub.objects import Server
 from jupyterhub.orm import Spawner
-from kubernetes.client import V1Namespace
-from kubernetes.client.models import V1Capabilities
-from kubernetes.client.models import V1Container
-from kubernetes.client.models import V1Pod
-from kubernetes.client.models import V1SecurityContext
-from kubernetes.config import load_kube_config
+from kubernetes_asyncio.client import V1Namespace
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1SecurityContext
+from kubernetes_asyncio.config import load_kube_config
 from traitlets.config import Config
 
 from kubespawner import KubeSpawner
diff --git a/tests/test_objects.py b/tests/test_objects.py
index 8e7310e6..dcaab1b3 100644
--- a/tests/test_objects.py
+++ b/tests/test_objects.py
@@ -2,7 +2,7 @@
 Test functions used to create k8s objects
 """
 import pytest
-from kubernetes.client import ApiClient
+from kubernetes_asyncio.client import ApiClient
 
 from kubespawner.objects import make_ingress
 from kubespawner.objects import make_namespace
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index bf2d04bf..df4dce35 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -7,11 +7,11 @@
 from jupyterhub.objects import Hub
 from jupyterhub.objects import Server
 from jupyterhub.orm import Spawner
-from kubernetes.client.models import V1Capabilities
-from kubernetes.client.models import V1Container
-from kubernetes.client.models import V1PersistentVolumeClaim
-from kubernetes.client.models import V1Pod
-from kubernetes.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1SecurityContext
 from traitlets.config import Config
 
 from kubespawner import KubeSpawner
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 129b1868..0e8c3425 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -2,11 +2,11 @@
 
 import pytest
 from conftest import ExecError
-from kubernetes.client.models import V1Capabilities
-from kubernetes.client.models import V1Container
-from kubernetes.client.models import V1Lifecycle
-from kubernetes.client.models import V1PodSpec
-from kubernetes.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1Lifecycle
+from kubernetes_asyncio.client.models import V1PodSpec
+from kubernetes_asyncio.client.models import V1SecurityContext
 
 from kubespawner.utils import _get_k8s_model_attribute
 from kubespawner.utils import get_k8s_model

From b921ffdcb4a510078f536bf236f509c4cbfa5c65 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 26 Jan 2022 17:20:34 -0700
Subject: [PATCH 02/56] remove asynchronize() and gen.with_timeout()

---
 kubespawner/clients.py   |  19 ++++++
 kubespawner/proxy.py     |  58 ++++------------
 kubespawner/reflector.py |  27 +++++---
 kubespawner/spawner.py   | 142 +++++++++++++++------------------------
 4 files changed, 107 insertions(+), 139 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 4f7a9d68..9278c8f9 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -46,3 +46,22 @@ def shared_client(ClientType, *args, **kwargs):
         # cache weakref so that clients can be garbage collected
         _client_cache[cache_key] = weakref.ref(client)
     return client
+
+
+async def set_k8s_client_configuration(client=None):
+    # Call this prior to using a client for readability /
+    # coupling with traitlets values.
+    try:
+	kubernetes_asyncio.config.load_incluster_config()
+    except kubernetes_asyncio.config.ConfigException:
+	await kubernetes_asyncio.config.load_kube_config()
+    if not client:
+	return
+    if client.k8s_api_ssl_ca_cert:
+	global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
+	global_conf.ssl_ca_cert = client.k8s_api_ssl_ca_cert
+	kubernetes_asyncio.client.Configuration.set_default(global_conf)
+    if client.k8s_api_host:
+	global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
+	global_conf.host = client.k8s_api_host
+	kubernetes_asyncio.client.Configuration.set_default(global_conf)
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 38acb7d2..39703ca2 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -113,7 +113,6 @@ def __init__(self, *args, **kwargs):
         self.executor = ThreadPoolExecutor(max_workers=self.app.concurrent_spawn_limit)
 
         # Global configuration before reflector.py code runs
-        self._set_k8s_client_configuration()
         self.core_api = shared_client('CoreV1Api')
         self.extension_api = shared_client('ExtensionsV1beta1Api')
 
@@ -121,40 +120,16 @@ def __init__(self, *args, **kwargs):
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
         }
-        self.ingress_reflector = IngressReflector(
+        self.ingress_reflector = IngressReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.service_reflector = ServiceReflector(
+        self.service_reflector = ServiceReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.endpoint_reflector = EndpointsReflector(
+        self.endpoint_reflector = EndpointsReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
 
-    def _set_k8s_client_configuration(self):
-        # The actual (singleton) Kubernetes client will be created
-        # in clients.py shared_client but the configuration
-        # for token / ca_cert / k8s api host is set globally
-        # in kubernetes.py syntax.  It is being set here
-        # and this method called prior to shared_client
-        # for readability / coupling with traitlets values
-        try:
-            kubernetes.config.load_incluster_config()
-        except kubernetes.config.ConfigException:
-            kubernetes.config.load_kube_config()
-        if self.k8s_api_ssl_ca_cert:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
-            client.Configuration.set_default(global_conf)
-        if self.k8s_api_host:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.host = self.k8s_api_host
-            client.Configuration.set_default(global_conf)
-
-    @run_on_executor
-    def asynchronize(self, method, *args, **kwargs):
-        return method(*args, **kwargs)
-
     def safe_name_for_routespec(self, routespec):
         safe_chars = set(string.ascii_lowercase + string.digits)
         safe_name = generate_hashed_slug(
@@ -189,9 +164,7 @@ async def add_route(self, routespec, target, data):
 
         async def ensure_object(create_func, patch_func, body, kind):
             try:
-                resp = await self.asynchronize(
-                    create_func, namespace=self.namespace, body=body
-                )
+                resp = await create_func(namespace=self.namespace, body=body),
                 self.log.info('Created %s/%s', kind, safe_name)
             except client.rest.ApiException as e:
                 if e.status == 409:
@@ -199,8 +172,7 @@ async def ensure_object(create_func, patch_func, body, kind):
                     self.log.warn(
                         "Trying to patch %s/%s, it already exists", kind, safe_name
                     )
-                    resp = await self.asynchronize(
-                        patch_func,
+                    resp = await patch_func(
                         namespace=self.namespace,
                         body=body,
                         name=body.metadata.name,
@@ -222,8 +194,7 @@ async def ensure_object(create_func, patch_func, body, kind):
                 'Could not find endpoints/%s after creating it' % safe_name,
             )
         else:
-            delete_endpoint = self.asynchronize(
-                self.core_api.delete_namespaced_endpoints,
+            delete_endpoint = await (self.core_api.delete_namespaced_endpoints(
                 name=safe_name,
                 namespace=self.namespace,
                 body=client.V1DeleteOptions(grace_period_seconds=0),
@@ -264,22 +235,19 @@ async def delete_route(self, routespec):
 
         delete_options = client.V1DeleteOptions(grace_period_seconds=0)
 
-        delete_endpoint = self.asynchronize(
-            self.core_api.delete_namespaced_endpoints,
+        delete_endpoint = await self.core_api.delete_namespaced_endpoints(
             name=safe_name,
             namespace=self.namespace,
             body=delete_options,
         )
 
-        delete_service = self.asynchronize(
-            self.core_api.delete_namespaced_service,
+        delete_service = await self.core_api.delete_namespaced_service(
             name=safe_name,
             namespace=self.namespace,
             body=delete_options,
         )
 
-        delete_ingress = self.asynchronize(
-            self.extension_api.delete_namespaced_ingress,
+        delete_ingress = await self.extension_api.delete_namespaced_ingress(
             name=safe_name,
             namespace=self.namespace,
             body=delete_options,
@@ -293,9 +261,11 @@ async def delete_route(self, routespec):
         # explicitly ourselves as well. In the future, we can probably try a
         # foreground cascading deletion (https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion)
         # instead, but for now this works well enough.
-        await self.delete_if_exists('endpoint', safe_name, delete_endpoint)
-        await self.delete_if_exists('service', safe_name, delete_service)
-        await self.delete_if_exists('ingress', safe_name, delete_ingress)
+        await gather(
+            self.delete_if_exists('endpoint', safe_name, delete_endpoint),
+            self.delete_if_exists('service', safe_name, delete_service),
+            self.delete_if_exists('ingress', safe_name, delete_ingress),
+        )
 
     async def get_all_routes(self):
         # copy everything, because iterating over this directly is not threadsafe
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index dddd99ce..67166dc7 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -16,7 +16,7 @@
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import shared_client
+from .clients import shared_client, set_k8s_client_configuration()
 
 # This is kubernetes client implementation specific, but we need to know
 # whether it was a network or watch timeout.
@@ -158,7 +158,9 @@ class ResourceReflector(LoggingConfigurable):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        # client configuration for kubernetes has already taken place
+        # client configuration for kubernetes may not have taken place,
+        # because it is asynchronous.  We will do it when we create the
+        # reflector from its reflector() classmethod.
         self.api = shared_client(self.api_group_name)
 
         # FIXME: Protect against malicious labels?
@@ -203,12 +205,21 @@ def __init__(self, *args, **kwargs):
         if not self.list_method_name:
             raise RuntimeError("Reflector list_method_name must be set!")
 
-        self.start()
+        # start is now asynchronous.  That means that the way to create a
+        # reflector is now with a classmethod (called "reflector") that
+        # awaits the start().
+
+    @classmethod
+    def reflector(cls, *args, **kwargs):
+        inst=cls(*args, **kwargs)
+	await set_k8s_client_configuration()
+	await inst.start()
+	return inst
 
     def __del__(self):
         self.stop()
 
-    def _list_and_update(self):
+    async def _list_and_update(self):
         """
         Update current list of resources by doing a full fetch.
 
@@ -234,7 +245,7 @@ def _list_and_update(self):
         # return the resource version so we can hook up a watch
         return initial_resources["metadata"]["resourceVersion"]
 
-    def _watch_and_update(self):
+    async def _watch_and_update(self):
         """
         Keeps the current list of resources up-to-date
 
@@ -362,7 +373,7 @@ def _watch_and_update(self):
                     break
         self.log.warning("%s watcher finished", self.kind)
 
-    def start(self):
+    async def start(self):
         """
         Start the reflection process!
 
@@ -381,10 +392,10 @@ def start(self):
         self.watch_thread.daemon = True
         self.watch_thread.start()
 
-    def stop(self):
+    async def stop(self):
         self._stop_event.set()
 
-    def stopped(self):
+    async def stopped(self):
         return self._stop_event.is_set()
 
 
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 815377dd..6c2ddb5b 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -39,7 +39,7 @@
 from traitlets import Union
 from traitlets import validate
 
-from .clients import shared_client
+from .clients import shared_client, set_k8s_client_configuration
 from .objects import make_namespace
 from .objects import make_owner_reference
 from .objects import make_pod
@@ -200,16 +200,13 @@ def __init__(self, *args, **kwargs):
                     max_workers=self.k8s_api_threadpool_workers
                 )
 
-            # Set global kubernetes client configurations
-            # before reflector.py code runs
-            self._set_k8s_client_configuration()
             self.api = shared_client('CoreV1Api')
 
+            # Starting our watchers is now async, so we cannot do it in
+            # __init()__.
+
             # This will start watching in __init__, so it'll start the first
             # time any spawner object is created. Not ideal but works!
-            self._start_watching_pods()
-            if self.events_enabled:
-                self._start_watching_events()
 
         # runs during both test and normal execution
         self.pod_name = self._expand_user_properties(self.pod_name_template)
@@ -227,25 +224,12 @@ def __init__(self, *args, **kwargs):
         # The attribute needs to exist, even though it is unset to start with
         self._start_future = None
 
-    def _set_k8s_client_configuration(self):
-        # The actual (singleton) Kubernetes client will be created
-        # in clients.py shared_client but the configuration
-        # for token / ca_cert / k8s api host is set globally
-        # in kubernetes.py syntax.  It is being set here
-        # and this method called prior to shared_client
-        # for readability / coupling with traitlets values
-        try:
-            kubernetes.config.load_incluster_config()
-        except kubernetes.config.ConfigException:
-            kubernetes.config.load_kube_config()
-        if self.k8s_api_ssl_ca_cert:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
-            client.Configuration.set_default(global_conf)
-        if self.k8s_api_host:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.host = self.k8s_api_host
-            client.Configuration.set_default(global_conf)
+    async def _initialize_reflectors_and_clients(self):
+        await set_client_k8s_configuration()
+        await self._start_watching_pods()
+        if self.events_enabled:
+            await self._start_watching_events()
+
 
     k8s_api_ssl_ca_cert = Unicode(
         "",
@@ -2067,7 +2051,12 @@ async def poll(self):
         Note that a clean exit will have an exit code of zero, so it is
         necessary to check that the returned value is None, rather than
         just Falsy, to determine that the pod is still running.
+
+        We cannot be sure the Hub will call start() before poll(), so
+        we need to load client configuration and start our reflectors
+        at the top of each of those methods.
         """
+        await self._initialize_reflectors_and_clients()
         # have to wait for first load of data before we have a valid answer
         if not self.pod_reflector.first_load_future.done():
             await asyncio.wrap_future(self.pod_reflector.first_load_future)
@@ -2125,10 +2114,6 @@ def _normalize_url(url):
         # pod doesn't exist or has been deleted
         return 1
 
-    @run_on_executor
-    def asynchronize(self, method, *args, **kwargs):
-        return method(*args, **kwargs)
-
     @property
     def events(self):
         """Filter event-reflector to just this pods events
@@ -2327,17 +2312,14 @@ async def _make_create_pod_request(self, pod, request_timeout):
             self.log.info(
                 f"Attempting to create pod {pod.metadata.name}, with timeout {request_timeout}"
             )
-            # Use tornado's timeout, _request_timeout seems unreliable?
-            await gen.with_timeout(
-                timedelta(seconds=request_timeout),
-                self.asynchronize(
-                    self.api.create_namespaced_pod,
+            # Use asyncio.wait_for, _request_timeout seems unreliable?
+            await asyncio.wait_for(
+                self.api.create_namespaced_pod(
                     self.namespace,
-                    pod,
-                ),
-            )
+                    pod),
+                request_timeout)
             return True
-        except gen.TimeoutError:
+        except asyncio.TimeoutError:
             # Just try again
             return False
         except ApiException as e:
@@ -2369,16 +2351,14 @@ async def _make_create_pvc_request(self, pvc, request_timeout):
             self.log.info(
                 f"Attempting to create pvc {pvc.metadata.name}, with timeout {request_timeout}"
             )
-            await gen.with_timeout(
-                timedelta(seconds=request_timeout),
-                self.asynchronize(
-                    self.api.create_namespaced_persistent_volume_claim,
+            await asyncio.wait_for(
+                self.api.create_namespaced_persistent_volume_claim(
                     namespace=self.namespace,
                     body=pvc,
                 ),
-            )
+                request_timeout)
             return True
-        except gen.TimeoutError:
+        except asyncio.TimeoutError:
             # Just try again
             return False
         except ApiException as e:
@@ -2391,8 +2371,7 @@ async def _make_create_pvc_request(self, pvc, request_timeout):
                 t, v, tb = sys.exc_info()
 
                 try:
-                    await self.asynchronize(
-                        self.api.read_namespaced_persistent_volume_claim,
+                    await self.api.read_namespaced_persistent_volume_claim(
                         name=pvc_name,
                         namespace=self.namespace,
                     )
@@ -2423,11 +2402,10 @@ async def _ensure_not_exists(self, kind, name):
         # first, attempt to delete the resource
         try:
             self.log.info(f"Deleting {kind}/{name}")
-            await gen.with_timeout(
-                timedelta(seconds=self.k8s_api_request_timeout),
-                self.asynchronize(delete, namespace=self.namespace, name=name),
-            )
-        except gen.TimeoutError:
+            await asyncio.wait_for(
+                delete(namespace=self.namespace, name=name),
+                self.k8s_api_request_timeout)
+        except asyncio.TimeoutError:
             # Just try again
             return False
         except ApiException as e:
@@ -2440,11 +2418,10 @@ async def _ensure_not_exists(self, kind, name):
 
         try:
             self.log.info(f"Checking for {kind}/{name}")
-            await gen.with_timeout(
-                timedelta(seconds=self.k8s_api_request_timeout),
-                self.asynchronize(read, namespace=self.namespace, name=name),
-            )
-        except gen.TimeoutError:
+            await asyncio.wait_for(
+                read(namespace=self.namespace, name=name),
+                self.k8s_api_request_timeout)
+        except asyncio.TimeoutError:
             # Just try again
             return False
         except ApiException as e:
@@ -2465,16 +2442,10 @@ async def _make_create_resource_request(self, kind, manifest):
         create = getattr(self.api, f"create_namespaced_{kind}")
         self.log.info(f"Attempting to create {kind} {manifest.metadata.name}")
         try:
-            # Use tornado's timeout, _request_timeout seems unreliable?
-            await gen.with_timeout(
-                timedelta(seconds=self.k8s_api_request_timeout),
-                self.asynchronize(
-                    create,
-                    self.namespace,
-                    manifest,
-                ),
-            )
-        except gen.TimeoutError:
+            await asyncio.wait_for(
+                create(self.namespace,manifest),
+                self.k8s_api_request_timeout)
+        except asyncio.TimeoutError:
             # Just try again
             return False
         except ApiException as e:
@@ -2494,6 +2465,10 @@ async def _start(self):
         # load user options (including profile)
         await self.load_user_options()
 
+        # Start watchers.  This might also be called from poll().  It also
+        #  configures our API clients.
+        await self._initialize_reflectors_and_clients()
+
         # If we have user_namespaces enabled, create the namespace.
         #  It's fine if it already exists.
         if self.enable_user_namespaces:
@@ -2644,18 +2619,15 @@ async def _make_delete_pod_request(
         ref_key = "{}/{}".format(self.namespace, pod_name)
         self.log.info("Deleting pod %s", ref_key)
         try:
-            await gen.with_timeout(
-                timedelta(seconds=request_timeout),
-                self.asynchronize(
-                    self.api.delete_namespaced_pod,
+            await asyncio.wait_for(
+                self.api.delete_namespaced_pod(
                     name=pod_name,
                     namespace=self.namespace,
                     body=delete_options,
-                    grace_period_seconds=grace_seconds,
-                ),
-            )
+                    grace_period_seconds=grace_seconds)
+                request_timeout)
             return True
-        except gen.TimeoutError:
+        except asyncio.TimeoutError:
             return False
         except ApiException as e:
             if e.status == 404:
@@ -2677,16 +2649,13 @@ async def _make_delete_pvc_request(self, pvc_name, request_timeout):
         """
         self.log.info("Deleting pvc %s", pvc_name)
         try:
-            await gen.with_timeout(
-                timedelta(seconds=request_timeout),
-                self.asynchronize(
-                    self.api.delete_namespaced_persistent_volume_claim,
+            await asyncio.wait_for(
+                self.api.delete_namespaced_persistent_volume_claim(
                     name=pvc_name,
-                    namespace=self.namespace,
-                ),
-            )
+                    namespace=self.namespace),
+                request_timeout)
             return True
-        except gen.TimeoutError:
+        except asyncio.TimeoutError:
             return False
         except ApiException as e:
             if e.status == 404:
@@ -2892,10 +2861,9 @@ async def _ensure_namespace(self):
         ns = make_namespace(self.namespace)
         api = self.api
         try:
-            await gen.with_timeout(
-                timedelta(seconds=self.k8s_api_request_timeout),
-                self.asynchronize(api.create_namespace, ns),
-            )
+            await asyncio.wait_for(
+                api.create_namespace(ns),
+                self.k8s_api_request_timeout)
         except ApiException as e:
             if e.status != 409:
                 # It's fine if it already exists

From 31eb2a99509e866a95bed88baa01f3232fa0e9ff Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 27 Jan 2022 14:14:11 -0700
Subject: [PATCH 03/56] Basic multinamespace functionality works

---
 kubespawner/clients.py                |  33 +++----
 kubespawner/proxy.py                  |  18 ++--
 kubespawner/reflector.py              | 125 ++++++++++++++------------
 kubespawner/spawner.py                |  26 +++---
 tests/conftest.py                     |   2 +-
 tests/test_multi_namespace_spawner.py |  16 ++--
 6 files changed, 122 insertions(+), 98 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 9278c8f9..b40da7e6 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -20,8 +20,7 @@
 
 _client_cache = {}
 
-
-def shared_client(ClientType, *args, **kwargs):
+async def shared_client(ClientType, *args, **kwargs):
     """Return a single shared kubernetes client instance
 
     A weak reference to the instance is cached,
@@ -41,27 +40,31 @@ def shared_client(ClientType, *args, **kwargs):
         # Kubernetes client configuration is handled globally
         # in kubernetes.py and is already called in spawner.py
         # or proxy.py prior to a shared_client being instantiated
-        Client = getattr(kubernetes.client, ClientType)
+        await load_config()
+        Client = getattr(kubernetes_asyncio.client, ClientType)
         client = Client(*args, **kwargs)
         # cache weakref so that clients can be garbage collected
         _client_cache[cache_key] = weakref.ref(client)
+        
     return client
 
+async def load_config():
+    try:
+        kubernetes_asyncio.config.load_incluster_config()
+    except kubernetes_asyncio.config.ConfigException:
+        await kubernetes_asyncio.config.load_kube_config()
 
 async def set_k8s_client_configuration(client=None):
     # Call this prior to using a client for readability /
     # coupling with traitlets values.
-    try:
-	kubernetes_asyncio.config.load_incluster_config()
-    except kubernetes_asyncio.config.ConfigException:
-	await kubernetes_asyncio.config.load_kube_config()
+    await load_config()
     if not client:
-	return
-    if client.k8s_api_ssl_ca_cert:
-	global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
-	global_conf.ssl_ca_cert = client.k8s_api_ssl_ca_cert
-	kubernetes_asyncio.client.Configuration.set_default(global_conf)
+        return
+    if hasattr(client, 'k8s_api_ssl_ca_cert') and client.k8s_api_ssl_ca_cert:
+        global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
+        global_conf.ssl_ca_cert = client.k8s_api_ssl_ca_cert
+        kubernetes_asyncio.client.Configuration.set_default(global_conf)
     if client.k8s_api_host:
-	global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
-	global_conf.host = client.k8s_api_host
-	kubernetes_asyncio.client.Configuration.set_default(global_conf)
+        global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
+        global_conf.host = client.k8s_api_host
+        kubernetes_asyncio.client.Configuration.set_default(global_conf)
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 39703ca2..b1a72dc2 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -113,20 +113,28 @@ def __init__(self, *args, **kwargs):
         self.executor = ThreadPoolExecutor(max_workers=self.app.concurrent_spawn_limit)
 
         # Global configuration before reflector.py code runs
-        self.core_api = shared_client('CoreV1Api')
-        self.extension_api = shared_client('ExtensionsV1beta1Api')
 
         labels = {
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
         }
-        self.ingress_reflector = IngressReflector.reflector(
+
+    @classmethod
+    async def initialize(cls, *args, **kwargs):
+        """
+        This is how you should get a proxy object.
+        """
+        inst=cls(*args, **kwargs)
+        inst.core_api = await shared_client('CoreV1Api')
+        inst.extension_api = await shared_client('ExtensionsV1beta1Api')
+        
+        inst.ingress_reflector = await IngressReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.service_reflector = ServiceReflector.reflector(
+        inst.service_reflector = await ServiceReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.endpoint_reflector = EndpointsReflector.reflector(
+        inst.endpoint_reflector = await EndpointsReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
 
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 67166dc7..25af7493 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -1,5 +1,6 @@
 # specifically use concurrent.futures for threadsafety
 # asyncio Futures cannot be used across threads
+import asyncio
 import json
 import threading
 import time
@@ -16,7 +17,7 @@
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import shared_client, set_k8s_client_configuration()
+from .clients import shared_client, set_k8s_client_configuration
 
 # This is kubernetes client implementation specific, but we need to know
 # whether it was a network or watch timeout.
@@ -158,11 +159,12 @@ class ResourceReflector(LoggingConfigurable):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        # client configuration for kubernetes may not have taken place,
-        # because it is asynchronous.  We will do it when we create the
-        # reflector from its reflector() classmethod.
-        self.api = shared_client(self.api_group_name)
+        # Client configuration for kubernetes may not have taken place.
+        # Because it is an asynchronous function, we can't do it here.
+        # Instead, we will do it when we create the reflector from its
+        # reflector() classmethod. 
 
+        
         # FIXME: Protect against malicious labels?
         self.label_selector = ','.join(
             ['{}={}'.format(k, v) for k, v in self.labels.items()]
@@ -210,11 +212,15 @@ def __init__(self, *args, **kwargs):
         # awaits the start().
 
     @classmethod
-    def reflector(cls, *args, **kwargs):
+    async def reflector(cls, *args, **kwargs):
+        """
+        This is how you should instantiate a reflector.
+        """
         inst=cls(*args, **kwargs)
-	await set_k8s_client_configuration()
-	await inst.start()
-	return inst
+        inst.api = await shared_client(inst.api_group_name)
+        await set_k8s_client_configuration()
+        await inst.start()
+        return inst
 
     def __del__(self):
         self.stop()
@@ -235,9 +241,14 @@ async def _list_and_update(self):
         if not self.omit_namespace:
             kwargs["namespace"] = self.namespace
 
-        initial_resources = getattr(self.api, self.list_method_name)(**kwargs)
+        method = getattr(self.api, self.list_method_name)
+        self.log.debug(f"method: {method}")
+        self.log.debug(f"kwargs: {kwargs}")
+        self.log.debug(self.api.api_client.configuration.host)
+        initial_resources_raw = await ((method)(**kwargs))
+        self.log.debug(f"initial resources {initial_resources_raw}")
         # This is an atomic operation on the dictionary!
-        initial_resources = json.loads(initial_resources.read())
+        initial_resources = json.loads(await initial_resources_raw.read())
         self.resources = {
             f'{p["metadata"]["namespace"]}/{p["metadata"]["name"]}': p
             for p in initial_resources["items"]
@@ -268,6 +279,10 @@ async def _watch_and_update(self):
         and as long as we don't try to mutate them (do a 'fetch / modify /
         update' cycle on them), we should be ok!
         """
+        #
+        # The preceding frightens me and I think we ought to care more
+        # about threadsafety.
+        #
         selectors = []
         log_name = ""
         if self.label_selector:
@@ -294,7 +309,7 @@ async def _watch_and_update(self):
             start = time.monotonic()
             w = watch.Watch()
             try:
-                resource_version = self._list_and_update()
+                resource_version = await self._list_and_update()
                 if not self.first_load_future.done():
                     # signal that we've loaded our initial data
                     self.first_load_future.set_result(None)
@@ -311,41 +326,41 @@ async def _watch_and_update(self):
                 if self.timeout_seconds:
                     # set watch timeout
                     watch_args['timeout_seconds'] = self.timeout_seconds
-                method = partial(
-                    getattr(self.api, self.list_method_name), _preload_content=False
-                )
-                # in case of timeout_seconds, the w.stream just exits (no exception thrown)
-                # -> we stop the watcher and start a new one
-                for watch_event in w.stream(method, **watch_args):
-                    # Remember that these events are k8s api related WatchEvents
-                    # objects, not k8s Event or Pod representations, they will
-                    # reside in the WatchEvent's object field depending on what
-                    # kind of resource is watched.
-                    #
-                    # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#watchevent-v1-meta
-                    # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#event-v1-core
-                    cur_delay = 0.1
-                    resource = watch_event['object']
-                    ref_key = "{}/{}".format(
-                        resource["metadata"]["namespace"], resource["metadata"]["name"]
-                    )
-                    if watch_event['type'] == 'DELETED':
-                        # This is an atomic delete operation on the dictionary!
-                        self.resources.pop(ref_key, None)
-                    else:
-                        # This is an atomic operation on the dictionary!
-                        self.resources[ref_key] = resource
-                    if self._stop_event.is_set():
-                        self.log.info("%s watcher stopped", self.kind)
-                        break
-                    watch_duration = time.monotonic() - start
-                    if watch_duration >= self.restart_seconds:
-                        self.log.debug(
-                            "Restarting %s watcher after %i seconds",
-                            self.kind,
-                            watch_duration,
+                watch_args['_preload_content'] = False
+                method = getattr(self.api, self.list_method_name)
+                async with w.stream(method(**watch_args)) as stream:
+                    async for watch_event in stream:
+                    # in case of timeout_seconds, the w.stream just exits (no exception thrown)
+                    # -> we stop the watcher and start a new one
+                        # Remember that these events are k8s api related WatchEvents
+                        # objects, not k8s Event or Pod representations, they will
+                        # reside in the WatchEvent's object field depending on what
+                        # kind of resource is watched.
+                        #
+                        # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#watchevent-v1-meta
+                        # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#event-v1-core
+                        cur_delay = 0.1
+                        resource = watch_event['object']
+                        ref_key = "{}/{}".format(
+                            resource["metadata"]["namespace"], resource["metadata"]["name"]
                         )
-                        break
+                        if watch_event['type'] == 'DELETED':
+                            # This is an atomic delete operation on the dictionary!
+                            self.resources.pop(ref_key, None)
+                        else:
+                            # This is an atomic operation on the dictionary!
+                            self.resources[ref_key] = resource
+                        if self._stop_event.is_set():
+                            self.log.info("%s watcher stopped", self.kind)
+                            break
+                        watch_duration = time.monotonic() - start
+                        if watch_duration >= self.restart_seconds:
+                            self.log.debug(
+                                "Restarting %s watcher after %i seconds",
+                                self.kind,
+                                watch_duration,
+                            )
+                            break
             except ReadTimeoutError:
                 # network read time out, just continue and restart the watch
                 # this could be due to a network problem or just low activity
@@ -361,7 +376,7 @@ async def _watch_and_update(self):
                 self.log.exception(
                     "Error when watching resources, retrying in %ss", cur_delay
                 )
-                time.sleep(cur_delay)
+                await asyncio.sleep(cur_delay)
                 continue
             else:
                 # no events on watch, reconnect
@@ -383,19 +398,17 @@ async def start(self):
         start of program initialization (when the singleton is being created),
         and not afterwards!
         """
-        if hasattr(self, 'watch_thread'):
-            raise ValueError('Thread watching for resources is already running')
+        if hasattr(self, 'watch_task') and not self.watch_task.done():
+            raise ValueError('Task watching for resources is already running')
 
-        self._list_and_update()
-        self.watch_thread = threading.Thread(target=self._watch_and_update)
-        # If the watch_thread is only thread left alive, exit app
-        self.watch_thread.daemon = True
-        self.watch_thread.start()
+        await self._list_and_update()
+        self.watch_task = asyncio.create_task(self._watch_and_update())
 
-    async def stop(self):
+    def stop(self):
         self._stop_event.set()
+        # We should do something here to cancel the watch task.
 
-    async def stopped(self):
+    def stopped(self):
         return self._stop_event.is_set()
 
 
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 6c2ddb5b..e43dc52b 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -200,8 +200,6 @@ def __init__(self, *args, **kwargs):
                     max_workers=self.k8s_api_threadpool_workers
                 )
 
-            self.api = shared_client('CoreV1Api')
-
             # Starting our watchers is now async, so we cannot do it in
             # __init()__.
 
@@ -224,8 +222,16 @@ def __init__(self, *args, **kwargs):
         # The attribute needs to exist, even though it is unset to start with
         self._start_future = None
 
+    @classmethod
+    async def initialize(cls,*args,**kwargs):
+        """In an ideal world, you'd get a new KubeSpawner with this."""
+        inst=cls(*args,**kwargs)
+        await inst._initialize_reflectors_and_clients()
+        return inst
+        
     async def _initialize_reflectors_and_clients(self):
-        await set_client_k8s_configuration()
+        self.api = await shared_client("CoreV1Api")
+        await set_k8s_client_configuration()
         await self._start_watching_pods()
         if self.events_enabled:
             await self._start_watching_events()
@@ -2201,7 +2207,7 @@ async def progress(self):
                 break
             await asyncio.sleep(1)
 
-    def _start_reflector(
+    async def _start_reflector(
         self,
         kind=None,
         reflector_class=ResourceReflector,
@@ -2238,7 +2244,7 @@ def on_reflector_failure():
         previous_reflector = self.__class__.reflectors.get(key)
 
         if replace or not previous_reflector:
-            self.__class__.reflectors[key] = ReflectorClass(
+            self.__class__.reflectors[key] = await ReflectorClass.reflector(
                 parent=self,
                 namespace=self.namespace,
                 on_failure=on_reflector_failure,
@@ -2252,7 +2258,7 @@ def on_reflector_failure():
         # return the current reflector
         return self.__class__.reflectors[key]
 
-    def _start_watching_events(self, replace=False):
+    async def _start_watching_events(self, replace=False):
         """Start the events reflector
 
         If replace=False and the event reflector is already running,
@@ -2261,7 +2267,7 @@ def _start_watching_events(self, replace=False):
         If replace=True, a running pod reflector will be stopped
         and a new one started (for recovering from possible errors).
         """
-        return self._start_reflector(
+        return await self._start_reflector(
             kind="events",
             reflector_class=EventReflector,
             fields={"involvedObject.kind": "Pod"},
@@ -2269,7 +2275,7 @@ def _start_watching_events(self, replace=False):
             replace=replace,
         )
 
-    def _start_watching_pods(self, replace=False):
+    async def _start_watching_pods(self, replace=False):
         """Start the pod reflector
 
         If replace=False and the pod reflector is already running,
@@ -2280,7 +2286,7 @@ def _start_watching_pods(self, replace=False):
         """
         pod_reflector_class = PodReflector
         pod_reflector_class.labels.update({"component": self.component_label})
-        return self._start_reflector(
+        return await self._start_reflector(
             "pods",
             PodReflector,
             omit_namespace=self.enable_user_namespaces,
@@ -2624,7 +2630,7 @@ async def _make_delete_pod_request(
                     name=pod_name,
                     namespace=self.namespace,
                     body=delete_options,
-                    grace_period_seconds=grace_seconds)
+                    grace_period_seconds=grace_seconds),
                 request_timeout)
             return True
         except asyncio.TimeoutError:
diff --git a/tests/conftest.py b/tests/conftest.py
index 3a852a63..7e6aab50 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -26,7 +26,7 @@
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
 #from kubernetes.stream import stream
-from kubernetes_asyncio.stream import stream
+#from kubernetes_asyncio.stream import stream
 from kubernetes_asyncio.watch import Watch
 from traitlets.config import Config
 
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index 47c77c74..87f85db5 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -62,20 +62,18 @@ async def test_multi_namespace_spawn():
 
     # get a client
     kube_ns = spawner.namespace
-    load_kube_config()
-    client = shared_client('CoreV1Api')
+    client = await shared_client('CoreV1Api')
 
     # the spawner will create the namespace on its own.
 
     # Wrap in a try block so we clean up the namespace.
 
-    saved_exception = None
     try:
         # start the spawner
         await spawner.start()
 
         # verify the pod exists
-        pods = client.list_namespaced_pod(kube_ns).items
+        pods = (await client.list_namespaced_pod(kube_ns)).items
         pod_names = [p.metadata.name for p in pods]
         assert "jupyter-%s" % spawner.user.name in pod_names
         # verify poll while running
@@ -84,15 +82,11 @@ async def test_multi_namespace_spawn():
         # stop the pod
         await spawner.stop()
         # verify pod is gone
-        pods = client.list_namespaced_pod(kube_ns).items
+        pods = (await client.list_namespaced_pod(kube_ns)).items
         pod_names = [p.metadata.name for p in pods]
         assert "jupyter-%s" % spawner.user.name not in pod_names
         # verify exit status
         status = await spawner.poll()
         assert isinstance(status, int)
-    except Exception as saved_exception:
-        pass  # We will raise after namespace removal
-    # remove namespace
-    client.delete_namespace(kube_ns, body={})
-    if saved_exception is not None:
-        raise saved_exception
+    finally:
+        await client.delete_namespace(kube_ns, body={})

From ef03248cd7b95b83ea5eb3b049e5b70cfd9183ff Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 27 Jan 2022 14:27:00 -0700
Subject: [PATCH 04/56] missed a stock k8s import

---
 kubespawner/spawner.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index e43dc52b..2450bf28 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -23,8 +23,8 @@
 from jupyterhub.spawner import Spawner
 from jupyterhub.traitlets import Command
 from jupyterhub.utils import exponential_backoff
-from kubernetes import client
-from kubernetes.client.rest import ApiException
+from kubernetes_asyncio import client
+from kubernetes_asyncio.client.rest import ApiException
 from slugify import slugify
 from tornado import gen
 from tornado.concurrent import run_on_executor

From 0a07bfee5ac1877f0090618e89bad3ba3b068a94 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 27 Jan 2022 14:40:20 -0700
Subject: [PATCH 05/56] Use dict rather than object representation of watched
 objects

---
 kubespawner/clients.py   | 20 +++-------------
 kubespawner/proxy.py     | 49 +++++++++++++++++++++++++++++-----------
 kubespawner/reflector.py | 20 ++++++++++------
 kubespawner/spawner.py   |  4 ++--
 4 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index b40da7e6..5e926d0b 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -27,6 +27,8 @@ async def shared_client(ClientType, *args, **kwargs):
     so that concurrent calls to shared_client
     will all return the same instance until
     all references to the client are cleared.
+
+    You should usually await load_config before calling this.
     """
     kwarg_key = tuple((key, kwargs[key]) for key in sorted(kwargs))
     cache_key = (ClientType, args, kwarg_key)
@@ -40,12 +42,11 @@ async def shared_client(ClientType, *args, **kwargs):
         # Kubernetes client configuration is handled globally
         # in kubernetes.py and is already called in spawner.py
         # or proxy.py prior to a shared_client being instantiated
-        await load_config()
         Client = getattr(kubernetes_asyncio.client, ClientType)
         client = Client(*args, **kwargs)
         # cache weakref so that clients can be garbage collected
         _client_cache[cache_key] = weakref.ref(client)
-        
+
     return client
 
 async def load_config():
@@ -53,18 +54,3 @@ async def load_config():
         kubernetes_asyncio.config.load_incluster_config()
     except kubernetes_asyncio.config.ConfigException:
         await kubernetes_asyncio.config.load_kube_config()
-
-async def set_k8s_client_configuration(client=None):
-    # Call this prior to using a client for readability /
-    # coupling with traitlets values.
-    await load_config()
-    if not client:
-        return
-    if hasattr(client, 'k8s_api_ssl_ca_cert') and client.k8s_api_ssl_ca_cert:
-        global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
-        global_conf.ssl_ca_cert = client.k8s_api_ssl_ca_cert
-        kubernetes_asyncio.client.Configuration.set_default(global_conf)
-    if client.k8s_api_host:
-        global_conf = kubernetes_asyncio.client.Configuration.get_default_copy()
-        global_conf.host = client.k8s_api_host
-        kubernetes_asyncio.client.Configuration.set_default(global_conf)
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index b1a72dc2..78dacbf3 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -12,7 +12,7 @@
 from tornado.concurrent import run_on_executor
 from traitlets import Unicode
 
-from .clients import shared_client
+from .clients import shared_client, load_config
 from .objects import make_ingress
 from .reflector import ResourceReflector
 from .utils import generate_hashed_slug
@@ -82,10 +82,10 @@ def _namespace_default(self):
         config=True,
         help="""
         Location (absolute filepath) for CA certs of the k8s API server.
-        
-        Typically this is unnecessary, CA certs are picked up by 
+
+        Typically this is unnecessary, CA certs are picked up by
         config.load_incluster_config() or config.load_kube_config.
-        
+
         In rare non-standard cases, such as using custom intermediate CA
         for your cluster, you may need to mount root CA's elsewhere in
         your Pod/Container and point this variable to that filepath
@@ -97,8 +97,8 @@ def _namespace_default(self):
         config=True,
         help="""
         Full host name of the k8s API server ("https://hostname:port").
-        
-        Typically this is unnecessary, the hostname is picked up by 
+
+        Typically this is unnecessary, the hostname is picked up by
         config.load_incluster_config() or config.load_kube_config.
         """,
     )
@@ -125,19 +125,42 @@ async def initialize(cls, *args, **kwargs):
         This is how you should get a proxy object.
         """
         inst=cls(*args, **kwargs)
-        inst.core_api = await shared_client('CoreV1Api')
-        inst.extension_api = await shared_client('ExtensionsV1beta1Api')
-        
-        inst.ingress_reflector = await IngressReflector.reflector(
+        await inst._initialize_resources()
+        return inst
+
+    async def _initialize_resources(self):
+        await load_config()
+        _set_k8s_client_configuration()
+        self.core_api = await shared_client('CoreV1Api')
+        self.extension_api = await shared_client('ExtensionsV1beta1Api')
+
+        self.ingress_reflector = await IngressReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        inst.service_reflector = await ServiceReflector.reflector(
+        self.service_reflector = await ServiceReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
-        inst.endpoint_reflector = await EndpointsReflector.reflector(
+        self.endpoint_reflector = await EndpointsReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
         )
 
+    def _set_k8s_client_configuration(self):
+        # The actual (singleton) Kubernetes client will be created
+        # in clients.py shared_client but the configuration
+        # for token / ca_cert / k8s api host is set globally
+        # in kubernetes.py syntax.  It is being set here
+        # and this method called prior to getting a shared_client
+        # (but after load_config)
+        # for readability / coupling with traitlets values
+        if self.k8s_api_ssl_ca_cert:
+            global_conf = client.Configuration.get_default_copy()
+            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
+            client.Configuration.set_default(global_conf)
+        if self.k8s_api_host:
+            global_conf = client.Configuration.get_default_copy()
+            global_conf.host = self.k8s_api_host
+            client.Configuration.set_default(global_conf)
+
     def safe_name_for_routespec(self, routespec):
         safe_chars = set(string.ascii_lowercase + string.digits)
         safe_name = generate_hashed_slug(
@@ -202,7 +225,7 @@ async def ensure_object(create_func, patch_func, body, kind):
                 'Could not find endpoints/%s after creating it' % safe_name,
             )
         else:
-            delete_endpoint = await (self.core_api.delete_namespaced_endpoints(
+            delete_endpoint = await self.core_api.delete_namespaced_endpoints(
                 name=safe_name,
                 namespace=self.namespace,
                 body=client.V1DeleteOptions(grace_period_seconds=0),
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 25af7493..a6697f2d 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -17,7 +17,7 @@
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import shared_client, set_k8s_client_configuration
+from .clients import shared_client, load_config
 
 # This is kubernetes client implementation specific, but we need to know
 # whether it was a network or watch timeout.
@@ -217,8 +217,8 @@ async def reflector(cls, *args, **kwargs):
         This is how you should instantiate a reflector.
         """
         inst=cls(*args, **kwargs)
+        await load_config()
         inst.api = await shared_client(inst.api_group_name)
-        await set_k8s_client_configuration()
         await inst.start()
         return inst
 
@@ -242,8 +242,6 @@ async def _list_and_update(self):
             kwargs["namespace"] = self.namespace
 
         method = getattr(self.api, self.list_method_name)
-        self.log.debug(f"method: {method}")
-        self.log.debug(f"kwargs: {kwargs}")
         self.log.debug(self.api.api_client.configuration.host)
         initial_resources_raw = await ((method)(**kwargs))
         self.log.debug(f"initial resources {initial_resources_raw}")
@@ -317,6 +315,8 @@ async def _watch_and_update(self):
                     "label_selector": self.label_selector,
                     "field_selector": self.field_selector,
                     "resource_version": resource_version,
+                    # You'd think this would work...but it does nothing.
+                    # "_preload_content": False
                 }
                 if not self.omit_namespace:
                     watch_args["namespace"] = self.namespace
@@ -326,9 +326,12 @@ async def _watch_and_update(self):
                 if self.timeout_seconds:
                     # set watch timeout
                     watch_args['timeout_seconds'] = self.timeout_seconds
-                watch_args['_preload_content'] = False
+                # Partial application of _preload_content=False causes
+                # a stream of errors of the form:
+                # AttributeError: module 'kubernetes_asyncio.client.models' has no attribute ''
+                # when unmarshaling the event into dict form.
                 method = getattr(self.api, self.list_method_name)
-                async with w.stream(method(**watch_args)) as stream:
+                async with w.stream(method,**watch_args) as stream:
                     async for watch_event in stream:
                     # in case of timeout_seconds, the w.stream just exits (no exception thrown)
                     # -> we stop the watcher and start a new one
@@ -340,7 +343,10 @@ async def _watch_and_update(self):
                         # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#watchevent-v1-meta
                         # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#event-v1-core
                         cur_delay = 0.1
-                        resource = watch_event['object']
+                        resource_obj = watch_event['object']
+                        # This, in theory, is exactly what _preload_content=False should do.
+                        # Only this actually works.
+                        resource = self.api.api_client.sanitize_for_serialization(resource_obj)
                         ref_key = "{}/{}".format(
                             resource["metadata"]["namespace"], resource["metadata"]["name"]
                         )
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 2450bf28..c34b0699 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -39,7 +39,7 @@
 from traitlets import Union
 from traitlets import validate
 
-from .clients import shared_client, set_k8s_client_configuration
+from .clients import shared_client, load_config
 from .objects import make_namespace
 from .objects import make_owner_reference
 from .objects import make_pod
@@ -230,8 +230,8 @@ async def initialize(cls,*args,**kwargs):
         return inst
         
     async def _initialize_reflectors_and_clients(self):
+        await load_config()
         self.api = await shared_client("CoreV1Api")
-        await set_k8s_client_configuration()
         await self._start_watching_pods()
         if self.events_enabled:
             await self._start_watching_events()

From cfd350cb51100bb3b1b7a7db8006e88f9c285703 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 27 Jan 2022 16:57:10 -0700
Subject: [PATCH 06/56] Make the initialization a public method, and call it
 during stop() too.

---
 kubespawner/spawner.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index c34b0699..4915c8b5 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -226,10 +226,10 @@ def __init__(self, *args, **kwargs):
     async def initialize(cls,*args,**kwargs):
         """In an ideal world, you'd get a new KubeSpawner with this."""
         inst=cls(*args,**kwargs)
-        await inst._initialize_reflectors_and_clients()
+        await inst.initialize_reflectors_and_clients()
         return inst
         
-    async def _initialize_reflectors_and_clients(self):
+    async def initialize_reflectors_and_clients(self):
         await load_config()
         self.api = await shared_client("CoreV1Api")
         await self._start_watching_pods()
@@ -2062,7 +2062,7 @@ async def poll(self):
         we need to load client configuration and start our reflectors
         at the top of each of those methods.
         """
-        await self._initialize_reflectors_and_clients()
+        await self.initialize_reflectors_and_clients()
         # have to wait for first load of data before we have a valid answer
         if not self.pod_reflector.first_load_future.done():
             await asyncio.wrap_future(self.pod_reflector.first_load_future)
@@ -2473,7 +2473,7 @@ async def _start(self):
 
         # Start watchers.  This might also be called from poll().  It also
         #  configures our API clients.
-        await self._initialize_reflectors_and_clients()
+        await self.initialize_reflectors_and_clients()
 
         # If we have user_namespaces enabled, create the namespace.
         #  It's fine if it already exists.
@@ -2675,6 +2675,10 @@ async def _make_delete_pvc_request(self, pvc_name, request_timeout):
                 raise
 
     async def stop(self, now=False):
+        # This could be the first method called; say the Hub has been
+        #  restarted, and the first thing someone does is hit it to stop
+        #  a running pod.  Hence the need to initialize reflectors/clients.
+        await self.initialize_reflectors_and_clients()
         delete_options = client.V1DeleteOptions()
 
         if now:

From ecda5fedb830536bec8d861c3d66e033a6b4decd Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 28 Jan 2022 15:37:40 -0700
Subject: [PATCH 07/56] Address review commentary, part 1

---
 kubespawner/proxy.py     |  16 ++-----
 kubespawner/reflector.py |  10 ++--
 kubespawner/spawner.py   | 100 ++++++++++++++++++---------------------
 3 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 78dacbf3..9aacfe97 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -1,15 +1,13 @@
+import asyncio
 import json
 import os
 import string
-from concurrent.futures import ThreadPoolExecutor
 
 import escapism
 import kubernetes.config
 from jupyterhub.proxy import Proxy
 from jupyterhub.utils import exponential_backoff
 from kubernetes_asyncio import client
-from tornado import gen
-from tornado.concurrent import run_on_executor
 from traitlets import Unicode
 
 from .clients import shared_client, load_config
@@ -106,12 +104,6 @@ def _namespace_default(self):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        # We use the maximum number of concurrent user server starts (and thus proxy adds)
-        # as our threadpool maximum. This ensures that contention here does not become
-        # an accidental bottleneck. Since we serialize our create operations, we only
-        # need 1x concurrent_spawn_limit, not 3x.
-        self.executor = ThreadPoolExecutor(max_workers=self.app.concurrent_spawn_limit)
-
         # Global configuration before reflector.py code runs
 
         labels = {
@@ -124,13 +116,13 @@ async def initialize(cls, *args, **kwargs):
         """
         This is how you should get a proxy object.
         """
-        inst=cls(*args, **kwargs)
+        inst = cls(*args, **kwargs)
         await inst._initialize_resources()
         return inst
 
     async def _initialize_resources(self):
         await load_config()
-        _set_k8s_client_configuration()
+        self._set_k8s_client_configuration()
         self.core_api = await shared_client('CoreV1Api')
         self.extension_api = await shared_client('ExtensionsV1beta1Api')
 
@@ -292,7 +284,7 @@ async def delete_route(self, routespec):
         # explicitly ourselves as well. In the future, we can probably try a
         # foreground cascading deletion (https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion)
         # instead, but for now this works well enough.
-        await gather(
+        await asyncio.gather(
             self.delete_if_exists('endpoint', safe_name, delete_endpoint),
             self.delete_if_exists('service', safe_name, delete_service),
             self.delete_if_exists('ingress', safe_name, delete_ingress),
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index a6697f2d..83baf8a9 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -162,9 +162,9 @@ def __init__(self, *args, **kwargs):
         # Client configuration for kubernetes may not have taken place.
         # Because it is an asynchronous function, we can't do it here.
         # Instead, we will do it when we create the reflector from its
-        # reflector() classmethod. 
+        # reflector() classmethod.
+
 
-        
         # FIXME: Protect against malicious labels?
         self.label_selector = ','.join(
             ['{}={}'.format(k, v) for k, v in self.labels.items()]
@@ -216,7 +216,7 @@ async def reflector(cls, *args, **kwargs):
         """
         This is how you should instantiate a reflector.
         """
-        inst=cls(*args, **kwargs)
+        inst = cls(*args, **kwargs)
         await load_config()
         inst.api = await shared_client(inst.api_group_name)
         await inst.start()
@@ -242,9 +242,7 @@ async def _list_and_update(self):
             kwargs["namespace"] = self.namespace
 
         method = getattr(self.api, self.list_method_name)
-        self.log.debug(self.api.api_client.configuration.host)
         initial_resources_raw = await ((method)(**kwargs))
-        self.log.debug(f"initial resources {initial_resources_raw}")
         # This is an atomic operation on the dictionary!
         initial_resources = json.loads(await initial_resources_raw.read())
         self.resources = {
@@ -331,7 +329,7 @@ async def _watch_and_update(self):
                 # AttributeError: module 'kubernetes_asyncio.client.models' has no attribute ''
                 # when unmarshaling the event into dict form.
                 method = getattr(self.api, self.list_method_name)
-                async with w.stream(method,**watch_args) as stream:
+                async with w.stream(method, **watch_args) as stream:
                     async for watch_event in stream:
                     # in case of timeout_seconds, the w.stream just exits (no exception thrown)
                     # -> we stop the watcher and start a new one
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 4915c8b5..979bfe7b 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -11,7 +11,6 @@
 import string
 import sys
 import warnings
-from concurrent.futures import ThreadPoolExecutor
 from datetime import timedelta
 from functools import partial
 from urllib.parse import urlparse
@@ -27,7 +26,6 @@
 from kubernetes_asyncio.client.rest import ApiException
 from slugify import slugify
 from tornado import gen
-from tornado.concurrent import run_on_executor
 from tornado.ioloop import IOLoop
 from traitlets import Bool
 from traitlets import default
@@ -123,12 +121,10 @@ class KubeSpawner(Spawner):
     spawned by a user will have its own KubeSpawner instance.
     """
 
-    # We want to have one single threadpool executor that is shared across all
-    # KubeSpawner instances, so we apply a Singleton pattern. We initialize this
-    # class variable from the first KubeSpawner instance that is created and
-    # then reference it from all instances. The same goes for the PodReflector
-    # and EventReflector.
-    executor = None
+    # The PodReflector and EventReflector are singletons.  Where to initialize
+    # them becomes a sort of thorny question in an asyncio world.  See the
+    # commentary on the initialize() method.
+
     reflectors = {
         "pods": None,
         "events": None,
@@ -188,25 +184,9 @@ def __init__(self, *args, **kwargs):
             self.namespace = self._expand_user_properties(self.user_namespace_template)
             self.log.info("Using user namespace: {}".format(self.namespace))
 
-        if not _mock:
-            # runs during normal execution only
-
-            if self.__class__.executor is None:
-                self.log.debug(
-                    'Starting executor thread pool with %d workers',
-                    self.k8s_api_threadpool_workers,
-                )
-                self.__class__.executor = ThreadPoolExecutor(
-                    max_workers=self.k8s_api_threadpool_workers
-                )
-
-            # Starting our watchers is now async, so we cannot do it in
-            # __init()__.
+        # Starting our watchers is now async, so we cannot do it in
+        # __init()__.
 
-            # This will start watching in __init__, so it'll start the first
-            # time any spawner object is created. Not ideal but works!
-
-        # runs during both test and normal execution
         self.pod_name = self._expand_user_properties(self.pod_name_template)
         self.dns_name = self.dns_name_template.format(
             namespace=self.namespace, name=self.pod_name
@@ -224,11 +204,26 @@ def __init__(self, *args, **kwargs):
 
     @classmethod
     async def initialize(cls,*args,**kwargs):
-        """In an ideal world, you'd get a new KubeSpawner with this."""
-        inst=cls(*args,**kwargs)
+        """In an ideal world, you'd get a new KubeSpawner with this.
+        That's not how we are called from JupyterHub, though; it just
+        instantiates whatever class you tell it to use as the spawner class.
+
+        Thus we need to do the initialization in poll(), start(), and stop()
+        since any of them could be the first thing called (imagine that the
+        hub restarts and the first thing someone does is try to stop their
+        running server).  Further (and this is actually true in the Rubin
+        case) a pre-spawn-start hook that depends on internal knowledge of
+        the spawner might be the first thing to run.
+
+        It would be nice if there were an agreed-upon mechanism for, say,
+        JupyterHub to look for an async initialize() method and then use
+        that, instead, to get its spawner instance.  I don't think our
+        use-case is likely to be unique.
+        """
+        inst = cls(*args,**kwargs)
         await inst.initialize_reflectors_and_clients()
         return inst
-        
+
     async def initialize_reflectors_and_clients(self):
         await load_config()
         self.api = await shared_client("CoreV1Api")
@@ -263,20 +258,6 @@ async def initialize_reflectors_and_clients(self):
         """,
     )
 
-    k8s_api_threadpool_workers = Integer(
-        # Set this explicitly, since this is the default in Python 3.5+
-        # but not in 3.4
-        5 * multiprocessing.cpu_count(),
-        config=True,
-        help="""
-        Number of threads in thread pool used to talk to the k8s API.
-
-        Increase this if you are dealing with a very large number of users.
-
-        Defaults to `5 * cpu_cores`, which is the default for `ThreadPoolExecutor`.
-        """,
-    )
-
     k8s_api_request_timeout = Integer(
         3,
         config=True,
@@ -2322,8 +2303,10 @@ async def _make_create_pod_request(self, pod, request_timeout):
             await asyncio.wait_for(
                 self.api.create_namespaced_pod(
                     self.namespace,
-                    pod),
-                request_timeout)
+                    pod,
+                ),
+                request_timeout,
+            )
             return True
         except asyncio.TimeoutError:
             # Just try again
@@ -2362,7 +2345,8 @@ async def _make_create_pvc_request(self, pvc, request_timeout):
                     namespace=self.namespace,
                     body=pvc,
                 ),
-                request_timeout)
+                request_timeout,
+            )
             return True
         except asyncio.TimeoutError:
             # Just try again
@@ -2410,7 +2394,8 @@ async def _ensure_not_exists(self, kind, name):
             self.log.info(f"Deleting {kind}/{name}")
             await asyncio.wait_for(
                 delete(namespace=self.namespace, name=name),
-                self.k8s_api_request_timeout)
+                self.k8s_api_request_timeout,
+            )
         except asyncio.TimeoutError:
             # Just try again
             return False
@@ -2426,7 +2411,8 @@ async def _ensure_not_exists(self, kind, name):
             self.log.info(f"Checking for {kind}/{name}")
             await asyncio.wait_for(
                 read(namespace=self.namespace, name=name),
-                self.k8s_api_request_timeout)
+                self.k8s_api_request_timeout
+            )
         except asyncio.TimeoutError:
             # Just try again
             return False
@@ -2450,7 +2436,8 @@ async def _make_create_resource_request(self, kind, manifest):
         try:
             await asyncio.wait_for(
                 create(self.namespace,manifest),
-                self.k8s_api_request_timeout)
+                self.k8s_api_request_timeout
+            )
         except asyncio.TimeoutError:
             # Just try again
             return False
@@ -2512,7 +2499,8 @@ async def _start(self):
         ref_key = "{}/{}".format(self.namespace, self.pod_name)
         # If there's a timeout, just let it propagate
         await exponential_backoff(
-            partial(self._make_create_pod_request, pod, self.k8s_api_request_timeout),
+            partial(self._make_create_pod_request, pod, self.k8s_api_request_timeout
+                    ),
             f'Could not create pod {ref_key}',
             timeout=self.k8s_api_request_retry_timeout,
         )
@@ -2631,7 +2619,8 @@ async def _make_delete_pod_request(
                     namespace=self.namespace,
                     body=delete_options,
                     grace_period_seconds=grace_seconds),
-                request_timeout)
+                request_timeout,
+            )
             return True
         except asyncio.TimeoutError:
             return False
@@ -2658,8 +2647,10 @@ async def _make_delete_pvc_request(self, pvc_name, request_timeout):
             await asyncio.wait_for(
                 self.api.delete_namespaced_persistent_volume_claim(
                     name=pvc_name,
-                    namespace=self.namespace),
-                request_timeout)
+                    namespace=self.namespace,
+                ),
+                request_timeout,
+            )
             return True
         except asyncio.TimeoutError:
             return False
@@ -2873,7 +2864,8 @@ async def _ensure_namespace(self):
         try:
             await asyncio.wait_for(
                 api.create_namespace(ns),
-                self.k8s_api_request_timeout)
+                self.k8s_api_request_timeout,
+            )
         except ApiException as e:
             if e.status != 409:
                 # It's fine if it already exists

From 59e45eca7ecef21d0bb08acb3cb4e0440cd5a185 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 28 Jan 2022 15:54:00 -0700
Subject: [PATCH 08/56] Attempt to skip serialization

---
 kubespawner/reflector.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 83baf8a9..d60a7e8a 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -341,10 +341,11 @@ async def _watch_and_update(self):
                         # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#watchevent-v1-meta
                         # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#event-v1-core
                         cur_delay = 0.1
-                        resource_obj = watch_event['object']
-                        # This, in theory, is exactly what _preload_content=False should do.
-                        # Only this actually works.
-                        resource = self.api.api_client.sanitize_for_serialization(resource_obj)
+                        resource = watch_event['raw_object']
+                        # Unfortunately the asyncio watch is still going to
+                        # deserialize it into watch_event['object'] so we may
+                        # well run into the performance issues from:
+                        # https://github.com/jupyterhub/kubespawner/pull/424
                         ref_key = "{}/{}".format(
                             resource["metadata"]["namespace"], resource["metadata"]["name"]
                         )

From 742e471ace84b4ce6b854f132e06cdf8a734fe94 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Sun, 30 Jan 2022 11:49:37 -0700
Subject: [PATCH 09/56] Terminate watch cleanly

---
 kubespawner/reflector.py | 42 +++++++++++++++++++++++++++++-----------
 kubespawner/spawner.py   |  2 +-
 setup.cfg                |  2 ++
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index d60a7e8a..c1a46590 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -207,6 +207,8 @@ def __init__(self, *args, **kwargs):
         if not self.list_method_name:
             raise RuntimeError("Reflector list_method_name must be set!")
 
+        self.watch_task = None  # Test this rather than absence of the attr
+
         # start is now asynchronous.  That means that the way to create a
         # reflector is now with a classmethod (called "reflector") that
         # awaits the start().
@@ -222,8 +224,9 @@ async def reflector(cls, *args, **kwargs):
         await inst.start()
         return inst
 
-    def __del__(self):
-        self.stop()
+    # We're removing __del__ -- there's no good way to call async
+    #  methods for it, and what we really want to do on the reflector stopping
+    #  is tell the watch task to exit, and then make sure it did.
 
     async def _list_and_update(self):
         """
@@ -276,8 +279,9 @@ async def _watch_and_update(self):
         update' cycle on them), we should be ok!
         """
         #
-        # The preceding frightens me and I think we ought to care more
-        # about threadsafety.
+        # I guess?  It is the case that the resources are read-only in the
+        #  spawner, so the worst that will happen will be that they're out-
+        #  of-date.  I think.
         #
         selectors = []
         log_name = ""
@@ -355,8 +359,8 @@ async def _watch_and_update(self):
                         else:
                             # This is an atomic operation on the dictionary!
                             self.resources[ref_key] = resource
-                        if self._stop_event.is_set():
-                            self.log.info("%s watcher stopped", self.kind)
+                        if self.stopped():
+                            self.log.info("%s watcher stopped: inner", self.kind)
                             break
                         watch_duration = time.monotonic() - start
                         if watch_duration >= self.restart_seconds:
@@ -366,6 +370,12 @@ async def _watch_and_update(self):
                                 watch_duration,
                             )
                             break
+                    if self.stopped():
+                        # We have an additional level of loop to break out
+                        #  of because of the asyncio watch.
+                        self.log.info("%s watcher stopped: middle", self.kind)
+                        break
+
             except ReadTimeoutError:
                 # network read time out, just continue and restart the watch
                 # this could be due to a network problem or just low activity
@@ -388,8 +398,8 @@ async def _watch_and_update(self):
                 self.log.debug("%s watcher timeout", self.kind)
             finally:
                 w.stop()
-                if self._stop_event.is_set():
-                    self.log.info("%s watcher stopped", self.kind)
+                if self.stopped():
+                    self.log.info("%s watcher stopped: outer", self.kind)
                     break
         self.log.warning("%s watcher finished", self.kind)
 
@@ -403,15 +413,25 @@ async def start(self):
         start of program initialization (when the singleton is being created),
         and not afterwards!
         """
-        if hasattr(self, 'watch_task') and not self.watch_task.done():
+        if self.watch_task and not self.watch_task.done():
             raise ValueError('Task watching for resources is already running')
 
         await self._list_and_update()
         self.watch_task = asyncio.create_task(self._watch_and_update())
 
-    def stop(self):
+    async def stop(self):
         self._stop_event.set()
-        # We should do something here to cancel the watch task.
+        # The watch task should be in the process of terminating.  Give it
+        # a bit...
+        if self.watch_task and not self.watch_task.done():
+            try:
+                timeout=5
+                await asyncio.wait_for(self.watch_task, timeout)
+            except asyncio.TimeoutError:
+                # This will cancel the task for us
+                self.log.warning(f"Watch task did not finish in {timeout}s" +
+                                 "and was cancelled")
+        self.watch_task = None
 
     def stopped(self):
         return self._stop_event.is_set()
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 979bfe7b..2b946ddd 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -2234,7 +2234,7 @@ def on_reflector_failure():
 
         if replace and previous_reflector:
             # we replaced the reflector, stop the old one
-            previous_reflector.stop()
+            await previous_reflector.stop()
 
         # return the current reflector
         return self.__class__.reflectors[key]
diff --git a/setup.cfg b/setup.cfg
index 7967ad45..0503fda6 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -4,3 +4,5 @@ test=pytest
 [tool:pytest]
 # Ignore thousands of tests in dependencies installed in a virtual environment
 norecursedirs = lib lib64
+# At some point probably want to make this "strict"
+asyncio_mode = auto

From b05e826734e58d7f21813c112b776831860883d5 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Mon, 31 Jan 2022 09:52:16 -0700
Subject: [PATCH 10/56] Make shared_client() sync

---
 kubespawner/clients.py                | 2 +-
 kubespawner/proxy.py                  | 4 ++--
 kubespawner/reflector.py              | 2 +-
 kubespawner/spawner.py                | 2 +-
 tests/test_multi_namespace_spawner.py | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 5e926d0b..2608445a 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -20,7 +20,7 @@
 
 _client_cache = {}
 
-async def shared_client(ClientType, *args, **kwargs):
+def shared_client(ClientType, *args, **kwargs):
     """Return a single shared kubernetes client instance
 
     A weak reference to the instance is cached,
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 9aacfe97..990a3871 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -123,8 +123,8 @@ async def initialize(cls, *args, **kwargs):
     async def _initialize_resources(self):
         await load_config()
         self._set_k8s_client_configuration()
-        self.core_api = await shared_client('CoreV1Api')
-        self.extension_api = await shared_client('ExtensionsV1beta1Api')
+        self.core_api = shared_client('CoreV1Api')
+        self.extension_api = shared_client('ExtensionsV1beta1Api')
 
         self.ingress_reflector = await IngressReflector.reflector(
             parent=self, namespace=self.namespace, labels=labels
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index c1a46590..b993d9df 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -220,7 +220,7 @@ async def reflector(cls, *args, **kwargs):
         """
         inst = cls(*args, **kwargs)
         await load_config()
-        inst.api = await shared_client(inst.api_group_name)
+        inst.api = shared_client(inst.api_group_name)
         await inst.start()
         return inst
 
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 2b946ddd..47b2ec87 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -226,7 +226,7 @@ async def initialize(cls,*args,**kwargs):
 
     async def initialize_reflectors_and_clients(self):
         await load_config()
-        self.api = await shared_client("CoreV1Api")
+        self.api = shared_client("CoreV1Api")
         await self._start_watching_pods()
         if self.events_enabled:
             await self._start_watching_events()
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index 87f85db5..b64321ef 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -62,7 +62,7 @@ async def test_multi_namespace_spawn():
 
     # get a client
     kube_ns = spawner.namespace
-    client = await shared_client('CoreV1Api')
+    client = shared_client('CoreV1Api')
 
     # the spawner will create the namespace on its own.
 

From abf99f107a849976194badf8968fe37b5d8cf799 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Tue, 1 Feb 2022 09:12:30 -0700
Subject: [PATCH 11/56] Address further review comments

---
 kubespawner/reflector.py | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index b993d9df..1072bb9c 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -28,6 +28,15 @@ class ResourceReflector(LoggingConfigurable):
     kubernetes resources.
 
     Must be subclassed once per kind of resource that needs watching.
+
+    Creating a reflector should be done with the reflector() classmethod,
+    since that, in addition to creating the instance, initializes the
+    Kubernetes configuration, acquires a K8s API client, and starts the
+    watch task.
+
+    Shutting down a reflector should be done by awaiting its stop() method.
+    JupyterHub does not currently do this, so the watch task runs until the
+    Hub exits.
     """
 
     labels = Dict(
@@ -159,12 +168,6 @@ class ResourceReflector(LoggingConfigurable):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        # Client configuration for kubernetes may not have taken place.
-        # Because it is an asynchronous function, we can't do it here.
-        # Instead, we will do it when we create the reflector from its
-        # reflector() classmethod.
-
-
         # FIXME: Protect against malicious labels?
         self.label_selector = ','.join(
             ['{}={}'.format(k, v) for k, v in self.labels.items()]
@@ -209,14 +212,17 @@ def __init__(self, *args, **kwargs):
 
         self.watch_task = None  # Test this rather than absence of the attr
 
-        # start is now asynchronous.  That means that the way to create a
-        # reflector is now with a classmethod (called "reflector") that
-        # awaits the start().
+        # Note that simply instantiating the class does not load
+        # Kubernetes config, acquire an API client, or start the
+        # watch task.
 
     @classmethod
     async def reflector(cls, *args, **kwargs):
         """
-        This is how you should instantiate a reflector.
+        This is how you should instantiate a reflector in order to bring
+        it up in a usable state, as this classmethod does load
+        Kubernetes config, acquires an API client, and starts the watch
+        task.
         """
         inst = cls(*args, **kwargs)
         await load_config()
@@ -224,10 +230,6 @@ async def reflector(cls, *args, **kwargs):
         await inst.start()
         return inst
 
-    # We're removing __del__ -- there's no good way to call async
-    #  methods for it, and what we really want to do on the reflector stopping
-    #  is tell the watch task to exit, and then make sure it did.
-
     async def _list_and_update(self):
         """
         Update current list of resources by doing a full fetch.
@@ -370,11 +372,6 @@ async def _watch_and_update(self):
                                 watch_duration,
                             )
                             break
-                    if self.stopped():
-                        # We have an additional level of loop to break out
-                        #  of because of the asyncio watch.
-                        self.log.info("%s watcher stopped: middle", self.kind)
-                        break
 
             except ReadTimeoutError:
                 # network read time out, just continue and restart the watch
@@ -420,15 +417,18 @@ async def start(self):
         self.watch_task = asyncio.create_task(self._watch_and_update())
 
     async def stop(self):
+        """
+        Cleanly shut down the watch task.
+        """
         self._stop_event.set()
-        # The watch task should be in the process of terminating.  Give it
-        # a bit...
+        # The watch task should now be in the process of terminating.  Give
+        # it a bit...
         if self.watch_task and not self.watch_task.done():
             try:
                 timeout=5
                 await asyncio.wait_for(self.watch_task, timeout)
             except asyncio.TimeoutError:
-                # This will cancel the task for us
+                # Raising the TimeoutError will cancel the task.
                 self.log.warning(f"Watch task did not finish in {timeout}s" +
                                  "and was cancelled")
         self.watch_task = None

From ef550462e987f70b11ac834ae8d2a771592889c3 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Tue, 1 Feb 2022 10:23:08 -0700
Subject: [PATCH 12/56] Rename package for upload

---
 setup.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/setup.py b/setup.py
index c8068345..564ede80 100644
--- a/setup.py
+++ b/setup.py
@@ -7,13 +7,13 @@
 
 v = sys.version_info
 if v[:2] < (3, 6):
-    error = "ERROR: jupyterhub-kubespawner requires Python version 3.6 or above."
+    error = "ERROR: rubin-kubespawner requires Python version 3.6 or above."
     print(error, file=sys.stderr)
     sys.exit(1)
 
 setup(
-    name='jupyterhub-kubespawner',
-    version='2.0.1.dev',
+    name='rubin-kubespawner',
+    version='2.0.1.dev1',
     install_requires=[
         'async_generator>=1.8',
         'escapism',
@@ -34,8 +34,8 @@
             'pytest-asyncio>=0.11.0',
         ]
     },
-    description='JupyterHub Spawner for Kubernetes',
-    url='http://github.com/jupyterhub/kubespawner',
+    description='JupyterHub Spawner for Kubernetes (Rubin asyncio version)',
+    url='http://github.com/lsst-sqre/kubespawner',
     author='Jupyter Contributors',
     author_email='jupyter@googlegroups.com',
     long_description=open("README.md").read(),

From c1278c378a5d3b284d092f1a92bd6476c868d602 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 2 Feb 2022 15:28:22 -0700
Subject: [PATCH 13/56] Brutal, but working

---
 tests/conftest.py     | 314 ++++++++++++++++++++++++++++++++++--------
 tests/test_spawner.py |  31 +++--
 2 files changed, 274 insertions(+), 71 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index 7e6aab50..48509174 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,4 +1,5 @@
 """pytest fixtures for kubespawner"""
+import asyncio
 import base64
 import inspect
 import io
@@ -7,9 +8,9 @@
 import sys
 import tarfile
 import time
-from distutils.version import LooseVersion as V
+from packaging.version import Version as V
 from functools import partial
-from threading import Thread
+from threading import Thread, Event
 
 import kubernetes_asyncio
 import pytest
@@ -25,16 +26,32 @@
 from kubernetes_asyncio.client import V1ServiceSpec
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
-#from kubernetes.stream import stream
-#from kubernetes_asyncio.stream import stream
+# from kubernetes_asyncio.stream import stream
 from kubernetes_asyncio.watch import Watch
 from traitlets.config import Config
 
 from kubespawner.clients import shared_client
 
+# Needed for the streaming stuff
+import kubernetes as sync_kubernetes
+from kubernetes.stream import stream as sync_stream
+from kubernetes.config import load_kube_config as sync_load_kube_config
+from kubernetes.client import CoreV1Api as sync_CoreV1Api
+from kubernetes.watch import Watch as sync_Watch
+from kubernetes.client.rest import ApiException as syncApiException
+
 here = os.path.abspath(os.path.dirname(__file__))
 jupyterhub_config_py = os.path.join(here, "jupyterhub_config.py")
 
+watch_task = {}
+sync_load_kube_config()
+sync_corev1api = sync_CoreV1Api()
+
+@pytest.fixture(scope="session")
+def event_loop():
+    loop=asyncio.get_event_loop()
+    yield loop
+    loop.close()
 
 @pytest.fixture(autouse=True)
 def traitlets_logging():
@@ -101,21 +118,23 @@ def ssl_app(tmpdir_factory, kube_ns):
     app.init_internal_ssl()
     return app
 
+@pytest.fixture(scope="session")
+def stop_event():
+    return Event()
+
 
-def watch_logs(kube_client, pod_info):
+async def watch_logs(kube_client, pod_info):
     """Stream a single pod's logs
 
     pod logs are streamed directly to sys.stderr,
     so that pytest capture can deal with it.
 
-    Blocking, should be run in a thread.
-
     Called for each new pod from watch_kubernetes
     """
     watch = Watch()
     while True:
         try:
-            for event in watch.stream(
+            async for event in watch.stream(
                 func=kube_client.read_namespaced_pod_log,
                 namespace=pod_info.namespace,
                 name=pod_info.name,
@@ -125,32 +144,69 @@ def watch_logs(kube_client, pod_info):
             if e.status == 400:
                 # 400 can occur if the container is not yet ready
                 # wait and retry
-                time.sleep(1)
+                await asyncio.sleep(1)
                 continue
             elif e.status == 404:
                 # pod is gone, we are done
                 return
             else:
-                # unexpeced error
+                # unexpected error
                 print(f"Error watching logs for {pod_info.name}: {e}", file=sys.stderr)
                 raise
         else:
             break
 
 
-def watch_kubernetes(kube_client, kube_ns):
+async def watch_kubernetes(kube_client, kube_ns, stop_event):
     """Stream kubernetes events to stdout
 
     so that pytest io capturing can include k8s events and logs
 
     All events are streamed to stdout
 
+    When a new pod is started, spawn an additional task to watch its logs
+    """
+
+    watch = Watch()
+    async for event in watch.stream(
+        func=kube_client.list_namespaced_event,
+        namespace=kube_ns,
+    ):
+
+        resource = event['object']
+        obj = resource.involved_object
+        print(f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}")
+
+        # new pod appeared, start streaming its logs
+        if (
+            obj.kind == "Pod"
+            and event["type"] == "ADDED"
+            and obj.name not in watch_task
+        ):
+            watch_task[obj.name] = asyncio.create_task(
+                watch_logs(
+                    kube_client,
+                    obj,
+                ),
+            )
+
+        # Were we asked to stop?
+        if stop_event.is_set():
+            break
+
+
+def sync_watch_kubernetes(sync_kube_client, kube_ns):
+    """Stream kubernetes events to stdout
+    so that pytest io capturing can include k8s events and logs
+    All events are streamed to stdout
     When a new pod is started, spawn an additional thread to watch its logs
+
+    Synchronous version.
     """
     log_threads = {}
-    watch = Watch()
+    watch = sync_Watch()
     for event in watch.stream(
-        func=kube_client.list_namespaced_event,
+        func=sync_kube_client.list_namespaced_event,
         namespace=kube_ns,
     ):
 
@@ -165,13 +221,71 @@ def watch_kubernetes(kube_client, kube_ns):
             and obj.name not in log_threads
         ):
             log_threads[obj.name] = t = Thread(
-                target=watch_logs, args=(kube_client, obj), daemon=True
+                target=sync_watch_logs, args=(kube_client, obj), daemon=True
             )
             t.start()
 
+def sync_watch_logs(sync_kube_client, pod_info):
+    """Stream a single pod's logs
+    pod logs are streamed directly to sys.stdout
+    so that pytest capture can deal with it.
+    Blocking, should be run in a thread.
+    Called for each new pod from sync watch_kubernetes.
+
+    Synchronous version.
+    """
+    watch = sync_Watch()
+    while True:
+        try:
+            for event in watch.stream(
+                func=sync_corev1api.read_namespaced_pod_log,
+                namespace=pod_info.namespace,
+                name=pod_info.name,
+            ):
+                print(f"[{pod_info.name}]: {event}")
+        except syncApiException as e:
+            if e.status == 400:
+                # 400 can occur if the container is not yet ready
+                # wait and retry
+                time.sleep(1)
+                continue
+            elif e.status == 404:
+                # pod is gone, we are done
+                return
+            else:
+                # unexpeced error
+                print(f"Error watching logs for {pod_info.name}: {e}", file=sys.stderr)
+                raise
+        else:
+            break
+
+@pytest.fixture(scope="session")
+def sync_kube_client(request, kube_ns):
+    """fixture for a synchronous Kubernetes client object, needed for
+    multi-channel websockets.
+    """
+    sync_load_kube_config()
+    client = sync_corev1api
+    try:
+        namespaces = client.list_namespace(_request_timeout=3)
+    except Exception as e:
+        pytest.skip("Kubernetes not found: %s" % e)
+
+    if not any(ns.metadata.name == kube_ns for ns in namespaces.items):
+        print("Creating namespace %s" % kube_ns)
+        client.create_namespace(V1Namespace(metadata=dict(name=kube_ns)))
+    else:
+        print("Using existing namespace %s" % kube_ns)
+
+    # begin streaming all logs and events in our test namespace
+    t = Thread(target=sync_watch_kubernetes, args=(client, kube_ns),
+               daemon=True)
+    t.start()
+
+    return client
 
 @pytest.fixture(scope="session")
-def kube_client(request, kube_ns):
+async def kube_client(request, kube_ns, stop_event):
     """fixture for the Kubernetes client object.
 
     skips test that require kubernetes if kubernetes cannot be contacted
@@ -180,29 +294,28 @@ def kube_client(request, kube_ns):
     - Hooks up kubernetes events and logs to pytest capture
     - Cleans up kubernetes namespace on exit
     """
-    load_kube_config()
+    await load_kube_config()
     client = shared_client("CoreV1Api")
     try:
-        namespaces = client.list_namespace(_request_timeout=3)
+        namespaces = await client.list_namespace(_request_timeout=3)
     except Exception as e:
         pytest.skip("Kubernetes not found: %s" % e)
 
     if not any(ns.metadata.name == kube_ns for ns in namespaces.items):
         print("Creating namespace %s" % kube_ns)
-        client.create_namespace(V1Namespace(metadata=dict(name=kube_ns)))
+        await client.create_namespace(V1Namespace(metadata=dict(name=kube_ns)))
     else:
         print("Using existing namespace %s" % kube_ns)
 
     # begin streaming all logs and events in our test namespace
-    t = Thread(target=watch_kubernetes, args=(client, kube_ns), daemon=True)
-    t.start()
+    t = asyncio.create_task(watch_kubernetes(client, kube_ns,stop_event))
 
     # delete the test namespace when we finish
-    def cleanup_namespace():
-        client.delete_namespace(kube_ns, body={}, grace_period_seconds=0)
-        for i in range(3):
+    async def cleanup_namespace():
+        await client.delete_namespace(kube_ns, body={}, grace_period_seconds=0)
+        for i in range(20):
             try:
-                ns = client.read_namespace(kube_ns)
+                ns = await client.read_namespace(kube_ns)
             except ApiException as e:
                 if e.status == 404:
                     return
@@ -210,19 +323,50 @@ def cleanup_namespace():
                     raise
             else:
                 print("waiting for %s to delete" % kube_ns)
-                time.sleep(1)
+                await asyncio.sleep(1)
+
+    yield client
 
+    stop_event.set()
+    # Give our watches a little while to cancel
+    await asyncio.sleep(3)
+    # This runs to clean up our watch tasks
+    for t in watch_task:
+        if watch_task[t] and not watch_task[t].done():
+            try:
+                watch_task[t].cancel()
+            except asyncio.CancelledError:
+                pass
     # allow opting out of namespace cleanup, for post-mortem debugging
     if not os.environ.get("KUBESPAWNER_DEBUG_NAMESPACE"):
-        request.addfinalizer(cleanup_namespace)
-    return client
+        await cleanup_namespace()
 
 
-def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
+async def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
     """Wait for a pod to be ready"""
     conditions = {}
     for i in range(int(timeout)):
-        pod = kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
+        pod = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
+        for condition in pod.status.conditions or []:
+            conditions[condition.type] = condition.status
+
+        if conditions.get("Ready") != "True":
+            print(
+                f"Waiting for pod {kube_ns}/{pod_name}; current status: {pod.status.phase}; {conditions}"
+            )
+            await asyncio.sleep(1)
+        else:
+            break
+
+    if conditions.get("Ready") != "True":
+        raise TimeoutError(f"pod {kube_ns}/{pod_name} failed to start: {pod.status}")
+    return pod
+
+def sync_wait_for_pod(sync_kube_client, kube_ns, pod_name, timeout=90):
+    """Wait for a pod to be ready"""
+    conditions = {}
+    for i in range(int(timeout)):
+        pod = sync_kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
         for condition in pod.status.conditions or []:
             conditions[condition.type] = condition.status
 
@@ -239,7 +383,8 @@ def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
     return pod
 
 
-def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
+
+async def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
     """Ensure an object doesn't exist
 
     Request deletion and wait for it to be gone
@@ -247,7 +392,7 @@ def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
     delete = getattr(kube_client, "delete_namespaced_{}".format(resource_type))
     read = getattr(kube_client, "read_namespaced_{}".format(resource_type))
     try:
-        delete(namespace=kube_ns, name=name)
+        await delete(namespace=kube_ns, name=name)
     except ApiException as e:
         if e.status != 404:
             raise
@@ -255,7 +400,7 @@ def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
     while True:
         # wait for delete
         try:
-            read(namespace=kube_ns, name=name)
+            await read(namespace=kube_ns, name=name)
         except ApiException as e:
             if e.status == 404:
                 # deleted
@@ -264,10 +409,35 @@ def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
                 raise
         else:
             print("waiting for {}/{} to delete".format(resource_type, name))
-            time.sleep(1)
+            await asyncio.sleep(1)
+
+def sync_ensure_not_exists(sync_kube_client, kube_ns, name, resource_type, timeout=30):
+    """Ensure an object doesn't exist
+    Request deletion and wait for it to be gone
+    """
+    delete = getattr(sync_kube_client, "delete_namespaced_{}".format(resource_type))
+    read = getattr(sync_kube_client, "read_namespaced_{}".format(resource_type))
+    try:
+        delete(namespace=kube_ns, name=name)
+    except syncApiException as e:
+        if e.status != 404:
+            raise
 
+    while True:
+        # wait for delete
+        try:
+            read(namespace=kube_ns, name=name)
+        except syncApiException as e:
+            if e.status == 404:
+                # deleted
+                break
+            else:
+                raise
+        else:
+            print("waiting for {}/{} to delete".format(resource_type, name))
+            time.sleep(1)
 
-def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=True):
+async def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=True):
     """Create a kubernetes resource
 
     handling 409 errors and others that can occur due to rapid startup
@@ -275,17 +445,49 @@ def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=
     """
     name = manifest.metadata["name"]
     if delete_first:
-        ensure_not_exists(kube_client, kube_ns, name, resource_type)
+        await ensure_not_exists(kube_client, kube_ns, name, resource_type)
     print(f"Creating {resource_type} {name}")
     create = getattr(kube_client, f"create_namespaced_{resource_type}")
     error = None
     for i in range(10):
         try:
-            create(
+            await create(
                 body=manifest,
                 namespace=kube_ns,
             )
         except ApiException as e:
+            if e.status == 409:
+                break
+            error = e
+            # need to retry since this can fail if run too soon after namespace creation
+            print(e, file=sys.stderr)
+            await asyncio.sleep(int(e.headers.get("Retry-After", 1)))
+        else:
+            break
+    else:
+        raise error
+
+
+def sync_create_resource(sync_kube_client, kube_ns, resource_type, manifest, delete_first=True):
+    """Create a kubernetes resource
+    handling 409 errors and others that can occur due to rapid startup
+    (typically: default service account doesn't exist yet
+
+    Synchronous version (for execcing in pod).
+    """
+    name = manifest.metadata["name"]
+    if delete_first:
+        sync_ensure_not_exists(sync_kube_client, kube_ns, name, resource_type)
+    print(f"Creating {resource_type} {name}")
+    create = getattr(sync_kube_client, f"create_namespaced_{resource_type}")
+    error = None
+    for i in range(10):
+        try:
+            create(
+                body=manifest,
+                namespace=kube_ns,
+            )
+        except syncApiException as e:
             if e.status == 409:
                 break
             error = e
@@ -298,7 +500,7 @@ def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=
         raise error
 
 
-def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
+async def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
     config_map_name = pod_name + "-config"
     secret_name = pod_name + "-secret"
     with open(jupyterhub_config_py) as f:
@@ -308,7 +510,7 @@ def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
         metadata={"name": config_map_name}, data={"jupyterhub_config.py": config}
     )
 
-    config_map = create_resource(
+    config_map = await create_resource(
         kube_client,
         kube_ns,
         "config_map",
@@ -361,14 +563,14 @@ def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
             ],
         ),
     )
-    pod = create_resource(kube_client, kube_ns, "pod", pod_manifest)
-    return wait_for_pod(kube_client, kube_ns, pod_name)
+    pod = await create_resource(kube_client, kube_ns, "pod", pod_manifest)
+    return await wait_for_pod(kube_client, kube_ns, pod_name)
 
 
 @pytest.fixture(scope="session")
-def hub_pod(kube_client, kube_ns):
+async def hub_pod(kube_client, kube_ns):
     """Create and return a pod running jupyterhub"""
-    return create_hub_pod(kube_client, kube_ns)
+    return await create_hub_pod(kube_client, kube_ns)
 
 
 @pytest.fixture
@@ -381,7 +583,7 @@ def hub(hub_pod):
 
 
 @pytest.fixture(scope="session")
-def hub_pod_ssl(kube_client, kube_ns, ssl_app):
+async def hub_pod_ssl(kube_client, kube_ns, ssl_app):
     """Start a hub pod with internal_ssl enabled"""
     # load ssl dir to tarfile
     buf = io.BytesIO()
@@ -394,7 +596,7 @@ def hub_pod_ssl(kube_client, kube_ns, ssl_app):
     secret_manifest = V1Secret(
         metadata={"name": secret_name}, data={"internal-ssl.tar": b64_certs}
     )
-    create_resource(kube_client, kube_ns, "secret", secret_manifest)
+    await create_resource(kube_client, kube_ns, "secret", secret_manifest)
 
     name = "hub-ssl"
 
@@ -407,9 +609,9 @@ def hub_pod_ssl(kube_client, kube_ns, ssl_app):
         ),
     )
 
-    create_resource(kube_client, kube_ns, "service", service_manifest)
+    await create_resource(kube_client, kube_ns, "service", service_manifest)
 
-    return create_hub_pod(
+    return await create_hub_pod(
         kube_client,
         kube_ns,
         pod_name=name,
@@ -444,7 +646,7 @@ def __str__(self):
         )
 
 
-def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0):
+def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0):
     """Run simple Python code in a pod
 
     code can be a str of code, or a 'simple' Python function,
@@ -452,11 +654,11 @@ def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retr
 
     kwargs are passed to the function, if it is given.
     """
-    if V(kubernetes.__version__) < V("11"):
+    if V(sync_kubernetes.__version__) < V("11"):
         pytest.skip(
             f"exec tests require kubernetes >= 11, got {kubernetes.__version__}"
         )
-    pod = wait_for_pod(kube_client, kube_ns, pod_name)
+    pod = sync_wait_for_pod(sync_kube_client, kube_ns, pod_name)
     original_code = code
     if not isinstance(code, str):
         # allow simple self-contained (no globals or args) functions
@@ -480,8 +682,8 @@ def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retr
     print("Running {} in {}".format(code, pod_name))
     # need to create ws client to get returncode,
     # see https://github.com/kubernetes-client/python/issues/812
-    client = stream(
-        kube_client.connect_get_namespaced_pod_exec,
+    client = sync_stream(
+        sync_kube_client.connect_get_namespaced_pod_exec,
         pod_name,
         namespace=kube_ns,
         command=exec_command,
@@ -517,16 +719,16 @@ def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retr
 
 
 @pytest.fixture
-def exec_python_pod(kube_client, kube_ns):
+def exec_python_pod(sync_kube_client, kube_ns):
     """Fixture to return callable to execute python in a pod by name
 
     Used as a fixture to contain references to client, namespace
     """
-    return partial(_exec_python_in_pod, kube_client, kube_ns)
+    return partial(_exec_python_in_pod, sync_kube_client, kube_ns)
 
 
 @pytest.fixture(scope="session")
-def exec_python(kube_ns, kube_client):
+def exec_python(kube_ns, sync_kube_client):
     """Return a callable to execute Python code in a pod in the test namespace
 
     This fixture creates a dedicated pod for executing commands
@@ -551,6 +753,6 @@ def exec_python(kube_ns, kube_client):
             termination_grace_period_seconds=0,
         ),
     )
-    pod = create_resource(kube_client, kube_ns, "pod", pod_manifest)
+    pod = sync_create_resource(sync_kube_client, kube_ns, "pod", pod_manifest)
 
-    yield partial(_exec_python_in_pod, kube_client, kube_ns, pod_name)
+    yield partial(_exec_python_in_pod, sync_kube_client, kube_ns, pod_name)
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index df4dce35..0cfe2038 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -1,3 +1,4 @@
+import asyncio
 import json
 import os
 import time
@@ -174,12 +175,12 @@ async def test_spawn_start(
     url = await spawner.start()
 
     # verify the pod exists
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert pod_name in pod_names
 
     # pod should be running when start returns
-    pod = kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
+    pod = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
     assert pod.status.phase == "Running"
 
     # verify poll while running
@@ -194,7 +195,7 @@ async def test_spawn_start(
     await spawner.stop()
 
     # verify pod is gone
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert pod_name not in pod_names
 
@@ -234,7 +235,7 @@ async def test_spawn_internal_ssl(
     url = await spawner.start()
     pod_name = "jupyter-%s" % spawner.user.name
     # verify the pod exists
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert pod_name in pod_names
     # verify poll while running
@@ -243,12 +244,12 @@ async def test_spawn_internal_ssl(
 
     # verify service and secret exist
     secret_name = spawner.secret_name
-    secrets = kube_client.list_namespaced_secret(kube_ns).items
+    secrets = (await kube_client.list_namespaced_secret(kube_ns)).items
     secret_names = [s.metadata.name for s in secrets]
     assert secret_name in secret_names
 
     service_name = pod_name
-    services = kube_client.list_namespaced_service(kube_ns).items
+    services = (await kube_client.list_namespaced_service(kube_ns)).items
     service_names = [s.metadata.name for s in services]
     assert service_name in service_names
 
@@ -279,20 +280,20 @@ async def test_spawn_internal_ssl(
     await spawner.stop()
 
     # verify pod is gone
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = await (kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert "jupyter-%s" % spawner.user.name not in pod_names
 
     # verify service and secret are gone
     # it may take a little while for them to get cleaned up
     for i in range(5):
-        secrets = kube_client.list_namespaced_secret(kube_ns).items
+        secrets = (await kube_client.list_namespaced_secret(kube_ns)).items
         secret_names = {s.metadata.name for s in secrets}
 
-        services = kube_client.list_namespaced_service(kube_ns).items
+        services = (await kube_client.list_namespaced_service(kube_ns)).items
         service_names = {s.metadata.name for s in services}
         if secret_name in secret_names or service_name in service_names:
-            time.sleep(1)
+            await asyncio.sleep(1)
         else:
             break
     assert secret_name not in secret_names
@@ -777,13 +778,13 @@ async def test_delete_pvc(kube_ns, kube_client, hub, config):
 
     # verify the pod exists
     pod_name = "jupyter-%s" % spawner.user.name
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert pod_name in pod_names
 
     # verify PVC is created
     pvc_name = spawner.pvc_name
-    pvc_list = kube_client.list_namespaced_persistent_volume_claim(kube_ns).items
+    pvc_list = (await kube_client.list_namespaced_persistent_volume_claim(kube_ns)).items
     pvc_names = [s.metadata.name for s in pvc_list]
     assert pvc_name in pvc_names
 
@@ -791,7 +792,7 @@ async def test_delete_pvc(kube_ns, kube_client, hub, config):
     await spawner.stop()
 
     # verify pod is gone
-    pods = kube_client.list_namespaced_pod(kube_ns).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert "jupyter-%s" % spawner.user.name not in pod_names
 
@@ -800,10 +801,10 @@ async def test_delete_pvc(kube_ns, kube_client, hub, config):
 
     # verify PVC is deleted, it may take a little while
     for i in range(5):
-        pvc_list = kube_client.list_namespaced_persistent_volume_claim(kube_ns).items
+        pvc_list = (await kube_client.list_namespaced_persistent_volume_claim(kube_ns)).items
         pvc_names = [s.metadata.name for s in pvc_list]
         if pvc_name in pvc_names:
-            time.sleep(1)
+            await asyncio.sleep(1)
         else:
             break
     assert pvc_name not in pvc_names

From b1080b2865db8cafffb77e1efb4dbb098d71a974 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 2 Feb 2022 17:42:52 -0700
Subject: [PATCH 14/56] Remove some brutality

---
 setup.py              |   1 +
 tests/conftest.py     | 197 ++++--------------------------------------
 tests/test_spawner.py |   4 +-
 tests/test_utils.py   |   8 +-
 4 files changed, 22 insertions(+), 188 deletions(-)

diff --git a/setup.py b/setup.py
index 564ede80..480f6360 100644
--- a/setup.py
+++ b/setup.py
@@ -29,6 +29,7 @@
         'test': [
             'bump2version',
             'flake8',
+            'kubernetes>=10.1.0',
             'pytest>=5.4',
             'pytest-cov',
             'pytest-asyncio>=0.11.0',
diff --git a/tests/conftest.py b/tests/conftest.py
index 48509174..f7a7d15f 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -33,12 +33,9 @@
 from kubespawner.clients import shared_client
 
 # Needed for the streaming stuff
-import kubernetes as sync_kubernetes
 from kubernetes.stream import stream as sync_stream
 from kubernetes.config import load_kube_config as sync_load_kube_config
 from kubernetes.client import CoreV1Api as sync_CoreV1Api
-from kubernetes.watch import Watch as sync_Watch
-from kubernetes.client.rest import ApiException as syncApiException
 
 here = os.path.abspath(os.path.dirname(__file__))
 jupyterhub_config_py = os.path.join(here, "jupyterhub_config.py")
@@ -194,96 +191,6 @@ async def watch_kubernetes(kube_client, kube_ns, stop_event):
         if stop_event.is_set():
             break
 
-
-def sync_watch_kubernetes(sync_kube_client, kube_ns):
-    """Stream kubernetes events to stdout
-    so that pytest io capturing can include k8s events and logs
-    All events are streamed to stdout
-    When a new pod is started, spawn an additional thread to watch its logs
-
-    Synchronous version.
-    """
-    log_threads = {}
-    watch = sync_Watch()
-    for event in watch.stream(
-        func=sync_kube_client.list_namespaced_event,
-        namespace=kube_ns,
-    ):
-
-        resource = event['object']
-        obj = resource.involved_object
-        print(f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}")
-
-        # new pod appeared, start streaming its logs
-        if (
-            obj.kind == "Pod"
-            and event["type"] == "ADDED"
-            and obj.name not in log_threads
-        ):
-            log_threads[obj.name] = t = Thread(
-                target=sync_watch_logs, args=(kube_client, obj), daemon=True
-            )
-            t.start()
-
-def sync_watch_logs(sync_kube_client, pod_info):
-    """Stream a single pod's logs
-    pod logs are streamed directly to sys.stdout
-    so that pytest capture can deal with it.
-    Blocking, should be run in a thread.
-    Called for each new pod from sync watch_kubernetes.
-
-    Synchronous version.
-    """
-    watch = sync_Watch()
-    while True:
-        try:
-            for event in watch.stream(
-                func=sync_corev1api.read_namespaced_pod_log,
-                namespace=pod_info.namespace,
-                name=pod_info.name,
-            ):
-                print(f"[{pod_info.name}]: {event}")
-        except syncApiException as e:
-            if e.status == 400:
-                # 400 can occur if the container is not yet ready
-                # wait and retry
-                time.sleep(1)
-                continue
-            elif e.status == 404:
-                # pod is gone, we are done
-                return
-            else:
-                # unexpeced error
-                print(f"Error watching logs for {pod_info.name}: {e}", file=sys.stderr)
-                raise
-        else:
-            break
-
-@pytest.fixture(scope="session")
-def sync_kube_client(request, kube_ns):
-    """fixture for a synchronous Kubernetes client object, needed for
-    multi-channel websockets.
-    """
-    sync_load_kube_config()
-    client = sync_corev1api
-    try:
-        namespaces = client.list_namespace(_request_timeout=3)
-    except Exception as e:
-        pytest.skip("Kubernetes not found: %s" % e)
-
-    if not any(ns.metadata.name == kube_ns for ns in namespaces.items):
-        print("Creating namespace %s" % kube_ns)
-        client.create_namespace(V1Namespace(metadata=dict(name=kube_ns)))
-    else:
-        print("Using existing namespace %s" % kube_ns)
-
-    # begin streaming all logs and events in our test namespace
-    t = Thread(target=sync_watch_kubernetes, args=(client, kube_ns),
-               daemon=True)
-    t.start()
-
-    return client
-
 @pytest.fixture(scope="session")
 async def kube_client(request, kube_ns, stop_event):
     """fixture for the Kubernetes client object.
@@ -346,7 +253,8 @@ async def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
     """Wait for a pod to be ready"""
     conditions = {}
     for i in range(int(timeout)):
-        pod = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
+        p = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
+        pod = p
         for condition in pod.status.conditions or []:
             conditions[condition.type] = condition.status
 
@@ -362,27 +270,6 @@ async def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
         raise TimeoutError(f"pod {kube_ns}/{pod_name} failed to start: {pod.status}")
     return pod
 
-def sync_wait_for_pod(sync_kube_client, kube_ns, pod_name, timeout=90):
-    """Wait for a pod to be ready"""
-    conditions = {}
-    for i in range(int(timeout)):
-        pod = sync_kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
-        for condition in pod.status.conditions or []:
-            conditions[condition.type] = condition.status
-
-        if conditions.get("Ready") != "True":
-            print(
-                f"Waiting for pod {kube_ns}/{pod_name}; current status: {pod.status.phase}; {conditions}"
-            )
-            time.sleep(1)
-        else:
-            break
-
-    if conditions.get("Ready") != "True":
-        raise TimeoutError(f"pod {kube_ns}/{pod_name} failed to start: {pod.status}")
-    return pod
-
-
 
 async def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=30):
     """Ensure an object doesn't exist
@@ -411,31 +298,6 @@ async def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=3
             print("waiting for {}/{} to delete".format(resource_type, name))
             await asyncio.sleep(1)
 
-def sync_ensure_not_exists(sync_kube_client, kube_ns, name, resource_type, timeout=30):
-    """Ensure an object doesn't exist
-    Request deletion and wait for it to be gone
-    """
-    delete = getattr(sync_kube_client, "delete_namespaced_{}".format(resource_type))
-    read = getattr(sync_kube_client, "read_namespaced_{}".format(resource_type))
-    try:
-        delete(namespace=kube_ns, name=name)
-    except syncApiException as e:
-        if e.status != 404:
-            raise
-
-    while True:
-        # wait for delete
-        try:
-            read(namespace=kube_ns, name=name)
-        except syncApiException as e:
-            if e.status == 404:
-                # deleted
-                break
-            else:
-                raise
-        else:
-            print("waiting for {}/{} to delete".format(resource_type, name))
-            time.sleep(1)
 
 async def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=True):
     """Create a kubernetes resource
@@ -468,36 +330,6 @@ async def create_resource(kube_client, kube_ns, resource_type, manifest, delete_
         raise error
 
 
-def sync_create_resource(sync_kube_client, kube_ns, resource_type, manifest, delete_first=True):
-    """Create a kubernetes resource
-    handling 409 errors and others that can occur due to rapid startup
-    (typically: default service account doesn't exist yet
-
-    Synchronous version (for execcing in pod).
-    """
-    name = manifest.metadata["name"]
-    if delete_first:
-        sync_ensure_not_exists(sync_kube_client, kube_ns, name, resource_type)
-    print(f"Creating {resource_type} {name}")
-    create = getattr(sync_kube_client, f"create_namespaced_{resource_type}")
-    error = None
-    for i in range(10):
-        try:
-            create(
-                body=manifest,
-                namespace=kube_ns,
-            )
-        except syncApiException as e:
-            if e.status == 409:
-                break
-            error = e
-            # need to retry since this can fail if run too soon after namespace creation
-            print(e, file=sys.stderr)
-            time.sleep(int(e.headers.get("Retry-After", 1)))
-        else:
-            break
-    else:
-        raise error
 
 
 async def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
@@ -564,7 +396,8 @@ async def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
         ),
     )
     pod = await create_resource(kube_client, kube_ns, "pod", pod_manifest)
-    return await wait_for_pod(kube_client, kube_ns, pod_name)
+    p = await wait_for_pod(kube_client, kube_ns, pod_name)
+    return p
 
 
 @pytest.fixture(scope="session")
@@ -646,7 +479,7 @@ def __str__(self):
         )
 
 
-def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0):
+async def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0):
     """Run simple Python code in a pod
 
     code can be a str of code, or a 'simple' Python function,
@@ -654,11 +487,11 @@ def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None,
 
     kwargs are passed to the function, if it is given.
     """
-    if V(sync_kubernetes.__version__) < V("11"):
+    if V(kubernetes_asyncio.__version__) < V("11"):
         pytest.skip(
             f"exec tests require kubernetes >= 11, got {kubernetes.__version__}"
         )
-    pod = sync_wait_for_pod(sync_kube_client, kube_ns, pod_name)
+    pod = await wait_for_pod(kube_client, kube_ns, pod_name)
     original_code = code
     if not isinstance(code, str):
         # allow simple self-contained (no globals or args) functions
@@ -683,7 +516,7 @@ def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None,
     # need to create ws client to get returncode,
     # see https://github.com/kubernetes-client/python/issues/812
     client = sync_stream(
-        sync_kube_client.connect_get_namespaced_pod_exec,
+        sync_corev1api.connect_get_namespaced_pod_exec,
         pod_name,
         namespace=kube_ns,
         command=exec_command,
@@ -706,8 +539,8 @@ def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None,
             raise ExecError(exit_code=returncode, message=stderr, command=code)
         else:
             # retry
-            time.sleep(1)
-            return _exec_python_in_pod(
+            await asyncio.sleep(1)
+            return await _exec_python_in_pod(
                 kube_client,
                 kube_ns,
                 pod_name,
@@ -719,16 +552,16 @@ def _exec_python_in_pod(sync_kube_client, kube_ns, pod_name, code, kwargs=None,
 
 
 @pytest.fixture
-def exec_python_pod(sync_kube_client, kube_ns):
+def exec_python_pod(kube_client, kube_ns):
     """Fixture to return callable to execute python in a pod by name
 
     Used as a fixture to contain references to client, namespace
     """
-    return partial(_exec_python_in_pod, sync_kube_client, kube_ns)
+    return partial(_exec_python_in_pod, kube_client, kube_ns)
 
 
 @pytest.fixture(scope="session")
-def exec_python(kube_ns, sync_kube_client):
+async def exec_python(kube_client, kube_ns):
     """Return a callable to execute Python code in a pod in the test namespace
 
     This fixture creates a dedicated pod for executing commands
@@ -753,6 +586,6 @@ def exec_python(kube_ns, sync_kube_client):
             termination_grace_period_seconds=0,
         ),
     )
-    pod = sync_create_resource(sync_kube_client, kube_ns, "pod", pod_manifest)
+    pod = await create_resource(kube_client, kube_ns, "pod", pod_manifest)
 
-    yield partial(_exec_python_in_pod, sync_kube_client, kube_ns, pod_name)
+    yield partial(_exec_python_in_pod, kube_client, kube_ns, pod_name)
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index 0cfe2038..93867d2b 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -188,7 +188,7 @@ async def test_spawn_start(
     assert status is None
 
     # make sure spawn url is correct
-    r = exec_python(check_up, {"url": url}, _retries=3)
+    r = await exec_python(check_up, {"url": url}, _retries=3)
     assert r == "302"
 
     # stop the pod
@@ -263,7 +263,7 @@ async def test_spawn_internal_ssl(
     hub_internal_cert = os.path.join(hub_internal, "hub-internal.crt")
     hub_internal_key = os.path.join(hub_internal, "hub-internal.key")
 
-    r = exec_python_pod(
+    r = await exec_python_pod(
         hub_pod_name,
         check_up,
         {
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 0e8c3425..845119c4 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -33,16 +33,16 @@ def exec_error():
     1 / 0
 
 
-def test_exec(exec_python):
+async def test_exec(exec_python):
     """Test the exec fixture itself"""
-    r = exec_python(print_hello)
+    r = await exec_python(print_hello)
     print("result: %r" % r)
 
 
-def test_exec_error(exec_python):
+async def test_exec_error(exec_python):
     """Test the exec fixture error handling"""
     with pytest.raises(ExecError) as e:
-        exec_python(exec_error)
+        await exec_python(exec_error)
 
 
 def test__get_k8s_model_attribute():

From 11fa7fe2294ef71883548f97ac1411b22e1fdafd Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 10:45:24 -0700
Subject: [PATCH 15/56] Update with non-deserializing kubernetes_asyncio

---
 kubespawner/reflector.py | 18 +++++++-----------
 setup.py                 |  2 +-
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 1072bb9c..bccef325 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -319,8 +319,6 @@ async def _watch_and_update(self):
                     "label_selector": self.label_selector,
                     "field_selector": self.field_selector,
                     "resource_version": resource_version,
-                    # You'd think this would work...but it does nothing.
-                    # "_preload_content": False
                 }
                 if not self.omit_namespace:
                     watch_args["namespace"] = self.namespace
@@ -330,11 +328,13 @@ async def _watch_and_update(self):
                 if self.timeout_seconds:
                     # set watch timeout
                     watch_args['timeout_seconds'] = self.timeout_seconds
-                # Partial application of _preload_content=False causes
-                # a stream of errors of the form:
-                # AttributeError: module 'kubernetes_asyncio.client.models' has no attribute ''
-                # when unmarshaling the event into dict form.
-                method = getattr(self.api, self.list_method_name)
+                # This is a little hack to defeat the Kubernetes client
+                # deserializing the objects it gets, which can use a lot
+                # of CPU in busy clusters.  cf
+                # https://github.com/jupyterhub/kubespawner/pull/424
+                method = partial(
+                    getattr(self.api, self.list_method_name),
+                    _preload_content=False)
                 async with w.stream(method, **watch_args) as stream:
                     async for watch_event in stream:
                     # in case of timeout_seconds, the w.stream just exits (no exception thrown)
@@ -348,10 +348,6 @@ async def _watch_and_update(self):
                         # ref: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#event-v1-core
                         cur_delay = 0.1
                         resource = watch_event['raw_object']
-                        # Unfortunately the asyncio watch is still going to
-                        # deserialize it into watch_event['object'] so we may
-                        # well run into the performance issues from:
-                        # https://github.com/jupyterhub/kubespawner/pull/424
                         ref_key = "{}/{}".format(
                             resource["metadata"]["namespace"], resource["metadata"]["name"]
                         )
diff --git a/setup.py b/setup.py
index 480f6360..df4fc400 100644
--- a/setup.py
+++ b/setup.py
@@ -20,7 +20,7 @@
         'python-slugify',
         'jupyterhub>=0.8',
         'jinja2',
-        'kubernetes_asyncio>=10.1.0',
+        'kubernetes_asyncio>=19.15.1',
         'urllib3',
         'pyYAML',
     ],

From 9597a5d6ce476530eda45753891edfc77765bfd2 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 11:34:01 -0700
Subject: [PATCH 16/56] Remove dead comment and bump version

---
 setup.py          | 2 +-
 tests/conftest.py | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/setup.py b/setup.py
index df4fc400..1fa8082b 100644
--- a/setup.py
+++ b/setup.py
@@ -13,7 +13,7 @@
 
 setup(
     name='rubin-kubespawner',
-    version='2.0.1.dev1',
+    version='2.0.1.dev2',
     install_requires=[
         'async_generator>=1.8',
         'escapism',
diff --git a/tests/conftest.py b/tests/conftest.py
index f7a7d15f..0d7e353f 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -26,7 +26,6 @@
 from kubernetes_asyncio.client import V1ServiceSpec
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
-# from kubernetes_asyncio.stream import stream
 from kubernetes_asyncio.watch import Watch
 from traitlets.config import Config
 

From 709a29b55298270b2cfcbf55ceeeaf85b09f02e5 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 15:59:29 -0700
Subject: [PATCH 17/56] Address review commentary

---
 setup.py          |  2 +-
 tests/conftest.py | 89 ++++++++++++++++++++++++++---------------------
 2 files changed, 50 insertions(+), 41 deletions(-)

diff --git a/setup.py b/setup.py
index 1fa8082b..9a41c3bf 100644
--- a/setup.py
+++ b/setup.py
@@ -13,7 +13,7 @@
 
 setup(
     name='rubin-kubespawner',
-    version='2.0.1.dev2',
+    version='2.0.1.dev3',
     install_requires=[
         'async_generator>=1.8',
         'escapism',
diff --git a/tests/conftest.py b/tests/conftest.py
index 0d7e353f..555bc08a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -14,6 +14,7 @@
 
 import kubernetes_asyncio
 import pytest
+import pytest_asyncio
 from jupyterhub.app import JupyterHub
 from jupyterhub.objects import Hub
 from kubernetes_asyncio.client import V1ConfigMap
@@ -39,7 +40,8 @@
 here = os.path.abspath(os.path.dirname(__file__))
 jupyterhub_config_py = os.path.join(here, "jupyterhub_config.py")
 
-watch_task = {}
+# We do these to set up the synchronous client, needed for executing
+# python inside pods.
 sync_load_kube_config()
 sync_corev1api = sync_CoreV1Api()
 
@@ -66,7 +68,6 @@ def kube_ns():
     """Fixture for the kubernetes namespace"""
     return os.environ.get("KUBESPAWNER_TEST_NAMESPACE") or "kubespawner-test"
 
-
 @pytest.fixture
 def config(kube_ns):
     """Return a traitlets Config object
@@ -114,15 +115,11 @@ def ssl_app(tmpdir_factory, kube_ns):
     app.init_internal_ssl()
     return app
 
-@pytest.fixture(scope="session")
-def stop_event():
-    return Event()
-
 
 async def watch_logs(kube_client, pod_info):
     """Stream a single pod's logs
 
-    pod logs are streamed directly to sys.stderr,
+    pod logs are streamed directly to sys.stdout,
     so that pytest capture can deal with it.
 
     Called for each new pod from watch_kubernetes
@@ -153,7 +150,7 @@ async def watch_logs(kube_client, pod_info):
             break
 
 
-async def watch_kubernetes(kube_client, kube_ns, stop_event):
+async def watch_kubernetes(kube_client, kube_ns):
     """Stream kubernetes events to stdout
 
     so that pytest io capturing can include k8s events and logs
@@ -164,6 +161,9 @@ async def watch_kubernetes(kube_client, kube_ns, stop_event):
     """
 
     watch = Watch()
+    watch_task = {}
+    stop_signal=kube_client.stop_signal
+    
     async for event in watch.stream(
         func=kube_client.list_namespaced_event,
         namespace=kube_ns,
@@ -187,11 +187,22 @@ async def watch_kubernetes(kube_client, kube_ns, stop_event):
             )
 
         # Were we asked to stop?
-        if stop_event.is_set():
+        if not stop_signal.empty():
             break
 
-@pytest.fixture(scope="session")
-async def kube_client(request, kube_ns, stop_event):
+    # This runs to clean up our watch tasks
+    for t in watch_task:
+        if watch_task[t] and not watch_task[t].done():
+            try:
+                watch_task[t].cancel()
+            except asyncio.CancelledError:
+                pass
+    await stop_signal.get()
+    stop_signal.done()
+
+
+@pytest_asyncio.fixture(scope="session")
+async def kube_client(request, kube_ns):
     """fixture for the Kubernetes client object.
 
     skips test that require kubernetes if kubernetes cannot be contacted
@@ -202,6 +213,7 @@ async def kube_client(request, kube_ns, stop_event):
     """
     await load_kube_config()
     client = shared_client("CoreV1Api")
+    client.stop_signal=asyncio.Queue()
     try:
         namespaces = await client.list_namespace(_request_timeout=3)
     except Exception as e:
@@ -214,12 +226,23 @@ async def kube_client(request, kube_ns, stop_event):
         print("Using existing namespace %s" % kube_ns)
 
     # begin streaming all logs and events in our test namespace
-    t = asyncio.create_task(watch_kubernetes(client, kube_ns,stop_event))
+    t = asyncio.create_task(watch_kubernetes(client, kube_ns))
 
-    # delete the test namespace when we finish
-    async def cleanup_namespace():
+    yield client
+
+    # Clean up at close
+    await client.stop_signal.put(1)
+    # Wait until tasks have been cancelled
+    #
+    # I do not understand why join() really really blocks everything and the
+    # queue never gets emptied.
+    #   await client.stop_signal.join()
+    if not client.stop_signal.empty():
+        await asyncio.sleep(1)
+    # allow opting out of namespace cleanup, for post-mortem debugging
+    if not os.environ.get("KUBESPAWNER_DEBUG_NAMESPACE"):
         await client.delete_namespace(kube_ns, body={}, grace_period_seconds=0)
-        for i in range(20):
+        for i in range(20):  # Usually finishes a good deal faster
             try:
                 ns = await client.read_namespace(kube_ns)
             except ApiException as e:
@@ -231,29 +254,12 @@ async def cleanup_namespace():
                 print("waiting for %s to delete" % kube_ns)
                 await asyncio.sleep(1)
 
-    yield client
-
-    stop_event.set()
-    # Give our watches a little while to cancel
-    await asyncio.sleep(3)
-    # This runs to clean up our watch tasks
-    for t in watch_task:
-        if watch_task[t] and not watch_task[t].done():
-            try:
-                watch_task[t].cancel()
-            except asyncio.CancelledError:
-                pass
-    # allow opting out of namespace cleanup, for post-mortem debugging
-    if not os.environ.get("KUBESPAWNER_DEBUG_NAMESPACE"):
-        await cleanup_namespace()
-
 
 async def wait_for_pod(kube_client, kube_ns, pod_name, timeout=90):
     """Wait for a pod to be ready"""
     conditions = {}
     for i in range(int(timeout)):
-        p = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
-        pod = p
+        pod = await kube_client.read_namespaced_pod(namespace=kube_ns, name=pod_name)
         for condition in pod.status.conditions or []:
             conditions[condition.type] = condition.status
 
@@ -329,8 +335,6 @@ async def create_resource(kube_client, kube_ns, resource_type, manifest, delete_
         raise error
 
 
-
-
 async def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
     config_map_name = pod_name + "-config"
     secret_name = pod_name + "-secret"
@@ -395,11 +399,10 @@ async def create_hub_pod(kube_client, kube_ns, pod_name="hub", ssl=False):
         ),
     )
     pod = await create_resource(kube_client, kube_ns, "pod", pod_manifest)
-    p = await wait_for_pod(kube_client, kube_ns, pod_name)
-    return p
+    return await wait_for_pod(kube_client, kube_ns, pod_name)
 
 
-@pytest.fixture(scope="session")
+@pytest_asyncio.fixture(scope="session")
 async def hub_pod(kube_client, kube_ns):
     """Create and return a pod running jupyterhub"""
     return await create_hub_pod(kube_client, kube_ns)
@@ -414,7 +417,7 @@ def hub(hub_pod):
     return Hub(ip=hub_pod.status.pod_ip, port=8081)
 
 
-@pytest.fixture(scope="session")
+@pytest_asyncio.fixture(scope="session")
 async def hub_pod_ssl(kube_client, kube_ns, ssl_app):
     """Start a hub pod with internal_ssl enabled"""
     # load ssl dir to tarfile
@@ -514,6 +517,12 @@ async def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None,
     print("Running {} in {}".format(code, pod_name))
     # need to create ws client to get returncode,
     # see https://github.com/kubernetes-client/python/issues/812
+    #
+    # That's why we are using the synchronous Kubernetes client here
+    # and why we imported them in the first place: kubernetes_asyncio
+    # does not yet support multichannel ws clients, which are needed
+    # to get the return code.
+    # cf https://github.com/tomplus/kubernetes_asyncio/issues/12
     client = sync_stream(
         sync_corev1api.connect_get_namespaced_pod_exec,
         pod_name,
@@ -559,7 +568,7 @@ def exec_python_pod(kube_client, kube_ns):
     return partial(_exec_python_in_pod, kube_client, kube_ns)
 
 
-@pytest.fixture(scope="session")
+@pytest_asyncio.fixture(scope="session")
 async def exec_python(kube_client, kube_ns):
     """Return a callable to execute Python code in a pod in the test namespace
 

From 6cf57db3cb36a3b8b02106806bfa96405f6a3558 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 17:00:34 -0700
Subject: [PATCH 18/56] I think this is how it *should* work

---
 tests/conftest.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index 555bc08a..c1e90e9e 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -191,14 +191,14 @@ async def watch_kubernetes(kube_client, kube_ns):
             break
 
     # This runs to clean up our watch tasks
+    await stop_signal.get()
     for t in watch_task:
         if watch_task[t] and not watch_task[t].done():
             try:
                 watch_task[t].cancel()
             except asyncio.CancelledError:
                 pass
-    await stop_signal.get()
-    stop_signal.done()
+    stop_signal.task_done()
 
 
 @pytest_asyncio.fixture(scope="session")
@@ -236,9 +236,9 @@ async def kube_client(request, kube_ns):
     #
     # I do not understand why join() really really blocks everything and the
     # queue never gets emptied.
-    #   await client.stop_signal.join()
-    if not client.stop_signal.empty():
-        await asyncio.sleep(1)
+    await client.stop_signal.join()
+    #if not client.stop_signal.empty():
+    #    await asyncio.sleep(1)
     # allow opting out of namespace cleanup, for post-mortem debugging
     if not os.environ.get("KUBESPAWNER_DEBUG_NAMESPACE"):
         await client.delete_namespace(kube_ns, body={}, grace_period_seconds=0)

From 2ca98b27028305eb5de380b10f370f9d1ff5c5c2 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 17:15:55 -0700
Subject: [PATCH 19/56] Use exceptions to handle watches

---
 tests/conftest.py | 84 +++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 43 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index c1e90e9e..4b1c438c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -162,43 +162,43 @@ async def watch_kubernetes(kube_client, kube_ns):
 
     watch = Watch()
     watch_task = {}
-    stop_signal=kube_client.stop_signal
-    
-    async for event in watch.stream(
-        func=kube_client.list_namespaced_event,
-        namespace=kube_ns,
-    ):
-
-        resource = event['object']
-        obj = resource.involved_object
-        print(f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}")
 
-        # new pod appeared, start streaming its logs
-        if (
-            obj.kind == "Pod"
-            and event["type"] == "ADDED"
-            and obj.name not in watch_task
+    try:
+        async for event in watch.stream(
+            func=kube_client.list_namespaced_event,
+            namespace=kube_ns,
         ):
-            watch_task[obj.name] = asyncio.create_task(
-                watch_logs(
-                    kube_client,
-                    obj,
-                ),
-            )
 
-        # Were we asked to stop?
-        if not stop_signal.empty():
-            break
+            resource = event['object']
+            obj = resource.involved_object
+            print(f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}")
 
-    # This runs to clean up our watch tasks
-    await stop_signal.get()
-    for t in watch_task:
-        if watch_task[t] and not watch_task[t].done():
-            try:
-                watch_task[t].cancel()
-            except asyncio.CancelledError:
-                pass
-    stop_signal.task_done()
+            # new pod appeared, start streaming its logs
+            if (
+                obj.kind == "Pod"
+                and event["type"] == "ADDED"
+                and obj.name not in watch_task
+            ):
+                watch_task[obj.name] = asyncio.create_task(
+                    watch_logs(
+                        kube_client,
+                        obj,
+                    ),
+                )
+
+    except asyncio.CancelledError as exc:
+        # kube_client cleanup cancelled us.  In turn, we should cancel
+        # the individual watch tasks.
+        await stop_signal.get()
+        for t in watch_task:
+            if watch_task[t] and not watch_task[t].done():
+                try:
+                    watch_task[t].cancel()
+                except asyncio.CancelledError:
+                    # Swallow these; they are what we expect.
+                    pass
+        # And re-raise so kube_client can finish cleanup
+        raise exc
 
 
 @pytest_asyncio.fixture(scope="session")
@@ -213,7 +213,7 @@ async def kube_client(request, kube_ns):
     """
     await load_kube_config()
     client = shared_client("CoreV1Api")
-    client.stop_signal=asyncio.Queue()
+    stop_signal=asyncio.Queue()
     try:
         namespaces = await client.list_namespace(_request_timeout=3)
     except Exception as e:
@@ -230,15 +230,13 @@ async def kube_client(request, kube_ns):
 
     yield client
 
-    # Clean up at close
-    await client.stop_signal.put(1)
-    # Wait until tasks have been cancelled
-    #
-    # I do not understand why join() really really blocks everything and the
-    # queue never gets emptied.
-    await client.stop_signal.join()
-    #if not client.stop_signal.empty():
-    #    await asyncio.sleep(1)
+    # Clean up at close by sending a cancel to watch_kubernetes and letting
+    # it handle the signal, cancel the tasks *it* started, and then raising
+    # it back to us. 
+    try:
+        t.cancel()
+    except asyncio.CancelledError:
+        pass
     # allow opting out of namespace cleanup, for post-mortem debugging
     if not os.environ.get("KUBESPAWNER_DEBUG_NAMESPACE"):
         await client.delete_namespace(kube_ns, body={}, grace_period_seconds=0)

From c4d557fc324abcb7b25ee1e4afefc3b8e6c229e5 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 17:30:45 -0700
Subject: [PATCH 20/56] Rename back to 'kubespawner' and bump major version

---
 setup.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/setup.py b/setup.py
index 9a41c3bf..03ede204 100644
--- a/setup.py
+++ b/setup.py
@@ -7,13 +7,13 @@
 
 v = sys.version_info
 if v[:2] < (3, 6):
-    error = "ERROR: rubin-kubespawner requires Python version 3.6 or above."
+    error = "ERROR: kubespawner requires Python version 3.6 or above."
     print(error, file=sys.stderr)
     sys.exit(1)
 
 setup(
-    name='rubin-kubespawner',
-    version='2.0.1.dev3',
+    name='kubespawner',
+    version='3.0.0',
     install_requires=[
         'async_generator>=1.8',
         'escapism',

From 6111628947a510cd62a7889ce5a262729750f0e8 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 17:42:00 -0700
Subject: [PATCH 21/56] flake8 cleanup

---
 kubespawner/proxy.py | 4 ++++
 tests/conftest.py    | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 990a3871..6f07fe28 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -121,6 +121,10 @@ async def initialize(cls, *args, **kwargs):
         return inst
 
     async def _initialize_resources(self):
+        labels = {
+            'component': self.component_label,
+            'hub.jupyter.org/proxy-route': 'true',
+        }
         await load_config()
         self._set_k8s_client_configuration()
         self.core_api = shared_client('CoreV1Api')
diff --git a/tests/conftest.py b/tests/conftest.py
index 4b1c438c..e1f9b0e6 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -189,7 +189,6 @@ async def watch_kubernetes(kube_client, kube_ns):
     except asyncio.CancelledError as exc:
         # kube_client cleanup cancelled us.  In turn, we should cancel
         # the individual watch tasks.
-        await stop_signal.get()
         for t in watch_task:
             if watch_task[t] and not watch_task[t].done():
                 try:
@@ -489,7 +488,7 @@ async def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None,
     """
     if V(kubernetes_asyncio.__version__) < V("11"):
         pytest.skip(
-            f"exec tests require kubernetes >= 11, got {kubernetes.__version__}"
+            f"exec tests require kubernetes >= 11, got {kubernetes_asyncio.__version__}"
         )
     pod = await wait_for_pod(kube_client, kube_ns, pod_name)
     original_code = code

From 67be97ec38ce81f0f6936ab8d27099d4655837f6 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 17:52:18 -0700
Subject: [PATCH 22/56] remove useless __init__

---
 kubespawner/proxy.py | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 6f07fe28..d607bac9 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -101,16 +101,6 @@ def _namespace_default(self):
         """,
     )
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-        # Global configuration before reflector.py code runs
-
-        labels = {
-            'component': self.component_label,
-            'hub.jupyter.org/proxy-route': 'true',
-        }
-
     @classmethod
     async def initialize(cls, *args, **kwargs):
         """

From 2c827490ef50fd68097fbda60a80df7bbc6e6192 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 18:31:17 -0700
Subject: [PATCH 23/56] Fix paren placement

---
 tests/test_spawner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index 93867d2b..217aa564 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -280,7 +280,7 @@ async def test_spawn_internal_ssl(
     await spawner.stop()
 
     # verify pod is gone
-    pods = await (kube_client.list_namespaced_pod(kube_ns)).items
+    pods = (await kube_client.list_namespaced_pod(kube_ns)).items
     pod_names = [p.metadata.name for p in pods]
     assert "jupyter-%s" % spawner.user.name not in pod_names
 

From fc51aa40c46971f3903a26a3c9cd798dac6b50f3 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 18:42:33 -0700
Subject: [PATCH 24/56] Blacken and isort

---
 kubespawner/clients.py                |   2 +
 kubespawner/objects.py                | 103 ++++++++++++++------------
 kubespawner/proxy.py                  |   4 +-
 kubespawner/reflector.py              |  29 ++++----
 kubespawner/spawner.py                |  58 +++++++--------
 setup.py                              |   3 +-
 tests/conftest.py                     |  53 +++++++------
 tests/test_multi_namespace_spawner.py |  13 ++--
 tests/test_objects.py                 |   5 +-
 tests/test_spawner.py                 |  25 ++++---
 tests/test_utils.py                   |  18 ++---
 11 files changed, 168 insertions(+), 145 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 2608445a..7db550a3 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -20,6 +20,7 @@
 
 _client_cache = {}
 
+
 def shared_client(ClientType, *args, **kwargs):
     """Return a single shared kubernetes client instance
 
@@ -49,6 +50,7 @@ def shared_client(ClientType, *args, **kwargs):
 
     return client
 
+
 async def load_config():
     try:
         kubernetes_asyncio.config.load_incluster_config()
diff --git a/kubespawner/objects.py b/kubespawner/objects.py
index e5b7d6ee..91cfe75c 100644
--- a/kubespawner/objects.py
+++ b/kubespawner/objects.py
@@ -9,50 +9,51 @@
 import re
 from urllib.parse import urlparse
 
-from kubernetes_asyncio.client.models import V1Affinity
-from kubernetes_asyncio.client.models import V1Container
-from kubernetes_asyncio.client.models import V1ContainerPort
-from kubernetes_asyncio.client.models import V1EndpointAddress
-from kubernetes_asyncio.client.models import V1Endpoints
-from kubernetes_asyncio.client.models import V1EndpointSubset
-from kubernetes_asyncio.client.models import V1EnvVar
-from kubernetes_asyncio.client.models import V1LabelSelector
-from kubernetes_asyncio.client.models import V1Lifecycle
-from kubernetes_asyncio.client.models import V1LocalObjectReference
-from kubernetes_asyncio.client.models import V1Namespace
-from kubernetes_asyncio.client.models import V1NodeAffinity
-from kubernetes_asyncio.client.models import V1NodeSelector
-from kubernetes_asyncio.client.models import V1NodeSelectorRequirement
-from kubernetes_asyncio.client.models import V1NodeSelectorTerm
-from kubernetes_asyncio.client.models import V1ObjectMeta
-from kubernetes_asyncio.client.models import V1OwnerReference
-from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
-from kubernetes_asyncio.client.models import V1PersistentVolumeClaimSpec
-from kubernetes_asyncio.client.models import V1Pod
-from kubernetes_asyncio.client.models import V1PodAffinity
-from kubernetes_asyncio.client.models import V1PodAffinityTerm
-from kubernetes_asyncio.client.models import V1PodAntiAffinity
-from kubernetes_asyncio.client.models import V1PodSecurityContext
-from kubernetes_asyncio.client.models import V1PodSpec
-from kubernetes_asyncio.client.models import V1PreferredSchedulingTerm
-from kubernetes_asyncio.client.models import V1ResourceRequirements
-from kubernetes_asyncio.client.models import V1Secret
-from kubernetes_asyncio.client.models import V1SecurityContext
-from kubernetes_asyncio.client.models import V1Service
-from kubernetes_asyncio.client.models import V1ServicePort
-from kubernetes_asyncio.client.models import V1ServiceSpec
-from kubernetes_asyncio.client.models import V1Toleration
-from kubernetes_asyncio.client.models import V1Volume
-from kubernetes_asyncio.client.models import V1VolumeMount
-from kubernetes_asyncio.client.models import V1WeightedPodAffinityTerm
+from kubernetes_asyncio.client.models import (
+    V1Affinity,
+    V1Container,
+    V1ContainerPort,
+    V1EndpointAddress,
+    V1Endpoints,
+    V1EndpointSubset,
+    V1EnvVar,
+    V1LabelSelector,
+    V1Lifecycle,
+    V1LocalObjectReference,
+    V1Namespace,
+    V1NodeAffinity,
+    V1NodeSelector,
+    V1NodeSelectorRequirement,
+    V1NodeSelectorTerm,
+    V1ObjectMeta,
+    V1OwnerReference,
+    V1PersistentVolumeClaim,
+    V1PersistentVolumeClaimSpec,
+    V1Pod,
+    V1PodAffinity,
+    V1PodAffinityTerm,
+    V1PodAntiAffinity,
+    V1PodSecurityContext,
+    V1PodSpec,
+    V1PreferredSchedulingTerm,
+    V1ResourceRequirements,
+    V1Secret,
+    V1SecurityContext,
+    V1Service,
+    V1ServicePort,
+    V1ServiceSpec,
+    V1Toleration,
+    V1Volume,
+    V1VolumeMount,
+    V1WeightedPodAffinityTerm,
+)
 
 try:
     from kubernetes_asyncio.client.models import CoreV1EndpointPort
 except ImportError:
     from kubernetes_asyncio.client.models import V1EndpointPort as CoreV1EndpointPort
 
-from kubespawner.utils import get_k8s_model
-from kubespawner.utils import update_k8s_model
+from kubespawner.utils import get_k8s_model, update_k8s_model
 
 
 def make_pod(
@@ -735,22 +736,32 @@ def make_ingress(name, routespec, target, labels, data):
 
     try:
         from kubernetes_asyncio.client.models import (
-            ExtensionsV1beta1Ingress,
-            ExtensionsV1beta1IngressSpec,
-            ExtensionsV1beta1IngressRule,
-            ExtensionsV1beta1HTTPIngressRuleValue,
             ExtensionsV1beta1HTTPIngressPath,
+            ExtensionsV1beta1HTTPIngressRuleValue,
+            ExtensionsV1beta1Ingress,
             ExtensionsV1beta1IngressBackend,
+            ExtensionsV1beta1IngressRule,
+            ExtensionsV1beta1IngressSpec,
         )
     except ImportError:
         from kubernetes_asyncio.client.models import (
-            V1beta1Ingress as ExtensionsV1beta1Ingress,
-            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
-            V1beta1IngressRule as ExtensionsV1beta1IngressRule,
-            V1beta1HTTPIngressRuleValue as ExtensionsV1beta1HTTPIngressRuleValue,
             V1beta1HTTPIngressPath as ExtensionsV1beta1HTTPIngressPath,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1HTTPIngressRuleValue as ExtensionsV1beta1HTTPIngressRuleValue,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1Ingress as ExtensionsV1beta1Ingress,
+        )
+        from kubernetes_asyncio.client.models import (
             V1beta1IngressBackend as ExtensionsV1beta1IngressBackend,
         )
+        from kubernetes_asyncio.client.models import (
+            V1beta1IngressRule as ExtensionsV1beta1IngressRule,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
+        )
 
     meta = V1ObjectMeta(
         name=name,
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index d607bac9..85d45475 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -10,7 +10,7 @@
 from kubernetes_asyncio import client
 from traitlets import Unicode
 
-from .clients import shared_client, load_config
+from .clients import load_config, shared_client
 from .objects import make_ingress
 from .reflector import ResourceReflector
 from .utils import generate_hashed_slug
@@ -181,7 +181,7 @@ async def add_route(self, routespec, target, data):
 
         async def ensure_object(create_func, patch_func, body, kind):
             try:
-                resp = await create_func(namespace=self.namespace, body=body),
+                resp = (await create_func(namespace=self.namespace, body=body),)
                 self.log.info('Created %s/%s', kind, safe_name)
             except client.rest.ApiException as e:
                 if e.status == 409:
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index bccef325..68cee01f 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -7,17 +7,12 @@
 from concurrent.futures import Future
 from functools import partial
 
-from kubernetes_asyncio import config
-from kubernetes_asyncio import watch
-from traitlets import Any
-from traitlets import Bool
-from traitlets import Dict
-from traitlets import Int
-from traitlets import Unicode
+from kubernetes_asyncio import config, watch
+from traitlets import Any, Bool, Dict, Int, Unicode
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import shared_client, load_config
+from .clients import load_config, shared_client
 
 # This is kubernetes client implementation specific, but we need to know
 # whether it was a network or watch timeout.
@@ -333,12 +328,12 @@ async def _watch_and_update(self):
                 # of CPU in busy clusters.  cf
                 # https://github.com/jupyterhub/kubespawner/pull/424
                 method = partial(
-                    getattr(self.api, self.list_method_name),
-                    _preload_content=False)
+                    getattr(self.api, self.list_method_name), _preload_content=False
+                )
                 async with w.stream(method, **watch_args) as stream:
                     async for watch_event in stream:
-                    # in case of timeout_seconds, the w.stream just exits (no exception thrown)
-                    # -> we stop the watcher and start a new one
+                        # in case of timeout_seconds, the w.stream just exits (no exception thrown)
+                        # -> we stop the watcher and start a new one
                         # Remember that these events are k8s api related WatchEvents
                         # objects, not k8s Event or Pod representations, they will
                         # reside in the WatchEvent's object field depending on what
@@ -349,7 +344,8 @@ async def _watch_and_update(self):
                         cur_delay = 0.1
                         resource = watch_event['raw_object']
                         ref_key = "{}/{}".format(
-                            resource["metadata"]["namespace"], resource["metadata"]["name"]
+                            resource["metadata"]["namespace"],
+                            resource["metadata"]["name"],
                         )
                         if watch_event['type'] == 'DELETED':
                             # This is an atomic delete operation on the dictionary!
@@ -421,12 +417,13 @@ async def stop(self):
         # it a bit...
         if self.watch_task and not self.watch_task.done():
             try:
-                timeout=5
+                timeout = 5
                 await asyncio.wait_for(self.watch_task, timeout)
             except asyncio.TimeoutError:
                 # Raising the TimeoutError will cancel the task.
-                self.log.warning(f"Watch task did not finish in {timeout}s" +
-                                 "and was cancelled")
+                self.log.warning(
+                    f"Watch task did not finish in {timeout}s" + "and was cancelled"
+                )
         self.watch_task = None
 
     def stopped(self):
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 47b2ec87..1c531a5c 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -17,8 +17,7 @@
 
 import escapism
 import kubernetes_asyncio.config
-from jinja2 import BaseLoader
-from jinja2 import Environment
+from jinja2 import BaseLoader, Environment
 from jupyterhub.spawner import Spawner
 from jupyterhub.traitlets import Command
 from jupyterhub.utils import exponential_backoff
@@ -27,23 +26,27 @@
 from slugify import slugify
 from tornado import gen
 from tornado.ioloop import IOLoop
-from traitlets import Bool
-from traitlets import default
-from traitlets import Dict
-from traitlets import Integer
-from traitlets import List
-from traitlets import observe
-from traitlets import Unicode
-from traitlets import Union
-from traitlets import validate
-
-from .clients import shared_client, load_config
-from .objects import make_namespace
-from .objects import make_owner_reference
-from .objects import make_pod
-from .objects import make_pvc
-from .objects import make_secret
-from .objects import make_service
+from traitlets import (
+    Bool,
+    Dict,
+    Integer,
+    List,
+    Unicode,
+    Union,
+    default,
+    observe,
+    validate,
+)
+
+from .clients import load_config, shared_client
+from .objects import (
+    make_namespace,
+    make_owner_reference,
+    make_pod,
+    make_pvc,
+    make_secret,
+    make_service,
+)
 from .reflector import ResourceReflector
 from .traitlets import Callable
 
@@ -203,7 +206,7 @@ def __init__(self, *args, **kwargs):
         self._start_future = None
 
     @classmethod
-    async def initialize(cls,*args,**kwargs):
+    async def initialize(cls, *args, **kwargs):
         """In an ideal world, you'd get a new KubeSpawner with this.
         That's not how we are called from JupyterHub, though; it just
         instantiates whatever class you tell it to use as the spawner class.
@@ -220,7 +223,7 @@ async def initialize(cls,*args,**kwargs):
         that, instead, to get its spawner instance.  I don't think our
         use-case is likely to be unique.
         """
-        inst = cls(*args,**kwargs)
+        inst = cls(*args, **kwargs)
         await inst.initialize_reflectors_and_clients()
         return inst
 
@@ -231,7 +234,6 @@ async def initialize_reflectors_and_clients(self):
         if self.events_enabled:
             await self._start_watching_events()
 
-
     k8s_api_ssl_ca_cert = Unicode(
         "",
         config=True,
@@ -2410,8 +2412,7 @@ async def _ensure_not_exists(self, kind, name):
         try:
             self.log.info(f"Checking for {kind}/{name}")
             await asyncio.wait_for(
-                read(namespace=self.namespace, name=name),
-                self.k8s_api_request_timeout
+                read(namespace=self.namespace, name=name), self.k8s_api_request_timeout
             )
         except asyncio.TimeoutError:
             # Just try again
@@ -2435,8 +2436,7 @@ async def _make_create_resource_request(self, kind, manifest):
         self.log.info(f"Attempting to create {kind} {manifest.metadata.name}")
         try:
             await asyncio.wait_for(
-                create(self.namespace,manifest),
-                self.k8s_api_request_timeout
+                create(self.namespace, manifest), self.k8s_api_request_timeout
             )
         except asyncio.TimeoutError:
             # Just try again
@@ -2499,8 +2499,7 @@ async def _start(self):
         ref_key = "{}/{}".format(self.namespace, self.pod_name)
         # If there's a timeout, just let it propagate
         await exponential_backoff(
-            partial(self._make_create_pod_request, pod, self.k8s_api_request_timeout
-                    ),
+            partial(self._make_create_pod_request, pod, self.k8s_api_request_timeout),
             f'Could not create pod {ref_key}',
             timeout=self.k8s_api_request_retry_timeout,
         )
@@ -2618,7 +2617,8 @@ async def _make_delete_pod_request(
                     name=pod_name,
                     namespace=self.namespace,
                     body=delete_options,
-                    grace_period_seconds=grace_seconds),
+                    grace_period_seconds=grace_seconds,
+                ),
                 request_timeout,
             )
             return True
diff --git a/setup.py b/setup.py
index 03ede204..6b80a6bf 100644
--- a/setup.py
+++ b/setup.py
@@ -2,8 +2,7 @@
 
 import sys
 
-from setuptools import find_packages
-from setuptools import setup
+from setuptools import find_packages, setup
 
 v = sys.version_info
 if v[:2] < (3, 6):
diff --git a/tests/conftest.py b/tests/conftest.py
index e1f9b0e6..76d6def3 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -8,35 +8,37 @@
 import sys
 import tarfile
 import time
-from packaging.version import Version as V
 from functools import partial
-from threading import Thread, Event
+from threading import Event, Thread
 
 import kubernetes_asyncio
 import pytest
 import pytest_asyncio
 from jupyterhub.app import JupyterHub
 from jupyterhub.objects import Hub
-from kubernetes_asyncio.client import V1ConfigMap
-from kubernetes_asyncio.client import V1Namespace
-from kubernetes_asyncio.client import V1Pod
-from kubernetes_asyncio.client import V1PodSpec
-from kubernetes_asyncio.client import V1Secret
-from kubernetes_asyncio.client import V1Service
-from kubernetes_asyncio.client import V1ServicePort
-from kubernetes_asyncio.client import V1ServiceSpec
+from kubernetes.client import CoreV1Api as sync_CoreV1Api
+from kubernetes.config import load_kube_config as sync_load_kube_config
+
+# Needed for the streaming stuff
+from kubernetes.stream import stream as sync_stream
+from kubernetes_asyncio.client import (
+    V1ConfigMap,
+    V1Namespace,
+    V1Pod,
+    V1PodSpec,
+    V1Secret,
+    V1Service,
+    V1ServicePort,
+    V1ServiceSpec,
+)
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
 from kubernetes_asyncio.watch import Watch
+from packaging.version import Version as V
 from traitlets.config import Config
 
 from kubespawner.clients import shared_client
 
-# Needed for the streaming stuff
-from kubernetes.stream import stream as sync_stream
-from kubernetes.config import load_kube_config as sync_load_kube_config
-from kubernetes.client import CoreV1Api as sync_CoreV1Api
-
 here = os.path.abspath(os.path.dirname(__file__))
 jupyterhub_config_py = os.path.join(here, "jupyterhub_config.py")
 
@@ -45,12 +47,14 @@
 sync_load_kube_config()
 sync_corev1api = sync_CoreV1Api()
 
+
 @pytest.fixture(scope="session")
 def event_loop():
-    loop=asyncio.get_event_loop()
+    loop = asyncio.get_event_loop()
     yield loop
     loop.close()
 
+
 @pytest.fixture(autouse=True)
 def traitlets_logging():
     """Ensure traitlets default logging is enabled
@@ -68,6 +72,7 @@ def kube_ns():
     """Fixture for the kubernetes namespace"""
     return os.environ.get("KUBESPAWNER_TEST_NAMESPACE") or "kubespawner-test"
 
+
 @pytest.fixture
 def config(kube_ns):
     """Return a traitlets Config object
@@ -171,7 +176,9 @@ async def watch_kubernetes(kube_client, kube_ns):
 
             resource = event['object']
             obj = resource.involved_object
-            print(f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}")
+            print(
+                f"k8s event ({event['type']} {obj.kind}/{obj.name}): {resource.message}"
+            )
 
             # new pod appeared, start streaming its logs
             if (
@@ -212,7 +219,7 @@ async def kube_client(request, kube_ns):
     """
     await load_kube_config()
     client = shared_client("CoreV1Api")
-    stop_signal=asyncio.Queue()
+    stop_signal = asyncio.Queue()
     try:
         namespaces = await client.list_namespace(_request_timeout=3)
     except Exception as e:
@@ -231,7 +238,7 @@ async def kube_client(request, kube_ns):
 
     # Clean up at close by sending a cancel to watch_kubernetes and letting
     # it handle the signal, cancel the tasks *it* started, and then raising
-    # it back to us. 
+    # it back to us.
     try:
         t.cancel()
     except asyncio.CancelledError:
@@ -301,7 +308,9 @@ async def ensure_not_exists(kube_client, kube_ns, name, resource_type, timeout=3
             await asyncio.sleep(1)
 
 
-async def create_resource(kube_client, kube_ns, resource_type, manifest, delete_first=True):
+async def create_resource(
+    kube_client, kube_ns, resource_type, manifest, delete_first=True
+):
     """Create a kubernetes resource
 
     handling 409 errors and others that can occur due to rapid startup
@@ -478,7 +487,9 @@ def __str__(self):
         )
 
 
-async def _exec_python_in_pod(kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0):
+async def _exec_python_in_pod(
+    kube_client, kube_ns, pod_name, code, kwargs=None, _retries=0
+):
     """Run simple Python code in a pod
 
     code can be a str of code, or a 'simple' Python function,
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index b64321ef..f44b9402 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -4,14 +4,15 @@
 from unittest.mock import Mock
 
 import pytest
-from jupyterhub.objects import Hub
-from jupyterhub.objects import Server
+from jupyterhub.objects import Hub, Server
 from jupyterhub.orm import Spawner
 from kubernetes_asyncio.client import V1Namespace
-from kubernetes_asyncio.client.models import V1Capabilities
-from kubernetes_asyncio.client.models import V1Container
-from kubernetes_asyncio.client.models import V1Pod
-from kubernetes_asyncio.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import (
+    V1Capabilities,
+    V1Container,
+    V1Pod,
+    V1SecurityContext,
+)
 from kubernetes_asyncio.config import load_kube_config
 from traitlets.config import Config
 
diff --git a/tests/test_objects.py b/tests/test_objects.py
index dcaab1b3..f9a48cfe 100644
--- a/tests/test_objects.py
+++ b/tests/test_objects.py
@@ -4,10 +4,7 @@
 import pytest
 from kubernetes_asyncio.client import ApiClient
 
-from kubespawner.objects import make_ingress
-from kubespawner.objects import make_namespace
-from kubespawner.objects import make_pod
-from kubespawner.objects import make_pvc
+from kubespawner.objects import make_ingress, make_namespace, make_pod, make_pvc
 
 api_client = ApiClient()
 
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index 217aa564..e708a148 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -5,14 +5,15 @@
 from unittest.mock import Mock
 
 import pytest
-from jupyterhub.objects import Hub
-from jupyterhub.objects import Server
+from jupyterhub.objects import Hub, Server
 from jupyterhub.orm import Spawner
-from kubernetes_asyncio.client.models import V1Capabilities
-from kubernetes_asyncio.client.models import V1Container
-from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
-from kubernetes_asyncio.client.models import V1Pod
-from kubernetes_asyncio.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import (
+    V1Capabilities,
+    V1Container,
+    V1PersistentVolumeClaim,
+    V1Pod,
+    V1SecurityContext,
+)
 from traitlets.config import Config
 
 from kubespawner import KubeSpawner
@@ -123,8 +124,8 @@ def check_up(url, ssl_ca=None, ssl_client_cert=None, ssl_client_key=None):
 
     Uses stdlib only because requests isn't always available in the target pod
     """
-    from urllib import request
     import ssl
+    from urllib import request
 
     if ssl_ca:
         context = ssl.create_default_context(
@@ -784,7 +785,9 @@ async def test_delete_pvc(kube_ns, kube_client, hub, config):
 
     # verify PVC is created
     pvc_name = spawner.pvc_name
-    pvc_list = (await kube_client.list_namespaced_persistent_volume_claim(kube_ns)).items
+    pvc_list = (
+        await kube_client.list_namespaced_persistent_volume_claim(kube_ns)
+    ).items
     pvc_names = [s.metadata.name for s in pvc_list]
     assert pvc_name in pvc_names
 
@@ -801,7 +804,9 @@ async def test_delete_pvc(kube_ns, kube_client, hub, config):
 
     # verify PVC is deleted, it may take a little while
     for i in range(5):
-        pvc_list = (await kube_client.list_namespaced_persistent_volume_claim(kube_ns)).items
+        pvc_list = (
+            await kube_client.list_namespaced_persistent_volume_claim(kube_ns)
+        ).items
         pvc_names = [s.metadata.name for s in pvc_list]
         if pvc_name in pvc_names:
             await asyncio.sleep(1)
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 845119c4..563df4c6 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -2,15 +2,15 @@
 
 import pytest
 from conftest import ExecError
-from kubernetes_asyncio.client.models import V1Capabilities
-from kubernetes_asyncio.client.models import V1Container
-from kubernetes_asyncio.client.models import V1Lifecycle
-from kubernetes_asyncio.client.models import V1PodSpec
-from kubernetes_asyncio.client.models import V1SecurityContext
-
-from kubespawner.utils import _get_k8s_model_attribute
-from kubespawner.utils import get_k8s_model
-from kubespawner.utils import update_k8s_model
+from kubernetes_asyncio.client.models import (
+    V1Capabilities,
+    V1Container,
+    V1Lifecycle,
+    V1PodSpec,
+    V1SecurityContext,
+)
+
+from kubespawner.utils import _get_k8s_model_attribute, get_k8s_model, update_k8s_model
 
 
 class MockLogger(object):

From ca76409da7794df24fe7f852258dcbb594bb9990 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 3 Feb 2022 19:02:07 -0700
Subject: [PATCH 25/56] isort, on its own

---
 kubespawner/objects.py                | 102 ++++++++++----------------
 kubespawner/spawner.py                |  23 +-----
 tests/conftest.py                     |  14 +---
 tests/test_multi_namespace_spawner.py |   8 +-
 tests/test_objects.py                 |   3 +-
 tests/test_spawner.py                 |  10 +--
 tests/test_utils.py                   |  15 ++--
 7 files changed, 60 insertions(+), 115 deletions(-)

diff --git a/kubespawner/objects.py b/kubespawner/objects.py
index 91cfe75c..c4681c26 100644
--- a/kubespawner/objects.py
+++ b/kubespawner/objects.py
@@ -9,44 +9,30 @@
 import re
 from urllib.parse import urlparse
 
-from kubernetes_asyncio.client.models import (
-    V1Affinity,
-    V1Container,
-    V1ContainerPort,
-    V1EndpointAddress,
-    V1Endpoints,
-    V1EndpointSubset,
-    V1EnvVar,
-    V1LabelSelector,
-    V1Lifecycle,
-    V1LocalObjectReference,
-    V1Namespace,
-    V1NodeAffinity,
-    V1NodeSelector,
-    V1NodeSelectorRequirement,
-    V1NodeSelectorTerm,
-    V1ObjectMeta,
-    V1OwnerReference,
-    V1PersistentVolumeClaim,
-    V1PersistentVolumeClaimSpec,
-    V1Pod,
-    V1PodAffinity,
-    V1PodAffinityTerm,
-    V1PodAntiAffinity,
-    V1PodSecurityContext,
-    V1PodSpec,
-    V1PreferredSchedulingTerm,
-    V1ResourceRequirements,
-    V1Secret,
-    V1SecurityContext,
-    V1Service,
-    V1ServicePort,
-    V1ServiceSpec,
-    V1Toleration,
-    V1Volume,
-    V1VolumeMount,
-    V1WeightedPodAffinityTerm,
-)
+from kubernetes_asyncio.client.models import (V1Affinity, V1Container,
+                                              V1ContainerPort,
+                                              V1EndpointAddress, V1Endpoints,
+                                              V1EndpointSubset, V1EnvVar,
+                                              V1LabelSelector, V1Lifecycle,
+                                              V1LocalObjectReference,
+                                              V1Namespace, V1NodeAffinity,
+                                              V1NodeSelector,
+                                              V1NodeSelectorRequirement,
+                                              V1NodeSelectorTerm, V1ObjectMeta,
+                                              V1OwnerReference,
+                                              V1PersistentVolumeClaim,
+                                              V1PersistentVolumeClaimSpec,
+                                              V1Pod, V1PodAffinity,
+                                              V1PodAffinityTerm,
+                                              V1PodAntiAffinity,
+                                              V1PodSecurityContext, V1PodSpec,
+                                              V1PreferredSchedulingTerm,
+                                              V1ResourceRequirements, V1Secret,
+                                              V1SecurityContext, V1Service,
+                                              V1ServicePort, V1ServiceSpec,
+                                              V1Toleration, V1Volume,
+                                              V1VolumeMount,
+                                              V1WeightedPodAffinityTerm)
 
 try:
     from kubernetes_asyncio.client.models import CoreV1EndpointPort
@@ -737,31 +723,23 @@ def make_ingress(name, routespec, target, labels, data):
     try:
         from kubernetes_asyncio.client.models import (
             ExtensionsV1beta1HTTPIngressPath,
-            ExtensionsV1beta1HTTPIngressRuleValue,
-            ExtensionsV1beta1Ingress,
-            ExtensionsV1beta1IngressBackend,
-            ExtensionsV1beta1IngressRule,
-            ExtensionsV1beta1IngressSpec,
-        )
+            ExtensionsV1beta1HTTPIngressRuleValue, ExtensionsV1beta1Ingress,
+            ExtensionsV1beta1IngressBackend, ExtensionsV1beta1IngressRule,
+            ExtensionsV1beta1IngressSpec)
     except ImportError:
-        from kubernetes_asyncio.client.models import (
-            V1beta1HTTPIngressPath as ExtensionsV1beta1HTTPIngressPath,
-        )
-        from kubernetes_asyncio.client.models import (
-            V1beta1HTTPIngressRuleValue as ExtensionsV1beta1HTTPIngressRuleValue,
-        )
-        from kubernetes_asyncio.client.models import (
-            V1beta1Ingress as ExtensionsV1beta1Ingress,
-        )
-        from kubernetes_asyncio.client.models import (
-            V1beta1IngressBackend as ExtensionsV1beta1IngressBackend,
-        )
-        from kubernetes_asyncio.client.models import (
-            V1beta1IngressRule as ExtensionsV1beta1IngressRule,
-        )
-        from kubernetes_asyncio.client.models import (
-            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
-        )
+        from kubernetes_asyncio.client.models import \
+            V1beta1HTTPIngressPath as ExtensionsV1beta1HTTPIngressPath
+        from kubernetes_asyncio.client.models import \
+            V1beta1HTTPIngressRuleValue as \
+            ExtensionsV1beta1HTTPIngressRuleValue
+        from kubernetes_asyncio.client.models import \
+            V1beta1Ingress as ExtensionsV1beta1Ingress
+        from kubernetes_asyncio.client.models import \
+            V1beta1IngressBackend as ExtensionsV1beta1IngressBackend
+        from kubernetes_asyncio.client.models import \
+            V1beta1IngressRule as ExtensionsV1beta1IngressRule
+        from kubernetes_asyncio.client.models import \
+            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec
 
     meta = V1ObjectMeta(
         name=name,
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 1c531a5c..f4803609 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -26,27 +26,12 @@
 from slugify import slugify
 from tornado import gen
 from tornado.ioloop import IOLoop
-from traitlets import (
-    Bool,
-    Dict,
-    Integer,
-    List,
-    Unicode,
-    Union,
-    default,
-    observe,
-    validate,
-)
+from traitlets import (Bool, Dict, Integer, List, Unicode, Union, default,
+                       observe, validate)
 
 from .clients import load_config, shared_client
-from .objects import (
-    make_namespace,
-    make_owner_reference,
-    make_pod,
-    make_pvc,
-    make_secret,
-    make_service,
-)
+from .objects import (make_namespace, make_owner_reference, make_pod, make_pvc,
+                      make_secret, make_service)
 from .reflector import ResourceReflector
 from .traitlets import Callable
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 76d6def3..41604f0a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -18,19 +18,11 @@
 from jupyterhub.objects import Hub
 from kubernetes.client import CoreV1Api as sync_CoreV1Api
 from kubernetes.config import load_kube_config as sync_load_kube_config
-
 # Needed for the streaming stuff
 from kubernetes.stream import stream as sync_stream
-from kubernetes_asyncio.client import (
-    V1ConfigMap,
-    V1Namespace,
-    V1Pod,
-    V1PodSpec,
-    V1Secret,
-    V1Service,
-    V1ServicePort,
-    V1ServiceSpec,
-)
+from kubernetes_asyncio.client import (V1ConfigMap, V1Namespace, V1Pod,
+                                       V1PodSpec, V1Secret, V1Service,
+                                       V1ServicePort, V1ServiceSpec)
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
 from kubernetes_asyncio.watch import Watch
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index f44b9402..a74c0aa3 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -7,12 +7,8 @@
 from jupyterhub.objects import Hub, Server
 from jupyterhub.orm import Spawner
 from kubernetes_asyncio.client import V1Namespace
-from kubernetes_asyncio.client.models import (
-    V1Capabilities,
-    V1Container,
-    V1Pod,
-    V1SecurityContext,
-)
+from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
+                                              V1Pod, V1SecurityContext)
 from kubernetes_asyncio.config import load_kube_config
 from traitlets.config import Config
 
diff --git a/tests/test_objects.py b/tests/test_objects.py
index f9a48cfe..64499e8b 100644
--- a/tests/test_objects.py
+++ b/tests/test_objects.py
@@ -4,7 +4,8 @@
 import pytest
 from kubernetes_asyncio.client import ApiClient
 
-from kubespawner.objects import make_ingress, make_namespace, make_pod, make_pvc
+from kubespawner.objects import (make_ingress, make_namespace, make_pod,
+                                 make_pvc)
 
 api_client = ApiClient()
 
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index e708a148..aba211a2 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -7,13 +7,9 @@
 import pytest
 from jupyterhub.objects import Hub, Server
 from jupyterhub.orm import Spawner
-from kubernetes_asyncio.client.models import (
-    V1Capabilities,
-    V1Container,
-    V1PersistentVolumeClaim,
-    V1Pod,
-    V1SecurityContext,
-)
+from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
+                                              V1PersistentVolumeClaim, V1Pod,
+                                              V1SecurityContext)
 from traitlets.config import Config
 
 from kubespawner import KubeSpawner
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 563df4c6..9a602678 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -2,15 +2,12 @@
 
 import pytest
 from conftest import ExecError
-from kubernetes_asyncio.client.models import (
-    V1Capabilities,
-    V1Container,
-    V1Lifecycle,
-    V1PodSpec,
-    V1SecurityContext,
-)
-
-from kubespawner.utils import _get_k8s_model_attribute, get_k8s_model, update_k8s_model
+from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
+                                              V1Lifecycle, V1PodSpec,
+                                              V1SecurityContext)
+
+from kubespawner.utils import (_get_k8s_model_attribute, get_k8s_model,
+                               update_k8s_model)
 
 
 class MockLogger(object):

From 5d5d7bc58f0dea5c72a0fad5bcc1206dc9131915 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 4 Feb 2022 08:46:49 -0700
Subject: [PATCH 26/56] address review commentary

---
 kubespawner/proxy.py     |  5 +++--
 kubespawner/reflector.py | 29 +++++++++++++----------------
 pyproject.toml           |  1 -
 setup.cfg                |  1 -
 setup.py                 | 13 +++++++------
 5 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 85d45475..a580714e 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -10,7 +10,8 @@
 from kubernetes_asyncio import client
 from traitlets import Unicode
 
-from .clients import load_config, shared_client
+from .clients import load_config
+from .clients import shared_client
 from .objects import make_ingress
 from .reflector import ResourceReflector
 from .utils import generate_hashed_slug
@@ -181,7 +182,7 @@ async def add_route(self, routespec, target, data):
 
         async def ensure_object(create_func, patch_func, body, kind):
             try:
-                resp = (await create_func(namespace=self.namespace, body=body),)
+                resp = await create_func(namespace=self.namespace, body=body)
                 self.log.info('Created %s/%s', kind, safe_name)
             except client.rest.ApiException as e:
                 if e.status == 409:
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 68cee01f..f25fb65b 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -7,12 +7,18 @@
 from concurrent.futures import Future
 from functools import partial
 
-from kubernetes_asyncio import config, watch
-from traitlets import Any, Bool, Dict, Int, Unicode
+from kubernetes_asyncio import config
+from kubernetes_asyncio import watch
+from traitlets import Any
+from traitlets import Bool
+from traitlets import Dict
+from traitlets import Int
+from traitlets import Unicode
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import load_config, shared_client
+from .clients import load_config
+from .clients import shared_client
 
 # This is kubernetes client implementation specific, but we need to know
 # whether it was a network or watch timeout.
@@ -256,8 +262,6 @@ async def _watch_and_update(self):
         """
         Keeps the current list of resources up-to-date
 
-        This method is to be run not on the main thread!
-
         We first fetch the list of current resources, and store that. Then we
         register to be notified of changes to those resources, and keep our
         local store up-to-date based on these notifications.
@@ -269,17 +273,10 @@ async def _watch_and_update(self):
         changes that might've been missed in the time we were not doing
         a watch.
 
-        Note that we're playing a bit with fire here, by updating a dictionary
-        in this thread while it is probably being read in another thread
-        without using locks! However, dictionary access itself is atomic,
-        and as long as we don't try to mutate them (do a 'fetch / modify /
-        update' cycle on them), we should be ok!
+        Since the resources are read-only in the Spawner (where they are
+        used), then this is safe.  The Spawner's view of the world might be
+        out-of-date, but it's not going to corrupt any data.
         """
-        #
-        # I guess?  It is the case that the resources are read-only in the
-        #  spawner, so the worst that will happen will be that they're out-
-        #  of-date.  I think.
-        #
         selectors = []
         log_name = ""
         if self.label_selector:
@@ -403,7 +400,7 @@ async def start(self):
         and not afterwards!
         """
         if self.watch_task and not self.watch_task.done():
-            raise ValueError('Task watching for resources is already running')
+            raise RuntimeError('Task watching for resources is already running')
 
         await self._list_and_update()
         self.watch_task = asyncio.create_task(self._watch_and_update())
diff --git a/pyproject.toml b/pyproject.toml
index a7630d48..99e00de3 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,7 +1,6 @@
 [tool.black]
 skip-string-normalization = true
 target_version = [
-    "py36",
     "py37",
     "py38",
 ]
diff --git a/setup.cfg b/setup.cfg
index 0503fda6..13dcd2d0 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -4,5 +4,4 @@ test=pytest
 [tool:pytest]
 # Ignore thousands of tests in dependencies installed in a virtual environment
 norecursedirs = lib lib64
-# At some point probably want to make this "strict"
 asyncio_mode = auto
diff --git a/setup.py b/setup.py
index 6b80a6bf..fc0fae12 100644
--- a/setup.py
+++ b/setup.py
@@ -2,16 +2,17 @@
 
 import sys
 
-from setuptools import find_packages, setup
+from setuptools import find_packages
+from setuptools import setup
 
 v = sys.version_info
-if v[:2] < (3, 6):
-    error = "ERROR: kubespawner requires Python version 3.6 or above."
+if v[:2] < (3, 7):
+    error = "ERROR: kubespawner requires Python version 3.7 or above."
     print(error, file=sys.stderr)
     sys.exit(1)
 
 setup(
-    name='kubespawner',
+    name='jupyterhub-kubespawner',
     version='3.0.0',
     install_requires=[
         'async_generator>=1.8',
@@ -34,8 +35,8 @@
             'pytest-asyncio>=0.11.0',
         ]
     },
-    description='JupyterHub Spawner for Kubernetes (Rubin asyncio version)',
-    url='http://github.com/lsst-sqre/kubespawner',
+    description='JupyterHub Spawner for Kubernetes',
+    url='http://github.com/jupyterhub/kubespawner',
     author='Jupyter Contributors',
     author_email='jupyter@googlegroups.com',
     long_description=open("README.md").read(),

From 71a86c111a894024981405dc967dcab0bd1525cb Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 4 Feb 2022 08:55:12 -0700
Subject: [PATCH 27/56] Check sync client version

---
 tests/conftest.py | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tests/conftest.py b/tests/conftest.py
index 41604f0a..5c301e83 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -9,20 +9,26 @@
 import tarfile
 import time
 from functools import partial
-from threading import Event, Thread
+from threading import Event
+from threading import Thread
 
 import kubernetes_asyncio
 import pytest
 import pytest_asyncio
 from jupyterhub.app import JupyterHub
 from jupyterhub.objects import Hub
+from kubernetes import __version__ as sync_version
 from kubernetes.client import CoreV1Api as sync_CoreV1Api
 from kubernetes.config import load_kube_config as sync_load_kube_config
-# Needed for the streaming stuff
 from kubernetes.stream import stream as sync_stream
-from kubernetes_asyncio.client import (V1ConfigMap, V1Namespace, V1Pod,
-                                       V1PodSpec, V1Secret, V1Service,
-                                       V1ServicePort, V1ServiceSpec)
+from kubernetes_asyncio.client import V1ConfigMap
+from kubernetes_asyncio.client import V1Namespace
+from kubernetes_asyncio.client import V1Pod
+from kubernetes_asyncio.client import V1PodSpec
+from kubernetes_asyncio.client import V1Secret
+from kubernetes_asyncio.client import V1Service
+from kubernetes_asyncio.client import V1ServicePort
+from kubernetes_asyncio.client import V1ServiceSpec
 from kubernetes_asyncio.client.rest import ApiException
 from kubernetes_asyncio.config import load_kube_config
 from kubernetes_asyncio.watch import Watch
@@ -489,10 +495,8 @@ async def _exec_python_in_pod(
 
     kwargs are passed to the function, if it is given.
     """
-    if V(kubernetes_asyncio.__version__) < V("11"):
-        pytest.skip(
-            f"exec tests require kubernetes >= 11, got {kubernetes_asyncio.__version__}"
-        )
+    if V(sync_version) < V("11"):
+        pytest.skip(f"exec tests require kubernetes >= 11, got {sync_version}")
     pod = await wait_for_pod(kube_client, kube_ns, pod_name)
     original_code = code
     if not isinstance(code, str):

From 73f78cc545ad9160911e14f6e57366ba75547913 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 4 Feb 2022 09:01:57 -0700
Subject: [PATCH 28/56] Rerun pre-commit

---
 kubespawner/objects.py                | 100 +++++++++++++++-----------
 kubespawner/spawner.py                |  27 +++++--
 tests/test_multi_namespace_spawner.py |   9 ++-
 tests/test_objects.py                 |   6 +-
 tests/test_spawner.py                 |  11 +--
 tests/test_utils.py                   |  15 ++--
 6 files changed, 106 insertions(+), 62 deletions(-)

diff --git a/kubespawner/objects.py b/kubespawner/objects.py
index c4681c26..787d8afc 100644
--- a/kubespawner/objects.py
+++ b/kubespawner/objects.py
@@ -9,30 +9,42 @@
 import re
 from urllib.parse import urlparse
 
-from kubernetes_asyncio.client.models import (V1Affinity, V1Container,
-                                              V1ContainerPort,
-                                              V1EndpointAddress, V1Endpoints,
-                                              V1EndpointSubset, V1EnvVar,
-                                              V1LabelSelector, V1Lifecycle,
-                                              V1LocalObjectReference,
-                                              V1Namespace, V1NodeAffinity,
-                                              V1NodeSelector,
-                                              V1NodeSelectorRequirement,
-                                              V1NodeSelectorTerm, V1ObjectMeta,
-                                              V1OwnerReference,
-                                              V1PersistentVolumeClaim,
-                                              V1PersistentVolumeClaimSpec,
-                                              V1Pod, V1PodAffinity,
-                                              V1PodAffinityTerm,
-                                              V1PodAntiAffinity,
-                                              V1PodSecurityContext, V1PodSpec,
-                                              V1PreferredSchedulingTerm,
-                                              V1ResourceRequirements, V1Secret,
-                                              V1SecurityContext, V1Service,
-                                              V1ServicePort, V1ServiceSpec,
-                                              V1Toleration, V1Volume,
-                                              V1VolumeMount,
-                                              V1WeightedPodAffinityTerm)
+from kubernetes_asyncio.client.models import V1Affinity
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1ContainerPort
+from kubernetes_asyncio.client.models import V1EndpointAddress
+from kubernetes_asyncio.client.models import V1Endpoints
+from kubernetes_asyncio.client.models import V1EndpointSubset
+from kubernetes_asyncio.client.models import V1EnvVar
+from kubernetes_asyncio.client.models import V1LabelSelector
+from kubernetes_asyncio.client.models import V1Lifecycle
+from kubernetes_asyncio.client.models import V1LocalObjectReference
+from kubernetes_asyncio.client.models import V1Namespace
+from kubernetes_asyncio.client.models import V1NodeAffinity
+from kubernetes_asyncio.client.models import V1NodeSelector
+from kubernetes_asyncio.client.models import V1NodeSelectorRequirement
+from kubernetes_asyncio.client.models import V1NodeSelectorTerm
+from kubernetes_asyncio.client.models import V1ObjectMeta
+from kubernetes_asyncio.client.models import V1OwnerReference
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaimSpec
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1PodAffinity
+from kubernetes_asyncio.client.models import V1PodAffinityTerm
+from kubernetes_asyncio.client.models import V1PodAntiAffinity
+from kubernetes_asyncio.client.models import V1PodSecurityContext
+from kubernetes_asyncio.client.models import V1PodSpec
+from kubernetes_asyncio.client.models import V1PreferredSchedulingTerm
+from kubernetes_asyncio.client.models import V1ResourceRequirements
+from kubernetes_asyncio.client.models import V1Secret
+from kubernetes_asyncio.client.models import V1SecurityContext
+from kubernetes_asyncio.client.models import V1Service
+from kubernetes_asyncio.client.models import V1ServicePort
+from kubernetes_asyncio.client.models import V1ServiceSpec
+from kubernetes_asyncio.client.models import V1Toleration
+from kubernetes_asyncio.client.models import V1Volume
+from kubernetes_asyncio.client.models import V1VolumeMount
+from kubernetes_asyncio.client.models import V1WeightedPodAffinityTerm
 
 try:
     from kubernetes_asyncio.client.models import CoreV1EndpointPort
@@ -723,23 +735,31 @@ def make_ingress(name, routespec, target, labels, data):
     try:
         from kubernetes_asyncio.client.models import (
             ExtensionsV1beta1HTTPIngressPath,
-            ExtensionsV1beta1HTTPIngressRuleValue, ExtensionsV1beta1Ingress,
-            ExtensionsV1beta1IngressBackend, ExtensionsV1beta1IngressRule,
-            ExtensionsV1beta1IngressSpec)
+            ExtensionsV1beta1HTTPIngressRuleValue,
+            ExtensionsV1beta1Ingress,
+            ExtensionsV1beta1IngressBackend,
+            ExtensionsV1beta1IngressRule,
+            ExtensionsV1beta1IngressSpec,
+        )
     except ImportError:
-        from kubernetes_asyncio.client.models import \
-            V1beta1HTTPIngressPath as ExtensionsV1beta1HTTPIngressPath
-        from kubernetes_asyncio.client.models import \
-            V1beta1HTTPIngressRuleValue as \
-            ExtensionsV1beta1HTTPIngressRuleValue
-        from kubernetes_asyncio.client.models import \
-            V1beta1Ingress as ExtensionsV1beta1Ingress
-        from kubernetes_asyncio.client.models import \
-            V1beta1IngressBackend as ExtensionsV1beta1IngressBackend
-        from kubernetes_asyncio.client.models import \
-            V1beta1IngressRule as ExtensionsV1beta1IngressRule
-        from kubernetes_asyncio.client.models import \
-            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec
+        from kubernetes_asyncio.client.models import (
+            V1beta1HTTPIngressPath as ExtensionsV1beta1HTTPIngressPath,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1HTTPIngressRuleValue as ExtensionsV1beta1HTTPIngressRuleValue,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1Ingress as ExtensionsV1beta1Ingress,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1IngressBackend as ExtensionsV1beta1IngressBackend,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1IngressRule as ExtensionsV1beta1IngressRule,
+        )
+        from kubernetes_asyncio.client.models import (
+            V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
+        )
 
     meta = V1ObjectMeta(
         name=name,
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index f4803609..7b89c1f5 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -17,7 +17,8 @@
 
 import escapism
 import kubernetes_asyncio.config
-from jinja2 import BaseLoader, Environment
+from jinja2 import BaseLoader
+from jinja2 import Environment
 from jupyterhub.spawner import Spawner
 from jupyterhub.traitlets import Command
 from jupyterhub.utils import exponential_backoff
@@ -26,12 +27,24 @@
 from slugify import slugify
 from tornado import gen
 from tornado.ioloop import IOLoop
-from traitlets import (Bool, Dict, Integer, List, Unicode, Union, default,
-                       observe, validate)
-
-from .clients import load_config, shared_client
-from .objects import (make_namespace, make_owner_reference, make_pod, make_pvc,
-                      make_secret, make_service)
+from traitlets import Bool
+from traitlets import default
+from traitlets import Dict
+from traitlets import Integer
+from traitlets import List
+from traitlets import observe
+from traitlets import Unicode
+from traitlets import Union
+from traitlets import validate
+
+from .clients import load_config
+from .clients import shared_client
+from .objects import make_namespace
+from .objects import make_owner_reference
+from .objects import make_pod
+from .objects import make_pvc
+from .objects import make_secret
+from .objects import make_service
 from .reflector import ResourceReflector
 from .traitlets import Callable
 
diff --git a/tests/test_multi_namespace_spawner.py b/tests/test_multi_namespace_spawner.py
index a74c0aa3..b64321ef 100644
--- a/tests/test_multi_namespace_spawner.py
+++ b/tests/test_multi_namespace_spawner.py
@@ -4,11 +4,14 @@
 from unittest.mock import Mock
 
 import pytest
-from jupyterhub.objects import Hub, Server
+from jupyterhub.objects import Hub
+from jupyterhub.objects import Server
 from jupyterhub.orm import Spawner
 from kubernetes_asyncio.client import V1Namespace
-from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
-                                              V1Pod, V1SecurityContext)
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1SecurityContext
 from kubernetes_asyncio.config import load_kube_config
 from traitlets.config import Config
 
diff --git a/tests/test_objects.py b/tests/test_objects.py
index 64499e8b..dcaab1b3 100644
--- a/tests/test_objects.py
+++ b/tests/test_objects.py
@@ -4,8 +4,10 @@
 import pytest
 from kubernetes_asyncio.client import ApiClient
 
-from kubespawner.objects import (make_ingress, make_namespace, make_pod,
-                                 make_pvc)
+from kubespawner.objects import make_ingress
+from kubespawner.objects import make_namespace
+from kubespawner.objects import make_pod
+from kubespawner.objects import make_pvc
 
 api_client = ApiClient()
 
diff --git a/tests/test_spawner.py b/tests/test_spawner.py
index aba211a2..fb2063be 100644
--- a/tests/test_spawner.py
+++ b/tests/test_spawner.py
@@ -5,11 +5,14 @@
 from unittest.mock import Mock
 
 import pytest
-from jupyterhub.objects import Hub, Server
+from jupyterhub.objects import Hub
+from jupyterhub.objects import Server
 from jupyterhub.orm import Spawner
-from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
-                                              V1PersistentVolumeClaim, V1Pod,
-                                              V1SecurityContext)
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1PersistentVolumeClaim
+from kubernetes_asyncio.client.models import V1Pod
+from kubernetes_asyncio.client.models import V1SecurityContext
 from traitlets.config import Config
 
 from kubespawner import KubeSpawner
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 9a602678..845119c4 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -2,12 +2,15 @@
 
 import pytest
 from conftest import ExecError
-from kubernetes_asyncio.client.models import (V1Capabilities, V1Container,
-                                              V1Lifecycle, V1PodSpec,
-                                              V1SecurityContext)
-
-from kubespawner.utils import (_get_k8s_model_attribute, get_k8s_model,
-                               update_k8s_model)
+from kubernetes_asyncio.client.models import V1Capabilities
+from kubernetes_asyncio.client.models import V1Container
+from kubernetes_asyncio.client.models import V1Lifecycle
+from kubernetes_asyncio.client.models import V1PodSpec
+from kubernetes_asyncio.client.models import V1SecurityContext
+
+from kubespawner.utils import _get_k8s_model_attribute
+from kubespawner.utils import get_k8s_model
+from kubespawner.utils import update_k8s_model
 
 
 class MockLogger(object):

From 05572bf3da19ec89b45cb16031d9a73a045199d5 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Tue, 8 Feb 2022 13:16:19 -0700
Subject: [PATCH 29/56] Address review comments

---
 kubespawner/reflector.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index f25fb65b..27bd0383 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -419,7 +419,7 @@ async def stop(self):
             except asyncio.TimeoutError:
                 # Raising the TimeoutError will cancel the task.
                 self.log.warning(
-                    f"Watch task did not finish in {timeout}s" + "and was cancelled"
+                    f"Watch task did not finish in {timeout}s and was cancelled"
                 )
         self.watch_task = None
 

From d23c6fc848dcad56fd8ea884b8fc707a781641a5 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 9 Feb 2022 08:07:25 -0700
Subject: [PATCH 30/56] fix python_requires

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index fc0fae12..569a9801 100644
--- a/setup.py
+++ b/setup.py
@@ -24,7 +24,7 @@
         'urllib3',
         'pyYAML',
     ],
-    python_requires='>=3.6',
+    python_requires='>=3.7',
     extras_require={
         'test': [
             'bump2version',

From 0df8b99a4e52746609e068b3be59873b52d94568 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 11:37:03 -0700
Subject: [PATCH 31/56] Address Erik Sundell's review commentary

---
 .github/workflows/test.yaml |  2 +-
 kubespawner/proxy.py        | 12 ++++++---
 kubespawner/reflector.py    | 19 ++++++++------
 kubespawner/spawner.py      | 52 +++++++++++++++++++++++++------------
 setup.py                    |  4 +--
 tests/conftest.py           |  2 --
 6 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 87c74c57..4a5900d5 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -44,7 +44,7 @@ jobs:
           # Tests with various k8s-python-client versions
           - python: 3.7
             k3s: v1.16
-            k8s-python-client: 10.* # supports k8s 1.14
+            k8s-python-client: 11.* # supports k8s 1.15
           - python: 3.8
             k3s: v1.16
             k8s-python-client: 11.* # supports k8s 1.15
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index a580714e..4bddf4d0 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -103,9 +103,13 @@ def _namespace_default(self):
     )
 
     @classmethod
-    async def initialize(cls, *args, **kwargs):
+    async def create(cls, *args, **kwargs):
         """
-        This is how you should get a proxy object.
+        This is a workaround: The `__init__` constructor cannot be async, but
+        we require that async methods be called when we request a new
+        KubeIngressProxy object.
+
+        Use this method to create a KubeIngressProxy object.
         """
         inst = cls(*args, **kwargs)
         await inst._initialize_resources()
@@ -133,11 +137,11 @@ async def _initialize_resources(self):
 
     def _set_k8s_client_configuration(self):
         # The actual (singleton) Kubernetes client will be created
-        # in clients.py shared_client but the configuration
+        # in clients.py shared_client() but the configuration
         # for token / ca_cert / k8s api host is set globally
         # in kubernetes.py syntax.  It is being set here
         # and this method called prior to getting a shared_client
-        # (but after load_config)
+        # (but after load_config())
         # for readability / coupling with traitlets values
         if self.k8s_api_ssl_ca_cert:
             global_conf = client.Configuration.get_default_copy()
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 27bd0383..ef168d12 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -30,7 +30,7 @@ class ResourceReflector(LoggingConfigurable):
 
     Must be subclassed once per kind of resource that needs watching.
 
-    Creating a reflector should be done with the reflector() classmethod,
+    Creating a reflector should be done with the create() classmethod,
     since that, in addition to creating the instance, initializes the
     Kubernetes configuration, acquires a K8s API client, and starts the
     watch task.
@@ -218,12 +218,15 @@ def __init__(self, *args, **kwargs):
         # watch task.
 
     @classmethod
-    async def reflector(cls, *args, **kwargs):
+    async def create(cls, *args, **kwargs):
         """
-        This is how you should instantiate a reflector in order to bring
-        it up in a usable state, as this classmethod does load
-        Kubernetes config, acquires an API client, and starts the watch
-        task.
+        This is a workaround: `__init__` cannot be async, but we want
+        to call async methods to instantiate a reflector in a usable state.
+
+        This method creates the reflector, and then loads Kubernetes config,
+        acquires an API client, and starts the watch task.
+
+        Use it to create a new Reflector object.
         """
         inst = cls(*args, **kwargs)
         await load_config()
@@ -247,8 +250,8 @@ async def _list_and_update(self):
         if not self.omit_namespace:
             kwargs["namespace"] = self.namespace
 
-        method = getattr(self.api, self.list_method_name)
-        initial_resources_raw = await ((method)(**kwargs))
+        list_method = getattr(self.api, self.list_method_name)
+        initial_resources_raw = await list_method(**kwargs)
         # This is an atomic operation on the dictionary!
         initial_resources = json.loads(await initial_resources_raw.read())
         self.resources = {
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 7b89c1f5..495f72af 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -173,10 +173,10 @@ def __init__(self, *args, **kwargs):
                 self.hub = hub
 
         # We have to set the namespace (if user namespaces are enabled)
-        #  before we start the reflectors, so this must run before
-        #  watcher start in normal execution.  We still want to get the
-        #  namespace right for test, though, so we need self.user to have
-        #  been set in order to do that.
+        # before we start the reflectors, so this must run before
+        # watcher start in normal execution.  We still want to get the
+        # namespace right for test, though, so we need self.user to have
+        # been set in order to do that.
 
         # By now, all the traitlets have been set, so we can use them to
         # compute other attributes
@@ -204,7 +204,7 @@ def __init__(self, *args, **kwargs):
         self._start_future = None
 
     @classmethod
-    async def initialize(cls, *args, **kwargs):
+    async def create(cls, *args, **kwargs):
         """In an ideal world, you'd get a new KubeSpawner with this.
         That's not how we are called from JupyterHub, though; it just
         instantiates whatever class you tell it to use as the spawner class.
@@ -212,14 +212,13 @@ async def initialize(cls, *args, **kwargs):
         Thus we need to do the initialization in poll(), start(), and stop()
         since any of them could be the first thing called (imagine that the
         hub restarts and the first thing someone does is try to stop their
-        running server).  Further (and this is actually true in the Rubin
-        case) a pre-spawn-start hook that depends on internal knowledge of
-        the spawner might be the first thing to run.
+        running server).  Further a pre-spawn-start hook that depends on
+        internal knowledge of the spawner might be the first thing to run.
 
         It would be nice if there were an agreed-upon mechanism for, say,
-        JupyterHub to look for an async initialize() method and then use
-        that, instead, to get its spawner instance.  I don't think our
-        use-case is likely to be unique.
+        JupyterHub to look for an async create() method and then use
+        that (instead of implicitly requesting __init__()) to get its
+        spawner instance.
         """
         inst = cls(*args, **kwargs)
         await inst.initialize_reflectors_and_clients()
@@ -227,6 +226,19 @@ async def initialize(cls, *args, **kwargs):
 
     async def initialize_reflectors_and_clients(self):
         await load_config()
+        # The actual (singleton) Kubernetes client will be created
+        # in clients.py shared_client() but the configuration
+        # for token / ca_cert / k8s api host is set globally
+        # in kubernetes.py syntax.  It is being set here (but after
+        # load_config()) for readability / coupling with traitlets values
+        if self.k8s_api_ssl_ca_cert:
+            global_conf = client.Configuration.get_default_copy()
+            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
+            client.Configuration.set_default(global_conf)
+        if self.k8s_api_host:
+            global_conf = client.Configuration.get_default_copy()
+            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
+            client.Configuration.set_default(global_conf)
         self.api = shared_client("CoreV1Api")
         await self._start_watching_pods()
         if self.events_enabled:
@@ -258,6 +270,15 @@ async def initialize_reflectors_and_clients(self):
         """,
     )
 
+    k8s_api_threadpool_workers = Integer(
+        config=True,
+        help="""
+        DEPRECATED.
+
+        No longer has any effect, as there is no threadpool anymore.
+        """,
+    )
+
     k8s_api_request_timeout = Integer(
         3,
         config=True,
@@ -2038,11 +2059,10 @@ async def poll(self):
         Note that a clean exit will have an exit code of zero, so it is
         necessary to check that the returned value is None, rather than
         just Falsy, to determine that the pod is still running.
-
-        We cannot be sure the Hub will call start() before poll(), so
-        we need to load client configuration and start our reflectors
-        at the top of each of those methods.
         """
+        # We cannot be sure the Hub will call start() before poll(), so
+        # we need to load client configuration and start our reflectors
+        # at the top of each of those methods.
         await self.initialize_reflectors_and_clients()
         # have to wait for first load of data before we have a valid answer
         if not self.pod_reflector.first_load_future.done():
@@ -2225,7 +2245,7 @@ def on_reflector_failure():
         previous_reflector = self.__class__.reflectors.get(key)
 
         if replace or not previous_reflector:
-            self.__class__.reflectors[key] = await ReflectorClass.reflector(
+            self.__class__.reflectors[key] = await ReflectorClass.create(
                 parent=self,
                 namespace=self.namespace,
                 on_failure=on_reflector_failure,
diff --git a/setup.py b/setup.py
index 569a9801..5f4558d6 100644
--- a/setup.py
+++ b/setup.py
@@ -13,7 +13,7 @@
 
 setup(
     name='jupyterhub-kubespawner',
-    version='3.0.0',
+    version='2.0.2.dev',
     install_requires=[
         'async_generator>=1.8',
         'escapism',
@@ -29,7 +29,7 @@
         'test': [
             'bump2version',
             'flake8',
-            'kubernetes>=10.1.0',
+            'kubernetes>=11',
             'pytest>=5.4',
             'pytest-cov',
             'pytest-asyncio>=0.11.0',
diff --git a/tests/conftest.py b/tests/conftest.py
index 5c301e83..d2355464 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -495,8 +495,6 @@ async def _exec_python_in_pod(
 
     kwargs are passed to the function, if it is given.
     """
-    if V(sync_version) < V("11"):
-        pytest.skip(f"exec tests require kubernetes >= 11, got {sync_version}")
     pod = await wait_for_pod(kube_client, kube_ns, pod_name)
     original_code = code
     if not isinstance(code, str):

From 5725ca27d17bf811ed9c577d92ec46ae29b282ac Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 15:16:43 -0700
Subject: [PATCH 32/56] move SSL config into load_config

---
 kubespawner/clients.py   | 16 +++++++++++++++-
 kubespawner/proxy.py     | 20 +-------------------
 kubespawner/reflector.py |  2 +-
 kubespawner/spawner.py   | 15 +--------------
 4 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 7db550a3..4bf9219c 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -8,6 +8,7 @@
 
 import kubernetes_asyncio.client
 from kubernetes_asyncio.client import api_client
+from kubernetes_asyncio.client import Configuration
 
 # FIXME: remove when instantiating a kubernetes client
 # doesn't create N-CPUs threads unconditionally.
@@ -51,8 +52,21 @@ def shared_client(ClientType, *args, **kwargs):
     return client
 
 
-async def load_config():
+async def load_config(caller=None):
     try:
         kubernetes_asyncio.config.load_incluster_config()
     except kubernetes_asyncio.config.ConfigException:
         await kubernetes_asyncio.config.load_kube_config()
+    # The ca_cert/k8s_api_host is set via traitlets on the objects
+    # that call load_config(); thus, they may pass a reference to themselves
+    # into load_config in order to set ca_cert/k8s_api_host.
+
+    if caller:
+        if hasattr(caller, "k8s_api_ssl_ca_cert") and caller.k8s_api_ssl_ca_cert:
+            global_conf = Configuration.get_default_copy()
+            global_conf.ssl_ca_cert = caller.k8s_api_ssl_ca_cert
+            Configuration.set_default(global_conf)
+        if hasattr(caller, "k8s_api_host") and caller.k8s_api_host:
+            global_conf = Configuration.get_default_copy()
+            global_conf.host = caller.k8s_api_host
+            Configuration.set_default(global_conf)
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 4bddf4d0..918d4175 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -120,8 +120,7 @@ async def _initialize_resources(self):
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
         }
-        await load_config()
-        self._set_k8s_client_configuration()
+        await load_config(caller=self)
         self.core_api = shared_client('CoreV1Api')
         self.extension_api = shared_client('ExtensionsV1beta1Api')
 
@@ -135,23 +134,6 @@ async def _initialize_resources(self):
             parent=self, namespace=self.namespace, labels=labels
         )
 
-    def _set_k8s_client_configuration(self):
-        # The actual (singleton) Kubernetes client will be created
-        # in clients.py shared_client() but the configuration
-        # for token / ca_cert / k8s api host is set globally
-        # in kubernetes.py syntax.  It is being set here
-        # and this method called prior to getting a shared_client
-        # (but after load_config())
-        # for readability / coupling with traitlets values
-        if self.k8s_api_ssl_ca_cert:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
-            client.Configuration.set_default(global_conf)
-        if self.k8s_api_host:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.host = self.k8s_api_host
-            client.Configuration.set_default(global_conf)
-
     def safe_name_for_routespec(self, routespec):
         safe_chars = set(string.ascii_lowercase + string.digits)
         safe_name = generate_hashed_slug(
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index ef168d12..32c82fc5 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -229,7 +229,7 @@ async def create(cls, *args, **kwargs):
         Use it to create a new Reflector object.
         """
         inst = cls(*args, **kwargs)
-        await load_config()
+        await load_config(caller=inst)
         inst.api = shared_client(inst.api_group_name)
         await inst.start()
         return inst
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 495f72af..87b996b9 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -225,20 +225,7 @@ async def create(cls, *args, **kwargs):
         return inst
 
     async def initialize_reflectors_and_clients(self):
-        await load_config()
-        # The actual (singleton) Kubernetes client will be created
-        # in clients.py shared_client() but the configuration
-        # for token / ca_cert / k8s api host is set globally
-        # in kubernetes.py syntax.  It is being set here (but after
-        # load_config()) for readability / coupling with traitlets values
-        if self.k8s_api_ssl_ca_cert:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
-            client.Configuration.set_default(global_conf)
-        if self.k8s_api_host:
-            global_conf = client.Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = self.k8s_api_ssl_ca_cert
-            client.Configuration.set_default(global_conf)
+        await load_config(caller=self)
         self.api = shared_client("CoreV1Api")
         await self._start_watching_pods()
         if self.events_enabled:

From 467af7c11778c93eb6c1b8b20dd8acf83c2f40f2 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 17:00:23 -0700
Subject: [PATCH 33/56] Address object instantiation/initialization

---
 kubespawner/proxy.py   | 37 ++++++++++++++++++++++---------------
 kubespawner/spawner.py | 40 +++++++++++++++++++---------------------
 2 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 918d4175..c4912814 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -43,6 +43,18 @@ def endpoints(self):
 
 
 class KubeIngressProxy(Proxy):
+    """
+    As the KubeIngressProxy class now depends on an asyncio-based
+    Kubernetes client, it is no longer sufficient to simply instantiate
+    an instance of the class with `__init__`.  Instance configuration is
+    required, and that depends on async functions, which cannot appear in
+    `__init__`.
+
+    Therefore, immediately after requesting a new instance, the requestor
+    should await the instance's `initialize_resources` method
+    to complete configuration of the instance.
+    """
+
     namespace = Unicode(
         config=True,
         help="""
@@ -102,20 +114,13 @@ def _namespace_default(self):
         """,
     )
 
-    @classmethod
-    async def create(cls, *args, **kwargs):
+    async def initialize_resources(self):
         """
-        This is a workaround: The `__init__` constructor cannot be async, but
-        we require that async methods be called when we request a new
-        KubeIngressProxy object.
-
-        Use this method to create a KubeIngressProxy object.
+        This contains logic that was formerly in `__init__`, but since
+        it now involves async/await, it can't be there anymore.  It is
+        intended that this method be called immediately after a new
+        KubeIngressProxy object is created via `__init__`.
         """
-        inst = cls(*args, **kwargs)
-        await inst._initialize_resources()
-        return inst
-
-    async def _initialize_resources(self):
         labels = {
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
@@ -124,13 +129,13 @@ async def _initialize_resources(self):
         self.core_api = shared_client('CoreV1Api')
         self.extension_api = shared_client('ExtensionsV1beta1Api')
 
-        self.ingress_reflector = await IngressReflector.reflector(
+        self.ingress_reflector = await IngressReflector.create(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.service_reflector = await ServiceReflector.reflector(
+        self.service_reflector = await ServiceReflector.create(
             parent=self, namespace=self.namespace, labels=labels
         )
-        self.endpoint_reflector = await EndpointsReflector.reflector(
+        self.endpoint_reflector = await EndpointsReflector.create(
             parent=self, namespace=self.namespace, labels=labels
         )
 
@@ -156,6 +161,7 @@ async def add_route(self, routespec, target, data):
         # Create a route with the name being escaped routespec
         # Use full routespec in label
         # 'data' is JSON encoded and put in an annotation - we don't need to query for it
+
         safe_name = self.safe_name_for_routespec(routespec).lower()
         labels = {
             'heritage': 'jupyterhub',
@@ -235,6 +241,7 @@ async def delete_route(self, routespec):
         # We just ensure that these objects are deleted.
         # This means if some of them are already deleted, we just let it
         # be.
+
         safe_name = self.safe_name_for_routespec(routespec).lower()
 
         delete_options = client.V1DeleteOptions(grace_period_seconds=0)
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 87b996b9..b73ecab7 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -152,6 +152,20 @@ def event_reflector(self):
             return self.__class__.reflectors['events']
 
     def __init__(self, *args, **kwargs):
+        """
+        We cannot call async methods from `__init__`.  Now that we use an
+        asyncio-based Kubernetes client, we have required initialization that
+        depends on async/await
+
+        Thus we need to do the initialization in `poll`, `start`, and `stop`
+        since any of them could be the first thing called (imagine that the
+        hub restarts and the first thing someone does is try to stop their
+        running server).  Further a pre-spawn-start hook that depends on
+        internal knowledge of the spawner might be the first thing to run,
+        and in a case like that, the caller would need to await the
+        initialization method (`initialize_reflectors_and_clients`) itself.
+        """
+
         _mock = kwargs.pop('_mock', False)
         super().__init__(*args, **kwargs)
 
@@ -203,28 +217,12 @@ def __init__(self, *args, **kwargs):
         # The attribute needs to exist, even though it is unset to start with
         self._start_future = None
 
-    @classmethod
-    async def create(cls, *args, **kwargs):
-        """In an ideal world, you'd get a new KubeSpawner with this.
-        That's not how we are called from JupyterHub, though; it just
-        instantiates whatever class you tell it to use as the spawner class.
-
-        Thus we need to do the initialization in poll(), start(), and stop()
-        since any of them could be the first thing called (imagine that the
-        hub restarts and the first thing someone does is try to stop their
-        running server).  Further a pre-spawn-start hook that depends on
-        internal knowledge of the spawner might be the first thing to run.
-
-        It would be nice if there were an agreed-upon mechanism for, say,
-        JupyterHub to look for an async create() method and then use
-        that (instead of implicitly requesting __init__()) to get its
-        spawner instance.
-        """
-        inst = cls(*args, **kwargs)
-        await inst.initialize_reflectors_and_clients()
-        return inst
-
     async def initialize_reflectors_and_clients(self):
+        """
+        This is functionality extracted from `__init__` because it requires
+        the use of async/await.  This method should be awaited before doing
+        anything with the object received from `__init__`.
+        """
         await load_config(caller=self)
         self.api = shared_client("CoreV1Api")
         await self._start_watching_pods()

From ce942c952d3813390750c3ca3950f7609197a43a Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 17:17:16 -0700
Subject: [PATCH 34/56] simplify SSL handling in load_config()

---
 kubespawner/clients.py | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 4bf9219c..06707a83 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -52,21 +52,21 @@ def shared_client(ClientType, *args, **kwargs):
     return client
 
 
-async def load_config(caller=None):
+async def load_config(caller):
     try:
         kubernetes_asyncio.config.load_incluster_config()
     except kubernetes_asyncio.config.ConfigException:
         await kubernetes_asyncio.config.load_kube_config()
     # The ca_cert/k8s_api_host is set via traitlets on the objects
-    # that call load_config(); thus, they may pass a reference to themselves
-    # into load_config in order to set ca_cert/k8s_api_host.
+    # that call load_config(); they must pass a reference to themselves
+    # into load_config, and then if the traitlets are set, they will
+    # be used.
 
-    if caller:
-        if hasattr(caller, "k8s_api_ssl_ca_cert") and caller.k8s_api_ssl_ca_cert:
-            global_conf = Configuration.get_default_copy()
-            global_conf.ssl_ca_cert = caller.k8s_api_ssl_ca_cert
-            Configuration.set_default(global_conf)
-        if hasattr(caller, "k8s_api_host") and caller.k8s_api_host:
-            global_conf = Configuration.get_default_copy()
-            global_conf.host = caller.k8s_api_host
-            Configuration.set_default(global_conf)
+    if caller.k8s_api_ssl_ca_cert:
+        global_conf = Configuration.get_default_copy()
+        global_conf.ssl_ca_cert = caller.k8s_api_ssl_ca_cert
+        Configuration.set_default(global_conf)
+    if caller.k8s_api_host:
+        global_conf = Configuration.get_default_copy()
+        global_conf.host = caller.k8s_api_host
+        Configuration.set_default(global_conf)

From 69b76cff45c19d406e359978b82d686bd92ea670 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 17:19:29 -0700
Subject: [PATCH 35/56] Improve load_config() doc

---
 kubespawner/clients.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 06707a83..8c9e9ed6 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -53,14 +53,15 @@ def shared_client(ClientType, *args, **kwargs):
 
 
 async def load_config(caller):
+    """
+    Loads configuration for the Python client we use to communicate with a
+    Kubernetes API server, and optionally tweaks that configuration based
+    on specific settings on the passed caller object.
+    """
     try:
         kubernetes_asyncio.config.load_incluster_config()
     except kubernetes_asyncio.config.ConfigException:
         await kubernetes_asyncio.config.load_kube_config()
-    # The ca_cert/k8s_api_host is set via traitlets on the objects
-    # that call load_config(); they must pass a reference to themselves
-    # into load_config, and then if the traitlets are set, they will
-    # be used.
 
     if caller.k8s_api_ssl_ca_cert:
         global_conf = Configuration.get_default_copy()

From e6c87dc242e61863f26a56e1381009a3f2651367 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Wed, 16 Feb 2022 17:41:36 -0700
Subject: [PATCH 36/56] Make Reflector's load_client reflect its parent

---
 kubespawner/reflector.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 32c82fc5..c18cf9cd 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -169,6 +169,10 @@ class ResourceReflector(LoggingConfigurable):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        parent = kwargs.get("parent", None)
+        if parent is None:
+            raise RuntimeError("Reflector's parent must be specified!")
+        self.parent = parent
         # FIXME: Protect against malicious labels?
         self.label_selector = ','.join(
             ['{}={}'.format(k, v) for k, v in self.labels.items()]
@@ -229,7 +233,7 @@ async def create(cls, *args, **kwargs):
         Use it to create a new Reflector object.
         """
         inst = cls(*args, **kwargs)
-        await load_config(caller=inst)
+        await load_config(caller=inst.parent)
         inst.api = shared_client(inst.api_group_name)
         await inst.start()
         return inst

From c2a257cda3d2de88a644860a94e270dda2ec9d49 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Thu, 17 Feb 2022 09:29:18 -0700
Subject: [PATCH 37/56] Clarify documentation

---
 kubespawner/clients.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index 8c9e9ed6..bda5d85d 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -30,7 +30,7 @@ def shared_client(ClientType, *args, **kwargs):
     will all return the same instance until
     all references to the client are cleared.
 
-    You should usually await load_config before calling this.
+    You must await load_config before calling this.
     """
     kwarg_key = tuple((key, kwargs[key]) for key in sorted(kwargs))
     cache_key = (ClientType, args, kwarg_key)

From 0cc6352c8c523d1db0c94b1bdde2c13d069a72f8 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 15:57:45 +0100
Subject: [PATCH 38/56] Update docstrings and comments

---
 kubespawner/clients.py | 46 ++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index bda5d85d..d2a4df00 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -1,7 +1,9 @@
-"""Shared clients for kubernetes
+"""Configures and instanciates REST API clients of various kinds to
+communicate with a Kubernetes api-server, but only one instance per kind is
+instanciated.
 
-avoids creating multiple kubernetes client objects,
-each of which spawns an unused max-size thread pool
+The instances of these REST API clients are also patched to avoid the creation
+of unused threads.
 """
 import weakref
 from unittest.mock import Mock
@@ -10,12 +12,15 @@
 from kubernetes_asyncio.client import api_client
 from kubernetes_asyncio.client import Configuration
 
-# FIXME: remove when instantiating a kubernetes client
-# doesn't create N-CPUs threads unconditionally.
-# monkeypatch threadpool in kubernetes api_client
-# to avoid instantiating ThreadPools.
-# This is known to work for kubernetes-4.0
-# and may need updating with later kubernetes clients
+# FIXME: Remove this workaround when instantiating a k8s client doesn't
+#        automatically create a ThreadPool with 1 thread that we won't use
+#        anyhow. To know if that has happened, reading
+#        https://github.com/jupyterhub/kubespawner/issues/567 may be helpful.
+#
+#        The workaround is to monkeypatch ThreadPool in the kubernetes
+#        api_client to avoid ThreadPools. This is known to work with both
+#        `kubernetes` and `kubernetes_asyncio`.
+#
 _dummy_pool = Mock()
 api_client.ThreadPool = lambda *args, **kwargs: _dummy_pool
 
@@ -23,14 +28,16 @@
 
 
 def shared_client(ClientType, *args, **kwargs):
-    """Return a single shared kubernetes client instance
+    """Return a shared kubernetes client instance
+    based on the provided arguments.
 
     A weak reference to the instance is cached,
     so that concurrent calls to shared_client
     will all return the same instance until
     all references to the client are cleared.
 
-    You must await load_config before calling this.
+    Note that we must await load_config before calling this function as it
+    relies on global client configuration being loaded.
     """
     kwarg_key = tuple((key, kwargs[key]) for key in sorted(kwargs))
     cache_key = (ClientType, args, kwarg_key)
@@ -41,9 +48,9 @@ def shared_client(ClientType, *args, **kwargs):
         client = _client_cache[cache_key]()
 
     if client is None:
-        # Kubernetes client configuration is handled globally
-        # in kubernetes.py and is already called in spawner.py
-        # or proxy.py prior to a shared_client being instantiated
+        # Kubernetes client configuration is handled globally and should already
+        # be configured from spawner.py or proxy.py via the load_config function
+        # prior to a shared_client being instantiated.
         Client = getattr(kubernetes_asyncio.client, ClientType)
         client = Client(*args, **kwargs)
         # cache weakref so that clients can be garbage collected
@@ -54,9 +61,14 @@ def shared_client(ClientType, *args, **kwargs):
 
 async def load_config(caller):
     """
-    Loads configuration for the Python client we use to communicate with a
-    Kubernetes API server, and optionally tweaks that configuration based
-    on specific settings on the passed caller object.
+    Loads global configuration for the Python client we use to communicate with
+    a Kubernetes API server, and optionally tweaks that configuration based on
+    specific settings on the passed caller object.
+
+    This needs to be called before creating a kubernetes client, so practically
+    before the shared_client function is called. The caller could be KubeSpawner
+    or KubeIngressProxy that both have `k8s_api_ssl_ca_cert` and `k8s_api_host`
+    configuration.
     """
     try:
         kubernetes_asyncio.config.load_incluster_config()

From 96a7358e51e869e9b1c9674c9c6409ffd20759d1 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 15:58:11 +0100
Subject: [PATCH 39/56] Let reflector instances assume k8s client config is
 loaded

The reflector isn't used directly, but indirectly by KubeSpawner and
KubeIngressProxy. A call to initialize a Reflector instance is always
done by KubeSpawner or KubeIngressProxy after they have loaded the
global k8s client config.
---
 kubespawner/reflector.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index c18cf9cd..72582bab 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -169,10 +169,12 @@ class ResourceReflector(LoggingConfigurable):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        parent = kwargs.get("parent", None)
-        if parent is None:
-            raise RuntimeError("Reflector's parent must be specified!")
-        self.parent = parent
+
+        # Client configuration for kubernetes, as done via the load_config
+        # function, has already taken place in KubeSpawner or KubeIngressProxy
+        # initialization steps.
+        self.api = shared_client(self.api_group_name)
+
         # FIXME: Protect against malicious labels?
         self.label_selector = ','.join(
             ['{}={}'.format(k, v) for k, v in self.labels.items()]
@@ -233,8 +235,6 @@ async def create(cls, *args, **kwargs):
         Use it to create a new Reflector object.
         """
         inst = cls(*args, **kwargs)
-        await load_config(caller=inst.parent)
-        inst.api = shared_client(inst.api_group_name)
         await inst.start()
         return inst
 

From 7c55baf5b8f28a5d2cb74edbeef790d8309aac4e Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 16:01:45 +0100
Subject: [PATCH 40/56] cleanup: remove unused reference to IOLoop.current()

---
 kubespawner/spawner.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index b73ecab7..ae0fc1de 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -2213,7 +2213,6 @@ async def _start_reflector(
         If replace=True, a running pod reflector will be stopped
         and a new one started (for recovering from possible errors).
         """
-        main_loop = IOLoop.current()
         key = kind
         ReflectorClass = reflector_class
 

From 4a04b8961fedd9e9d545851fea616c412b874f45 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 17:59:42 +0100
Subject: [PATCH 41/56] Remove unused imports

---
 kubespawner/reflector.py | 1 -
 kubespawner/spawner.py   | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 72582bab..bddba6b2 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -17,7 +17,6 @@
 from traitlets.config import LoggingConfigurable
 from urllib3.exceptions import ReadTimeoutError
 
-from .clients import load_config
 from .clients import shared_client
 
 # This is kubernetes client implementation specific, but we need to know
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index ae0fc1de..df83bb55 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -5,18 +5,15 @@
 implementation that should be used by JupyterHub.
 """
 import asyncio
-import multiprocessing
 import os
 import signal
 import string
 import sys
 import warnings
-from datetime import timedelta
 from functools import partial
 from urllib.parse import urlparse
 
 import escapism
-import kubernetes_asyncio.config
 from jinja2 import BaseLoader
 from jinja2 import Environment
 from jupyterhub.spawner import Spawner
@@ -26,7 +23,6 @@
 from kubernetes_asyncio.client.rest import ApiException
 from slugify import slugify
 from tornado import gen
-from tornado.ioloop import IOLoop
 from traitlets import Bool
 from traitlets import default
 from traitlets import Dict

From 1578cb4b291cfae4294f7dc115927fbfb66d2388 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 18:40:38 +0100
Subject: [PATCH 42/56] ci: update test matrix and remove redundant flake8 test

---
 .github/workflows/publish.yaml |  2 +-
 .github/workflows/test.yaml    | 65 ++++++++--------------------------
 2 files changed, 15 insertions(+), 52 deletions(-)

diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml
index f38c4c8a..864969b2 100644
--- a/.github/workflows/publish.yaml
+++ b/.github/workflows/publish.yaml
@@ -25,7 +25,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: "3.9"
 
       - name: install build package
         run: |
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 4a5900d5..eede51a7 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -38,41 +38,24 @@ jobs:
         # gain meaning on how job steps use them.
         #
         # k3s-channel:        https://update.k3s.io/v1-release/channels
-        # k8s-python-client:  https://github.com/kubernetes-client/python#compatibility
+        # kubernetes_asyncio:  https://github.com/tomplus/kubernetes_asyncio/tags
         #
         include:
-          # Tests with various k8s-python-client versions
-          - python: 3.7
-            k3s: v1.16
-            k8s-python-client: 11.* # supports k8s 1.15
-          - python: 3.8
-            k3s: v1.16
-            k8s-python-client: 11.* # supports k8s 1.15
-          - python: 3.9
-            k3s: v1.16
-            k8s-python-client: 12.* # supports k8s 1.16
-          - python: 3.9
+          # Tests with oldest supported Python, k8s, and k8s client
+          - python: "3.7"
             k3s: v1.17
-            k8s-python-client: 17.* # supports k8s 1.17
+            test_dependencies: kubernetes_asyncio==19.*
 
-          # Test with pre-releases of k8s-python-client
-          - python: 3.9
-            k3s: v1.18
-            k8s-python-client: pre
-
-          # Test with various recent k8s versions
-          - python: 3.8
+          # Test with modern python and k8s versions
+          - python: "3.9"
             k3s: stable
-            k8s-python-client: 17.*
-          - python: 3.8
+          - python: "3.9"
             k3s: latest
-            k8s-python-client: 17.*
 
-          # Test with JupyterHub in main branch
-          - python: 3.8
-            k3s: v1.16
-            k8s-python-client: 17.*
-            jupyterhub: main
+          # Test with latest python and JupyterHub in main branch
+          - python: "3.10"
+            k3s: latest
+            test_dependencies: git+https://github.com/jupyterhub/jupyterhub
 
     steps:
       - uses: actions/checkout@v2
@@ -81,31 +64,11 @@ jobs:
           python-version: "${{ matrix.python }}"
 
       - name: Install package and test dependencies
-        env:
-          K8S_PYTHON_CLIENT_VERSION: "${{ matrix.k8s-python-client }}"
-          JUPYTERHUB_VERSION: ${{ matrix.jupyterhub }}
         run: |
-          if [[ "$K8S_PYTHON_CLIENT_VERSION" == "pre" ]]; then
-              PRE="--pre"
-              PINS=""
-          else
-              PRE=""
-              PINS="kubernetes==${K8S_PYTHON_CLIENT_VERSION}"
-          fi
-          if [ "$JUPYTERHUB_VERSION" == "main" ]; then
-              PINS="$PINS git+https://github.com/jupyterhub/jupyterhub"
-          fi
-
-          pip install --upgrade setuptools pip
-          pip install -e ".[test]" ${PRE} ${PINS}
+          pip install --upgrade pip
+          pip install -e ".[test]" ${{ matrix.test_dependencies }}
           pip freeze
 
-      # flake8 runs a very quick code analysis without running any of the code
-      # it analyses
-      - name: Run flake8
-        run: |
-          flake8 kubespawner
-
       # Starts a k8s cluster with NetworkPolicy enforcement and installs both
       # kubectl and helm. We won't need network policy enforcement or helm
       # though.
@@ -141,7 +104,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: "3.9"
 
       - name: install build package
         run: |

From 46b6a0a518fb12268fb1b9913fee2d305bb29dae Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Thu, 17 Feb 2022 19:29:57 +0100
Subject: [PATCH 43/56] Address review comments by Adam

---
 kubespawner/clients.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index d2a4df00..ddbe3779 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -1,6 +1,6 @@
-"""Configures and instanciates REST API clients of various kinds to
+"""Configures and instantiates REST API clients of various kinds to
 communicate with a Kubernetes api-server, but only one instance per kind is
-instanciated.
+instantiated.
 
 The instances of these REST API clients are also patched to avoid the creation
 of unused threads.
@@ -66,9 +66,9 @@ async def load_config(caller):
     specific settings on the passed caller object.
 
     This needs to be called before creating a kubernetes client, so practically
-    before the shared_client function is called. The caller could be KubeSpawner
-    or KubeIngressProxy that both have `k8s_api_ssl_ca_cert` and `k8s_api_host`
-    configuration.
+    before the shared_client function is called. The caller must have both the
+    k8s_api_ssl_ca_cert and k8s_api_host attributes. KubeSpawner and
+    KubeIngressProxy both have these attributes.
     """
     try:
         kubernetes_asyncio.config.load_incluster_config()

From 5d34e64b57e42ccc7796e51cbac7257a858e234a Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Thu, 17 Feb 2022 12:24:55 -0700
Subject: [PATCH 44/56] Update kubespawner/reflector.py

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/reflector.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index bddba6b2..07dfaf49 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -326,10 +326,9 @@ async def _watch_and_update(self):
                 if self.timeout_seconds:
                     # set watch timeout
                     watch_args['timeout_seconds'] = self.timeout_seconds
-                # This is a little hack to defeat the Kubernetes client
-                # deserializing the objects it gets, which can use a lot
-                # of CPU in busy clusters.  cf
-                # https://github.com/jupyterhub/kubespawner/pull/424
+                # Calling the method with _preload_content=False is a performance
+                # optimization making the Kubernetes client do less work. See
+                # https://github.com/jupyterhub/kubespawner/pull/424.
                 method = partial(
                     getattr(self.api, self.list_method_name), _preload_content=False
                 )

From f3e6cbc04fefc24b28758fbe50a290b72c7505e8 Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Thu, 17 Feb 2022 12:25:43 -0700
Subject: [PATCH 45/56] Update kubespawner/spawner.py

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/spawner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index df83bb55..902158fd 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -120,7 +120,7 @@ class KubeSpawner(Spawner):
 
     # The PodReflector and EventReflector are singletons.  Where to initialize
     # them becomes a sort of thorny question in an asyncio world.  See the
-    # commentary on the initialize() method.
+    # commentary on the initialize_reflectors_and_clients() method.
 
     reflectors = {
         "pods": None,

From a38246e3c4aa84664880027285f0d68e3b7d978a Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Thu, 17 Feb 2022 12:30:00 -0700
Subject: [PATCH 46/56] Update kubespawner/spawner.py

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/spawner.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 902158fd..3ae5241f 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -195,9 +195,6 @@ def __init__(self, *args, **kwargs):
             self.namespace = self._expand_user_properties(self.user_namespace_template)
             self.log.info("Using user namespace: {}".format(self.namespace))
 
-        # Starting our watchers is now async, so we cannot do it in
-        # __init()__.
-
         self.pod_name = self._expand_user_properties(self.pod_name_template)
         self.dns_name = self.dns_name_template.format(
             namespace=self.namespace, name=self.pod_name

From b283953cd2fe3c352e3a8df7961a76b38450d716 Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Thu, 17 Feb 2022 12:32:09 -0700
Subject: [PATCH 47/56] Update kubespawner/spawner.py

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/spawner.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 3ae5241f..b53087f5 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -251,7 +251,7 @@ async def initialize_reflectors_and_clients(self):
     k8s_api_threadpool_workers = Integer(
         config=True,
         help="""
-        DEPRECATED.
+        DEPRECATED in KubeSpawner 3.0.0.
 
         No longer has any effect, as there is no threadpool anymore.
         """,

From 358c768b3579809e8467640cd2d419393745ed9c Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Fri, 18 Feb 2022 13:19:53 +0100
Subject: [PATCH 48/56] Apply Min's idea on awaiting async initialization logic

---
 kubespawner/proxy.py   | 72 ++++++++++++++++++++++++++-----------
 kubespawner/spawner.py | 82 +++++++++++++++++++++++-------------------
 2 files changed, 97 insertions(+), 57 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index c4912814..9df3d025 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -1,4 +1,5 @@
 import asyncio
+import functools
 import json
 import os
 import string
@@ -43,18 +44,6 @@ def endpoints(self):
 
 
 class KubeIngressProxy(Proxy):
-    """
-    As the KubeIngressProxy class now depends on an asyncio-based
-    Kubernetes client, it is no longer sufficient to simply instantiate
-    an instance of the class with `__init__`.  Instance configuration is
-    required, and that depends on async functions, which cannot appear in
-    `__init__`.
-
-    Therefore, immediately after requesting a new instance, the requestor
-    should await the instance's `initialize_resources` method
-    to complete configuration of the instance.
-    """
-
     namespace = Unicode(
         config=True,
         help="""
@@ -114,21 +103,46 @@ def _namespace_default(self):
         """,
     )
 
-    async def initialize_resources(self):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # Schedules async initialization logic that is to be awaited by async
+        # functions by decorating them with @_await_async_init.
+        self._async_init_future = asyncio.ensure_future(self._async_init())
+
+    async def _async_init(self):
         """
-        This contains logic that was formerly in `__init__`, but since
-        it now involves async/await, it can't be there anymore.  It is
-        intended that this method be called immediately after a new
-        KubeIngressProxy object is created via `__init__`.
+        This method is scheduled to run from `__init__`, but not awaited there
+        as it can't me marked as async.
+
+        Since JupyterHub won't await this method, we ensure the async methods
+        JupyterHub may call on this object will await this method until
+        continuing. To do this, we decorate them with `_await_async_init`.
+
+        But, how do we figure out the methods to decorate? Likely only those
+        exposed by the base class that JupyterHub would know about. The base
+        class is Proxy, as declared in proxy.py:
+        https://github.com/jupyterhub/jupyterhub/blob/HEAD/jupyterhub/proxy.py.
+
+        From the Proxy class docstring we can conclude that the following
+        methods, if implemented, could be what we need to decorate with
+        _await_async_init:
+
+          - get_all_routes (implemented and decorated)
+          - add_route (implemented and decorated)
+          - delete_route (implemented and decorated)
+          - start
+          - stop
+          - get_route
         """
-        labels = {
-            'component': self.component_label,
-            'hub.jupyter.org/proxy-route': 'true',
-        }
         await load_config(caller=self)
         self.core_api = shared_client('CoreV1Api')
         self.extension_api = shared_client('ExtensionsV1beta1Api')
 
+        labels = {
+            'component': self.component_label,
+            'hub.jupyter.org/proxy-route': 'true',
+        }
         self.ingress_reflector = await IngressReflector.create(
             parent=self, namespace=self.namespace, labels=labels
         )
@@ -139,6 +153,19 @@ async def initialize_resources(self):
             parent=self, namespace=self.namespace, labels=labels
         )
 
+    def _await_async_init(method):
+        """A decorator to await the _async_init method after having been
+        scheduled to run in the `__init__` method."""
+
+        @functools.wraps(method)
+        async def async_method(self, *args, **kwargs):
+            if self._async_init_future is not None:
+                await self._async_init_future
+                self._async_init_future = None
+            return await method(self, *args, **kwargs)
+
+        return async_method
+
     def safe_name_for_routespec(self, routespec):
         safe_chars = set(string.ascii_lowercase + string.digits)
         safe_name = generate_hashed_slug(
@@ -157,6 +184,7 @@ async def delete_if_exists(self, kind, safe_name, future):
                 raise
             self.log.warn("Could not delete %s/%s: does not exist", kind, safe_name)
 
+    @_await_async_init
     async def add_route(self, routespec, target, data):
         # Create a route with the name being escaped routespec
         # Use full routespec in label
@@ -237,6 +265,7 @@ async def ensure_object(create_func, patch_func, body, kind):
             'Could not find ingress/%s after creating it' % safe_name,
         )
 
+    @_await_async_init
     async def delete_route(self, routespec):
         # We just ensure that these objects are deleted.
         # This means if some of them are already deleted, we just let it
@@ -278,6 +307,7 @@ async def delete_route(self, routespec):
             self.delete_if_exists('ingress', safe_name, delete_ingress),
         )
 
+    @_await_async_init
     async def get_all_routes(self):
         # copy everything, because iterating over this directly is not threadsafe
         # FIXME: is this performance intensive? It could be! Measure?
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index b53087f5..0289fb39 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -11,6 +11,7 @@
 import sys
 import warnings
 from functools import partial
+from functools import wraps
 from urllib.parse import urlparse
 
 import escapism
@@ -118,10 +119,6 @@ class KubeSpawner(Spawner):
     spawned by a user will have its own KubeSpawner instance.
     """
 
-    # The PodReflector and EventReflector are singletons.  Where to initialize
-    # them becomes a sort of thorny question in an asyncio world.  See the
-    # commentary on the initialize_reflectors_and_clients() method.
-
     reflectors = {
         "pods": None,
         "events": None,
@@ -148,23 +145,13 @@ def event_reflector(self):
             return self.__class__.reflectors['events']
 
     def __init__(self, *args, **kwargs):
-        """
-        We cannot call async methods from `__init__`.  Now that we use an
-        asyncio-based Kubernetes client, we have required initialization that
-        depends on async/await
-
-        Thus we need to do the initialization in `poll`, `start`, and `stop`
-        since any of them could be the first thing called (imagine that the
-        hub restarts and the first thing someone does is try to stop their
-        running server).  Further a pre-spawn-start hook that depends on
-        internal knowledge of the spawner might be the first thing to run,
-        and in a case like that, the caller would need to await the
-        initialization method (`initialize_reflectors_and_clients`) itself.
-        """
-
         _mock = kwargs.pop('_mock', False)
         super().__init__(*args, **kwargs)
 
+        # Schedules async initialization logic that is to be awaited by async
+        # functions by decorating them with @_await_async_init.
+        self._async_init_future = asyncio.ensure_future(self._async_init())
+
         if _mock:
             # runs during test execution only
             if 'user' not in kwargs:
@@ -210,11 +197,32 @@ def __init__(self, *args, **kwargs):
         # The attribute needs to exist, even though it is unset to start with
         self._start_future = None
 
-    async def initialize_reflectors_and_clients(self):
+    async def _async_init(self):
         """
-        This is functionality extracted from `__init__` because it requires
-        the use of async/await.  This method should be awaited before doing
-        anything with the object received from `__init__`.
+        This method is scheduled to run from `__init__`, but not awaited there
+        as it can't me marked as async.
+
+        Since JupyterHub won't await this method, we ensure the async methods
+        JupyterHub may call on this object will await this method until
+        continuing. To do this, we decorate them with `_await_async_init`.
+
+        But, how do we figure out the methods to decorate? Likely only those
+        exposed by the base class that JupyterHub would know about. The base
+        class is Spawner, as declared in spawner.py:
+        https://github.com/jupyterhub/jupyterhub/blob/HEAD/jupyterhub/spawner.py.
+
+        From the Proxy class docstring we can conclude that the following
+        methods, if implemented, could be what we need to decorate with
+        _await_async_init:
+
+          - load_state (implemented)
+          - get_state (implemented)
+          - start (implemented and decorated)
+          - stop (implemented and decorated)
+          - poll (implemented and decorated)
+
+        Out of these, it seems that only `start`, `stop`, and `poll` would need
+        the initialization logic in this method to have completed.
         """
         await load_config(caller=self)
         self.api = shared_client("CoreV1Api")
@@ -222,6 +230,19 @@ async def initialize_reflectors_and_clients(self):
         if self.events_enabled:
             await self._start_watching_events()
 
+    def _await_async_init(method):
+        """A decorator to await the _async_init method after having been
+        scheduled to run in the `__init__` method."""
+
+        @wraps(method)
+        async def async_method(self, *args, **kwargs):
+            if self._async_init_future is not None:
+                await self._async_init_future
+                self._async_init_future = None
+            return await method(self, *args, **kwargs)
+
+        return async_method
+
     k8s_api_ssl_ca_cert = Unicode(
         "",
         config=True,
@@ -2025,6 +2046,7 @@ def load_state(self, state):
         if 'pod_name' in state:
             self.pod_name = state['pod_name']
 
+    @_await_async_init
     async def poll(self):
         """
         Check if the pod is still running.
@@ -2038,10 +2060,6 @@ async def poll(self):
         necessary to check that the returned value is None, rather than
         just Falsy, to determine that the pod is still running.
         """
-        # We cannot be sure the Hub will call start() before poll(), so
-        # we need to load client configuration and start our reflectors
-        # at the top of each of those methods.
-        await self.initialize_reflectors_and_clients()
         # have to wait for first load of data before we have a valid answer
         if not self.pod_reflector.first_load_future.done():
             await asyncio.wrap_future(self.pod_reflector.first_load_future)
@@ -2271,8 +2289,7 @@ async def _start_watching_pods(self, replace=False):
             replace=replace,
         )
 
-    # record a future for the call to .start()
-    # so we can use it to terminate .progress()
+    @_await_async_init
     def start(self):
         """Thin wrapper around self._start
 
@@ -2453,10 +2470,6 @@ async def _start(self):
         # load user options (including profile)
         await self.load_user_options()
 
-        # Start watchers.  This might also be called from poll().  It also
-        #  configures our API clients.
-        await self.initialize_reflectors_and_clients()
-
         # If we have user_namespaces enabled, create the namespace.
         #  It's fine if it already exists.
         if self.enable_user_namespaces:
@@ -2660,11 +2673,8 @@ async def _make_delete_pvc_request(self, pvc_name, request_timeout):
             else:
                 raise
 
+    @_await_async_init
     async def stop(self, now=False):
-        # This could be the first method called; say the Hub has been
-        #  restarted, and the first thing someone does is hit it to stop
-        #  a running pod.  Hence the need to initialize reflectors/clients.
-        await self.initialize_reflectors_and_clients()
         delete_options = client.V1DeleteOptions()
 
         if now:

From e77791094f72d9849bf1127f4359eab3ddfd649a Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Fri, 18 Feb 2022 14:15:09 +0100
Subject: [PATCH 49/56] Remove unused import in proxy.py

---
 kubespawner/proxy.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 9df3d025..291d4ead 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -5,7 +5,6 @@
 import string
 
 import escapism
-import kubernetes.config
 from jupyterhub.proxy import Proxy
 from jupyterhub.utils import exponential_backoff
 from kubernetes_asyncio import client

From 56cb6303104380712b7b74609c74871145e6a441 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 18 Feb 2022 10:14:35 -0700
Subject: [PATCH 50/56] Move @_async_init decorator to _start()

---
 kubespawner/spawner.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 0289fb39..b3743e62 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -223,6 +223,10 @@ class is Spawner, as declared in spawner.py:
 
         Out of these, it seems that only `start`, `stop`, and `poll` would need
         the initialization logic in this method to have completed.
+
+        This is slightly complicated by the fact that `start` is already a
+        synchronous method that returns a future, so where we want the
+        decorator is actually on the async `_start` that `start` calls.
         """
         await load_config(caller=self)
         self.api = shared_client("CoreV1Api")
@@ -2289,7 +2293,6 @@ async def _start_watching_pods(self, replace=False):
             replace=replace,
         )
 
-    @_await_async_init
     def start(self):
         """Thin wrapper around self._start
 
@@ -2464,6 +2467,7 @@ async def _make_create_resource_request(self, kind, manifest):
         else:
             return True
 
+    @_await_async_init
     async def _start(self):
         """Start the user's pod"""
 

From 3d9b9503ff36409143575121ce89325026965120 Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Fri, 18 Feb 2022 11:47:57 -0700
Subject: [PATCH 51/56] Apply suggestions from code review

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/clients.py   |  3 ---
 kubespawner/proxy.py     |  4 ++--
 kubespawner/reflector.py | 16 +++++-----------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/kubespawner/clients.py b/kubespawner/clients.py
index ddbe3779..3ed378ab 100644
--- a/kubespawner/clients.py
+++ b/kubespawner/clients.py
@@ -35,9 +35,6 @@ def shared_client(ClientType, *args, **kwargs):
     so that concurrent calls to shared_client
     will all return the same instance until
     all references to the client are cleared.
-
-    Note that we must await load_config before calling this function as it
-    relies on global client configuration being loaded.
     """
     kwarg_key = tuple((key, kwargs[key]) for key in sorted(kwargs))
     cache_key = (ClientType, args, kwarg_key)
diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 291d4ead..1ccbd9b0 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -112,10 +112,10 @@ def __init__(self, *args, **kwargs):
     async def _async_init(self):
         """
         This method is scheduled to run from `__init__`, but not awaited there
-        as it can't me marked as async.
+        as `__init__` can't be marked as async.
 
         Since JupyterHub won't await this method, we ensure the async methods
-        JupyterHub may call on this object will await this method until
+        JupyterHub may call on this object will await this method before
         continuing. To do this, we decorate them with `_await_async_init`.
 
         But, how do we figure out the methods to decorate? Likely only those
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 07dfaf49..7e762607 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -30,13 +30,11 @@ class ResourceReflector(LoggingConfigurable):
     Must be subclassed once per kind of resource that needs watching.
 
     Creating a reflector should be done with the create() classmethod,
-    since that, in addition to creating the instance, initializes the
-    Kubernetes configuration, acquires a K8s API client, and starts the
-    watch task.
+    since that, in addition to creating the instance starts the watch task.
 
-    Shutting down a reflector should be done by awaiting its stop() method.
-    JupyterHub does not currently do this, so the watch task runs until the
-    Hub exits.
+    Shutting down a shared reflector should be done by awaiting its stop()
+    method. KubeSpawner doesn't gracefully do this currently, so the watch
+    task runs until the Hub exits.
     """
 
     labels = Dict(
@@ -216,11 +214,7 @@ def __init__(self, *args, **kwargs):
         if not self.list_method_name:
             raise RuntimeError("Reflector list_method_name must be set!")
 
-        self.watch_task = None  # Test this rather than absence of the attr
-
-        # Note that simply instantiating the class does not load
-        # Kubernetes config, acquire an API client, or start the
-        # watch task.
+        self.watch_task = None
 
     @classmethod
     async def create(cls, *args, **kwargs):

From f452361747fc0e688a80d5d213fadcf4beef1434 Mon Sep 17 00:00:00 2001
From: adam <athornton@lsst.org>
Date: Fri, 18 Feb 2022 11:57:58 -0700
Subject: [PATCH 52/56] Clarify stop() and shared reflectors

---
 kubespawner/reflector.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 7e762607..7c3c915a 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -32,9 +32,11 @@ class ResourceReflector(LoggingConfigurable):
     Creating a reflector should be done with the create() classmethod,
     since that, in addition to creating the instance starts the watch task.
 
-    Shutting down a shared reflector should be done by awaiting its stop()
-    method. KubeSpawner doesn't gracefully do this currently, so the watch
-    task runs until the Hub exits.
+    Shutting down a reflector should be done by awaiting its stop() method.
+
+    KubeSpawner does not do this, because its reflectors are singleton
+    instances shared among multiple spawners.  The watch task therefore runs
+    until JupyterHub exits.
     """
 
     labels = Dict(

From 10910116791eabd1b37c21da01f921dbfb362952 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.i.sundell@gmail.com>
Date: Fri, 18 Feb 2022 19:56:42 +0100
Subject: [PATCH 53/56] Use asyncio.gather when creating three reflectors

---
 kubespawner/proxy.py | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index 1ccbd9b0..d86f311a 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -142,14 +142,20 @@ class is Proxy, as declared in proxy.py:
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
         }
-        self.ingress_reflector = await IngressReflector.create(
-            parent=self, namespace=self.namespace, labels=labels
-        )
-        self.service_reflector = await ServiceReflector.create(
-            parent=self, namespace=self.namespace, labels=labels
-        )
-        self.endpoint_reflector = await EndpointsReflector.create(
-            parent=self, namespace=self.namespace, labels=labels
+        (
+            self.ingress_reflector,
+            self.service_reflector,
+            self.endpoint_reflector,
+        ) = asyncio.gather(
+            IngressReflector.create(
+                parent=self, namespace=self.namespace, labels=labels
+            ),
+            ServiceReflector.create(
+                parent=self, namespace=self.namespace, labels=labels
+            ),
+            EndpointsReflector.create(
+                parent=self, namespace=self.namespace, labels=labels
+            ),
         )
 
     def _await_async_init(method):

From 90231cb24c26eb939ca92eba2e33fc42119f95fb Mon Sep 17 00:00:00 2001
From: Adam Thornton <athornton@users.noreply.github.com>
Date: Fri, 18 Feb 2022 12:16:55 -0700
Subject: [PATCH 54/56] Update kubespawner/spawner.py

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 kubespawner/spawner.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index b3743e62..596835a7 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -200,10 +200,10 @@ def __init__(self, *args, **kwargs):
     async def _async_init(self):
         """
         This method is scheduled to run from `__init__`, but not awaited there
-        as it can't me marked as async.
+        as it can't be marked as async.
 
         Since JupyterHub won't await this method, we ensure the async methods
-        JupyterHub may call on this object will await this method until
+        JupyterHub may call on this object will await this method before
         continuing. To do this, we decorate them with `_await_async_init`.
 
         But, how do we figure out the methods to decorate? Likely only those

From 8c69b2a5e7e112597b9f66eb90f77930f226d922 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik@sundellopensource.se>
Date: Sat, 19 Feb 2022 02:20:20 +0100
Subject: [PATCH 55/56] Refactor away the create function

---
 kubespawner/proxy.py     | 27 +++++++++++++--------------
 kubespawner/reflector.py | 15 ---------------
 kubespawner/spawner.py   |  3 ++-
 3 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py
index d86f311a..b019c412 100644
--- a/kubespawner/proxy.py
+++ b/kubespawner/proxy.py
@@ -142,20 +142,19 @@ class is Proxy, as declared in proxy.py:
             'component': self.component_label,
             'hub.jupyter.org/proxy-route': 'true',
         }
-        (
-            self.ingress_reflector,
-            self.service_reflector,
-            self.endpoint_reflector,
-        ) = asyncio.gather(
-            IngressReflector.create(
-                parent=self, namespace=self.namespace, labels=labels
-            ),
-            ServiceReflector.create(
-                parent=self, namespace=self.namespace, labels=labels
-            ),
-            EndpointsReflector.create(
-                parent=self, namespace=self.namespace, labels=labels
-            ),
+        self.ingress_reflector = IngressReflector(
+            parent=self, namespace=self.namespace, labels=labels
+        )
+        self.service_reflector = ServiceReflector(
+            parent=self, namespace=self.namespace, labels=labels
+        )
+        self.endpoint_reflector = EndpointsReflector(
+            self, namespace=self.namespace, labels=labels
+        )
+        await asyncio.gather(
+            self.ingress_reflector.start(),
+            self.service_reflector.start(),
+            self.endpoint_reflector.start(),
         )
 
     def _await_async_init(method):
diff --git a/kubespawner/reflector.py b/kubespawner/reflector.py
index 7c3c915a..af57c6e5 100644
--- a/kubespawner/reflector.py
+++ b/kubespawner/reflector.py
@@ -218,21 +218,6 @@ def __init__(self, *args, **kwargs):
 
         self.watch_task = None
 
-    @classmethod
-    async def create(cls, *args, **kwargs):
-        """
-        This is a workaround: `__init__` cannot be async, but we want
-        to call async methods to instantiate a reflector in a usable state.
-
-        This method creates the reflector, and then loads Kubernetes config,
-        acquires an API client, and starts the watch task.
-
-        Use it to create a new Reflector object.
-        """
-        inst = cls(*args, **kwargs)
-        await inst.start()
-        return inst
-
     async def _list_and_update(self):
         """
         Update current list of resources by doing a full fetch.
diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index 596835a7..ced2946c 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -2244,12 +2244,13 @@ def on_reflector_failure():
         previous_reflector = self.__class__.reflectors.get(key)
 
         if replace or not previous_reflector:
-            self.__class__.reflectors[key] = await ReflectorClass.create(
+            self.__class__.reflectors[key] = ReflectorClass(
                 parent=self,
                 namespace=self.namespace,
                 on_failure=on_reflector_failure,
                 **kwargs,
             )
+            await self.__class__.reflectors[key].start()
 
         if replace and previous_reflector:
             # we replaced the reflector, stop the old one

From 3a82fc01a5edc0068db2054264be6d84bc658109 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik@sundellopensource.se>
Date: Sat, 19 Feb 2022 02:26:45 +0100
Subject: [PATCH 56/56] Remove outdated comment

---
 kubespawner/spawner.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
index ced2946c..27f53082 100644
--- a/kubespawner/spawner.py
+++ b/kubespawner/spawner.py
@@ -2317,7 +2317,6 @@ async def _make_create_pod_request(self, pod, request_timeout):
             self.log.info(
                 f"Attempting to create pod {pod.metadata.name}, with timeout {request_timeout}"
             )
-            # Use asyncio.wait_for, _request_timeout seems unreliable?
             await asyncio.wait_for(
                 self.api.create_namespaced_pod(
                     self.namespace,