feature: Support choosing Kubernetes QoS class through the decorator #2155
Conversation
| # Guaranteed - has both cpu/memory limits. requests not required, as these will be inferred. | ||
| qos_limits = { | ||
| "cpu": str(cpu), | ||
| "memory": "%sM" % str(memory), |
There was a problem hiding this comment.
storage limit/request does not affect the QoS class from what I could tell, so I kept it out of this portion. Same withgpu requests. I can pre-emptively move these into the same function for introducing custom QoS classes, but for now it shouldn't have any effect.
should I make the change so ephemeral-storage and gpu requests/limits get the same treatment as cpu/memory?
There was a problem hiding this comment.
just ephemeral-storage. gpu's always need to specified with both requests and limits. also, can you verify airflow too?
There was a problem hiding this comment.
handling storage as well now. For the BestEffort case, I figure we keep the ephemeral-storage in the requests, or do we omit it completely?
| # Guaranteed - has both cpu/memory limits. requests not required, as these will be inferred. | ||
| qos_limits = { | ||
| "cpu": str(cpu), | ||
| "memory": "%sM" % str(memory), | ||
| "ephemeral-storage": "%sM" % str(storage), | ||
| } | ||
| elif qos == "BestEffort": | ||
| elif qos == "besteffort": | ||
| # BestEffort - no limit or requests for cpu/memory | ||
| qos_requests = {"ephemeral-storage": "%sM" % str(storage)} |
There was a problem hiding this comment.
should we even support best effort at the moment? there doesn't seem to be a use case for it at the moment
There was a problem hiding this comment.
removed this and left a comment on adding support once there is a proper use case.
|
lgtm! can you verify airflow works as expected? |
enables choosing the Kubernetes QoS class out of
Guaranteed/Burstable/BestEffortthrough the decorator.