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

feature: backwards compatible Kubernetes list and kill #2023

Merged
merged 6 commits into from
Sep 16, 2024

Conversation

saikonen
Copy link
Collaborator

@saikonen saikonen commented Sep 10, 2024

supersedes #1998 as the first attempt, as this approach is backwards compatible.

closes #1631

caveat
this approach iterates through all active pods in a namespace indiscriminately (with a limit of 1000 per request), only filtering on the client side for relevant ones. As a result, in deployments with enormous amounts of active pods, the operation can take quite long.

savingoyal
savingoyal previously approved these changes Sep 11, 2024
Copy link
Collaborator

@savingoyal savingoyal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! just need to enhance the killing logic so that it works internally.

metaflow/plugins/kubernetes/kubernetes_cli.py Outdated Show resolved Hide resolved
metaflow/plugins/kubernetes/kubernetes_cli.py Outdated Show resolved Hide resolved
metaflow/plugins/kubernetes/kubernetes_cli.py Outdated Show resolved Hide resolved
metaflow/plugins/kubernetes/kubernetes_client.py Outdated Show resolved Hide resolved
metaflow/plugins/kubernetes/kubernetes_client.py Outdated Show resolved Hide resolved
* add fallbacks to kube kill

* block killing pods managed by argo-workflows
# Metaflows Kubernetes job names are of the format
# t-{uid}-{k8s_job_generate_name_suffix}-{k8s_pod_generate_name_suffix}
# so we can infer the job name from the pod name by dropping the last suffix
job_name = "-".join(pod.metadata.name.split("-")[:-1])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we get this from pod's owner reference instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems so, or upon further inspection its also part of the pod labels as well so I'll go with that as the simplest source

@savingoyal savingoyal merged commit a287fd6 into master Sep 16, 2024
26 checks passed
@savingoyal savingoyal deleted the feature/backwards-compatible-kubernetes-kill branch September 16, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add list and kill support for kubernetes CLI
2 participants