T5586: Disable by default SNMP for Keeplived VRRP service

AgentX does not work stable. From time to time we see the system
service crashing/degrading if something is wrong with SNMP from
util net-snmp.
We should disable it by default and enable it only if configured.

set high-availability vrrp snmp

3 files changed, 39 insertions, 4 deletions

diff --git a/data/templates/vrrp/10-override.conf.j2 b/data/templates/vrrp/10-override.conf.j2
new file mode 100644
index 000000000..4a16012b6
--- /dev/null
+++ b/data/templates/vrrp/10-override.conf.j2
@@ -0,0 +1,16 @@
+### Autogenerated by ${vyos_conf_scripts_dir}/vrrp.py ###
+{% set snmp = '--snmp' if snmp is defined else '' %}
+[Unit]
+After=vyos-router.service
+# Only start if there is our configuration file - remove Debian default
+# config file from the condition list
+ConditionFileNotEmpty=
+ConditionFileNotEmpty=/run/keepalived/keepalived.conf
+
+[Service]
+KillMode=process
+Type=simple
+# Read configuration variable file if it is present
+ExecStart=
+ExecStart=/usr/sbin/keepalived --use-file /run/keepalived/keepalived.conf --pid /run/keepalived/keepalived.pid --dont-fork {{ snmp }}
+PIDFile=/run/keepalived/keepalived.pid
diff --git a/interface-definitions/vrrp.xml.in b/interface-definitions/vrrp.xml.in
index 0684a7e4e..829845385 100644
--- a/interface-definitions/vrrp.xml.in
+++ b/interface-definitions/vrrp.xml.in
@@ -351,6 +351,12 @@
               </leafNode>
             </children>
           </tagNode>
+          <leafNode name="snmp">
+            <properties>
+              <valueless/>
+              <help>Enable SNMP</help>
+            </properties>
+          </leafNode>
           <tagNode name="sync-group">
             <properties>
               <help>VRRP sync group</help>
diff --git a/src/conf_mode/vrrp.py b/src/conf_mode/vrrp.py
index 86b11b6c4..b53294e64 100755
--- a/src/conf_mode/vrrp.py
+++ b/src/conf_mode/vrrp.py
@@ -24,6 +24,7 @@ from ipaddress import IPv6Interface
 
 from vyos.config import Config
 from vyos.configdict import dict_merge
+from vyos.configdict import leaf_node_changed
 from vyos.ifconfig.vrrp import VRRP
 from vyos.template import render
 from vyos.template import is_ipv4
@@ -36,6 +37,10 @@ from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
 
+
+systemd_override = r'/run/systemd/system/keepalived.service.d/10-override.conf'
+
+
 def get_config(config=None):
     if config:
         conf = config
@@ -60,6 +65,9 @@ def get_config(config=None):
     if conf.exists(conntrack_path):
         vrrp['conntrack_sync_group'] = conf.return_value(conntrack_path)
 
+    if leaf_node_changed(conf, base + ['snmp']):
+        vrrp.update({'restart_required': {}})
+
     return vrrp
 
 def verify(vrrp):
@@ -138,13 +146,17 @@ def verify(vrrp):
 
 def generate(vrrp):
     if not vrrp:
+        if os.path.isfile(systemd_override):
+            os.unlink(systemd_override)
         return None
 
     render(VRRP.location['config'], 'vrrp/keepalived.conf.tmpl', vrrp)
+    render(systemd_override, 'vrrp/10-override.conf.j2', vrrp)
     return None
 
 def apply(vrrp):
     service_name = 'keepalived.service'
+    call('systemctl daemon-reload')
     if not vrrp:
         call(f'systemctl stop {service_name}')
         return None
@@ -163,10 +175,11 @@ def apply(vrrp):
 
     # XXX: T3944 - reload keepalived configuration if service is already running
     # to not cause any service disruption when applying changes.
-    if is_systemd_service_running(service_name):
-        call(f'systemctl reload {service_name}')
-    else:
-        call(f'systemctl restart {service_name}')
+    systemd_action = 'reload-or-restart'
+    if 'restart_required' in vrrp:
+        systemd_action = 'restart'
+
+    call(f'systemctl {systemd_action} {service_name}')
     return None
 
 if __name__ == '__main__':