From b2291c5825c61b769e438db6f5083b06165e72e6 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sat, 4 Jul 2020 22:03:10 +0200
Subject: snmp: vrf: T2682: support restart on failure indefinitely.

Linux tries to bind sshd to the VRF but it is yet not ready - for any arbitrary
reason. After restarting SSH to often (rate-limiting) it is blocked by systemd.

Using Restart/RestartSec is not enough - systemd services use start rate
limiting (enabled by default). If service is started more than StartLimitBurst
times in StartLimitIntervalSec seconds is it not permitted to start any more.
Parameters are inherited from DefaultStartLimitIntervalSec (default 10s) and
DefaultStartLimitBurst (default 5).
---
 data/templates/snmp/override.conf.tmpl | 14 +++++++++-----
 src/conf_mode/snmp.py                  | 21 ++++++++++++++-------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/data/templates/snmp/override.conf.tmpl b/data/templates/snmp/override.conf.tmpl
index 1eb8f20a9..e6302a9e1 100644
--- a/data/templates/snmp/override.conf.tmpl
+++ b/data/templates/snmp/override.conf.tmpl
@@ -1,9 +1,13 @@
+{% set vrf_command = '/sbin/ip vrf exec ' + vrf + ' ' if vrf is defined else '' %}
+[Unit]
+StartLimitIntervalSec=0
+After=vyos-router.service
+
 [Service]
 Environment=
 Environment="MIBSDIR=/usr/share/snmp/mibs:/usr/share/snmp/mibs/iana:/usr/share/snmp/mibs/ietf:/usr/share/mibs/site:/usr/share/snmp/mibs:/usr/share/mibs/iana:/usr/share/mibs/ietf:/usr/share/mibs/netsnmp"
 ExecStart=
-{% if vrf %}
-ExecStart=/sbin/ip vrf exec {{ vrf }} /usr/sbin/snmpd -LS0-5d -Lf /dev/null -u Debian-snmp -g Debian-snmp -I -ipCidrRouteTable,inetCidrRouteTable -f -p /run/snmpd.pid
-{% else %}
-ExecStart=/usr/sbin/snmpd -LS0-5d -Lf /dev/null -u Debian-snmp -g Debian-snmp -I -ipCidrRouteTable,inetCidrRouteTable -f -p /run/snmpd.pid
-{% endif %}
+ExecStart={{vrf_command}}/usr/sbin/snmpd -LS0-5d -Lf /dev/null -u Debian-snmp -g Debian-snmp -I -ipCidrRouteTable,inetCidrRouteTable -f -p /run/snmpd.pid
+Restart=on-failure
+RestartSec=10
+
diff --git a/src/conf_mode/snmp.py b/src/conf_mode/snmp.py
index eb0d20654..bafd26edc 100755
--- a/src/conf_mode/snmp.py
+++ b/src/conf_mode/snmp.py
@@ -22,6 +22,7 @@ from time import sleep
 from sys import exit
 
 from vyos.config import Config
+from vyos.configverify import verify_vrf
 from vyos.validate import is_ipv4, is_addr_assigned
 from vyos.version import get_version_data
 from vyos import ConfigError
@@ -67,8 +68,7 @@ default_config_data = {
     'v3_traps': [],
     'v3_users': [],
     'v3_views': [],
-    'script_ext': [],
-    'vrf': ''
+    'script_ext': []
 }
 
 def rmfile(file):
@@ -191,6 +191,9 @@ def get_config():
             snmp['script_ext'].append(extension)
 
     if conf.exists('vrf'):
+        # Append key to dict but don't place it in the default dictionary.
+        # This is required to make the override.conf.tmpl work until we
+        # migrate to get_config_dict().
         snmp['vrf'] = conf.return_value('vrf')
 
 
@@ -416,8 +419,7 @@ def verify(snmp):
         else:
             print('WARNING: SNMP listen address {0} not configured!'.format(addr))
 
-    if snmp['vrf'] and snmp['vrf'] not in interfaces():
-        raise ConfigError('VRF "{vrf}" does not exist'.format(**snmp))
+    verify_vrf(snmp)
 
     # bail out early if SNMP v3 is not configured
     if not snmp['v3_enabled']:
@@ -550,15 +552,20 @@ def apply(snmp):
     # start SNMP daemon
     call("systemctl restart snmpd.service")
 
-    while (call('systemctl -q is-active snmpd.service') != 0):
-        print("service not yet started")
-        sleep(0.5)
+    if 'vrf' not in snmp.keys():
+        # service will be restarted multiple times later on
+        while (call('systemctl -q is-active snmpd.service') != 0):
+            sleep(0.5)
 
     # net-snmp is now regenerating the configuration file in the background
     # thus we need to re-open and re-read the file as the content changed.
     # After that we can no read the encrypted password from the config and
     # replace the CLI plaintext password with its encrypted version.
     os.environ["vyos_libexec_dir"] = "/usr/libexec/vyos"
+
+    # XXX: actually this whole logic makes less sense - why not calculate the
+    # password hashed on our own and write them back into the config? I see
+    # no valid reason in waiting for a third party process to do so.
     with open(config_file_user, 'r') as f:
         engineID = ''
         for line in f:
-- 
cgit v1.2.3