Skip to content

Commit

Permalink
Make use of a reset_pod_reflector fixture when needed
Browse files Browse the repository at this point in the history
  • Loading branch information
consideRatio committed Nov 10, 2022
1 parent da764c2 commit 5e5cbd2
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 21 deletions.
45 changes: 24 additions & 21 deletions kubespawner/spawner.py
Expand Up @@ -55,6 +55,11 @@ class PodReflector(ResourceReflector):

kind = "pods"

# The default component label can be over-ridden by specifying the component_label property
labels = {
'component': 'singleuser-server',
}

@property
def pods(self):
"""
Expand Down Expand Up @@ -113,25 +118,24 @@ class KubeSpawner(Spawner):
spawned by a user will have its own KubeSpawner instance.
"""

# Reflectors are stored in class variable for performance reasons.
# If every pod will start its own reflector, Kubelet will not be happy
# with attaching 20k event watchers
reflectors = {}
# Reflectors keeping track of the k8s api-server's state for various k8s
# resources are singletons as that state can be tracked and shared by all
# KubeSpawner objects.
reflectors = {
"pods": None,
"events": None,
}

# Characters as defined by safe for DNS
# Note: '-' is not in safe_chars, as it is being used as escape character
safe_chars = set(string.ascii_lowercase + string.digits)

def get_reflector_key(self, kind: str) -> tuple:
namespace = None if self.enable_user_namespaces else self.namespace
return (kind, self.component_label, namespace)

@property
def pod_reflector(self):
"""
A convenience alias to the class variable reflectors['pods'].
"""
return self.__class__.reflectors.get(self.get_reflector_key('pods'))
return self.__class__.reflectors['pods']

@property
def event_reflector(self):
Expand All @@ -140,9 +144,7 @@ def event_reflector(self):
spawner instance has events_enabled.
"""
if self.events_enabled:
return self.__class__.reflectors.get(self.get_reflector_key('events'))

return None
return self.__class__.reflectors['events']

def __init__(self, *args, **kwargs):
_mock = kwargs.pop('_mock', False)
Expand Down Expand Up @@ -2378,9 +2380,9 @@ async def progress(self):

def _start_reflector(
self,
kind: str,
reflector_class: ResourceReflector,
replace: bool = False,
kind=None,
reflector_class=ResourceReflector,
replace=False,
**kwargs,
):
"""Start a shared reflector on the KubeSpawner class
Expand All @@ -2395,7 +2397,8 @@ def _start_reflector(
If replace=True, a running pod reflector will be stopped
and a new one started (for recovering from possible errors).
"""
key = self.get_reflector_key(kind)
key = kind
ReflectorClass = reflector_class

def on_reflector_failure():
self.log.critical(
Expand All @@ -2407,7 +2410,7 @@ def on_reflector_failure():
previous_reflector = self.__class__.reflectors.get(key)

if replace or not previous_reflector:
self.__class__.reflectors[key] = reflector_class(
self.__class__.reflectors[key] = ReflectorClass(
parent=self,
namespace=self.namespace,
on_failure=on_reflector_failure,
Expand Down Expand Up @@ -2457,10 +2460,11 @@ def _start_watching_pods(self, replace=False):
If replace=True, a running pod reflector will be stopped
and a new one started (for recovering from possible errors).
"""
pod_reflector_class = PodReflector
pod_reflector_class.labels.update({"component": self.component_label})
return self._start_reflector(
kind="pods",
reflector_class=PodReflector,
labels={"component": self.component_label},
"pods",
PodReflector,
omit_namespace=self.enable_user_namespaces,
replace=replace,
)
Expand Down Expand Up @@ -2637,7 +2641,6 @@ async def _make_create_resource_request(self, kind, manifest):
else:
return True

@_await_pod_reflector
async def _start(self):
"""Start the user's pod"""

Expand Down
20 changes: 20 additions & 0 deletions tests/conftest.py
Expand Up @@ -31,6 +31,7 @@
from kubernetes_asyncio.watch import Watch
from traitlets.config import Config

from kubespawner import KubeSpawner
from kubespawner.clients import shared_client

here = os.path.abspath(os.path.dirname(__file__))
Expand Down Expand Up @@ -623,3 +624,22 @@ async def exec_python(kube_client, kube_ns):
pod = await create_resource(kube_client, kube_ns, "pod", pod_manifest)

yield partial(_exec_python_in_pod, kube_client, kube_ns, pod_name)


@pytest.fixture(scope="function")
def reset_pod_reflector():
"""
Resets the class state KubeSpawner.reflectors["pods"] before and after the
test function executes. This enables us to start fresh if a test needs to
test configuration influencing our singleton pod reflector.
"""

def _reset_pod_reflector():
pods_reflector = KubeSpawner.reflectors["pods"]
KubeSpawner.reflectors["pods"] = None
if pods_reflector:
asyncio.ensure_future(pods_reflector.stop())

_reset_pod_reflector()
yield
_reset_pod_reflector()
2 changes: 2 additions & 0 deletions tests/test_spawner.py
Expand Up @@ -211,6 +211,7 @@ async def test_spawn_component_label(
kube_client,
config,
hub,
reset_pod_reflector,
):
spawner = KubeSpawner(
hub=hub,
Expand Down Expand Up @@ -339,6 +340,7 @@ async def test_spawn_services_enabled(
kube_client,
hub,
config,
reset_pod_reflector,
):
spawner = KubeSpawner(
config=config,
Expand Down

0 comments on commit 5e5cbd2

Please sign in to comment.