Skip to content

Commit e830491

Browse files
authored
[system-health] When disabling a feature the SYSTEM_READY|SYSTEM_STATE was not updated (#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
1 parent 6852fcd commit e830491

File tree

3 files changed

+34
-0
lines changed

3 files changed

+34
-0
lines changed

src/system-health/health_checker/sysmonitor.py

+6
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,12 @@ def check_unit_status(self, event):
406406
if event in self.dnsrvs_name:
407407
self.dnsrvs_name.remove(event)
408408

409+
if len(self.dnsrvs_name) == 0:
410+
astate = "UP"
411+
else:
412+
astate = "DOWN"
413+
self.publish_system_status(astate)
414+
409415
srv_name,last = event.split('.')
410416
# stop on service maybe propagated to timers and in that case,
411417
# the state_db entry for the service should not be deleted

src/system-health/tests/mock_connector.py

+3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ def keys(self, db_id, pattern):
2323
def get_all(self, db_id, key):
2424
return MockConnector.data[key]
2525

26+
def exists(self, db_id, key):
27+
return key in MockConnector.data
28+
2629
def set(self, db_id, key, field, value):
2730
self.data[key] = {}
2831
self.data[key][field] = value

src/system-health/tests/test_system_health.py

+25
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,31 @@ def test_check_unit_status():
667667
assert 'mock_bgp.service' in sysmon.dnsrvs_name
668668

669669

670+
@patch('health_checker.sysmonitor.Sysmonitor.get_all_service_list', MagicMock(side_effect=[
671+
['mock_snmp.service', 'mock_bgp.service', 'mock_ns.service'],
672+
['mock_snmp.service', 'mock_ns.service']
673+
]))
674+
@patch('health_checker.sysmonitor.Sysmonitor.run_systemctl_show', MagicMock(return_value=mock_srv_props['mock_bgp.service']))
675+
@patch('health_checker.sysmonitor.Sysmonitor.get_app_ready_status', MagicMock(return_value=('Down','-','-')))
676+
@patch('health_checker.sysmonitor.Sysmonitor.post_unit_status', MagicMock())
677+
@patch('health_checker.sysmonitor.Sysmonitor.print_console_message', MagicMock())
678+
def test_system_status_up_after_service_removed():
679+
sysmon = Sysmonitor()
680+
sysmon.publish_system_status('UP')
681+
682+
sysmon.check_unit_status('mock_bgp.service')
683+
assert 'mock_bgp.service' in sysmon.dnsrvs_name
684+
result = swsscommon.SonicV2Connector.get(MockConnector, 0, "SYSTEM_READY|SYSTEM_STATE", 'Status')
685+
print("system status result before service was removed from system: {}".format(result))
686+
assert result == "DOWN"
687+
688+
sysmon.check_unit_status('mock_bgp.service')
689+
assert 'mock_bgp.service' not in sysmon.dnsrvs_name
690+
result = swsscommon.SonicV2Connector.get(MockConnector, 0, "SYSTEM_READY|SYSTEM_STATE", 'Status')
691+
print("system status result after service was removed from system: {}".format(result))
692+
assert result == "UP"
693+
694+
670695
@patch('health_checker.sysmonitor.Sysmonitor.get_all_service_list', MagicMock(return_value=['mock_snmp.service']))
671696
def test_check_unit_status_timer():
672697
sysmon = Sysmonitor()

0 commit comments

Comments
 (0)