[docker-dhcp-relay] Start dhcp6relay unconditionally; apply IPv6 relay config at runtime

dockers/docker-dhcp-relay/cli-plugin-tests/test_config_dhcp_relay.py

@@ -18,6 +18,15 @@
 Removed DHCP relay address [{}] from Vlan1000
 Restarting DHCP relay service...
 """
+# dhcp6relay applies DHCP_RELAY (dhcpv6_servers) changes at runtime, so the IPv6
+# path does not restart the dhcp_relay container: no "Restarting..." line and no
+# systemctl calls.
+config_dhcp_relay_add_output_no_restart = """\
+Added DHCP relay address [{}] to Vlan1000
+"""
+config_dhcp_relay_del_output_no_restart = """\
+Removed DHCP relay address [{}] from Vlan1000
+"""
 expected_dhcp_relay_add_config_db_output = {
     "ipv4": {
         "dhcp_servers": [
@@ -140,6 +149,15 @@ def test_plugin_registration(self):
         cli = mock.MagicMock()
         dhcp_relay.register(cli)
 
+    def test_restart_dhcp_relay_service_skipped_for_ipv6(self, mock_cfgdb):
+        # dhcp6relay applies DHCP_RELAY changes at runtime, so the IPv6 path must
+        # not restart the dhcp_relay container.
+        db = Db()
+        db.cfgdb = mock_cfgdb
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
+            dhcp_relay.restart_dhcp_relay_service(db, dhcp_relay.IPV6)
+            assert mock_run_command.call_count == 0
+
     def test_config_dhcp_relay_add_del_with_nonexist_vlanid_ipv4(self, op):
         runner = CliRunner()
 
@@ -260,10 +278,12 @@ def test_config_add_del_dhcp_relay(self, mock_cfgdb, version):
             print(result.output)
             assert result.exit_code == 0
             if version != "ipv4_dhcp":
-                assert result.output == config_dhcp_relay_add_output.format(test_ip)
+                expected_add = (config_dhcp_relay_add_output_no_restart if version == "ipv6"
+                                else config_dhcp_relay_add_output)
+                assert result.output == expected_add.format(test_ip)
                 assert db.cfgdb.get_entry(config_db_table, "Vlan1000") \
                     == expected_dhcp_relay_add_config_db_output[version]
-                assert mock_run_command.call_count == 3
+                assert mock_run_command.call_count == (0 if version == "ipv6" else 3)
 
             if version == "ipv4_dhcp" :
                 assert result.output == config_dhcp_relay_update_output.format(test_ip)
@@ -287,10 +307,12 @@ def test_config_add_del_dhcp_relay(self, mock_cfgdb, version):
             print(result.output)
             assert result.exit_code == 0
             if version != "ipv4_dhcp":
-                assert result.output == config_dhcp_relay_del_output.format(test_ip)
+                expected_del = (config_dhcp_relay_del_output_no_restart if version == "ipv6"
+                                else config_dhcp_relay_del_output)
+                assert result.output == expected_del.format(test_ip)
                 assert db.cfgdb.get_entry(config_db_table, "Vlan1000") \
                     == expected_dhcp_relay_del_config_db_output[version]
-                assert mock_run_command.call_count == 3
+                assert mock_run_command.call_count == (0 if version == "ipv6" else 3)
 
             if version == "ipv4_dhcp" :
                 assert "Removed DHCP relay address [{}] from Vlan1000".format(test_ip) in result.output
@@ -358,10 +380,12 @@ def test_config_add_del_multiple_dhcp_relay(self, mock_cfgdb, version):
             print(result.output)
             assert result.exit_code == 0
             if version != "ipv4_dhcp":
-                assert result.output == config_dhcp_relay_add_output.format(",".join(test_ips))
+                expected_add = (config_dhcp_relay_add_output_no_restart if version == "ipv6"
+                                else config_dhcp_relay_add_output)
+                assert result.output == expected_add.format(",".join(test_ips))
                 assert db.cfgdb.get_entry(config_db_table, "Vlan1000") \
                     == expected_dhcp_relay_add_multi_config_db_output[version]
-                assert mock_run_command.call_count == 3
+                assert mock_run_command.call_count == (0 if version == "ipv6" else 3)
 
             if version == "ipv4_dhcp" :
                 assert result.output == config_dhcp_relay_update_output.format(",".join(test_ips))
@@ -385,8 +409,10 @@ def test_config_add_del_multiple_dhcp_relay(self, mock_cfgdb, version):
             print(result.output)
             assert result.exit_code == 0
             if version != "ipv4_dhcp":
-                assert result.output == config_dhcp_relay_del_output.format(",".join(test_ips))
-                assert mock_run_command.call_count == 3
+                expected_del = (config_dhcp_relay_del_output_no_restart if version == "ipv6"
+                                else config_dhcp_relay_del_output)
+                assert result.output == expected_del.format(",".join(test_ips))
+                assert mock_run_command.call_count == (0 if version == "ipv6" else 3)
                 assert db.cfgdb.get_entry(config_db_table, "Vlan1000") \
                     == expected_dhcp_relay_del_config_db_output[version]
             if ip_version == "ipv4_dhcp":
@@ -446,7 +472,7 @@ def test_config_add_dhcp_relay_ipv6_with_non_entry(self, mock_cfgdb):
             print(result.output)
             assert result.exit_code == 0
             assert db.cfgdb.get_entry(table, "Vlan1000") == {"dhcpv6_servers": [test_ip]}
-            assert mock_run_command.call_count == 3
+            assert mock_run_command.call_count == 0
 
     def test_add_dhcpv4_relay_compatibility_check(self, mock_cfgdb):
         ip_version = "ipv4_dhcp"

dockers/docker-dhcp-relay/cli-plugin-tests/test_config_vlan_dhcp_relay.py

@@ -17,14 +17,15 @@
 Restarting DHCP relay service...
 """
 
