Skip to content

Conversation

@haykking
Copy link
Collaborator

@haykking haykking commented Aug 1, 2025

New variables responsible for Limits and Requests were created and changed.
A new Limits section was created in the Kubernetes manifest.

@anksena
Copy link
Contributor

anksena commented Aug 6, 2025

Please add verification steps for the benchmark

CPUS_PER_POD = flags.DEFINE_string(
'kubernetes_scale_pod_cpus', '250m', 'CPU limit per pod'
REQUEST_CPUS_PER_POD = flags.DEFINE_string(
'kubernetes_scale_min_pod_cpus', '250m', 'Min CPU limit per pod'
Copy link
Collaborator

Choose a reason for hiding this comment

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

No real need to change the flag name. It's somewhat inconvenient on my end as I then need to modify the scheduling code we have to change this as well. Updating the description is sufficient documentation.

MEMORY_PER_POD = flags.DEFINE_string(
'kubernetes_scale_pod_memory', '250M', 'Memory limit per pod'
LIMIT_CPUS_PER_POD = flags.DEFINE_string(
'kubernetes_scale_max_pod_cpus', '250m', 'Max CPU limit per pod'
Copy link
Collaborator

Choose a reason for hiding this comment

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

But I think we also chatted offline about just needing one number for both limits & requests, so will wait on the larger modification to give next review.

@haykking
Copy link
Collaborator Author

haykking commented Aug 7, 2025

I returned the old names to the variables
Now I use only one variable for Requests
As a result, the changes were very small after all :)

@hubatish
Copy link
Collaborator

hubatish commented Aug 8, 2025

Fair enough, very small indeed but all that's needed. I'll approve & we can check back in with Rich when he's back as well.

@hubatish
Copy link
Collaborator

Per our chat conversation with Rich, this actually does not make sense right? Should it be reverted?

@hubatish hubatish requested a review from rsgowman August 18, 2025 16:49
@haykking
Copy link
Collaborator Author

After chat conversation with Rich, we have two options:

  1. Leave it as it was, with limits
  2. Make both limits and requests in the manifest. And so that they are always equal to the same value (more preferable as Rich said)

I can do it as we decide is more convenient.

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.

4 participants