refactor kubernetes providers: each block in kube refers to a pod #1073
Conversation
| @@ -65,7 +65,7 @@ def __init__(self, | |||
| max_blocks=10, | |||
| parallelism=1, | |||
| worker_init="", | |||
| deployment_name=None, | |||
| pod_name=None, | |||
| @@ -161,9 +159,10 @@ def cancel(self, job_ids): | |||
| for job in job_ids: | |||
| logger.debug("Terminating job/proc_id: {0}".format(job)) | |||
| # Here we are assuming that for local, the job_ids are the process id's | |||
| self._delete_deployment(job) | |||
There was a problem hiding this comment.
Maybe remove _delete_deployment from the source file?
| formatted_cmd = template_string.format(command=cmd_string, | ||
| worker_init=self.worker_init) | ||
|
|
||
| self.deployment_obj = self._create_deployment_object(job_name, |
There was a problem hiding this comment.
_create_deployment and _create_deployment_object are dead code that should be removed maybe now.
parsl/providers/kubernetes/kube.py
Outdated
| port=80, | ||
| cmd_string=None, | ||
| volumes=[]): | ||
| """ Create a kubernetes non-deployment pod for the job. |
There was a problem hiding this comment.
Refer to this as this a "pod", not a "non-deployment pod".
parsl/providers/kubernetes/kube.py
Outdated
| for volume in volumes: | ||
| volume_mounts.append(client.V1VolumeMount(mount_path=volume[1], | ||
| name=volume[0])) | ||
| # Configureate Pod template container |
parsl/providers/kubernetes/kube.py
Outdated
| name=volume[0])) | ||
| # Configureate Pod template container | ||
| container = None | ||
| if security_context: |
There was a problem hiding this comment.
I think the only purpose of this is to supply or not supply the security_context parameter. It looks (from the generated python code of V1Container in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/models/v1_container.py) that it is fine to pass in None for security_context and so this if-statement could collapse into a single code path.
|
This looks ok but I'd like someone to confirm they've definitely run this code and seen it scale correctly. |
@benclifford I have tested this on petrelkube with DLHub. This includes the following tests.
|
fix #853.
Now kubernetes provider creates non-deployment pods for each executor, rather than deployments.
The scaling of pods will be managed by Parsl.
Refer the details to the discussions in #853