Merge pull request #5151 from natali-rs1985/T8293

T8293: Add ability to set timeout for high-availability health-check

4 files changed, 50 insertions, 0 deletions

diff --git a/data/templates/high-availability/keepalived.conf.j2 b/data/templates/high-availability/keepalived.conf.j2
index f0ed7909c..a45befdb6 100644
--- a/data/templates/high-availability/keepalived.conf.j2
+++ b/data/templates/high-availability/keepalived.conf.j2
@@ -47,6 +47,9 @@ vrrp_script healthcheck_sg_{{ name }} {
     script "/usr/bin/ping -c1 {{ sync_group_config.health_check.ping }}"
 {%             endif %}
     interval {{ sync_group_config.health_check.interval }}
+{%             if sync_group_config.health_check.timeout is vyos_defined %}
+    timeout {{ sync_group_config.health_check.timeout }}
+{%             endif %}
     fall {{ sync_group_config.health_check.failure_count }}
     rise 1
 }
@@ -64,6 +67,9 @@ vrrp_script healthcheck_{{ name }} {
     script "/usr/bin/ping -c1 {{ group_config.health_check.ping }}"
 {%             endif %}
     interval {{ group_config.health_check.interval }}
+{%             if group_config.health_check.timeout is vyos_defined %}
+    timeout {{ group_config.health_check.timeout }}
+{%             endif %}
     fall {{ group_config.health_check.failure_count }}
     rise 1
 }
diff --git a/interface-definitions/high-availability.xml.in b/interface-definitions/high-availability.xml.in
index bb132e337..70ac308a3 100644
--- a/interface-definitions/high-availability.xml.in
+++ b/interface-definitions/high-availability.xml.in
@@ -170,6 +170,18 @@
                       </constraint>
                     </properties>
                   </leafNode>
+                  <leafNode name="timeout">
+                    <properties>
+                      <help>Health check script timeout in seconds</help>
+                      <valueHelp>
+                        <format>u32</format>
+                        <description>Timeout in seconds</description>
+                      </valueHelp>
+                      <constraint>
+                        <validator name="numeric" argument="--positive"/>
+                      </constraint>
+                    </properties>
+                  </leafNode>
                 </children>
               </node>
               <leafNode name="hello-source-address">
@@ -393,6 +405,18 @@
                       </constraint>
                     </properties>
                   </leafNode>
+                  <leafNode name="timeout">
+                    <properties>
+                      <help>Health check script timeout in seconds</help>
+                      <valueHelp>
+                        <format>u32</format>
+                        <description>Timeout in seconds</description>
+                      </valueHelp>
+                      <constraint>
+                        <validator name="numeric" argument="--positive"/>
+                      </constraint>
+                    </properties>
+                  </leafNode>
                 </children>
               </node>
               #include <include/vrrp-transition-script.xml.i>
diff --git a/smoketest/scripts/cli/test_high-availability_vrrp.py b/smoketest/scripts/cli/test_high-availability_vrrp.py
index f1f1c7df2..93ef9a09b 100755
--- a/smoketest/scripts/cli/test_high-availability_vrrp.py
+++ b/smoketest/scripts/cli/test_high-availability_vrrp.py
@@ -279,6 +279,7 @@ class TestVRRP(VyOSUnitTestSHIM.TestCase):
 
     def test_check_health_script(self):
         sync_group = 'VyOS'
+        timeout = '100'
 
         for group in groups:
             vlan_id = group.lstrip('VLAN')
@@ -329,6 +330,16 @@ class TestVRRP(VyOSUnitTestSHIM.TestCase):
         config = getConfig(f'vrrp_sync_group {sync_group}')
         self.assertIn(f'track_script', config)
 
+        self.cli_set(
+            base_path
+            + ['vrrp', 'sync-group', sync_group, 'health-check', 'timeout', timeout]
+        )
+        # commit changes
+        self.cli_commit()
+
+        config = getConfig(f'vrrp_script healthcheck_sg_{sync_group}')
+        self.assertIn(f'timeout {timeout}', config)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())
diff --git a/src/conf_mode/high-availability.py b/src/conf_mode/high-availability.py
index 48a7d8abc..175929547 100755
--- a/src/conf_mode/high-availability.py
+++ b/src/conf_mode/high-availability.py
@@ -188,6 +188,15 @@ def _validate_health_check(group, group_config):
         # to avoid generating useless config statements in keepalived.conf
         del group_config["health_check"]
 
+    if 'timeout' in group_config.get('health_check', {}):
+        interval = int(group_config['health_check']['interval'])
+        timeout = int(group_config['health_check']['timeout'])
+        if timeout < interval:
+            Warning(
+                f'Health check timeout ({timeout}s) is less than interval ({interval}s) '
+                f'for VRRP group "{group}", script may be killed before completion'
+            )
+
 
 def generate(ha):
     if not ha or 'disable' in ha: