[hostcfgd] Fix the state machine during eth0 default route check failure (#196)

The last del command in the following flow will fail because of a bug in MgmtIfaceCfg. During vrf add, it tries to update the default route and if the route is not present, the command will write an error log

MgmtIfaceCfg is not saving the saving the state in the internal variable. This will cause problem next time and update comes.

config vrf add mgmt
config vrf del mgmt
config vrf add mgmt
config vrf del mgmt
Added a UT and verified the fix

Dec 18 03:57:43 sonic hostcfgd[98362]: MgmtIfaceCfg: mgmt vrf state update
Dec 18 03:57:43 sonic hostcfgd[98362]: Set mgmt vrf state true
Dec 18 03:57:50 sonic hostcfgd[98362]: MgmtIfaceCfg: Could not delete eth0 route
Dec 18 03:58:05 sonic hostcfgd[98362]: MgmtIfaceCfg: mgmt vrf state update
Dec 18 03:58:05 sonic hostcfgd[98362]: Set mgmt vrf state false

Author: vivekrnv

scripts/hostcfgd

@@ -125,7 +125,7 @@ def run_cmd_pipe(cmd0, cmd1, cmd2, log_err=True, raise_exception=False):
         check_output_pipe(cmd0, cmd1, cmd2)
     except Exception as err:
         if log_err:
-            syslog.syslog(syslog.LOG_ERR, "{} - failed: return code - {}, output:\n{}"
+            syslog.syslog(syslog.LOG_WARNING, "{} - failed: return code - {}, output:\n{}"
                   .format(err.cmd, err.returncode, err.output))
         if raise_exception:
             raise
@@ -1559,6 +1559,9 @@ class MgmtIfaceCfg(object):
                           'services')
             return
 
+        # Update cache
+        self.mgmt_vrf_enabled = enabled
+
         # Remove mgmt if route
         if enabled == 'true':
             """
@@ -1577,16 +1580,12 @@ class MgmtIfaceCfg(object):
                 cmd2 = ['wc', '-l']
                 run_cmd_pipe(cmd0, cmd1, cmd2, True, True)
             except subprocess.CalledProcessError:
-                syslog.syslog(syslog.LOG_ERR, 'MgmtIfaceCfg: Could not delete '
+                syslog.syslog(syslog.LOG_WARNING, 'MgmtIfaceCfg: Could not delete '
                                               'eth0 route')
                 return
 
             run_cmd(["ip", "-4", "route", "del", "default", "dev", "eth0", "metric", "202"], False)
 
-        # Update cache
-        self.mgmt_vrf_enabled = enabled
-
-
 class RSyslogCfg(object):
     """Remote syslog config daemon
 

tests/hostcfgd/hostcfgd_test.py

@@ -4,6 +4,7 @@
 import signal
 import psutil
 import swsscommon as swsscommon_package
+from subprocess import CalledProcessError
 from sonic_py_common import device_info
 from swsscommon import swsscommon
 from parameterized import parameterized
@@ -324,6 +325,29 @@ def test_mgmtiface_event(self):
                 ]
                 mocked_check_output.assert_has_calls(expected)
 
+    def test_mgmtvrf_route_check_failed(self):
+        mgmtiface = hostcfgd.MgmtIfaceCfg()
+        mgmtiface.load({}, {'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()
+                attrs = {'communicate.return_value': ('output', 'error')}
+                popen_mock.configure_mock(**attrs)
+                mocked_subprocess.Popen.return_value = popen_mock
+                mocked_subprocess.CalledProcessError = CalledProcessError
+                # Simulate the case where there is no default route
+                mocked_check_output.side_effect = CalledProcessError(returncode=1, cmd="", output="")
+
+                mgmtiface.update_mgmt_vrf({'mgmtVrfEnabled' : "true"})
+                expected = [
+                    call(['cat', '/proc/net/route'], ['grep', '-E', r"eth0\s+00000000\s+[0-9A-Z]+\s+[0-9]+\s+[0-9]+\s+[0-9]+\s+202"], ['wc', '-l'])
+                ]
+                mocked_check_output.assert_has_calls(expected)
+                assert mgmtiface.mgmt_vrf_enabled == "true"
+
+                mgmtiface.update_mgmt_vrf({'mgmtVrfEnabled' : "false"})
+                assert mgmtiface.mgmt_vrf_enabled == "false"
+
     def test_dns_events(self):
         MockConfigDb.set_config_db(HOSTCFG_DAEMON_CFG_DB)
         MockConfigDb.event_queue = [('DNS_NAMESERVER', '1.1.1.1')]