Skip to content

[system-health] When disabling a feature the SYSTEM_READY|SYSTEM_STAT…#14823

Merged
liat-grozovik merged 3 commits intosonic-net:masterfrom
DavidZagury:fix_feature_disable_issue
May 30, 2023
Merged

[system-health] When disabling a feature the SYSTEM_READY|SYSTEM_STAT…#14823
liat-grozovik merged 3 commits intosonic-net:masterfrom
DavidZagury:fix_feature_disable_issue

Conversation

@DavidZagury
Copy link
Contributor

@DavidZagury DavidZagury commented Apr 24, 2023

…E was not updated
Fix issue #14916

Why I did it

If you enable feature and then disable it, System Ready status change to Not Ready


root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay enabled 
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 
System is ready

Service-Name            Service-Status    App-Ready-Status    Down-Reason
----------------------  ----------------  ------------------  -------------
containerd              OK                OK                  -
dhcp_relay              OK                OK                  -
docker                  OK                OK                  -
...


root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay disabled 
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 
System is not ready - one or more services are not up

Service-Name            Service-Status    App-Ready-Status    Down-Reason
----------------------  ----------------  ------------------  -------------
containerd              OK                OK                  -
docker                  OK                OK                  -
...

A disabled feature should not affect the system ready status.

Work item tracking
  • Microsoft ADO (number only):

How I did it

During the disable flow of dhcp_relay, it entered the dnsrvs_name list, which caused the SYSTEM_STATE key to be set to DOWN. Right after that, the dhcp_relay service was removed from the full service list, however, but, when it was removed from the dnsrvs_name, there was no flow to reset the system state back to UP even though there was no more services in down state.

How to verify it

root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay enabled 
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 

root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay disabled
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 

Should see
System is ready

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

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)

@DavidZagury DavidZagury requested a review from lguohan as a code owner April 24, 2023 07:14
@DavidZagury
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@DavidZagury is there a GitHub issue which explain the issue and what cause it? if so, can you refer to it?
Please also tag the feature owner/contributor to provide feedback on the PR.

@DavidZagury
Copy link
Contributor Author

DavidZagury commented Apr 27, 2023

@DavidZagury is there a GitHub issue which explain the issue and what cause it? if so, can you refer to it? Please also tag the feature owner/contributor to provide feedback on the PR.

No there is no GitHub issue opened.

@sg893052 I see that you were the original author of this feature, will apricate your review

@liat-grozovik
Copy link
Collaborator

@DavidZagury can you please resolve conflicts?

@liat-grozovik liat-grozovik requested a review from qiluo-msft May 2, 2023 07:38
@DavidZagury DavidZagury force-pushed the fix_feature_disable_issue branch from 2ad7731 to da5737c Compare May 2, 2023 08:00
@DavidZagury DavidZagury requested a review from xumia as a code owner May 2, 2023 08:00
@DavidZagury DavidZagury force-pushed the fix_feature_disable_issue branch 2 times, most recently from 497e98f to 72a8807 Compare May 2, 2023 08:05
@DavidZagury
Copy link
Contributor Author

@liat-grozovik conflicts resolved

@gechiang gechiang requested a review from adyeung May 10, 2023 15:55
@liat-grozovik
Copy link
Collaborator

@DavidZagury please handle conflicts.

@DavidZagury DavidZagury marked this pull request as draft May 28, 2023 17:16
@DavidZagury DavidZagury force-pushed the fix_feature_disable_issue branch from 92d1690 to 72a8807 Compare May 28, 2023 17:23
@DavidZagury DavidZagury force-pushed the fix_feature_disable_issue branch from 72a8807 to 9592595 Compare May 28, 2023 17:32
@DavidZagury DavidZagury marked this pull request as ready for review May 28, 2023 17:32
@DavidZagury
Copy link
Contributor Author

@liat-grozovik conflicts handled

@liat-grozovik liat-grozovik merged commit e830491 into sonic-net:master May 30, 2023
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
…E was not updated (sonic-net#14823)

- Why I did it
If you enable feature and then disable it, System Ready status change to Not Ready

A disabled feature should not affect the system ready status.

- How I did it
During the disable flow of dhcp_relay, it entered the dnsrvs_name list, which caused the SYSTEM_STATE key to be set to DOWN. Right after that, the dhcp_relay service was removed from the full service list, however, but, when it was removed from the dnsrvs_name, there was no flow to reset the system state back to UP even though there was no more services in down state.

- How to verify it
root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay enabled 
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 

root@qa-eth-vt01-2-3700v:/home/admin# config feature state dhcp_relay disabled
root@qa-eth-vt01-2-3700v:/home/admin# show system-health sysready-status 

Should see
System is ready
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants