From a8ccf72c222caad8cd7aaca9bca773be39e87f5c Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 19 Sep 2021 10:51:15 +0200
Subject: dhcp-server: T3672: only one failover peer is supported
---
data/templates/dhcp-server/dhcpd.conf.tmpl | 37 ++++-------
interface-definitions/dhcp-server.xml.in | 98 ++++++++++++++----------------
src/conf_mode/dhcp_server.py | 36 +++++------
src/migration-scripts/dhcp-server/5-to-6 | 25 ++++++--
4 files changed, 97 insertions(+), 99 deletions(-)
diff --git a/data/templates/dhcp-server/dhcpd.conf.tmpl b/data/templates/dhcp-server/dhcpd.conf.tmpl
index 108c9cc85..54fff3ded 100644
--- a/data/templates/dhcp-server/dhcpd.conf.tmpl
+++ b/data/templates/dhcp-server/dhcpd.conf.tmpl
@@ -31,32 +31,25 @@ option wpad-url code 252 = text;
{% endfor %}
{% endif %}
-{% if shared_network_name is defined and shared_network_name is not none %}
-{% for network, network_config in shared_network_name.items() if network_config.disable is not defined %}
-{% if network_config.subnet is defined and network_config.subnet is not none %}
-{% for subnet, subnet_config in network_config.subnet.items() %}
-{% if subnet_config.failover is defined and subnet_config.failover is defined and subnet_config.failover.name is defined and subnet_config.failover.name is not none %}
-# Failover configuration for {{ subnet }}
-failover peer "{{ subnet_config.failover.name }}" {
-{% if subnet_config.failover.status == 'primary' %}
+{% if failover is defined and failover is not none %}
+{% set dhcp_failover_name = 'VyOS-DHCP-failover-peer' %}
+# DHCP failover configuration
+failover peer "{{ dhcp_failover_name }}" {
+{% if failover.status == 'primary' %}
primary;
mclt 1800;
split 128;
-{% elif subnet_config.failover.status == 'secondary' %}
+{% elif failover.status == 'secondary' %}
secondary;
-{% endif %}
- address {{ subnet_config.failover.local_address }};
+{% endif %}
+ address {{ failover.source_address }};
port 520;
- peer address {{ subnet_config.failover.peer_address }};
+ peer address {{ failover.remote }};
peer port 520;
max-response-delay 30;
max-unacked-updates 10;
load balance max seconds 3;
}
-{% endif %}
-{% endfor %}
-{% endif %}
-{% endfor %}
{% endif %}
{% if listen_address is defined and listen_address is not none %}
@@ -184,23 +177,17 @@ shared-network {{ network | replace('_','-') }} {
}
{% endfor %}
{% endif %}
-{% if subnet_config.failover is defined and subnet_config.failover.name is defined and subnet_config.failover.name is not none %}
pool {
- failover peer "{{ subnet_config.failover.name }}";
+{% if subnet_config.enable_failover is defined %}
+ failover peer "{{ dhcp_failover_name }}";
deny dynamic bootp clients;
+{% endif %}
{% if subnet_config.range is defined and subnet_config.range is not none %}
{% for range, range_options in subnet_config.range.items() %}
range {{ range_options.start }} {{ range_options.stop }};
{% endfor %}
{% endif %}
}
-{% else %}
-{% if subnet_config.range is defined and subnet_config.range is not none %}
-{% for range, range_options in subnet_config.range.items() %}
- range {{ range_options.start }} {{ range_options.stop }};
-{% endfor %}
-{% endif %}
-{% endif %}
}
{% endfor %}
{% endif %}
diff --git a/interface-definitions/dhcp-server.xml.in b/interface-definitions/dhcp-server.xml.in
index e629d96ab..960b8a4f0 100644
--- a/interface-definitions/dhcp-server.xml.in
+++ b/interface-definitions/dhcp-server.xml.in
@@ -16,6 +16,46 @@
<valueless/>
</properties>
</leafNode>
+ <node name="failover">
+ <properties>
+ <help>DHCP failover configuration</help>
+ </properties>
+ <children>
+ #include <include/source-address-ipv4.xml.i>
+ <leafNode name="remote">
+ <properties>
+ <help>IPv4 remote address used for connectio</help>
+ <valueHelp>
+ <format>ipv4</format>
+ <description>IPv4 address of failover peer</description>
+ </valueHelp>
+ <constraint>
+ <validator name="ipv4-address"/>
+ </constraint>
+ </properties>
+ </leafNode>
+ <leafNode name="status">
+ <properties>
+ <help>Failover hierarchy</help>
+ <completionHelp>
+ <list>primary secondary</list>
+ </completionHelp>
+ <valueHelp>
+ <format>primary</format>
+ <description>Configure this server to be the primary node</description>
+ </valueHelp>
+ <valueHelp>
+ <format>secondary</format>
+ <description>Configure this server to be the secondary node</description>
+ </valueHelp>
+ <constraint>
+ <regex>^(primary|secondary)$</regex>
+ </constraint>
+ <constraintErrorMessage>Invalid DHCP failover peer status</constraintErrorMessage>
+ </properties>
+ </leafNode>
+ </children>
+ </node>
<leafNode name="global-parameters">
<properties>
<help>Additional global parameters for DHCP server. You must
@@ -118,6 +158,12 @@
#include <include/name-server-ipv4.xml.i>
#include <include/dhcp-domain-name.xml.i>
#include <include/dhcp-server-domain-search.xml.i>
+ <leafNode name="enable-failover">
+ <properties>
+ <help>Enable DHCP failover support for this subnet</help>
+ <valueless/>
+ </properties>
+ </leafNode>
<leafNode name="exclude">
<properties>
<help>IP address to exclude from DHCP lease range</help>
@@ -131,58 +177,6 @@
<multi/>
</properties>
</leafNode>
- <node name="failover">
- <properties>
- <help>DHCP failover parameters</help>
- </properties>
- <children>
- <leafNode name="local-address">
- <properties>
- <help>IP address for failover peer to connect [REQUIRED]</help>
- <valueHelp>
- <format>ipv4</format>
- <description>IPv4 address to exclude from lease range</description>
- </valueHelp>
- <constraint>
- <validator name="ipv4-address"/>
- </constraint>
- </properties>
- </leafNode>
- <leafNode name="name">
- <properties>
- <help>DHCP failover peer name [REQUIRED]</help>
- <constraint>
- <regex>[-_a-zA-Z0-9.]+</regex>
- </constraint>
- <constraintErrorMessage>Invalid failover peer name. May only contain letters, numbers and .-_</constraintErrorMessage>
- </properties>
- </leafNode>
- <leafNode name="peer-address">
- <properties>
- <help>IP address of failover peer [REQUIRED]</help>
- <valueHelp>
- <format>ipv4</format>
- <description>IPv4 address of failover peer</description>
- </valueHelp>
- <constraint>
- <validator name="ipv4-address"/>
- </constraint>
- </properties>
- </leafNode>
- <leafNode name="status">
- <properties>
- <help>DHCP failover peer status (primary|secondary) [REQUIRED]</help>
- <completionHelp>
- <list>primary secondary</list>
- </completionHelp>
- <constraint>
- <regex>^(primary|secondary)$</regex>
- </constraint>
- <constraintErrorMessage>Invalid DHCP failover peer status</constraintErrorMessage>
- </properties>
- </leafNode>
- </children>
- </node>
<leafNode name="ip-forwarding">
<properties>
<help>Enable IP forwarding on client</help>
diff --git a/src/conf_mode/dhcp_server.py b/src/conf_mode/dhcp_server.py
index 8d6cef8b7..5b3809017 100755
--- a/src/conf_mode/dhcp_server.py
+++ b/src/conf_mode/dhcp_server.py
@@ -148,9 +148,9 @@ def verify(dhcp):
'At least one DHCP shared network must be configured.')
# Inspect shared-network/subnet
- failover_names = []
listen_ok = False
subnets = []
+ failover_ok = False
# A shared-network requires a subnet definition
for network, network_config in dhcp['shared_network_name'].items():
@@ -159,11 +159,19 @@ def verify(dhcp):
'lease subnet must be configured.')
for subnet, subnet_config in network_config['subnet'].items():
+ # All delivered static routes require a next-hop to be set
if 'static_route' in subnet_config:
for route, route_option in subnet_config['static_route'].items():
if 'next_hop' not in route_option:
raise ConfigError(f'DHCP static-route "{route}" requires router to be defined!')
+ # 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!')
+ failover_ok = True
+
# Check if DHCP address range is inside configured subnet declaration
if 'range' in subnet_config:
networks = []
@@ -192,23 +200,6 @@ def verify(dhcp):
tmp = IPRange(range_config['start'], range_config['stop'])
networks.append(tmp)
- if 'failover' in subnet_config:
- for key in ['local_address', 'peer_address', 'name', 'status']:
- if key not in subnet_config['failover']:
- raise ConfigError(f'Missing DHCP failover parameter "{key}"!')
-
- # Failover names must be uniquie
- if subnet_config['failover']['name'] in failover_names:
- name = subnet_config['failover']['name']
- raise ConfigError(f'DHCP failover names must be unique:\n' \
- f'{name} has already been configured!')
- failover_names.append(subnet_config['failover']['name'])
-
- # Failover requires start/stop ranges for pool
- if 'range' not in subnet_config:
- raise ConfigError(f'DHCP failover requires at least one start-stop range to be configured\n'\
- f'within shared-network "{network}, {subnet}" for using failover!')
-
# Exclude addresses must be in bound
if 'exclude' in subnet_config:
for exclude in subnet_config['exclude']:
@@ -252,6 +243,15 @@ def verify(dhcp):
if net.overlaps(net2):
raise ConfigError('Conflicting subnet ranges: "{net}" overlaps "{net2}"!')
+ if 'failover' in dhcp:
+ if not failover_ok:
+ raise ConfigError('DHCP failover must be enabled for at least one subnet!')
+
+ for key in ['source_address', 'remote', 'status']:
+ if key not in dhcp['failover']:
+ tmp = key.replace('_', '-')
+ raise ConfigError(f'DHCP failover requires "{tmp}" to be specified!')
+
for address in (dict_search('listen_address', dhcp) or []):
if is_addr_assigned(address):
listen_ok = True
diff --git a/src/migration-scripts/dhcp-server/5-to-6 b/src/migration-scripts/dhcp-server/5-to-6
index 7f447ac17..39bbb9f50 100755
--- a/src/migration-scripts/dhcp-server/5-to-6
+++ b/src/migration-scripts/dhcp-server/5-to-6
@@ -29,16 +29,16 @@ file_name = sys.argv[1]
with open(file_name, 'r') as f:
config_file = f.read()
-base = ['service', 'dhcp-server', 'shared-network-name']
+base = ['service', 'dhcp-server']
config = ConfigTree(config_file)
-if not config.exists(base):
+if not config.exists(base + ['shared-network-name']):
# Nothing to do
exit(0)
# Run this for every instance if 'shared-network-name'
-for network in config.list_nodes(base):
- base_network = base + [network]
+for network in config.list_nodes(base + ['shared-network-name']):
+ base_network = base + ['shared-network-name', network]
if not config.exists(base_network + ['subnet']):
continue
@@ -60,6 +60,23 @@ for network in config.list_nodes(base):
if config.exists(base_subnet + ['dns-server']):
config.rename(base_subnet + ['dns-server'], 'name-server')
+
+ # T3672: ISC DHCP server only supports one failover peer
+ if config.exists(base_subnet + ['failover']):
+ # There can only be one failover configuration, if none is present
+ # we add the first one
+ if not config.exists(base + ['failover']):
+ local = config.return_value(base_subnet + ['failover', 'local-address'])
+ remote = config.return_value(base_subnet + ['failover', 'peer-address'])
+ status = config.return_value(base_subnet + ['failover', 'status'])
+
+ config.set(base + ['failover', 'remote'], value=remote)
+ config.set(base + ['failover', 'source-address'], value=local)
+ config.set(base + ['failover', 'status'], value=status)
+
+ config.delete(base_subnet + ['failover'])
+ config.set(base_subnet + ['enable-failover'])
+
try:
with open(file_name, 'w') as f:
f.write(config.to_string())
--
cgit v1.2.3