Merge pull request #5008 from alexandr-san4ez/T8297-current

vpp: T8297: Fixed double enabling of VPP

3 files changed, 61 insertions, 16 deletions

diff --git a/python/vyos/vpp/configdb.py b/python/vyos/vpp/configdb.py
index 13c1cd3a0..e86b32fb7 100644
--- a/python/vyos/vpp/configdb.py
+++ b/python/vyos/vpp/configdb.py
@@ -34,6 +34,10 @@ class JSONStorage:
         """
         # If a name is not provided, this is a temporary one-time storage
         # this use case is strange, but let's allow this
+
+        # Indicates that the object has already been closed and cannot be used again
+        self.__closed = False
+
         if not name:
             self.__temporary = True
             self.__cache: dict[Any, Any] = {}
@@ -68,17 +72,34 @@ class JSONStorage:
             self.__cache = self.__load_file()
 
     def __del__(self) -> None:
+        self.close()
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *exc):
+        self.close()
+
+    def close(self):
         """Dump data to persistent storage and unlock it"""
-        if self.__temporary:
+
+        # Do not do anything if it is already closed storage
+        if self.__closed:
             return
-        # dump a cache to storage
-        if self.__cache:
-            self.__dump_file()
-        # or remove a file
-        else:
-            self.__storage.unlink()
-        # unlock a storage
-        self.__lock_file.unlink()
+
+        try:
+            if self.__temporary:
+                return
+            # dump a cache to storage
+            if self.__cache:
+                self.__dump_file()
+            # or remove a file
+            else:
+                self.__storage.unlink(missing_ok=True)
+            # unlock a storage
+            self.__lock_file.unlink(missing_ok=True)
+        finally:
+            self.__closed = True
 
     def __check_types(self, data: Any) -> None:
         """Check if all the data have supported types
diff --git a/smoketest/scripts/cli/test_vpp.py b/smoketest/scripts/cli/test_vpp.py
index a7bb19882..88c146022 100755
--- a/smoketest/scripts/cli/test_vpp.py
+++ b/smoketest/scripts/cli/test_vpp.py
@@ -1730,6 +1730,29 @@ class TestVPP(VyOSUnitTestSHIM.TestCase):
         _, out = rc_cmd('sudo vppctl show flowprobe feature')
         self.assertIn(required_str, out)
 
+    def test_22_double_enabling_vpp(self):
+        # Verify double enabling of VPP
+
+        # Delete already defined settings from 'setUp' method
+        self.cli_delete(base_path)
+
+        # First commit changes
+        self.cli_set(base_path + ['settings', 'interface', interface])
+        self.cli_set(base_path + ['settings', 'poll-sleep-usec', '20'])
+        self.cli_commit()
+
+        # Delete all VPP changes
+        self.cli_delete(base_path)
+        self.cli_commit()
+
+        # Second commit changes
+        self.cli_set(base_path + ['settings', 'interface', interface])
+        self.cli_set(base_path + ['settings', 'poll-sleep-usec', '30'])
+        self.cli_commit()
+
+        # Ensure that VPP process is active
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())
diff --git a/src/conf_mode/vpp.py b/src/conf_mode/vpp.py
index b02c809ec..90356e6f4 100755
--- a/src/conf_mode/vpp.py
+++ b/src/conf_mode/vpp.py
@@ -231,10 +231,10 @@ def get_config(config=None):
     # this is required because some interfaces after they are connected
     # to VPP is really hard or impossible to restore without knowing
     # their original parameters (like IDs)
-    persist_config = JSONStorage('vpp_conf')
-    eth_ifaces_persist: dict[str, dict[str, str]] = persist_config.read(
-        'eth_ifaces', {}
-    )
+    with JSONStorage('vpp_conf') as persist_config:
+        eth_ifaces_persist: dict[str, dict[str, str]] = persist_config.read(
+            'eth_ifaces', {}
+        )
 
     if config:
         conf = config
@@ -692,10 +692,10 @@ def apply(config):
     modules = ('vfio_iommu_type1', 'vfio_pci', 'vfio_pci_core', 'vfio')
     # Open persistent config
     # It is required for operations with interfaces
-    persist_config = JSONStorage('vpp_conf')
     if not config or ('removed_ifaces' in config and 'settings' not in config):
         # Cleanup persistent config
-        persist_config.delete()
+        with JSONStorage('vpp_conf') as persist_config:
+            persist_config.delete()
         # And stop the service
         call(f'systemctl stop {service_name}.service')
         # Unlod modules (modprobe -r)
@@ -869,7 +869,8 @@ def apply(config):
 
     # Save persistent config
     if 'persist_config' in config and config['persist_config']:
-        persist_config.write('eth_ifaces', config['persist_config'])
+        with JSONStorage('vpp_conf') as persist_config:
+            persist_config.write('eth_ifaces', config['persist_config'])
 
     # reinitialize interfaces, but not during the first boot
     if boot_configuration_complete():