From da07f317f9e458b50481cb3d610f6ffa767770b9 Mon Sep 17 00:00:00 2001 From: psolomin Date: Sat, 29 Apr 2023 15:46:24 +0100 Subject: [PATCH 1/7] Drop unnecessary imports --- test/test_kubernetes_job_task.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/test_kubernetes_job_task.py b/test/test_kubernetes_job_task.py index f6a36d9..7c8bc11 100644 --- a/test/test_kubernetes_job_task.py +++ b/test/test_kubernetes_job_task.py @@ -1,13 +1,8 @@ import yaml - - -from mock import patch, MagicMock import pytest from kubeluigi import KubernetesJobTask -from kubernetes.client import BatchV1Api - class DummyTask(KubernetesJobTask): @property From affb757249328fb174612c28ab80ecbe0d727028 Mon Sep 17 00:00:00 2001 From: psolomin Date: Sun, 30 Apr 2023 16:32:19 +0100 Subject: [PATCH 2/7] Fixes --- kubeluigi/__init__.py | 4 +--- kubeluigi/k8s.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/kubeluigi/__init__.py b/kubeluigi/__init__.py index 389814a..0432384 100644 --- a/kubeluigi/__init__.py +++ b/kubeluigi/__init__.py @@ -25,9 +25,7 @@ class KubernetesJobTask: volumes: List[AttachableVolume] = [] - - def __init__(self): - self.tolerations: List[V1Toleration] = [] + tolerations: List[V1Toleration] = [] def _init_task_metadata(self): self.uu_name = self.name diff --git a/kubeluigi/k8s.py b/kubeluigi/k8s.py index 62406aa..ca82e79 100644 --- a/kubeluigi/k8s.py +++ b/kubeluigi/k8s.py @@ -134,6 +134,17 @@ def kick_off_job(k8s_client: ApiClient, job: V1Job) -> V1Job: job = k8s_client.read_namespaced_job( job.metadata.name, job.metadata.namespace ) + # TODO: improve design of this + # Problem: of a job failed, it's currently tracked and keeps + # the Luigi task failing. This is a quick patch to avoid that. + if not job.status.active: + condition = job.status.conditions[0] + if condition.type == "Failed" and condition.reason == "BackoffLimitExceeded": + logger.warning( + "The job you tried to start was in BackoffLimitExceeded state, deleting it" + ) + clean_job_resources(k8s_client, job) + raise RuntimeError("Found orphan failed job with the same spec, deleted it.") else: raise e From f4b61ce1f14ce1ab5f6e06e4a68c2e9f1dccdf1f Mon Sep 17 00:00:00 2001 From: psolomin Date: Mon, 1 May 2023 13:06:49 +0100 Subject: [PATCH 3/7] Fix attribute error --- kubeluigi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kubeluigi/__init__.py b/kubeluigi/__init__.py index 0432384..75fbd2e 100644 --- a/kubeluigi/__init__.py +++ b/kubeluigi/__init__.py @@ -96,7 +96,7 @@ def build_job_definition(self) -> V1Job: def onpodstarted(self, pods): for pod in pods: logger.info( - f"Tail the Pod logs using: kubectl logs -f -n {pod.namespace} {pod.name}" + f"Tail the Pod logs using: kubectl logs -f -n {pod.metadata.namespace} {pod.metadata.name}" ) def as_yaml(self): From 72c47b849fe5a31b49d7516cd13667e89c63c6ce Mon Sep 17 00:00:00 2001 From: psolomin Date: Mon, 1 May 2023 19:42:06 +0100 Subject: [PATCH 4/7] Preserve volume spec items Otherwise things like read_only, sub_path, etc are dropped from the final spec. --- kubeluigi/k8s.py | 7 ++++--- test/kubernetes_helpers_test.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/kubeluigi/k8s.py b/kubeluigi/k8s.py index ca82e79..020f1e3 100644 --- a/kubeluigi/k8s.py +++ b/kubeluigi/k8s.py @@ -91,10 +91,11 @@ def get_container_with_volume_mounts(container): """ volumes_spec = container["volume_mounts"] mount_volumes = [] + keys_to_omit = {"host_path"} for volume in volumes_spec: - mount_path = volume["mountPath"] - name = volume["name"] - mount_volumes.append(V1VolumeMount(mount_path=mount_path, name=name)) + # we need things like read_only, sub_path, etc: + volume_std_spec = {k: v for k, v in volume.items() if k not in keys_to_omit} + mount_volumes.append(V1VolumeMount(**volume_std_spec)) container["volume_mounts"] = mount_volumes return container diff --git a/test/kubernetes_helpers_test.py b/test/kubernetes_helpers_test.py index a16cc83..bcbda22 100644 --- a/test/kubernetes_helpers_test.py +++ b/test/kubernetes_helpers_test.py @@ -56,7 +56,7 @@ def test_pod_spec_from_dict(): "imagePullPolicy": "Always", "env": [{"name": "my_env", "value": "env"}], "volume_mounts": [ - {"name": "Vname", "mountPath": "VmountPath", "host_path": "VhostPath"} + {"name": "Vname", "mount_path": "VmountPath", "host_path": "VhostPath"} ], } ], @@ -105,7 +105,7 @@ def test_pod_spec_with_volume_from_dict(): "imagePullPolicy": "Always", "env": [{"name": "my_env", "value": "env"}], "volume_mounts": [ - {"name": "Vname", "mountPath": "VmountPath", "host_path": "VhostPath"} + {"name": "Vname", "mount_path": "VmountPath", "host_path": "VhostPath"} ], } From 3749f79874bdd5d23ef3686ffa7d231ec950c13d Mon Sep 17 00:00:00 2001 From: psolomin Date: Tue, 2 May 2023 00:07:40 +0100 Subject: [PATCH 5/7] Add test which fails ``` ================================================================ short test summary info ================================================================= FAILED test/test_task.py::test_task_can_be_scheduled_by_luigi - luigi.worker.TaskException: Task of class DummyK8sTask not initialized. Did you override __init__ and forget to call super(...).__init__? ======================================================== 1 failed, 18 passed, 1 warning in 1.69s ========================================================= nox > Command pytest failed with exit code 1 ``` --- test/test_task.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/test_task.py diff --git a/test/test_task.py b/test/test_task.py new file mode 100644 index 0000000..3f81702 --- /dev/null +++ b/test/test_task.py @@ -0,0 +1,21 @@ +from typing import Any, Dict + +import luigi + +from kubeluigi import KubernetesJobTask + + +class DummyK8sTask(KubernetesJobTask, luigi.Task): + @property + def name(self) -> str: + return "dummy-tsk" + + def spec_schema(self) -> Dict[str, Any]: + return {} + + def run(self): + pass + + +def test_task_can_be_scheduled_by_luigi(): + luigi.build([DummyK8sTask()], local_scheduler=True, log_level="WARNING") From 31e431e1fb4965fd440a95d7cef13b9193f87e58 Mon Sep 17 00:00:00 2001 From: blackwolf Date: Wed, 19 Jul 2023 09:35:08 -0500 Subject: [PATCH 6/7] remove pyyaml version (#1) Co-authored-by: blackwolf --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 28209fd..f85b153 100644 --- a/setup.py +++ b/setup.py @@ -40,6 +40,6 @@ def run_tests(self): license="Apache License 2.0", packages=find_packages(exclude=["tests"]), cmdclass={"test": PyTest}, - install_requires=["kubernetes>=17.17.0", "luigi", "PyYaml==5.4.1"], + install_requires=["kubernetes>=17.17.0", "luigi", "PyYaml"], entry_points={}, ) From cca3d7cd567104aaa03b3e2aac93c66be15f7e97 Mon Sep 17 00:00:00 2001 From: psolomin Date: Sun, 13 Aug 2023 01:41:58 +0100 Subject: [PATCH 7/7] Add job name to error message --- kubeluigi/k8s.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kubeluigi/k8s.py b/kubeluigi/k8s.py index 020f1e3..15d57e0 100644 --- a/kubeluigi/k8s.py +++ b/kubeluigi/k8s.py @@ -32,7 +32,7 @@ class FailedJob(Exception): def __init__(self, job, pods, message): self.job = job self.pods = pods - self.message = message + self.message = job.metadata.name + ": " + message super().__init__(self.message)