From 4a8a6522276e3a0b815b8133af0f471531a9bea2 Mon Sep 17 00:00:00 2001 From: Andrew Bezzubtsev Date: Thu, 25 Jun 2026 10:50:58 +0300 Subject: [PATCH] fix(hostcfgd): correct `mgmtVrfEnabled` retrieval from nested `vrf_global` table The `MgmtIfaceCfg.load()` method was incorrectly reading `mgmtVrfEnabled` directly from the top level of the `mgmt_vrf` dict, but the actual data structure from ConfigDB is a full table dict keyed by row name (e.g., `{'vrf_global': {'mgmtVrfEnabled': 'true'}}`). This caused the internal cache to always be initialized to `''` (empty string), `making update_mgmt_vrf()` detect a state change on `config load` and unnecessarily restart the interfaces-config service. The issue was discovered during repetitive execution of `config load -y`, which caused unexpected SSH session teardowns as the networking stack was being reconfigured unnecessarily. Change the retrieval to access the nested `vrf_global` row following the `sonic-mgmt_vrf` YANG model leaf path: `/sonic-mgmt_vrf:sonic-mgmt_vrf/MGMT_VRF_CONFIG/vrf_global/mgmtVrfEnabled` Signed-off-by: Andrew Bezzubtsev --- scripts/hostcfgd | 2 +- tests/hostcfgd/hostcfgd_test.py | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts/hostcfgd b/scripts/hostcfgd index 9b67ba41..bb7faea4 100644 --- a/scripts/hostcfgd +++ b/scripts/hostcfgd @@ -1617,7 +1617,7 @@ class MgmtIfaceCfg(object): def load(self, mgmt_iface={}, mgmt_vrf={}): # Get initial data self.iface_config_data = mgmt_iface - self.mgmt_vrf_enabled = mgmt_vrf.get('mgmtVrfEnabled', '') + self.mgmt_vrf_enabled = mgmt_vrf.get('vrf_global', {}).get('mgmtVrfEnabled', '') syslog.syslog(syslog.LOG_DEBUG, f'Initial mgmt interface conf: {self.iface_config_data}') syslog.syslog(syslog.LOG_DEBUG, diff --git a/tests/hostcfgd/hostcfgd_test.py b/tests/hostcfgd/hostcfgd_test.py index 3b21d962..0e1af7f6 100644 --- a/tests/hostcfgd/hostcfgd_test.py +++ b/tests/hostcfgd/hostcfgd_test.py @@ -415,9 +415,30 @@ def test_mgmtiface_event(self): ] mocked_check_output.assert_has_calls(expected) + def test_mgmtvrf_no_restart_on_same_state(self): + """ + Verify that load() correctly reads mgmtVrfEnabled from the nested + 'vrf_global' row so that a subsequent update_mgmt_vrf() call with the + same value does NOT trigger an interfaces-config restart. + + This is a regression test for the bug where load() used + mgmt_vrf.get('mgmtVrfEnabled') instead of + mgmt_vrf.get('vrf_global', {}).get('mgmtVrfEnabled'), causing the + cache to always be initialised to '' and every config load to + incorrectly detect a state change and restart interfaces-config. + """ + mgmtiface = hostcfgd.MgmtIfaceCfg() + mgmtiface.load({}, {'vrf_global': {'mgmtVrfEnabled': 'true'}}) + assert mgmtiface.mgmt_vrf_enabled == 'true' + + with mock.patch('hostcfgd.subprocess') as mocked_subprocess: + mocked_subprocess.CalledProcessError = CalledProcessError + mgmtiface.update_mgmt_vrf({'mgmtVrfEnabled': 'true'}) + mocked_subprocess.check_call.assert_not_called() + def test_mgmtvrf_route_check_failed(self): mgmtiface = hostcfgd.MgmtIfaceCfg() - mgmtiface.load({}, {'mgmtVrfEnabled' : "false"}) + mgmtiface.load({}, {'vrf_global': {'mgmtVrfEnabled': "false"}}) with mock.patch('hostcfgd.check_output_pipe') as mocked_check_output: with mock.patch('hostcfgd.subprocess') as mocked_subprocess: popen_mock = mock.Mock()