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

[MM-56184] Allow passing Kubernetes resource requirements to jobs #49

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

streamer45
Copy link
Contributor

Summary

We want to allow setting resource limits and requests for jobs we manage so that it's easier for Kubernetes to schedule them properly based on available nodes resources.

I kept the implementation simple to avoid redefining a bunch of K8S types. The downside is that we need to pass some potentially ugly JSON but the alternative seemed just a lot of repetition and potentially less powerful overall.

This is how it would look like to define specific limits and requests for both jobs we support today from a helm chart:

- name: JOBS_KUBERNETES_JOBSRESOURCEREQUIREMENTS
  value: "{\"transcribing\":{\"limits\":{\"cpu\":\"4000m\"},\"requests\":{\"cpu\":\"2000m\"}},\"recording\":{\"limits\":{\"cpu\":\"2000m\"},\"requests\":{\"cpu\":\"1000m\"}}}"

Again, thanks @gabrieljackson for the tip :)

Ticket Link

https://mattermost.atlassian.net/browse/MM-56184

@streamer45 streamer45 added the 2: Dev Review Requires review by a core committer label Dec 22, 2023
@streamer45 streamer45 self-assigned this Dec 22, 2023
@streamer45 streamer45 added this to the v0.6.0 milestone Dec 22, 2023
Copy link

@stylianosrigas stylianosrigas left a comment

Choose a reason for hiding this comment

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

Nice one, LGTM

Copy link
Member

@cpoile cpoile left a comment

Choose a reason for hiding this comment

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

Seems like json is the way to go 👍

@streamer45 streamer45 added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Jan 9, 2024
@streamer45 streamer45 merged commit 94edfd5 into master Jan 22, 2024
3 checks passed
@streamer45 streamer45 deleted the MM-56184 branch January 22, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants