[System ready] Extend sysmonitor functionality to wait for host daemons #18817

fastiuk · 2024-04-28T00:09:44Z

Why I did it

To extend sysmonitor to report services states according to HLD: sonic-net/SONiC#1363

How I did it

Add timeout monitor task to sysmonitor to declare system ready state forcibly when timout happened.
sysmonitor now has a state machine, so the status now doesn't get back to "Not OK" after it was declared as "OK"
A new "irrel_for_sysready" configuration was added to "FEATURE" table to handle services for which sysmonitor shouldn't wait for.
Add "wait_services" field to platform health config file to explicitly specify services sysmonitor has to wait for.

How to verify it

Start NOS on the box
Make sure it defines system ready status
Make sure sysreadyshow shows all services as OK

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

Device metadata config

A picture of a cute animal (it is my cat Finn)

liushilongbuaa · 2024-04-28T10:06:11Z

/azpw ms_conflict

liushilongbuaa · 2024-04-28T10:08:15Z

@fastiuk , PR need to rebase master branch.

files/build_templates/docker_image_ctl.j2

fastiuk · 2024-05-16T16:16:00Z

files/build_templates/docker_image_ctl.j2

@@ -108,6 +108,37 @@ function preStartAction()
    fi
 {%- elif docker_container_name == "snmp" %}
    $SONIC_DB_CLI STATE_DB HSET 'DEVICE_METADATA|localhost' chassis_serial_number $(decode-syseeprom -s)
+


Are you enabling this feature by default for snmp? Shouldn't it be based on configuration?

@dgsudharsan It is disabled for snmp by default: see files/build_templates/init_cfg.json.j2

My question is different. In the init_cfg.j2 you are considering snmp as irrelevent for system ready but using system ready as a criteria to spawn snmp. Is this correct?

Can you consider handling this logic through featured? Today we have a similar flow in featured where certain daemons are spawned after portinitdone. In this case the critera can be systemd. It can be much easy to extend this to multiple daemons

My question is different. In the init_cfg.j2 you are considering snmp as irrelevent for system ready but using system ready as a criteria to spawn snmp. Is this correct?

Yes, it is indeed correct. Irrelevant for system ready means that system ready feature won't just collect readiness status of snmp systemd service. But SNMP must be started after system is ready. The logic actually does't break each other.

Can you consider handling this logic through featured? Today we have a similar flow in featured where certain daemons are spawned after portinitdone. In this case the critera can be systemd. It can be much easy to extend this to multiple daemons

I looked at the code in featured and I don't want to break anything there by adding wait for system ready. Most of the delayed services there depend on portinit done, and I don't want to add another wait point there as it will just make the logic there more complex and less clear.

files/build_templates/init_cfg.json.j2

fastiuk · 2024-05-16T16:20:13Z

files/scripts/syncd.sh