+# dhcp6relay applies DHCP_RELAY (dhcpv6_servers) changes at runtime, so the IPv6
+# path does not restart the dhcp_relay container: no "Restarting..." line and no
+# systemctl calls.
 config_vlan_add_dhcpv6_relay_output="""\
 Added DHCP relay destination addresses ['fc02:2000::1'] to Vlan1000
-Restarting DHCP relay service...
 """
 
 config_vlan_add_multiple_dhcpv6_relay_output="""\
 Added DHCP relay destination addresses ['fc02:2000::1', 'fc02:2000::2', 'fc02:2000::3'] to Vlan1000
-Restarting DHCP relay service...
 """
 
 config_vlan_del_dhcp_relay_output="""\
@@ -34,12 +35,10 @@
 
 config_vlan_del_dhcpv6_relay_output="""\
 Removed DHCP relay destination addresses ('fc02:2000::1',) from Vlan1000
-Restarting DHCP relay service...
 """
 
 config_vlan_del_multiple_dhcpv6_relay_output="""\
 Removed DHCP relay destination addresses ('fc02:2000::1', 'fc02:2000::2', 'fc02:2000::3') from Vlan1000
-Restarting DHCP relay service...
 """
 
 @pytest.fixture(scope="module", params=["isc", "new"])
@@ -199,7 +198,7 @@ def test_config_vlan_add_del_dhcpv6_relay_dest(self, mock_cfgdb):
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_add_dhcpv6_relay_output
-            assert mock_run_command.call_count == 3
+            assert mock_run_command.call_count == 0
             db.cfgdb.set_entry.assert_called_once_with('VLAN', 'Vlan1000', {'dhcp_servers': ['192.0.0.1'], 'dhcpv6_servers': ['fc02:2000::1']})
 
         db.cfgdb.set_entry.reset_mock()
@@ -212,7 +211,7 @@ def test_config_vlan_add_del_dhcpv6_relay_dest(self, mock_cfgdb):
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_del_dhcpv6_relay_output
-            assert mock_run_command.call_count == 3
+            assert mock_run_command.call_count == 0
             db.cfgdb.set_entry.assert_called_once_with('VLAN', 'Vlan1000', {'dhcp_servers': ['192.0.0.1']})
 
     def test_config_vlan_add_del_multiple_dhcpv6_relay_dest(self, mock_cfgdb):
@@ -228,7 +227,7 @@ def test_config_vlan_add_del_multiple_dhcpv6_relay_dest(self, mock_cfgdb):
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_add_multiple_dhcpv6_relay_output
-            assert mock_run_command.call_count == 3
+            assert mock_run_command.call_count == 0
             db.cfgdb.set_entry.assert_called_once_with('VLAN', 'Vlan1000', {'dhcp_servers': ['192.0.0.1'], 'dhcpv6_servers': ['fc02:2000::1', 'fc02:2000::2', 'fc02:2000::3']})
 
         db.cfgdb.set_entry.reset_mock()
@@ -241,7 +240,7 @@ def test_config_vlan_add_del_multiple_dhcpv6_relay_dest(self, mock_cfgdb):
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_del_multiple_dhcpv6_relay_output
-            assert mock_run_command.call_count == 3
+            assert mock_run_command.call_count == 0
             db.cfgdb.set_entry.assert_called_once_with('VLAN', 'Vlan1000', {'dhcp_servers': ['192.0.0.1']})
 
     def test_config_vlan_remove_nonexist_dhcp_relay_dest(self, mock_cfgdb):

dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py

@@ -52,6 +52,10 @@ def restart_dhcp_relay_service(db, ip_version):
     """
     Restart dhcp_relay service
     """
+    if ip_version == IPV6:
+        # dhcp6relay applies DHCP_RELAY (dhcpv6_servers) changes at runtime,
+        # so no dhcp_relay container restart is required.
+        return
     if(ip_version == IPV4 and check_sonic_dhcpv4_relay_flag(db)):
         # if 'has_sonic_dhcpv4_relay' flag is present in DEVICE_METADATA['localhost'] and is 'true'
         return

dockers/docker-dhcp-relay/dhcp-relay.programs.j2

@@ -1,37 +1,7 @@
 [group:dhcp-relay]
-programs=dhcprelayd
-{%- if 'has_sonic_dhcpv4_relay' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['has_sonic_dhcpv4_relay'] == 'True' %}
-{%- set relay_for_ipv6 = { 'flag': False } %}
-{%- set add_preceding_comma = { 'flag': True } %}
-{% for vlan_name in VLAN_INTERFACE %}
-{% if DHCP_RELAY and vlan_name in DHCP_RELAY and DHCP_RELAY[vlan_name]['dhcpv6_servers']|length > 0 %}
-{% set _dummy = relay_for_ipv6.update({'flag': True})  %}
-{%- endif %}
-{% endfor %}
-{# Append DHCPv6 agents #}
-{% if relay_for_ipv6.flag %}
-{% if add_preceding_comma.flag %},{% endif %}
-{% set _dummy = add_preceding_comma.update({'flag': True}) %}
-dhcp6relay
-{%- endif %}
-{# Create a program entry for sonic dhcpv4 relay agent #}
-{%- set add_preceding_comma = { 'flag': True } %}
-{# Append sonic DHCPv4 agent #}
-{% if add_preceding_comma.flag %},{% endif %}
-{% set _dummy = add_preceding_comma.update({'flag': True}) %}
-dhcp4relay
-{% else %}
-{%- set relay_for_ipv6 = { 'flag': False } %}
-{%- set add_preceding_comma = { 'flag': True } %}
-{% for vlan_name in VLAN_INTERFACE %}
-{% if DHCP_RELAY and vlan_name in DHCP_RELAY and DHCP_RELAY[vlan_name]['dhcpv6_servers']|length > 0 %}
-{% set _dummy = relay_for_ipv6.update({'flag': True})  %}
-{%- endif %}
-{% endfor %}
-{# Append DHCPv6 agents #}
-{% if relay_for_ipv6.flag %}
-{% if add_preceding_comma.flag %},{% endif %}
-{% set _dummy = add_preceding_comma.update({'flag': True}) %}
-dhcp6relay
-{% endif %}
+{# The DHCPv6 relay agent is always started so DHCPv6 relay configuration added at runtime is applied without a container restart #}
+{% if 'has_sonic_dhcpv4_relay' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['has_sonic_dhcpv4_relay'] == 'True' -%}
+programs=dhcprelayd,dhcp6relay,dhcp4relay
+{% else -%}
+programs=dhcprelayd,dhcp6relay
 {% endif %}

dockers/docker-dhcp-relay/dhcpv6-relay.agents.j2

@@ -1,15 +1,6 @@
-{# Append DHCPv6 agents #}
-{% for vlan_name in VLAN_INTERFACE %}
-{% if DHCP_RELAY and vlan_name in DHCP_RELAY and DHCP_RELAY[vlan_name]['dhcpv6_servers']|length > 0 %}
-{% for dhcpv6_server in DHCP_RELAY[vlan_name]['dhcpv6_servers'] %}
-{% if dhcpv6_server | ipv6 %}
-{% set _dummy = relay_for_ipv6.update({'flag': True}) %}
-{% endif %}
-{% endfor %}
-{% endif %}
-{% endfor %}
-{% if relay_for_ipv6.flag %}
-{% set _dummy = relay_for_ipv6.update({'flag': False}) %}
+{# Append the DHCPv6 agent. It is always started so DHCPv6 relay configuration
+   added at runtime is applied by its config monitor without restarting the
+   dhcp_relay container. #}
 [program:dhcp6relay]
 command=/usr/sbin/dhcp6relay
 {#- Dual ToR Option #}
@@ -25,4 +16,4 @@ stderr_syslog=true
 dependent_startup=true
 dependent_startup_wait_for=start:exited
 
-{% endif %}
+

dockers/docker-dhcp-relay/wait_for_intf.sh.j2

@@ -1,24 +1,63 @@
 #!/usr/bin/env bash
 
+# Maximum time (in seconds) to wait for a single interface to become ready in
+# STATE_DB before proceeding with relay startup. A value of 0 means "wait
+# indefinitely" (the historical behavior). Two bounds are used, one per agent:
+#
+# WAIT_IFACE_READY_TIMEOUT governs the IPv4 interface waits (INTERFACE, VLAN and
+# PortChannel v4 addresses). It is bounded (300s) only when has_sonic_dhcpv4_relay
+# == 'True': the sonic dhcp4relay watches STATE_DB INTERFACE_TABLE and reconciles
+# a late-ready interface at runtime, so proceeding without one and picking it up
+# later is safe. For the legacy ISC dhcrelay it stays 0 (wait forever): that agent
+# binds its interfaces once at process start and never reconciles, so shortening
+# the wait could make it permanently miss an interface.
+#
+# WAIT_IFACE_READY_TIMEOUT_V6 governs the IPv6 VLAN-interface waits, which are for
+# dhcp6relay. dhcp6relay is always the native v6 agent and always reconciles at
+# runtime (STATE_DB INTERFACE_TABLE + periodic link-local check), so these are
+# always bounded (300s) regardless of the v4 relay type -- a never-ready v6 VLAN
+# interface must not block container startup. Operators can override either bound
+# by exporting the corresponding variable.
+{% if 'has_sonic_dhcpv4_relay' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['has_sonic_dhcpv4_relay'] == 'True' %}
+WAIT_IFACE_READY_TIMEOUT="${WAIT_IFACE_READY_TIMEOUT:-300}"
+{% else %}
+WAIT_IFACE_READY_TIMEOUT="${WAIT_IFACE_READY_TIMEOUT:-0}"
+{% endif %}
+# dhcp6relay always reconciles late-ready interfaces at runtime, so the v6
+# VLAN-interface waits below are always bounded, independent of the v4 relay type.
+WAIT_IFACE_READY_TIMEOUT_V6="${WAIT_IFACE_READY_TIMEOUT_V6:-300}"
+
 function wait_until_iface_ready
 {
     IFACE_NAME=$1
     IFACE_CIDR=$2
+    # Optional per-call timeout; defaults to WAIT_IFACE_READY_TIMEOUT (the v4/ISC
+    # bound). The IPv6 VLAN waits pass WAIT_IFACE_READY_TIMEOUT_V6 instead.
+    local timeout="${3:-${WAIT_IFACE_READY_TIMEOUT}}"
 
     echo "Waiting until interface ${IFACE_NAME} is ready..."
 
     # Wait for the interface to come up
-    # (i.e., interface is present in STATE_DB and state is "ok")
+    # (i.e., interface is present in STATE_DB and state is "ok"). When
+    # timeout > 0 the wait is bounded so a never-ready interface cannot hang
+    # startup forever; when it is 0 we wait indefinitely (legacy ISC dhcrelay
+    # behavior).
+    local waited=0
     while true; do
         RESULT=$(sonic-db-cli STATE_DB HGET "INTERFACE_TABLE|${IFACE_NAME}|${IFACE_CIDR}" "state" 2> /dev/null)
         if [ x"$RESULT" == x"ok" ]; then
-            break
+            echo "Interface ${IFACE_NAME} is ready!"
+            return 0
+        fi
+
+        if [ "${timeout}" -gt 0 ] && [ "${waited}" -ge "${timeout}" ]; then
+            echo "WARNING: interface ${IFACE_NAME} (${IFACE_CIDR}) not ready after ${timeout}s; proceeding without it (relay agents reconcile it at runtime)"
+            return 0
         fi
 
         sleep 1
+        waited=$((waited + 1))
     done
-
-    echo "Interface ${IFACE_NAME} is ready!"
 }
 
 # Wait for all interfaces with IPv4 addresses to be up and ready
@@ -35,7 +74,7 @@ wait_until_iface_ready {{ name }} {{ prefix }}
 {% endif %}
 {% if prefix | ipv6 %}
 {% if DHCP_RELAY and name in DHCP_RELAY %}
-wait_until_iface_ready {{ name }} {{ prefix }}
+wait_until_iface_ready {{ name }} {{ prefix }} "${WAIT_IFACE_READY_TIMEOUT_V6}"
 {% endif %}
 {% endif %}
 {% endfor %}

src/sonic-config-engine/tests/dhcp-relay-dualtor-sample.json

@@ -0,0 +1,7 @@
+{
+    "DEVICE_METADATA": {
+        "localhost": {
+            "subtype": "DualToR"
+        }
+    }
+}

src/sonic-config-engine/tests/sample_output/py3/docker-dhcp-relay-sonic-agent-no-relay-cfg.supervisord.conf

@@ -44,7 +44,7 @@ dependent_startup=true
 dependent_startup_wait_for=rsyslogd:running
 
 [group:dhcp-relay]
-programs=dhcprelayd,dhcp4relay
+programs=dhcprelayd,dhcp6relay,dhcp4relay
 
 [program:dhcp4relay]
 command=/usr/sbin/dhcp4relay
@@ -58,6 +58,18 @@ stderr_syslog=true
 dependent_startup=true
 dependent_startup_wait_for=start:exited
 
+[program:dhcp6relay]
+command=/usr/sbin/dhcp6relay
+priority=3
+autostart=false
+autorestart=false
+stdout_logfile=NONE
+stdout_syslog=true
+stderr_logfile=NONE
+stderr_syslog=true
+dependent_startup=true
+dependent_startup_wait_for=start:exited
+
 [group:dhcpmon]
 programs=