Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rely on the event loop: use kubernetes_asyncio instead of kubernetes and dedicated threads #563

Merged
merged 63 commits into from
Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
9160ea9
Change kubernetes to kubernetes_asyncio in imports
athornton Jan 26, 2022
b921ffd
remove asynchronize() and gen.with_timeout()
athornton Jan 27, 2022
31eb2a9
Basic multinamespace functionality works
athornton Jan 27, 2022
ef03248
missed a stock k8s import
athornton Jan 27, 2022
0a07bfe
Use dict rather than object representation of watched objects
athornton Jan 27, 2022
cfd350c
Make the initialization a public method, and call it during stop() too.
athornton Jan 27, 2022
ecda5fe
Address review commentary, part 1
athornton Jan 28, 2022
59e45ec
Attempt to skip serialization
athornton Jan 28, 2022
742e471
Terminate watch cleanly
athornton Jan 30, 2022
b05e826
Make shared_client() sync
athornton Jan 31, 2022
abf99f1
Address further review comments
athornton Feb 1, 2022
ef55046
Rename package for upload
athornton Feb 1, 2022
31833bd
Merge pull request #2 from lsst-sqre/tickets/DM-33044E
athornton Feb 1, 2022
c1278c3
Brutal, but working
athornton Feb 2, 2022
b1080b2
Remove some brutality
athornton Feb 3, 2022
11fa7fe
Update with non-deserializing kubernetes_asyncio
athornton Feb 3, 2022
9597a5d
Remove dead comment and bump version
athornton Feb 3, 2022
709a29b
Address review commentary
athornton Feb 3, 2022
6cf57db
I think this is how it *should* work
athornton Feb 4, 2022
2ca98b2
Use exceptions to handle watches
athornton Feb 4, 2022
bc8bfae
Merge pull request #3 from lsst-sqre/tickets/DM-33509B
athornton Feb 4, 2022
c4d557f
Rename back to 'kubespawner' and bump major version
athornton Feb 4, 2022
6111628
flake8 cleanup
athornton Feb 4, 2022
67be97e
remove useless __init__
athornton Feb 4, 2022
2c82749
Fix paren placement
athornton Feb 4, 2022
fc51aa4
Blacken and isort
athornton Feb 4, 2022
ca76409
isort, on its own
athornton Feb 4, 2022
5d5d7bc
address review commentary
athornton Feb 4, 2022
71a86c1
Check sync client version
athornton Feb 4, 2022
73f78cc
Rerun pre-commit
athornton Feb 4, 2022
05572bf
Address review comments
athornton Feb 8, 2022
d23c6fc
fix python_requires
athornton Feb 9, 2022
2b8a3f3
Merge branch 'main' into tickets/DM-33560
athornton Feb 15, 2022
0df8b99
Address Erik Sundell's review commentary
athornton Feb 16, 2022
5725ca2
move SSL config into load_config
athornton Feb 16, 2022
467af7c
Address object instantiation/initialization
athornton Feb 17, 2022
ce942c9
simplify SSL handling in load_config()
athornton Feb 17, 2022
69b76cf
Improve load_config() doc
athornton Feb 17, 2022
e6c87dc
Make Reflector's load_client reflect its parent
athornton Feb 17, 2022
c2a257c
Clarify documentation
athornton Feb 17, 2022
0cc6352
Update docstrings and comments
consideRatio Feb 17, 2022
96a7358
Let reflector instances assume k8s client config is loaded
consideRatio Feb 17, 2022
7c55baf
cleanup: remove unused reference to IOLoop.current()
consideRatio Feb 17, 2022
4a04b89
Remove unused imports
consideRatio Feb 17, 2022
1578cb4
ci: update test matrix and remove redundant flake8 test
consideRatio Feb 17, 2022
46b6a0a
Address review comments by Adam
consideRatio Feb 17, 2022
98c9528
Merge pull request #8 from consideRatio/tickets/DM-33560
athornton Feb 17, 2022
5d34e64
Update kubespawner/reflector.py
athornton Feb 17, 2022
f3e6cbc
Update kubespawner/spawner.py
athornton Feb 17, 2022
a38246e
Update kubespawner/spawner.py
athornton Feb 17, 2022
b283953
Update kubespawner/spawner.py
athornton Feb 17, 2022
358c768
Apply Min's idea on awaiting async initialization logic
consideRatio Feb 18, 2022
e777910
Remove unused import in proxy.py
consideRatio Feb 18, 2022
75e4a55
Merge pull request #10 from consideRatio/tickets/DM-33560
athornton Feb 18, 2022
56cb630
Move @_async_init decorator to _start()
athornton Feb 18, 2022
3d9b950
Apply suggestions from code review
athornton Feb 18, 2022
f452361
Clarify stop() and shared reflectors
athornton Feb 18, 2022
1091011
Use asyncio.gather when creating three reflectors
consideRatio Feb 18, 2022
8d3a0fd
Merge pull request #11 from consideRatio/tickets/DM-33560
athornton Feb 18, 2022
90231cb
Update kubespawner/spawner.py
athornton Feb 18, 2022
8c69b2a
Refactor away the create function
consideRatio Feb 19, 2022
3a82fc0
Remove outdated comment
consideRatio Feb 19, 2022
dc18487
Merge pull request #12 from consideRatio/tickets/DM-33560
athornton Feb 19, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
65 changes: 14 additions & 51 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: 10.* # supports k8s 1.14
- 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
Expand All @@ -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.
Expand Down Expand Up @@ -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: |
Expand Down
66 changes: 50 additions & 16 deletions kubespawner/clients.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
"""Shared clients for kubernetes
"""Configures and instantiates REST API clients of various kinds to
communicate with a Kubernetes api-server, but only one instance per kind is
instantiated.

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

import kubernetes.client
from kubernetes.client import api_client
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.
# 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

_client_cache = {}


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
Expand All @@ -38,11 +45,38 @@ 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
Client = getattr(kubernetes.client, ClientType)
# 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
_client_cache[cache_key] = weakref.ref(client)

return client


async def load_config(caller):
"""
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 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()
except kubernetes_asyncio.config.ConfigException:
await kubernetes_asyncio.config.load_kube_config()

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)
109 changes: 59 additions & 50 deletions kubespawner/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,50 +9,49 @@
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
from kubespawner.utils import get_k8s_model, update_k8s_model


def make_pod(
Expand Down Expand Up @@ -734,23 +733,33 @@ def make_ingress(name, routespec, target, labels, data):
# to keep compatibility with older K8S versions

try:
from kubernetes.client.models import (
ExtensionsV1beta1Ingress,
ExtensionsV1beta1IngressSpec,
ExtensionsV1beta1IngressRule,
ExtensionsV1beta1HTTPIngressRuleValue,
from kubernetes_asyncio.client.models import (
ExtensionsV1beta1HTTPIngressPath,
ExtensionsV1beta1HTTPIngressRuleValue,
ExtensionsV1beta1Ingress,
ExtensionsV1beta1IngressBackend,
ExtensionsV1beta1IngressRule,
ExtensionsV1beta1IngressSpec,
)
except ImportError:
from kubernetes.client.models import (
V1beta1Ingress as ExtensionsV1beta1Ingress,
V1beta1IngressSpec as ExtensionsV1beta1IngressSpec,
V1beta1IngressRule as ExtensionsV1beta1IngressRule,
V1beta1HTTPIngressRuleValue as ExtensionsV1beta1HTTPIngressRuleValue,
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,
Expand Down
Loading