-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[memory_checker] Do not check memory usage of containers which are not created #11129
Changes from all commits
add2fbf
28a1071
6eb68bd
298949d
c40919f
76737e3
0c7f104
7ff329a
766b0e9
8b38168
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,8 @@ import sys | |
import syslog | ||
import re | ||
|
||
import docker | ||
|
||
|
||
def get_command_result(command): | ||
"""Executes the command and return the resulting output. | ||
|
@@ -86,14 +88,36 @@ def check_memory_usage(container_name, threshold_value): | |
print("[{}]: Memory usage ({} Bytes) is larger than the threshold ({} Bytes)!" | ||
.format(container_name, mem_usage_bytes, threshold_value)) | ||
syslog.syslog(syslog.LOG_INFO, "[{}]: Memory usage ({} Bytes) is larger than the threshold ({} Bytes)!" | ||
.format(container_name, mem_usage_bytes, threshold_value)) | ||
.format(container_name, mem_usage_bytes, threshold_value)) | ||
sys.exit(3) | ||
else: | ||
syslog.syslog(syslog.LOG_ERR, "[memory_checker] Failed to retrieve memory value from '{}'" | ||
.format(mem_usage)) | ||
sys.exit(4) | ||
|
||
|
||
def get_running_container_names(): | ||
"""Retrieves names of running containers by talking to the docker daemon. | ||
|
||
Args: | ||
None. | ||
|
||
Returns: | ||
running_container_names: A list indicates names of running containers. | ||
""" | ||
try: | ||
docker_client = docker.DockerClient(base_url='unix://var/run/docker.sock') | ||
running_container_list = docker_client.containers.list(filters={"status": "running"}) | ||
running_container_names = [ container.name for container in running_container_list ] | ||
except (docker.errors.APIError, docker.errors.DockerException) as err: | ||
syslog.syslog(syslog.LOG_ERR, | ||
"Failed to retrieve the running container list from docker daemon! Error message is: '{}'" | ||
.format(err)) | ||
sys.exit(5) | ||
|
||
return running_container_names | ||
|
||
|
||
def main(): | ||
parser = argparse.ArgumentParser(description="Check memory usage of a container \ | ||
and an alerting message will be written into syslog if memory usage \ | ||
|
@@ -104,7 +128,13 @@ def main(): | |
parser.add_argument("threshold_value", type=int, help="threshold value in bytes") | ||
args = parser.parse_args() | ||
|
||
check_memory_usage(args.container_name, args.threshold_value) | ||
running_container_names = get_running_container_names() | ||
if args.container_name in running_container_names: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Working on the test case and will post the PR link at here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sonic-mgmt test case is posted: sonic-net/sonic-mgmt#5823. If you are available, can you please help me review? |
||
check_memory_usage(args.container_name, args.threshold_value) | ||
else: | ||
syslog.syslog(syslog.LOG_INFO, | ||
"[memory_checker] Exits without checking memory usage since container '{}' is not running!" | ||
.format(args.container_name)) | ||
|
||
|
||
if __name__ == "__main__": | ||
|
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.
@yozhao101 i would suggest to rewrite like this:
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.
Updated.