@@ -44,6 +44,8 @@ function startplatform() {
        /usr/bin/mlnx-fw-upgrade.sh -v
        if [[ "$?" -ne "${EXIT_SUCCESS}" ]]; then
            debug "Failed to upgrade fw. " "$?" "Restart syncd"
+            sonic-db-cli STATE_DB HSET "FEATURE|$DEV_SRV" fail_reason \


Why are we considering just the asic firmware update as sysready indication for syncd. Shouldn't it be the create switch success?

@dgsudharsan not only, we have PortInitDone event in swss and SFP ready in pmon.

My question is syncd up_status is just set base on startplatform completion. Is it possible that in the actual syncd flow, switch_create might fail and we will still see the up_status as true. Can we include the switch create in the definition of syncd up_status?

Yes, it can happen I think. Set system ready condition you see here is just an internal NV requirement, but at the same time it is very generic, so won't conflict with any other platform. I prefer to leave it as it is.

src/system-health/health_checker/sysmonitor.py

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

qiluo-msft · 2024-11-14T02:13:51Z

files/build_templates/init_cfg.json.j2

@@ -82,11 +82,17 @@
            "auto_restart": "{{autorestart}}",
            "support_syslog_rate_limit" : "true",
 {# Set check_up_status to true here when app readiness will be marked in state db  #}
+{%- if feature in ["swss", "syncd", "pmon"] %}
+            "check_up_status" : "true",


Please do not change image config generation behavior. We will rely on external service to generate golden config, and ask sonic to config reload -f golden_config.json to consume them.

@wen587 to review.

liat-grozovik · 2024-12-03T17:27:27Z

@fastiuk i hope that you can handle conflict and we will be able to merge it finally before branch out this week.

fastiuk requested review from qiluo-msft and lguohan as code owners April 28, 2024 00:09

fastiuk self-assigned this Apr 28, 2024

fastiuk mentioned this pull request Apr 28, 2024

Extend sysmonitor functionality to wait for host daemons sonic-net/SONiC#1363

Merged

fastiuk force-pushed the dev-sysready-extension branch from cac3ed8 to fa52f3f Compare May 3, 2024 13:47

fastiuk force-pushed the dev-sysready-extension branch from 93e5740 to fbaa10a Compare May 16, 2024 00:29

fastiuk commented May 16, 2024

View reviewed changes

fastiuk force-pushed the dev-sysready-extension branch from 11a3471 to 0dc3fde Compare May 16, 2024 16:37

fastiuk added 9 commits May 20, 2024 14:54

System-ready: Update YANG model

6e975c3

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Add YANG model tests

376a4d1

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Update configuration reference

e50e33c

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Extend sysmonitor to listen for daemons

bcd3883

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Report syncd ready status

592dcb7

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Update features' configuration

981243a

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: SNMP should wait for system ready event

85ca8a9

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

System-ready: Add sysmonitor tests

39d1e8d

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

🐞 sysmonitor: Skip error logging of inactive srvs

de110cf

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

fastiuk force-pushed the dev-sysready-extension branch from 0dc3fde to de110cf Compare May 20, 2024 12:00

fastiuk requested a review from prgeor May 20, 2024 20:38

Merge branch 'master' into dev-sysready-extension

c210287

fastiuk closed this Aug 15, 2024

fastiuk reopened this Aug 15, 2024

fastiuk added 3 commits August 18, 2024 00:18

System ready: Update overlap wait time to 1 minute

544dfa2

Signed-off-by: Yevhen Fastiuk <yfastiuk@nvidia.com>

Merge branch 'master' into dev-sysready-extension

7f21c07

Merge branch 'master' into dev-sysready-extension

8dc3b19

fastiuk closed this Nov 11, 2024

fastiuk reopened this Nov 11, 2024

Merge branch 'master' into dev-sysready-extension

a9b865e

qiluo-msft reviewed Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[System ready] Extend sysmonitor functionality to wait for host daemons #18817

[System ready] Extend sysmonitor functionality to wait for host daemons #18817

fastiuk commented Apr 28, 2024 •

edited

Loading

liushilongbuaa commented Apr 28, 2024

liushilongbuaa commented Apr 28, 2024

fastiuk May 16, 2024

dgsudharsan Jun 6, 2024

dgsudharsan Jun 6, 2024

fastiuk Aug 15, 2024

fastiuk Aug 15, 2024

fastiuk May 16, 2024

dgsudharsan Jun 6, 2024

fastiuk Aug 15, 2024

qiluo-msft Nov 14, 2024

qiluo-msft Nov 14, 2024

liat-grozovik commented Dec 3, 2024

[System ready] Extend sysmonitor functionality to wait for host daemons #18817

Are you sure you want to change the base?

[System ready] Extend sysmonitor functionality to wait for host daemons #18817

Conversation

fastiuk commented Apr 28, 2024 • edited Loading

Why I did it

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 (it is my cat Finn)

liushilongbuaa commented Apr 28, 2024

liushilongbuaa commented Apr 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liat-grozovik commented Dec 3, 2024

fastiuk commented Apr 28, 2024 •

edited

Loading