[memory_checker] Do not check memory usage of containers which are not created #11129

yozhao101 · 2022-06-14T05:27:40Z

Signed-off-by: Yong Zhao yozhao@microsoft.com

Why I did it

This PR aims to fix an issue (#10088) by enhancing the script memory_checker.

Specifically, if container is not created successfully during device is booted/rebooted, then memory_checker do not need check its memory usage.

How I did it

In the script memory_checker, a function is added to get names of running containers. If the specified container name is not in current running container list, then this script will exit without checking its memory usage.

How to verify it

I tested on a lab device by following the steps:

Stops telemetry container with command sudo systemctl stop telemetry.service
Removes telemetry container with command docker rm telemetry

Checks whether the script memory_checker ran by Monit will generate the syslog message saying it will exit without checking memory usage of telemetry:

 Jun 14 15:42:08.580612 str-s6000-on-2 INFO /memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running!

Which release branch to backport (provide reason below if selected)

201811
[x ] 201911
[x ] 202006
[ x] 202012
[x ] 202106
[x ] 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

device is (re)booted, then exits without checking its memory usage. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

This reverts commit a477dbb.

This reverts commit 28a1071.

This reverts commit add2fbf.

booted/rebooted, then `memory_checker` will not check its memory usage. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 · 2022-06-14T15:44:01Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2022-06-14T15:44:13Z

Azure Pipelines successfully started running 1 pipeline(s).

list could not be retrieved. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 · 2022-06-15T17:15:33Z

@qiluo-msft Can you help me review this PR please?

qiluo-msft · 2022-06-15T18:32:14Z

files/image_config/monit/memory_checker

@@ -104,7 +131,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:


running_container_names

Add a sonic-mgmt test case? #Closed

Working on the test case and will post the PR link at here.

sonic-mgmt test case is posted: sonic-net/sonic-mgmt#5823. If you are available, can you please help me review?

nazariig · 2022-06-15T19:31:07Z

files/image_config/monit/memory_checker

+        running_container_names: A list indicates names of running containers.
+    """
+    running_container_names = []
+    docker_client = docker.DockerClient(base_url='unix://var/run/docker.sock')


@yozhao101 do we need handle the case when docker engine is not started? Let's say memory checker starts simultaneously with docker service

memory_checker script will be ran by Monit periodically and Monit is delayed to run this script after device is up 300 seconds.

Docker daemon will be started by systemd immediately once device is booted/rebooted.

If docker daemon crashed or is not started successfully, then memory_checker will exit with error code and log an error message into syslog as well.

@yozhao101 ok

…rted successfully. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 · 2022-06-15T20:11:40Z

@nazariig Can you please review again to see whether logging an error message is good way or not to address your question?

nazariig · 2022-06-16T09:07:12Z

files/image_config/monit/memory_checker

+        container_list = container_obj.list(filters={"status": "running"})
+        for container in container_list:
+            running_container_names.append(container.name)
+    except (docker.errors.APIError, docker.errors.DockerException) as err:


@yozhao101 i would suggest to rewrite like this:

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:

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

qiluo-msft

LGTM. Please wait for other reviewers.

yozhao101 · 2022-06-17T01:24:28Z

/AzurePipelines run Azure.sonic-buildimage

1.What is the motivation for this PR? This PR aims to add a new test case which will check whether memory_checker can log a message into syslog if a container is not created during device is booted/rebooted. The image PR is: sonic-net/sonic-buildimage#11129. 2.How did you do it? This test case has the following steps: - Removes a container explicitly from DuT - Leverages Loganalyzer to analyze whether the message from memory_checker appears in syslog - Restarts the corresponding container on DuT and do health check 3.How did you verify/test it? I verified this new test case on lab device str-s6000-on-2

…t created (#11129) Signed-off-by: Yong Zhao yozhao@microsoft.com Why I did it This PR aims to fix an issue (#10088) by enhancing the script memory_checker. Specifically, if container is not created successfully during device is booted/rebooted, then memory_checker do not need check its memory usage. How I did it In the script memory_checker, a function is added to get names of running containers. If the specified container name is not in current running container list, then this script will exit without checking its memory usage. How to verify it I tested on a lab device by following the steps: Stops telemetry container with command sudo systemctl stop telemetry.service Removes telemetry container with command docker rm telemetry Checks whether the script memory_checker ran by Monit will generate the syslog message saying it will exit without checking memory usage of telemetry.

1.What is the motivation for this PR? This PR aims to add a new test case which will check whether memory_checker can log a message into syslog if a container is not created during device is booted/rebooted. The image PR is: sonic-net/sonic-buildimage#11129. 2.How did you do it? This test case has the following steps: - Removes a container explicitly from DuT - Leverages Loganalyzer to analyze whether the message from memory_checker appears in syslog - Restarts the corresponding container on DuT and do health check 3.How did you verify/test it? I verified this new test case on lab device str-s6000-on-2

…t created (#11129) Signed-off-by: Yong Zhao yozhao@microsoft.com Why I did it This PR aims to fix an issue (#10088) by enhancing the script memory_checker. Specifically, if container is not created successfully during device is booted/rebooted, then memory_checker do not need check its memory usage. How I did it In the script memory_checker, a function is added to get names of running containers. If the specified container name is not in current running container list, then this script will exit without checking its memory usage. How to verify it I tested on a lab device by following the steps: Stops telemetry container with command sudo systemctl stop telemetry.service Removes telemetry container with command docker rm telemetry Checks whether the script memory_checker ran by Monit will generate the syslog message saying it will exit without checking memory usage of telemetry.

…emon is not running (#11476) Fix in Monit memory_checker plugin. Skip fetching running containers if docker engine is down (can happen in deinit). This PR fixes issue #11472. Signed-off-by: liora liora@nvidia.com Why I did it In the case where Monit runs during deinit flow, memory_checker plugin is fetching the running containers without checking if Docker service is still running. I added this check. How I did it Use systemctl is-active to check if Docker engine is still running. How to verify it Use systemctl to stop docker engine and reload Monit, no errors in log and relevant print appears in log. Which release branch to backport (provide reason below if selected) The fix is required in 202205 and 202012 since the PR that introduced the issue was cherry picked to those branches (#11129).

…emon is not running (sonic-net#11476) Fix in Monit memory_checker plugin. Skip fetching running containers if docker engine is down (can happen in deinit). This PR fixes issue sonic-net#11472. Signed-off-by: liora liora@nvidia.com Why I did it In the case where Monit runs during deinit flow, memory_checker plugin is fetching the running containers without checking if Docker service is still running. I added this check. How I did it Use systemctl is-active to check if Docker engine is still running. How to verify it Use systemctl to stop docker engine and reload Monit, no errors in log and relevant print appears in log. Which release branch to backport (provide reason below if selected) The fix is required in 202205 and 202012 since the PR that introduced the issue was cherry picked to those branches (sonic-net#11129).

… service is not running. (#16018) #### Why I did it To fix the logic introduced by [[memory_checker] Do not check memory usage of containers which are not created #11129](#11129). There could be a scenario before the reboot, where 1. The `docker service` has stopped 2. In a very short period of time, the monit service performs the `root@sonic:/home/admin# monit status container_memory_telemetry` In such scenario, the `memory_checker` script will throw an error to the syslog: ``` ERR memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))' ``` But, actually, this scenario is a correct behavior, because when the docker service is stopped, the Unix socket is destroyed and that is why we could see the `FileNotFoundError(2, 'No such file or directory'` exception in the syslog. #### How I did it Change the log severity to the warning and changed the return value. #### How to verify it It is really hard to catch the exact moment described in the `Why I did it` section. In order to check the logic: 1. Change the Unix socket path to non-existing in [/usr/bin/memory_checker](https://github.com/sonic-net/sonic-buildimage/blob/47742dfc2c0d1fa27198d69c9183ddc044e11b22/files/image_config/monit/memory_checker#L139) file on the switch. 2. Execute the `root@sonic:/home/admin# monit restart container_memory_telemetry` 3. Check the syslog for such messages: ``` WARNING memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborte d.', FileNotFoundError(2, 'No such file or directory'))' INFO memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running! ```

… service is not running. (sonic-net#16018) #### Why I did it To fix the logic introduced by [[memory_checker] Do not check memory usage of containers which are not created sonic-net#11129](sonic-net#11129). There could be a scenario before the reboot, where 1. The `docker service` has stopped 2. In a very short period of time, the monit service performs the `root@sonic:/home/admin# monit status container_memory_telemetry` In such scenario, the `memory_checker` script will throw an error to the syslog: ``` ERR memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))' ``` But, actually, this scenario is a correct behavior, because when the docker service is stopped, the Unix socket is destroyed and that is why we could see the `FileNotFoundError(2, 'No such file or directory'` exception in the syslog. #### How I did it Change the log severity to the warning and changed the return value. #### How to verify it It is really hard to catch the exact moment described in the `Why I did it` section. In order to check the logic: 1. Change the Unix socket path to non-existing in [/usr/bin/memory_checker](https://github.com/sonic-net/sonic-buildimage/blob/47742dfc2c0d1fa27198d69c9183ddc044e11b22/files/image_config/monit/memory_checker#L139) file on the switch. 2. Execute the `root@sonic:/home/admin# monit restart container_memory_telemetry` 3. Check the syslog for such messages: ``` WARNING memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborte d.', FileNotFoundError(2, 'No such file or directory'))' INFO memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running! ```

… service is not running. (#16018) #### Why I did it To fix the logic introduced by [[memory_checker] Do not check memory usage of containers which are not created #11129](#11129). There could be a scenario before the reboot, where 1. The `docker service` has stopped 2. In a very short period of time, the monit service performs the `root@sonic:/home/admin# monit status container_memory_telemetry` In such scenario, the `memory_checker` script will throw an error to the syslog: ``` ERR memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))' ``` But, actually, this scenario is a correct behavior, because when the docker service is stopped, the Unix socket is destroyed and that is why we could see the `FileNotFoundError(2, 'No such file or directory'` exception in the syslog. #### How I did it Change the log severity to the warning and changed the return value. #### How to verify it It is really hard to catch the exact moment described in the `Why I did it` section. In order to check the logic: 1. Change the Unix socket path to non-existing in [/usr/bin/memory_checker](https://github.com/sonic-net/sonic-buildimage/blob/47742dfc2c0d1fa27198d69c9183ddc044e11b22/files/image_config/monit/memory_checker#L139) file on the switch. 2. Execute the `root@sonic:/home/admin# monit restart container_memory_telemetry` 3. Check the syslog for such messages: ``` WARNING memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborte d.', FileNotFoundError(2, 'No such file or directory'))' INFO memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running! ```

… service is not running. (sonic-net#16018) #### Why I did it To fix the logic introduced by [[memory_checker] Do not check memory usage of containers which are not created sonic-net#11129](sonic-net#11129). There could be a scenario before the reboot, where 1. The `docker service` has stopped 2. In a very short period of time, the monit service performs the `root@sonic:/home/admin# monit status container_memory_telemetry` In such scenario, the `memory_checker` script will throw an error to the syslog: ``` ERR memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))' ``` But, actually, this scenario is a correct behavior, because when the docker service is stopped, the Unix socket is destroyed and that is why we could see the `FileNotFoundError(2, 'No such file or directory'` exception in the syslog. #### How I did it Change the log severity to the warning and changed the return value. #### How to verify it It is really hard to catch the exact moment described in the `Why I did it` section. In order to check the logic: 1. Change the Unix socket path to non-existing in [/usr/bin/memory_checker](https://github.com/sonic-net/sonic-buildimage/blob/47742dfc2c0d1fa27198d69c9183ddc044e11b22/files/image_config/monit/memory_checker#L139) file on the switch. 2. Execute the `root@sonic:/home/admin# monit restart container_memory_telemetry` 3. Check the syslog for such messages: ``` WARNING memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborte d.', FileNotFoundError(2, 'No such file or directory'))' INFO memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running! ```

… service is not running. (#16018) #### Why I did it To fix the logic introduced by [[memory_checker] Do not check memory usage of containers which are not created #11129](#11129). There could be a scenario before the reboot, where 1. The `docker service` has stopped 2. In a very short period of time, the monit service performs the `root@sonic:/home/admin# monit status container_memory_telemetry` In such scenario, the `memory_checker` script will throw an error to the syslog: ``` ERR memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))' ``` But, actually, this scenario is a correct behavior, because when the docker service is stopped, the Unix socket is destroyed and that is why we could see the `FileNotFoundError(2, 'No such file or directory'` exception in the syslog. #### How I did it Change the log severity to the warning and changed the return value. #### How to verify it It is really hard to catch the exact moment described in the `Why I did it` section. In order to check the logic: 1. Change the Unix socket path to non-existing in [/usr/bin/memory_checker](https://github.com/sonic-net/sonic-buildimage/blob/47742dfc2c0d1fa27198d69c9183ddc044e11b22/files/image_config/monit/memory_checker#L139) file on the switch. 2. Execute the `root@sonic:/home/admin# monit restart container_memory_telemetry` 3. Check the syslog for such messages: ``` WARNING memory_checker: Failed to retrieve the running container list from docker daemon! Error message is: 'Error while fetching server API version: ('Connection aborte d.', FileNotFoundError(2, 'No such file or directory'))' INFO memory_checker: [memory_checker] Exits without checking memory usage since container 'telemetry' is not running! ```

yozhao101 added 5 commits June 13, 2022 22:12

[memory_checker] If containers are not created during

add2fbf

device is (re)booted, then exits without checking its memory usage. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

Revert "Frr 8.2 upgrade (sonic-net#10691)"

28a1071

This reverts commit a477dbb.

Revert "Revert "Frr 8.2 upgrade (sonic-net#10691)""

6eb68bd

This reverts commit 28a1071.

Revert "[memory_checker] If containers are not created during"

298949d

This reverts commit add2fbf.

[memory_checker] If containers are not created during device is

c40919f

booted/rebooted, then `memory_checker` will not check its memory usage. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 requested a review from lguohan as a code owner June 14, 2022 05:27

yozhao101 marked this pull request as draft June 14, 2022 05:27

yozhao101 added Request for 202012 Branch Request for 202111 Branch For PRs being requested for 202111 branch Request for 202205 Branch Bug 🐛 labels Jun 14, 2022

yozhao101 added 2 commits June 15, 2022 09:44

[memory_checker] Write error message into syslog if running contianer

76737e3

list could not be retrieved. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

[memory_checker] Import docker module.

0c7f104

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 marked this pull request as ready for review June 15, 2022 16:59

yozhao101 requested a review from qiluo-msft June 15, 2022 16:59

qiluo-msft requested a review from nazariig June 15, 2022 18:31

qiluo-msft reviewed Jun 15, 2022

View reviewed changes

nazariig reviewed Jun 15, 2022

View reviewed changes

yozhao101 added 2 commits June 15, 2022 12:59

[memory_checker] Log an error message if the docker daemon is not sta…

7ff329a

…rted successfully. Signed-off-by: Yong Zhao <yozhao@microsoft.com>

[memory_checker] Fix a typo of exception name.

766b0e9

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

nazariig reviewed Jun 16, 2022

View reviewed changes

[memory_checker] Simplify and clean the implementation.

8b38168

Signed-off-by: Yong Zhao <yozhao@microsoft.com>

yozhao101 mentioned this pull request Jun 16, 2022

[memory_checker] Log syslog message if container was not created sonic-net/sonic-mgmt#5823

Merged

3 tasks

qiluo-msft reviewed Jun 16, 2022

View reviewed changes

nazariig approved these changes Jun 16, 2022

View reviewed changes

qiluo-msft approved these changes Jun 17, 2022

View reviewed changes

yozhao101 merged commit 241f445 into sonic-net:master Jun 17, 2022

yozhao101 deleted the enhancement_memory_checker branch June 17, 2022 19:13

yxieca added the Included in 202205 Branch label Jun 19, 2022

qiluo-msft added the Included in 202012 Branch label Jul 5, 2022

This was referenced Jul 18, 2022

[Monit] Fix in memory_checker, skip fetching running containers if Docker engine is not running liorghub/sonic-buildimage#30

Closed

[memory_checker] Do not check memory usage of containers if docker daemon is not running #11476

Merged

This was referenced Jul 26, 2023

[memory_checker] Change log severity to WARNING in a case when the docker service is not running. vadymhlushko-mlnx/sonic-buildimage#27

Closed

[memory_checker] Change log severity to WARNING in a case when the docker service is not running #16018

Merged

mssonicbld mentioned this pull request Sep 3, 2023

[action] [PR:16018] [memory_checker] Change log severity to WARNING in a case when the docker service is not running #16405

Merged

11 tasks

mssonicbld mentioned this pull request Oct 10, 2023

[action] [PR:16018] [memory_checker] Change log severity to WARNING in a case when the docker service is not running #16821

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[memory_checker] Do not check memory usage of containers which are not created #11129

[memory_checker] Do not check memory usage of containers which are not created #11129

yozhao101 commented Jun 14, 2022 •

edited

Loading

yozhao101 commented Jun 14, 2022

azure-pipelines bot commented Jun 14, 2022

yozhao101 commented Jun 15, 2022

qiluo-msft Jun 15, 2022 •

edited

Loading

yozhao101 Jun 15, 2022

yozhao101 Jun 16, 2022

nazariig Jun 15, 2022

yozhao101 Jun 15, 2022 •

edited

Loading

nazariig Jun 16, 2022

yozhao101 commented Jun 15, 2022

nazariig Jun 16, 2022

yozhao101 Jun 16, 2022

qiluo-msft left a comment

yozhao101 commented Jun 17, 2022

[memory_checker] Do not check memory usage of containers which are not created #11129

[memory_checker] Do not check memory usage of containers which are not created #11129

Conversation

yozhao101 commented Jun 14, 2022 • edited Loading

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

yozhao101 commented Jun 14, 2022

azure-pipelines bot commented Jun 14, 2022

yozhao101 commented Jun 15, 2022

qiluo-msft Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

yozhao101 Jun 15, 2022

Choose a reason for hiding this comment

yozhao101 Jun 16, 2022

Choose a reason for hiding this comment

nazariig Jun 15, 2022

Choose a reason for hiding this comment

yozhao101 Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

nazariig Jun 16, 2022

Choose a reason for hiding this comment

yozhao101 commented Jun 15, 2022

nazariig Jun 16, 2022

Choose a reason for hiding this comment

yozhao101 Jun 16, 2022

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

yozhao101 commented Jun 17, 2022

yozhao101 commented Jun 14, 2022 •

edited

Loading

qiluo-msft Jun 15, 2022 •

edited

Loading

yozhao101 Jun 15, 2022 •

edited

Loading