-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: create TES task as k8s job #200
base: main
Are you sure you want to change the base?
Changes from 6 commits
6a578b0
4edd278
009ca53
84dfc46
2429b0f
cebb7b2
eb652f4
89e2dfb
f9b630b
5f44926
ad17a00
2b7a251
da720a7
38f2b46
0cd3110
aaad8fb
46bbe2a
099c528
d96b6ae
d545d24
9d1b89c
e2adc6f
104aa05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ host_name: "" | |
# | ||
|
||
# 'openstack' or 's3' | ||
storage: none | ||
storage: s3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: turn back to none. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏽 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be described in more detail somewhere. Also, if |
||
|
||
# Configurable storage class. | ||
storageClass: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,15 @@ | |
|
||
import logging | ||
|
||
# from connexion import request # type: ignore | ||
from foca.utils.logging import log_traffic # type: ignore | ||
|
||
from tesk.api.ga4gh.tes.models import TesTask | ||
from tesk.api.ga4gh.tes.service_info.service_info import ServiceInfo | ||
from tesk.api.ga4gh.tes.task.create_task import CreateTesTask | ||
from tesk.api.kubernetes.convert.converter import TesKubernetesConverter | ||
from tesk.api.kubernetes.convert.template import KubernetesTemplateSupplier | ||
from tesk.exceptions import BadRequest, InternalServerError | ||
from tesk.utils import get_custom_config | ||
|
||
# Get logger instance | ||
logger = logging.getLogger(__name__) | ||
|
@@ -26,14 +31,24 @@ def CancelTask(id, *args, **kwargs) -> dict: # type: ignore | |
|
||
# POST /tasks | ||
@log_traffic | ||
def CreateTask(*args, **kwargs) -> dict: # type: ignore | ||
def CreateTask(**kwargs) -> dict: # type: ignore | ||
"""Create task. | ||
|
||
Args: | ||
*args: Variable length argument list. | ||
**kwargs: Arbitrary keyword arguments. | ||
""" | ||
pass | ||
try: | ||
request_body = kwargs.get("body") | ||
if request_body is None: | ||
logger("Nothing recieved in request body.") | ||
raise BadRequest("No request body recieved.") | ||
tes_task = TesTask(**request_body) | ||
namespace = "tesk" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have just seen that this value is hardwired to I am not sure what others think, but this will be an issue for CSC There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't be an issue I plan on removing it from here, I'll add this to docs config or as env variable so when launching the api anyone can configure it. |
||
task_creater = CreateTesTask(tes_task, namespace) | ||
task_creater.response() | ||
except Exception as e: | ||
raise InternalServerError from e | ||
|
||
|
||
# GET /tasks/service-info | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
"""Task API controller logic.""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
"""TESK API module for creating a task.""" | ||
|
||
import logging | ||
|
||
from tesk.api.ga4gh.tes.models import TesTask | ||
from tesk.api.kubernetes.client_wrapper import KubernetesClientWrapper | ||
from tesk.api.kubernetes.constants import Constants | ||
from tesk.api.kubernetes.convert.converter import TesKubernetesConverter | ||
from tesk.constants import TeskConstants | ||
from tesk.exceptions import KubernetesError | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class CreateTesTask: | ||
"""Create TES task.""" | ||
|
||
# TODO: Add user to the class when auth implemented in FOCA | ||
def __init__(self, task: TesTask, namespace=TeskConstants.tesk_namespace): | ||
"""Initialize the CreateTask class. | ||
|
||
Args: | ||
task: TES task to create. | ||
namespace: Kubernetes namespace where the task is created. | ||
""" | ||
self.task = task | ||
# self.user = user | ||
self.kubernetes_client_wrapper = KubernetesClientWrapper() | ||
self.namespace = namespace | ||
self.tes_kubernetes_converter = TesKubernetesConverter(self.namespace) | ||
self.constants = Constants() | ||
|
||
def create_task(self): | ||
"""Create TES task.""" | ||
attempts_no = 0 | ||
while attempts_no < self.constants.job_create_attempts_no: | ||
try: | ||
attempts_no += 1 | ||
resources = self.task.resources | ||
|
||
if resources and resources.ram_gb: | ||
minimum_ram_gb = self.kubernetes_client_wrapper.minimum_ram_gb() | ||
if resources.ram_gb < minimum_ram_gb: | ||
self.task.resources.ram_gb = minimum_ram_gb | ||
|
||
task_master_job = ( | ||
self.tes_kubernetes_converter.from_tes_task_to_k8s_job( | ||
self.task, | ||
# self.user | ||
) | ||
) | ||
|
||
task_master_config_map = ( | ||
self.tes_kubernetes_converter.from_tes_task_to_k8s_config_map( | ||
self.task, | ||
task_master_job, | ||
# user | ||
) | ||
) | ||
_ = self.kubernetes_client_wrapper.create_config_map( | ||
task_master_config_map | ||
) | ||
created_job = self.kubernetes_client_wrapper.create_job(task_master_job) | ||
print(task_master_config_map) | ||
print(task_master_job) | ||
return created_job.metadata.name | ||
|
||
except KubernetesError as e: | ||
if ( | ||
not e.is_object_name_duplicated() | ||
or attempts_no >= self.constants.job_create_attempts_no | ||
): | ||
raise e | ||
|
||
except Exception as exc: | ||
logging.error("ERROR: In createTask", exc_info=True) | ||
raise exc | ||
|
||
def response(self) -> dict: | ||
"""Create response.""" | ||
return self.create_task() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
"""Kubernetes API module.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just trying to understand k8s flow, please ignore, maybe will remove later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine for me, but the rendered version does not look good to me:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle it's nice to have such a schema in the docs.
I am, however, wondering about the FTP secret. Maybe this will become clearer to me after looking at the code below, but seeing it represented like this in the doc, I have the concern that FTP is somehow treated special, when, ideally, storage solutions should all be treated in an abstract manner, like previously discussed: abstract storage handler and individual implementations for different storage/file transfer solutions. And that should probably extend to managing secrets/credentials as well, no?