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

vadymhlushko-mlnx · 2023-08-02T13:07:47Z

Why I did it

To fix the logic introduced by [memory_checker] Do not check memory usage of containers which are not created #11129.
There could be a scenario before the reboot, where

The docker service has stopped
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.

Work item tracking

Microsoft ADO (number only):

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:

Change the Unix socket path to non-existing in /usr/bin/memory_checker file on the switch.
Execute the root@sonic:/home/admin# monit restart container_memory_telemetry
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!

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

vadymhlushko-mlnx · 2023-08-08T07:27:05Z

@yozhao101 could you please take a look at this PR?

vadymhlushko-mlnx · 2023-08-16T08:41:04Z

@yozhao101 kind reminder

liat-grozovik · 2023-08-16T08:44:51Z

@qiluo-msft @yozhao101 could you please help to review? this is a fix following a PR merged/reviewed by you so are the optimal reviewer for this one

qiluo-msft · 2023-08-20T00:03:05Z

files/image_config/monit/memory_checker

@@ -140,10 +140,10 @@ def get_running_container_names():
        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,


LOG_ERR

Is it possible to check first docker service is running, if Failed to retrieve the running container list, treat it still as ERR? #Closed

That is the idea of get_running_container_names(), if the docker service is not running it just gives us a warning message to the syslog and returns the empty list of docker containers.

To clarify:

check docker service is running, if not running, no ERR, no WARNING. Low frequent INFO may be good.

if docker service is running, retrieve the running container list. If failed to retrieve, treat it still as ERR.

We already have the check if the docker service is running, but in some cases (which I described in the PR description) is not enough.
This check is right before the get_running_container_names() function call (161 line number).

Thanks for the explanation! Let me update my proposal:

when you catch an exception, if you are sure docker service is not running, log INFO in low frequency

if not a docker service issue, something really bad happens, let's still use ERR.

The implementation is aligned according to your last comment, please take a look

@qiluo-msft please take a look

files/image_config/monit/memory_checker

… service is not running. Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>

… 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! ```

mssonicbld · 2023-09-03T03:21:47Z

Cherry-pick PR to 202305: #16405

… 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! ```

mssonicbld · 2023-10-10T06:33:11Z

Cherry-pick PR to 202211: #16821

mssonicbld · 2023-10-13T22:33:26Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-14T22:33:26Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-15T22:33:39Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-16T22:33:31Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-17T01:31:53Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-17T22:33:50Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-18T22:33:58Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

mssonicbld · 2023-10-19T22:33:56Z

@vadymhlushko-mlnx cherry pick PR didn't pass PR checker. Please check!!!
#16821

… 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! ```

vadymhlushko-mlnx requested a review from lguohan as a code owner August 2, 2023 13:07

vadymhlushko-mlnx requested a review from yozhao101 August 2, 2023 13:08

nazariig approved these changes Aug 2, 2023

View reviewed changes

volodymyrsamotiy added Request for 202205 Branch Request for 202211 Branch Request for 202305 Branch labels Aug 4, 2023

qiluo-msft reviewed Aug 20, 2023

View reviewed changes

vadymhlushko-mlnx force-pushed the master-memory-checker-docker-log-severity branch from bdbe756 to 8e70bf0 Compare August 28, 2023 13:34

nazariig reviewed Aug 28, 2023

View reviewed changes

files/image_config/monit/memory_checker Outdated Show resolved Hide resolved

[memory_checker] Add a specific log message in a case when the docker…

e0df3f4

… service is not running. Signed-off-by: vadymhlushko-mlnx <vadymh@nvidia.com>

vadymhlushko-mlnx force-pushed the master-memory-checker-docker-log-severity branch from 8e70bf0 to e0df3f4 Compare August 30, 2023 15:36

nazariig approved these changes Aug 30, 2023

View reviewed changes

qiluo-msft approved these changes Aug 31, 2023

View reviewed changes

qiluo-msft merged commit 43340cd into sonic-net:master Aug 31, 2023
17 checks passed

StormLiangMS added the Approved for 202305 Branch label Sep 3, 2023

mssonicbld added the Created PR to 202305 Branch label Sep 3, 2023

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 added Included in 202305 Branch and removed Created PR to 202305 Branch labels Sep 3, 2023

StormLiangMS added the Approved for 202211 Branch label Oct 10, 2023

mssonicbld added the Created PR to 202211 Branch label Oct 10, 2023

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

mssonicbld added Included in 202211 Branch and removed Approved for 202211 Branch Created PR to 202211 Branch labels Oct 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

vadymhlushko-mlnx commented Aug 2, 2023 •

edited

Loading

vadymhlushko-mlnx commented Aug 8, 2023

vadymhlushko-mlnx commented Aug 16, 2023

liat-grozovik commented Aug 16, 2023

qiluo-msft Aug 20, 2023 •

edited

Loading

vadymhlushko-mlnx Aug 21, 2023 •

edited

Loading

qiluo-msft Aug 21, 2023

vadymhlushko-mlnx Aug 23, 2023 •

edited

Loading

qiluo-msft Aug 23, 2023

vadymhlushko-mlnx Aug 28, 2023

vadymhlushko-mlnx Aug 30, 2023

mssonicbld commented Sep 3, 2023

mssonicbld commented Oct 10, 2023

mssonicbld commented Oct 13, 2023

mssonicbld commented Oct 14, 2023

mssonicbld commented Oct 15, 2023

mssonicbld commented Oct 16, 2023

mssonicbld commented Oct 17, 2023

mssonicbld commented Oct 17, 2023

mssonicbld commented Oct 18, 2023

mssonicbld commented Oct 19, 2023

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

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

Conversation

vadymhlushko-mlnx commented Aug 2, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

vadymhlushko-mlnx commented Aug 8, 2023

vadymhlushko-mlnx commented Aug 16, 2023

liat-grozovik commented Aug 16, 2023

qiluo-msft Aug 20, 2023 • edited Loading

Choose a reason for hiding this comment

vadymhlushko-mlnx Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Aug 21, 2023

Choose a reason for hiding this comment

vadymhlushko-mlnx Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Aug 23, 2023

Choose a reason for hiding this comment

vadymhlushko-mlnx Aug 28, 2023

Choose a reason for hiding this comment

vadymhlushko-mlnx Aug 30, 2023

Choose a reason for hiding this comment

mssonicbld commented Sep 3, 2023

mssonicbld commented Oct 10, 2023

mssonicbld commented Oct 13, 2023

mssonicbld commented Oct 14, 2023

mssonicbld commented Oct 15, 2023

mssonicbld commented Oct 16, 2023

mssonicbld commented Oct 17, 2023

mssonicbld commented Oct 17, 2023

mssonicbld commented Oct 18, 2023

mssonicbld commented Oct 19, 2023

vadymhlushko-mlnx commented Aug 2, 2023 •

edited

Loading

qiluo-msft Aug 20, 2023 •

edited

Loading

vadymhlushko-mlnx Aug 21, 2023 •

edited

Loading

vadymhlushko-mlnx Aug 23, 2023 •

edited

Loading