[docker-dhcp-relay] Add server_vrf to the DHCPv6 relay YANG and config CLI

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,48 @@ 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_add_dhcp_relay_ipv6_with_server_vrf(self, mock_cfgdb):
+        # The v6 relay accepts a server_vrf (servers reachable in a different VRF):
+        # the value is written to DHCP_RELAY and the container is not restarted
+        # (runtime reconfiguration).
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb = mock_cfgdb
+        test_ip = "fc02:2000::3"
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
+            result = runner.invoke(
+                dhcp_relay.dhcp_relay.commands["ipv6"].commands["destination"].commands["add"],
+                ["1000", test_ip, "--server-vrf", "VrfRED"], obj=db)
+            print(result.output)
+            assert result.exit_code == 0
+            assert mock_run_command.call_count == 0
+            entry = db.cfgdb.get_entry("DHCP_RELAY", "Vlan1000")
+            assert entry.get("server_vrf") == "VrfRED"
+            assert test_ip in entry.get("dhcpv6_servers", [])
+        db.cfgdb.set_entry.reset_mock()
+
+    def test_config_add_dhcp_relay_ipv6_with_invalid_server_vrf(self, mock_cfgdb):
+        # A server_vrf that is not in the VRF table is rejected before any change.
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb = mock_cfgdb
+        with mock.patch("utilities_common.cli.run_command"):
+            result = runner.invoke(
+                dhcp_relay.dhcp_relay.commands["ipv6"].commands["destination"].commands["add"],
+                ["1000", "fc02:2000::3", "--server-vrf", "NoSuchVrf"], obj=db)
+            assert result.exit_code != 0
+            assert "does not exist" in result.output
+        db.cfgdb.set_entry.reset_mock()
+
     def test_config_dhcp_relay_add_del_with_nonexist_vlanid_ipv4(self, op):
         runner = CliRunner()
 
@@ -260,10 +311,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 +340,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 +413,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 +442,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 +505,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
@@ -487,9 +491,24 @@ def dhcp_relay_ipv6_destination():
 @dhcp_relay_ipv6_destination.command("add")
 @click.argument("vid", metavar="<vid>", required=True, type=int)
 @click.argument("dhcp_relay_destinations", nargs=-1, required=True)
+@click.option("--server-vrf", required=False,
+              help="VRF in which the DHCPv6 servers are reachable, when different "
+                   "from the VLAN's own VRF (use 'default' for the global table)")
 @clicommon.pass_db
-def add_dhcp_relay_ipv6_destination(db, vid, dhcp_relay_destinations):
+def add_dhcp_relay_ipv6_destination(db, vid, dhcp_relay_destinations, server_vrf):
+    ctx = click.get_current_context()
+    # Validate the server VRF up front so an invalid VRF rejects the whole command
+    # rather than leaving the relay servers half-applied. "default" (global table)
+    # is always valid and is not present in the VRF table.
+    if server_vrf and server_vrf != "default" and not validate_vrf_exists(db, server_vrf):
+        ctx.fail("VRF {} does not exist in the VRF table.".format(server_vrf))
     add_dhcp_relay(vid, dhcp_relay_destinations, db, IPV6)
+    if server_vrf:
+        vlan_name = "Vlan{}".format(vid)
+        relay_entry = db.cfgdb.get_entry(DHCP_RELAY_TABLE, vlan_name)
+        relay_entry["server_vrf"] = server_vrf
+        db.cfgdb.set_entry(DHCP_RELAY_TABLE, vlan_name, relay_entry)
+        click.echo("Set DHCPv6 relay server VRF {} on {}".format(server_vrf, vlan_name))
 
 
 @dhcp_relay_ipv6_destination.command("del")

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,5 +1,27 @@
 #!/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).
+#
+# The bound is applied only when has_sonic_dhcpv4_relay == 'True'. In that mode
+# the relay agents reconcile a late-ready interface at runtime (dhcp4relay and
+# dhcp6relay both watch STATE_DB INTERFACE_TABLE; dhcp6relay also re-checks the
+# link-local address periodically), so an interface that becomes ready after the
+# timeout is still picked up without a restart, and one never-ready interface
+# must not block the whole container. The default (300s) covers normal startup
+# convergence.
+#
+# Otherwise (legacy ISC dhcrelay) the default stays 0 (wait forever): that agent
+# binds its interfaces at process start and does not reconcile readiness at
+# runtime, so shortening the wait could make it permanently miss an interface.
+# Operators can still opt into a bound by exporting WAIT_IFACE_READY_TIMEOUT.
+{% 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 %}
+
 function wait_until_iface_ready
 {
     IFACE_NAME=$1
@@ -8,17 +30,26 @@ function wait_until_iface_ready
     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
+    # WAIT_IFACE_READY_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 [ "${WAIT_IFACE_READY_TIMEOUT}" -gt 0 ] && [ "${waited}" -ge "${WAIT_IFACE_READY_TIMEOUT}" ]; then
+            echo "WARNING: interface ${IFACE_NAME} (${IFACE_CIDR}) not ready after ${WAIT_IFACE_READY_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

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=