keepalived: T1350: Replaced transition scripts logic

In old implementation, all notify scripts are configured directly inside a keepalived.conf. This way is not recommended even by keepalived as scripts execution is not tracked and they may be running not in order and killed before the actual work will be finished. We have observed such situations in very rare cases during tests.
New implementation use FIFO pipe, to which keepalived send any state changes. And these notifications are read by a daemon, saved in a queue and processed one by one, which eliminates the situation when the scripts creating inconsistent config or environment.

1 files changed, 25 insertions, 22 deletions

diff --git a/src/conf_mode/vrrp.py b/src/conf_mode/vrrp.py
index a09e55a2f..b17f1ce82 100755
--- a/src/conf_mode/vrrp.py
+++ b/src/conf_mode/vrrp.py
@@ -19,16 +19,18 @@ import os
 import sys
 import subprocess
 import ipaddress
-
+import json
 import jinja2
 
 import vyos.config
 import vyos.keepalived
 
 from vyos import ConfigError
+from pathlib import Path
 
 daemon_file = "/etc/default/keepalived"
 config_file = "/etc/keepalived/keepalived.conf"
+config_dict_path = "/run/keepalived_config.dict"
 
 config_tmpl = """
 # Autogenerated by VyOS
@@ -38,16 +40,18 @@ config_tmpl = """
 global_defs {
     dynamic_interfaces
     script_user root
+    notify_fifo /run/keepalived_notify_fifo
+    notify_fifo_script /usr/libexec/vyos/system/keepalived-fifo.py
 }
 
 {% for group in groups -%}
 
 {% if group.health_check_script -%}
 vrrp_script healthcheck_{{ group.name }} {
-	script "{{ group.health_check_script }}"
-	interval {{ group.health_check_interval }}
-	fall {{ group.health_check_count }}
-	rise 1
+    script "{{ group.health_check_script }}"
+    interval {{ group.health_check_interval }}
+    fall {{ group.health_check_count }}
+    rise 1
 
 }
 {% endif %}
@@ -106,22 +110,6 @@ vrrp_instance {{ group.name }} {
         healthcheck_{{ group.name }}
     }
     {% endif -%}
-
-    {% if group.master_script -%}
-        notify_master "/usr/libexec/vyos/system/vrrp-script-wrapper.py --state master --group {{ group.name }} --interface {{ group.interface }} {{ group.master_script }}"
-    {% endif -%}
-
-    {% if group.backup_script -%}
-        notify_backup "/usr/libexec/vyos/system/vrrp-script-wrapper.py --state backup --group {{ group.name }} --interface {{ group.interface }} {{ group.backup_script }}"
-    {% endif -%}
-
-    {% if group.fault_script -%}
-        notify_fault "/usr/libexec/vyos/system/vrrp-script-wrapper.py --state fault --group {{ group.name }} --interface {{ group.interface }} {{ group.fault_script }}"
-    {% endif -%}
-
-    {% if group.stop_script -%}
-        notify_stop "/usr/libexec/vyos/system/vrrp-script-wrapper.py --state stop --group {{ group.name }} --interface {{ group.interface }} {{ group.stop_script }}"
-    {% endif -%}
 }
 
 {% endfor -%}
@@ -153,6 +141,7 @@ daemon_tmpl = """
 DAEMON_ARGS="--snmp"
 """
 
+
 def get_config():
     vrrp_groups = []
     sync_groups = []
@@ -240,8 +229,13 @@ def get_config():
 
         sync_groups.append(sync_group)
 
+    # create a file with dict with proposed configuration
+    with open("{}.temp".format(config_dict_path), 'w') as dict_file:
+        dict_file.write(json.dumps({'vrrp_groups': vrrp_groups, 'sync_groups': sync_groups}))
+
     return (vrrp_groups, sync_groups)
 
+
 def verify(data):
     vrrp_groups, sync_groups = data
 
@@ -308,6 +302,7 @@ def verify(data):
             if not (m in vrrp_group_names):
                 raise ConfigError("VRRP sync-group {0} refers to VRRP group {1}, but group {1} does not exist".format(sync_group["name"], m))
 
+
 def generate(data):
     vrrp_groups, sync_groups = data
 
@@ -318,7 +313,7 @@ def generate(data):
             if g["disable"]:
                 print("Warning: ignoring disabled VRRP group {0} in sync-group {1}".format(g["name"], sync_group["name"]))
     # Filter out disabled groups
-    vrrp_groups = list(filter(lambda x: x["disable"] != True, vrrp_groups))
+    vrrp_groups = list(filter(lambda x: x["disable"] is not True, vrrp_groups))
 
     tmpl = jinja2.Template(config_tmpl)
     config_text = tmpl.render({"groups": vrrp_groups, "sync_groups": sync_groups})
@@ -330,9 +325,17 @@ def generate(data):
 
     return None
 
+
 def apply(data):
     vrrp_groups, sync_groups = data
     if vrrp_groups:
+        # safely rename a temporary file with configuration dict
+        try:
+            dict_file = Path("{}.temp".format(config_dict_path))
+            dict_file.rename(Path(config_dict_path))
+        except Exception as err:
+            print("Unable to rename the file with keepalived config for FIFO pipe: {}".format(err))
+
         if not vyos.keepalived.vrrp_running():
             print("Starting the VRRP process")
             ret = subprocess.call("sudo systemctl restart keepalived.service", shell=True)