-
Notifications
You must be signed in to change notification settings - Fork 241
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
Add support for disk resource isolation #241
Conversation
disk = self.get_disk() | ||
if disk is not None: | ||
if not isinstance(disk, float) and not isinstance(disk, int): | ||
return False, 'The specified disk value "%s" is not a valid float.' % disk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't valid - the value can be a float or an int.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional allows for floats and ints. It is only the error message that is specific to floats. This code is based on the check above for memory. I can fix up both of the messages to be a bit more clear.
can you also update the docs for yelpsoa-configs? |
This looks really comprehensive! Fix docs and ship i say. |
disk = self.get_disk() | ||
if disk is not None: | ||
if not isinstance(disk, float) and not isinstance(disk, int): | ||
return False, 'The specified disk value "%s" is not a valid float or int.' % disk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isinstance can be passed a tuple for its second argument so this could just be isinstance(cpus, (float, int))
Ditto for line 202 and 209
Add support for disk resource isolation
@@ -83,6 +85,11 @@ def check_thresholds(percent): | |||
over_threshold = True | |||
else: | |||
output += "OK: CPU usage is under %d%%. (Currently at %f%%)\n" % cpu_print_tuple | |||
if current_disk >= percent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add blank lines between each if/else in here? These blocks kind of run into each other.
This is a first pass at adding this functionality. I went through trying to find all of the places we deal with cpu/mem and adjusted the code to also deal with disk space. I'm sure I've missed a spot or two in the code, but all of the tests are currently passing. I wanted to get this pull request tossed up so that I can get some feedback.