openvpn: T9363: multiply the site-to-site keepalive timeout

"failure-count" counts missed intervals, so the timeout it describes is
interval * failure-count. Server mode renders and verifies it that way;
site-to-site passed the bare count to "ping-restart", where it lands as
seconds - 60s instead of 600s with the defaults.

Render the product. That puts a timeout past the 86400 "ping-restart"
accepts within reach, so verify() rejects it and the 5-to-6 migration
clamps configurations already beyond it.

This changes behaviour on upgrade: site-to-site tunnels now tolerate a
longer silence, 600 seconds instead of 60 with the defaults.

1 files changed, 28 insertions, 2 deletions

diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index 922695ff2..7c7c3f69e 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -1381,7 +1381,8 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
         self.assertNotIn('ping-restart', directives)
         self.assertServiceRunning(interface)
 
-        # an enabled keepalive still renders both
+        # an enabled keepalive renders both, and the timeout is the interval
+        # times the number of failures it tolerates
         self.cli_set(path + ['keep-alive', 'interval', '10'])
         self.cli_set(path + ['keep-alive', 'failure-count', '60'])
         self.cli_commit()
@@ -1391,9 +1392,34 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
         config = read_file(f'/run/openvpn/{interface}.conf')
         lines = config.splitlines()
         self.assertIn('ping 10', lines)
-        self.assertIn('ping-restart 60', lines)
+        self.assertIn('ping-restart 600', lines)
         self.assertServiceRunning(interface)
 
+        # check validate() - OpenVPN caps ping-restart at 24 hours, and
+        # nothing doubles the value here the way it does for the server
+        self.cli_set(path + ['keep-alive', 'interval', '600'])
+        self.cli_set(path + ['keep-alive', 'failure-count', '145'])
+        with self.assertRaisesRegex(ConfigSessionError, r'cannot\s+exceed\s+86400'):
+            self.cli_commit()
+
+        # 600 * 144 is exactly 24 hours, which OpenVPN still accepts
+        self.cli_set(path + ['keep-alive', 'failure-count', '144'])
+        self.cli_commit()
+
+        config = read_file(f'/run/openvpn/{interface}.conf')
+        self.assertIn('ping-restart 86400', config.splitlines())
+        self.assertServiceRunning(interface)
+
+        # a failure-count below 2 is deliberately allowed here, unlike the
+        # server: it renders "ping-restart 10", a working "restart after one
+        # missed ping"
+        self.cli_set(path + ['keep-alive', 'interval', '10'])
+        self.cli_set(path + ['keep-alive', 'failure-count', '1'])
+        self.cli_commit()
+
+        config = read_file(f'/run/openvpn/{interface}.conf')
+        self.assertIn('ping-restart 10', config.splitlines())
+
         self.cli_delete(base_path)
         self.cli_commit()