From df503c3a09d828c019951ffe02515caa8f3a10b1 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 24 Apr 2024 09:03:34 +0200 Subject: [PATCH] Add modern app.kubernetes.io labels alongside old --- kubespawner/proxy.py | 27 +++++++++-- kubespawner/spawner.py | 33 ++++++++++++-- tests/test_objects.py | 101 +++++++++++------------------------------ tests/test_spawner.py | 16 ++++++- 4 files changed, 93 insertions(+), 84 deletions(-) diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py index 5dfa9930..f1b14088 100644 --- a/kubespawner/proxy.py +++ b/kubespawner/proxy.py @@ -156,14 +156,21 @@ def _namespace_default(self): 'singleuser-server', config=True, help=""" - The component label used to tag the user pods. This can be used to override - the spawner behavior when dealing with multiple hub instances in the same - namespace. Usually helpful for CI workflows. + The value of the labels app.kubernetes.io/component and component, used + to identify user pods kubespawner is to manage. + + This can be used to override the spawner behavior when dealing with + multiple hub instances in the same namespace. Usually helpful for CI + workflows. """, ) common_labels = Dict( { + 'app.kubernetes.io/name': 'jupyterhub', + 'app.kubernetes.io/managed-by': 'kubespawner', + # app and heritage are older variants of the modern + # app.kubernetes.io labels 'app': 'jupyterhub', 'heritage': 'jupyterhub', }, @@ -320,6 +327,13 @@ def __init__(self, *args, **kwargs): self.networking_api = shared_client('NetworkingV1Api') labels = { + # NOTE: We monitor resources with the old component label instead of + # the modern app.kubernetes.io/component label. A change here + # is only non-breaking if we can assume the running resources + # monitored can be detected by either old or new labels. + # + # Related to https://github.com/jupyterhub/kubespawner/issues/834 + # 'component': self.component_label, 'hub.jupyter.org/proxy-route': 'true', } @@ -421,7 +435,12 @@ async def add_route(self, routespec, target, data): full_name = f'{self.namespace}/{safe_name}' common_labels = self._expand_all(self.common_labels, routespec, data) - common_labels.update({'component': self.component_label}) + common_labels.update( + { + 'app.kubernetes.io/component': self.component_label, + 'component': self.component_label, + } + ) ingress_extra_labels = self._expand_all( self.ingress_extra_labels, routespec, data diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py index fd53a8e7..0948cf21 100644 --- a/kubespawner/spawner.py +++ b/kubespawner/spawner.py @@ -596,9 +596,12 @@ def _namespace_default(self): 'singleuser-server', config=True, help=""" - The component label used to tag the user pods. This can be used to override - the spawner behavior when dealing with multiple hub instances in the same - namespace. Usually helpful for CI workflows. + The value of the labels app.kubernetes.io/component and component, used + to identify user pods kubespawner is to manage. + + This can be used to override the spawner behavior when dealing with + multiple hub instances in the same namespace. Usually helpful for CI + workflows. """, ) @@ -655,6 +658,10 @@ def _deprecated_changed(self, change): common_labels = Dict( { + 'app.kubernetes.io/name': 'jupyterhub', + 'app.kubernetes.io/managed-by': 'kubespawner', + # app and heritage are older variants of the modern + # app.kubernetes.io labels 'app': 'jupyterhub', 'heritage': 'jupyterhub', }, @@ -1878,6 +1885,7 @@ def _build_pod_labels(self, extra_labels): labels = self._build_common_labels(extra_labels) labels.update( { + 'app.kubernetes.io/component': self.component_label, 'component': self.component_label, 'hub.jupyter.org/servername': escapism.escape( self.name, safe=self.safe_chars, escape_char='-' @@ -2102,7 +2110,17 @@ def get_pvc_manifest(self): Make a pvc manifest that will spawn current user's pvc. """ labels = self._build_common_labels(self._expand_all(self.storage_extra_labels)) - labels.update({'component': 'singleuser-storage'}) + labels.update( + { + # The component label has been set to singleuser-storage, but should + # probably have been set to singleuser-server (self.component_label) + # as that ties it to the user pods kubespawner creates. Due to that, + # the newly introduced label app.kubernetes.io/component gets + # singleuser-server (self.component_label) as a value instead. + 'app.kubernetes.io/component': self.component_label, + 'component': 'singleuser-storage', + } + ) annotations = self._build_common_annotations( self._expand_all(self.storage_extra_annotations) @@ -2473,6 +2491,13 @@ async def _start_watching_pods(self, replace=False): return await self._start_reflector( kind="pods", reflector_class=PodReflector, + # NOTE: We monitor resources with the old component label instead of + # the modern app.kubernetes.io/component label. A change here + # is only non-breaking if we can assume the running resources + # monitored can be detected by either old or new labels. + # + # Related to https://github.com/jupyterhub/kubespawner/issues/834 + # labels={"component": self.component_label}, omit_namespace=self.enable_user_namespaces, replace=replace, diff --git a/tests/test_objects.py b/tests/test_objects.py index 90aba79c..afe5cc9d 100644 --- a/tests/test_objects.py +++ b/tests/test_objects.py @@ -1986,9 +1986,7 @@ def test_make_ingress_for_ip(reuse_existing_services, target, ip): Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } ingress_extra_labels = { 'extra/label': 'value1', @@ -2020,9 +2018,7 @@ def test_make_ingress_for_ip(reuse_existing_services, target, ip): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2039,9 +2035,7 @@ def test_make_ingress_for_ip(reuse_existing_services, target, ip): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2062,9 +2056,7 @@ def test_make_ingress_for_ip(reuse_existing_services, target, ip): 'extra/annotation': 'value2', }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', 'extra/label': 'value1', }, @@ -2112,8 +2104,7 @@ def test_make_ingress_for_service_reuse_existing_services_enabled(target): leads to reusing the same service which was created by KubeSpawner """ common_labels = { - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } endpoint, service, ingress = api_client.sanitize_for_serialization( make_ingress( @@ -2139,8 +2130,7 @@ def test_make_ingress_for_service_reuse_existing_services_enabled(target): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'component': 'singleuser-server', - 'heritage': 'jupyterhub', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2201,8 +2191,7 @@ def test_make_ingress_for_service_reuse_existing_services_disabled( leads to creating service with type External name pointing to the pod's service """ common_labels = { - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } endpoint, service, ingress = api_client.sanitize_for_serialization( make_ingress( @@ -2227,8 +2216,7 @@ def test_make_ingress_for_service_reuse_existing_services_disabled( 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'component': 'singleuser-server', - 'heritage': 'jupyterhub', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2250,8 +2238,7 @@ def test_make_ingress_for_service_reuse_existing_services_disabled( 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'component': 'singleuser-server', - 'heritage': 'jupyterhub', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2315,9 +2302,7 @@ def test_make_ingress_for_service_reuse_existing_services_ignored( or `KubeSpawner.enable_user_namespaces=True` """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } endpoint, service, ingress = api_client.sanitize_for_serialization( make_ingress( @@ -2342,9 +2327,7 @@ def test_make_ingress_for_service_reuse_existing_services_ignored( 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2366,9 +2349,7 @@ def test_make_ingress_for_service_reuse_existing_services_ignored( 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2412,9 +2393,7 @@ def test_make_ingress_with_subdomain_host(target): Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } _endpoint, _service, ingress = api_client.sanitize_for_serialization( make_ingress( @@ -2436,9 +2415,7 @@ def test_make_ingress_with_subdomain_host(target): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2483,9 +2460,7 @@ def test_make_ingress_with_specifications(target, ip): Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } ingress_specifications = [ { @@ -2517,9 +2492,7 @@ def test_make_ingress_with_specifications(target, ip): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2536,9 +2509,7 @@ def test_make_ingress_with_specifications(target, ip): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2558,9 +2529,7 @@ def test_make_ingress_with_specifications(target, ip): 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2621,9 +2590,7 @@ def test_make_ingress_external_name_with_specifications(): Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } ingress_specifications = [ { @@ -2657,9 +2624,7 @@ def test_make_ingress_external_name_with_specifications(): 'hub.jupyter.org/proxy-target': 'http://my-pod-name:9000', }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2680,9 +2645,7 @@ def test_make_ingress_external_name_with_specifications(): 'hub.jupyter.org/proxy-target': 'http://my-pod-name:9000', }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2762,9 +2725,7 @@ def test_make_ingress_with_specifications_and_matching_subdomain_host(target, ho Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } ingress_specifications = [ { @@ -2794,9 +2755,7 @@ def test_make_ingress_with_specifications_and_matching_subdomain_host(target, ho 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2861,9 +2820,7 @@ def test_make_ingress_with_specifications_and_not_matching_subdomain_host(target Test specification of the ingress objects """ common_labels = { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', } ingress_specifications = [ { @@ -2893,9 +2850,7 @@ def test_make_ingress_with_specifications_and_not_matching_subdomain_host(target 'hub.jupyter.org/proxy-target': target, }, 'labels': { - 'app': 'jupyterhub', - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'common/label': 'value0', 'hub.jupyter.org/proxy-route': 'true', }, 'name': 'jupyter-test', @@ -2954,8 +2909,7 @@ def test_make_ingress_with_specifications_and_not_matching_subdomain_host(target def test_make_namespace(): labels = { - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', + 'some/label': 'value0', } namespace = api_client.sanitize_for_serialization( make_namespace(name='test-namespace', labels=labels) @@ -2964,8 +2918,7 @@ def test_make_namespace(): 'metadata': { 'annotations': {}, 'labels': { - 'component': 'singleuser-server', - 'heritage': 'jupyterhub', + 'some/label': 'value0', }, 'name': 'test-namespace', }, diff --git a/tests/test_spawner.py b/tests/test_spawner.py index 897f3d89..e3859179 100644 --- a/tests/test_spawner.py +++ b/tests/test_spawner.py @@ -332,7 +332,7 @@ async def test_spawn_start_enable_user_namespaces( assert isinstance(status, int) -async def test_spawn_component_label( +async def test_spawn_component_label_and_other_labels( kube_ns, kube_client, config, @@ -360,8 +360,16 @@ async def test_spawn_component_label( # component label is same as expected pod = pods[0] + assert pod.metadata.labels["app.kubernetes.io/component"] == "something" assert pod.metadata.labels["component"] == "something" + # other labels are the same as expected + assert pod.metadata.labels["app.kubernetes.io/name"] == "jupyterhub" + assert pod.metadata.labels["app.kubernetes.io/managed-by"] == "kubespawner" + assert pod.metadata.labels["heritage"] == "jupyterhub" + assert pod.metadata.labels["app"] == "jupyterhub" + assert pod.metadata.labels["heritage"] == "jupyterhub" + # stop the pod await spawner.stop() @@ -510,6 +518,7 @@ async def test_spawn_services_enabled( # verify selector contains component_label, common_labels and extra_labels # as well as user and server name selector = services[0].spec.selector + assert selector["app.kubernetes.io/component"] == "something" assert selector["component"] == "something" assert selector["some/label"] == "value1" assert selector["extra/label"] == "value2" @@ -1513,9 +1522,12 @@ async def test_get_pvc_manifest(): assert manifest.metadata.labels == { "user": "mock-5fname", "hub.jupyter.org/username": "mock-5fname", + "app.kubernetes.io/name": "jupyterhub", + "app.kubernetes.io/managed-by": "kubespawner", + "app.kubernetes.io/component": "singleuser-server", "app": "jupyterhub", - "component": "singleuser-storage", "heritage": "jupyterhub", + "component": "singleuser-storage", } assert manifest.metadata.annotations == { "user": "mock-5fname",