Merge pull request #3239 from nicolas-fort/T6068-sag

T6068: T6171: change <fail-over> node to <high-availability>; add <mode> parameter

6 files changed, 151 insertions, 26 deletions

diff --git a/data/templates/dhcp-server/dhcpd.conf.j2 b/data/templates/dhcp-server/dhcpd.conf.j2
index 639526532..d00837389 100644
--- a/data/templates/dhcp-server/dhcpd.conf.j2
+++ b/data/templates/dhcp-server/dhcpd.conf.j2
@@ -41,19 +41,20 @@ class "ubnt" {
 {%     endfor %}
 
 {% endif %}
-{% if failover is vyos_defined %}
-# DHCP failover configuration
-failover peer "{{ failover.name }}" {
-{%     if failover.status == 'primary' %}
+{% if high_availability is vyos_defined %}
+# DHCP HA configuration
+{%     set split_value = '256' if high_availability.mode == 'active-passive' else '128' %}
+failover peer "{{ high_availability.name }}" {
+{%     if high_availability.status == 'primary' %}
     primary;
     mclt 1800;
-    split 128;
-{%     elif failover.status == 'secondary' %}
+    split {{ split_value }};
+{%     elif high_availability.status == 'secondary' %}
     secondary;
 {%     endif %}
-    address {{ failover.source_address }};
+    address {{ high_availability.source_address }};
     port 647;
-    peer address {{ failover.remote }};
+    peer address {{ high_availability.remote }};
     peer port 647;
     max-response-delay 30;
     max-unacked-updates 10;
@@ -215,7 +216,7 @@ shared-network {{ network }} {
         pool {
 {%                 endif %}
 {%                 if subnet_config.enable_failover is vyos_defined %}
-            failover peer "{{ failover.name }}";
+            failover peer "{{ high_availability.name }}";
             deny dynamic bootp clients;
 {%                 endif %}
 {%                 if subnet_config.range is vyos_defined %}
diff --git a/interface-definitions/include/version/dhcp-server-version.xml.i b/interface-definitions/include/version/dhcp-server-version.xml.i
index 7c4b5633e..cc84ea8b9 100644
--- a/interface-definitions/include/version/dhcp-server-version.xml.i
+++ b/interface-definitions/include/version/dhcp-server-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/dhcp-server-version.xml.i -->
-<syntaxVersion component='dhcp-server' version='7'></syntaxVersion>
+<syntaxVersion component='dhcp-server' version='8'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/service_dhcp-server.xml.in b/interface-definitions/service_dhcp-server.xml.in
index a3c48afca..495d1a00b 100644
--- a/interface-definitions/service_dhcp-server.xml.in
+++ b/interface-definitions/service_dhcp-server.xml.in
@@ -16,12 +16,33 @@
               <valueless/>
             </properties>
           </leafNode>
-          <node name="failover">
+          <node name="high-availability">
             <properties>
-              <help>DHCP failover configuration</help>
+              <help>DHCP high availability configuration</help>
             </properties>
             <children>
               #include <include/source-address-ipv4.xml.i>
+              <leafNode name="mode">
+                <properties>
+                  <help>Configure high availability mode</help>
+                  <completionHelp>
+                    <list>active-active active-passive</list>
+                  </completionHelp>
+                  <valueHelp>
+                    <format>active-active</format>
+                    <description>Both server attend DHCP requests</description>
+                  </valueHelp>
+                  <valueHelp>
+                    <format>active-passive</format>
+                    <description>Only primary server attends DHCP requests</description>
+                  </valueHelp>
+                  <constraint>
+                    <regex>(active-active|active-passive)</regex>
+                  </constraint>
+                  <constraintErrorMessage>Invalid DHCP high availability mode</constraintErrorMessage>
+                </properties>
+                <defaultValue>active-active</defaultValue>
+              </leafNode>
               <leafNode name="remote">
                 <properties>
                   <help>IPv4 remote address used for connectio</help>
diff --git a/smoketest/scripts/cli/test_service_dhcp-server.py b/smoketest/scripts/cli/test_service_dhcp-server.py
index 3d1f47ece..b962cc636 100755
--- a/smoketest/scripts/cli/test_service_dhcp-server.py
+++ b/smoketest/scripts/cli/test_service_dhcp-server.py
@@ -439,7 +439,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         # Check for running process
         self.assertTrue(process_named_running(PROCESS_NAME))
 
-    def test_dhcp_failover(self):
+    def test_dhcp_high_availability(self):
         shared_net_name = 'FAILOVER'
         failover_name = 'VyOS-Failover'
 
@@ -449,10 +449,6 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         pool = base_path + ['shared-network-name', shared_net_name, 'subnet', subnet]
         # we use the first subnet IP address as default gateway
         self.cli_set(pool + ['default-router', router])
-
-        # check validate() - No DHCP address range or active static-mapping set
-        with self.assertRaises(ConfigSessionError):
-            self.cli_commit()
         self.cli_set(pool + ['range', '0', 'start', range_0_start])
         self.cli_set(pool + ['range', '0', 'stop', range_0_stop])
 
@@ -460,10 +456,10 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         failover_local = router
         failover_remote = inc_ip(router, 1)
 
-        self.cli_set(base_path + ['failover', 'source-address', failover_local])
-        self.cli_set(base_path + ['failover', 'name', failover_name])
-        self.cli_set(base_path + ['failover', 'remote', failover_remote])
-        self.cli_set(base_path + ['failover', 'status', 'primary'])
+        self.cli_set(base_path + ['high-availability', 'source-address', failover_local])
+        self.cli_set(base_path + ['high-availability', 'name', failover_name])
+        self.cli_set(base_path + ['high-availability', 'remote', failover_remote])
+        self.cli_set(base_path + ['high-availability', 'status', 'primary'])
 
         # check validate() - failover needs to be enabled for at least one subnet
         with self.assertRaises(ConfigSessionError):
@@ -501,6 +497,61 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         # Check for running process
         self.assertTrue(process_named_running(PROCESS_NAME))
 
+    def test_dhcp_high_availability_mode(self):
+        shared_net_name = 'FAILOVER'
+        failover_name = 'VyOS-Failover'
+
+        range_0_start = inc_ip(subnet, 10)
+        range_0_stop  = inc_ip(subnet, 20)
+
+        pool = base_path + ['shared-network-name', shared_net_name, 'subnet', subnet]
+        # we use the first subnet IP address as default gateway
+        self.cli_set(pool + ['default-router', router])
+        self.cli_set(pool + ['range', '0', 'start', range_0_start])
+        self.cli_set(pool + ['range', '0', 'stop', range_0_stop])
+
+        # failover
+        failover_local = router
+        failover_remote = inc_ip(router, 1)
+
+        self.cli_set(base_path + ['high-availability', 'source-address', failover_local])
+        self.cli_set(base_path + ['high-availability', 'name', failover_name])
+        self.cli_set(base_path + ['high-availability', 'remote', failover_remote])
+        self.cli_set(base_path + ['high-availability', 'status', 'primary'])
+        self.cli_set(base_path + ['high-availability', 'mode', 'active-passive'])
+        self.cli_set(pool + ['enable-failover'])
+
+        # commit changes
+        self.cli_commit()
+
+        config = read_file(DHCPD_CONF)
+
+        self.assertIn(f'failover peer "{failover_name}"' + r' {', config)
+        self.assertIn(f'primary;', config)
+        self.assertIn(f'mclt 1800;', config)
+        self.assertIn(f'mclt 1800;', config)
+        self.assertIn(f'split 256;', config)
+        self.assertIn(f'port 647;', config)
+        self.assertIn(f'peer port 647;', config)
+        self.assertIn(f'max-response-delay 30;', config)
+        self.assertIn(f'max-unacked-updates 10;', config)
+        self.assertIn(f'load balance max seconds 3;', config)
+        self.assertIn(f'address {failover_local};', config)
+        self.assertIn(f'peer address {failover_remote};', config)
+
+        network = address_from_cidr(subnet)
+        netmask = netmask_from_cidr(subnet)
+        self.assertIn(f'ddns-update-style none;', config)
+        self.assertIn(f'subnet {network} netmask {netmask}' + r' {', config)
+        self.assertIn(f'option routers {router};', config)
+        self.assertIn(f'range {range_0_start} {range_0_stop};', config)
+        self.assertIn(f'set shared-networkname = "{shared_net_name}";', config)
+        self.assertIn(f'failover peer "{failover_name}";', config)
+        self.assertIn(f'deny dynamic bootp clients;', config)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
     def test_dhcp_on_interface_with_vrf(self):
         self.cli_set(['interfaces', 'ethernet', 'eth1', 'address', '10.1.1.1/30'])
         self.cli_set(['interfaces', 'ethernet', 'eth1', 'vrf', 'SMOKE-DHCP'])
diff --git a/src/conf_mode/service_dhcp-server.py b/src/conf_mode/service_dhcp-server.py
index e910ecdf7..3cfd74a19 100755
--- a/src/conf_mode/service_dhcp-server.py
+++ b/src/conf_mode/service_dhcp-server.py
@@ -131,6 +131,10 @@ def get_config(config=None):
                         dhcp['shared_network_name'][network]['subnet'][subnet].update(
                                 {'range' : new_range_dict})
 
+    if len(dhcp['high_availability']) == 1:
+        ## only default value for mode is set, need to remove ha node
+        del dhcp['high_availability']
+
     return dhcp
 
 def verify(dhcp):
@@ -178,9 +182,9 @@ def verify(dhcp):
 
             # DHCP failover needs at least one subnet that uses it
             if 'enable_failover' in subnet_config:
-                if 'failover' not in dhcp:
-                    raise ConfigError(f'Can not enable failover for "{subnet}" in "{network}".\n' \
-                                      'Failover is not configured globally!')
+                if 'high_availability' not in dhcp:
+                    raise ConfigError(f'Can not enable high availability for "{subnet}" in "{network}".\n' \
+                                      'High availability is not configured globally!')
                 failover_ok = True
 
             # Check if DHCP address range is inside configured subnet declaration
@@ -270,12 +274,12 @@ def verify(dhcp):
     if (shared_networks - disabled_shared_networks) < 1:
         raise ConfigError(f'At least one shared network must be active!')
 
-    if 'failover' in dhcp:
+    if 'high_availability' in dhcp:
         if not failover_ok:
             raise ConfigError('DHCP failover must be enabled for at least one subnet!')
 
         for key in ['name', 'remote', 'source_address', 'status']:
-            if key not in dhcp['failover']:
+            if key not in dhcp['high_availability']:
                 tmp = key.replace('_', '-')
                 raise ConfigError(f'DHCP failover requires "{tmp}" to be specified!')
 
diff --git a/src/migration-scripts/dhcp-server/7-to-8 b/src/migration-scripts/dhcp-server/7-to-8
new file mode 100755
index 000000000..a0dc96ad0
--- /dev/null
+++ b/src/migration-scripts/dhcp-server/7-to-8
@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2024 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# T6171: rename "service dhcp-server failover" to "service dhcp-server high-availability"
+
+from sys import argv
+from sys import exit
+
+from vyos.configtree import ConfigTree
+
+if len(argv) < 2:
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+base = ['service', 'dhcp-server']
+config = ConfigTree(config_file)
+
+if not config.exists(base):
+    # Nothing to do
+    exit(0)
+
+if config.exists(base + ['failover']):
+    config.rename(base + ['failover'],'high-availability')
+
+try:
+    with open(file_name, 'w') as f:
+        f.write(config.to_string())
+except OSError as e:
+    print(f'Failed to save the modified config: {e}')
+    exit(1)
\ No newline at end of file