Refactor the cpu limit check - #4373
Conversation
sidtuladhar
changed the title
| config_file_path = os.path.join( | ||
| soa_dir, | ||
| service, | ||
| f"{instance_config.get_instance_type()}-{cluster}.yaml", | ||
| ) |
There was a problem hiding this comment.
@mbrankin-art has a nice helper for this in his validation PR, but if this gets merged first, we can always switch this to the new helper later :p
| if instance_config.get_instance_type() not in ("kubernetes", "eks"): | ||
| continue |
There was a problem hiding this comment.
i think similar to mbrankin's PR: we might want to use PaastaServiceConfigLoader so that we can quickly filter and just load the configs we care about
| returncode = False | ||
| print( | ||
| failure( | ||
| f"{service}.{instance} in {cluster} has {cpu} CPUs, which exceeds the limit of {pool_limits['max_cpus']} for the {pool} pool." |
There was a problem hiding this comment.
| f"{service}.{instance} in {cluster} has {cpu} CPUs, which exceeds the limit of {pool_limits['max_cpus']} for the {pool} pool." | |
| f"{service}.{instance} in {cluster} has {cpu} CPUs, which exceeds the recommended limit of {pool_limits['max_cpus']} for the {pool} pool." |
There was a problem hiding this comment.
and hmm, maybe we want to also include what the true max is so that folks don't try to do something like cpus: 512 during an incident to give something a ton of resources and get confused as to why stuff isn't running :p
There was a problem hiding this comment.
my concern for adding a true max is that we'd always need to have it updated with whatever the max we've set in the karpenter nodepool. In your experience, do engineers just use an unrealistic big number during DARs for mitigation? In most cases, if they do double the CPU + override comment should go to the fallback pool ig
2 participants
Refactor the cpu limit check:
cpuin yelpsoa to override