diff --git a/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_dhcp_relay.py b/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_dhcp_relay.py index 74d320e5e30..f623b223919 100644 --- a/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_dhcp_relay.py +++ b/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" diff --git a/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_vlan_dhcp_relay.py b/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_vlan_dhcp_relay.py index b94ab3609dd..90489ffade5 100644 --- a/dockers/docker-dhcp-relay/cli-plugin-tests/test_config_vlan_dhcp_relay.py +++ b/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): diff --git a/dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py b/dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py index ef889beb589..40e8a35bb03 100644 --- a/dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py +++ b/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 diff --git a/dockers/docker-dhcp-relay/dhcp-relay.programs.j2 b/dockers/docker-dhcp-relay/dhcp-relay.programs.j2 index 73b12060453..4265bad937d 100644 --- a/dockers/docker-dhcp-relay/dhcp-relay.programs.j2 +++ b/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 %} diff --git a/dockers/docker-dhcp-relay/dhcpv6-relay.agents.j2 b/dockers/docker-dhcp-relay/dhcpv6-relay.agents.j2 index ffa73909f8a..5da1857fcac 100644 --- a/dockers/docker-dhcp-relay/dhcpv6-relay.agents.j2 +++ b/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 %} + diff --git a/dockers/docker-dhcp-relay/wait_for_intf.sh.j2 b/dockers/docker-dhcp-relay/wait_for_intf.sh.j2 index c00eaa35686..8da7cb55e09 100644 --- a/dockers/docker-dhcp-relay/wait_for_intf.sh.j2 +++ b/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 %} diff --git a/src/sonic-config-engine/tests/dhcp-relay-dualtor-sample.json b/src/sonic-config-engine/tests/dhcp-relay-dualtor-sample.json new file mode 100644 index 00000000000..fbe5b2ed06d --- /dev/null +++ b/src/sonic-config-engine/tests/dhcp-relay-dualtor-sample.json @@ -0,0 +1,7 @@ +{ + "DEVICE_METADATA": { + "localhost": { + "subtype": "DualToR" + } + } +} diff --git a/src/sonic-config-engine/tests/sample_output/py3/docker-dhcp-relay-sonic-agent-no-relay-cfg.supervisord.conf b/src/sonic-config-engine/tests/sample_output/py3/docker-dhcp-relay-sonic-agent-no-relay-cfg.supervisord.conf index 2598ee08f06..c403158305a 100644 --- a/src/sonic-config-engine/tests/sample_output/py3/docker-dhcp-relay-sonic-agent-no-relay-cfg.supervisord.conf +++ b/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= diff --git a/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf.sh b/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf.sh index 3e0e303e71d..23e53ea7eb4 100644 --- a/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf.sh +++ b/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf.sh @@ -1,30 +1,65 @@ #!/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. +WAIT_IFACE_READY_TIMEOUT="${WAIT_IFACE_READY_TIMEOUT:-0}" +# 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 # dhcp6relay binds to ipv6 addresses configured on these vlan ifaces # Thus check if they are ready before launching dhcp6relay -wait_until_iface_ready Vlan1000 fc02:2000::2/24 +wait_until_iface_ready Vlan1000 fc02:2000::2/24 "${WAIT_IFACE_READY_TIMEOUT_V6}" wait_until_iface_ready Vlan1000 192.168.0.1/27 wait_until_iface_ready Vlan2000 192.168.200.1/27 wait_until_iface_ready PortChannel01 10.0.0.56/31 diff --git a/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf_v4_relay.sh b/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf_v4_relay.sh new file mode 100644 index 00000000000..af340228152 --- /dev/null +++ b/src/sonic-config-engine/tests/sample_output/py3/wait_for_intf_v4_relay.sh @@ -0,0 +1,73 @@ +#!/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. +WAIT_IFACE_READY_TIMEOUT="${WAIT_IFACE_READY_TIMEOUT:-300}" +# 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"). 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 + 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 +} + +# Wait for all interfaces with IPv4 addresses to be up and ready +# dhcp6relay binds to ipv6 addresses configured on these vlan ifaces +# Thus check if they are ready before launching dhcp6relay +wait_until_iface_ready Vlan1000 fc02:2000::2/24 "${WAIT_IFACE_READY_TIMEOUT_V6}" +wait_until_iface_ready Vlan1000 192.168.0.1/27 +wait_until_iface_ready Vlan2000 192.168.200.1/27 +wait_until_iface_ready PortChannel01 10.0.0.56/31 +wait_until_iface_ready PortChannel02 10.0.0.58/31 +wait_until_iface_ready PortChannel03 10.0.0.60/31 +wait_until_iface_ready PortChannel04 10.0.0.62/31 + +# Wait 10 seconds for the rest of interfaces to get added/populated. +# dhcrelay listens on each of the interfaces (in addition to the port +# channels and vlan interfaces) +sleep 10 diff --git a/src/sonic-config-engine/tests/test_j2files.py b/src/sonic-config-engine/tests/test_j2files.py index 4ba6f319c1f..35d194c5c93 100644 --- a/src/sonic-config-engine/tests/test_j2files.py +++ b/src/sonic-config-engine/tests/test_j2files.py @@ -186,6 +186,15 @@ def test_dhcp_relay(self): self.run_script(argument, output_file=self.output_file) self.assertTrue(utils.cmp(os.path.join(self.test_dir, 'sample_output', utils.PYvX_DIR, 'wait_for_intf.sh'), self.output_file)) + # Test generation of wait_for_intf.sh when has_sonic_dhcpv4_relay is True. + # This exercises the has_sonic_dhcpv4_relay == 'True' branch: the IPv4 waits + # become bounded (WAIT_IFACE_READY_TIMEOUT=300), while the IPv6 VLAN waits + # stay bounded via WAIT_IFACE_READY_TIMEOUT_V6 (dhcp6relay always reconciles). + v4relay_sample_data = os.path.join(self.test_dir, "dhcp-sonic-relay-enabled-sample.json") + argument = ['-m', self.t0_minigraph, '-j', v4relay_sample_data, '-p', self.t0_port_config, '-t', template_path] + self.run_script(argument, output_file=self.output_file) + self.assertTrue(utils.cmp(os.path.join(self.test_dir, 'sample_output', utils.PYvX_DIR, 'wait_for_intf_v4_relay.sh'), self.output_file)) + template_path = os.path.join(self.test_dir, '..', '..', '..', 'dockers', 'docker-dhcp-relay', 'docker-dhcp-relay.supervisord.conf.j2') argument = ['-m', self.t0_minigraph_common_dhcp_relay, '-p', self.t0_port_config, '-t', template_path] self.run_script(argument, output_file=self.output_file) @@ -234,6 +243,25 @@ def test_dhcp_relay(self): self.assertTrue(utils.cmp(os.path.join(self.test_dir, 'sample_output', utils.PYvX_DIR, 'docker-dhcp-relay.supervisord.conf'), self.output_file)) + # Test that the DualToR option (-u Loopback0) is rendered for the dhcp6relay agent only when + # DEVICE_METADATA.localhost.subtype is 'DualToR'. This is asserted on the rendered content + # directly (not only through a golden file) so the template-to-command-line contract is pinned + # independently of golden regeneration: the dhcp6relay binary treats '-u' as "enable Dual-ToR + # mode", so an unconditional '-u' would silently turn every device into Dual-ToR. + template_path = os.path.join(self.test_dir, '..', '..', '..', 'dockers', 'docker-dhcp-relay', + 'docker-dhcp-relay.supervisord.conf.j2') + # Non-DualToR (no subtype): dhcp6relay must start without '-u Loopback0'. + argument = ['-m', self.t0_minigraph_common_dhcp_relay, '-p', self.t0_port_config, '-t', template_path] + output = self.run_script(argument) + self.assertIn('command=/usr/sbin/dhcp6relay\n', output) + self.assertNotIn('dhcp6relay -u Loopback0', output) + # DualToR (subtype=DualToR): dhcp6relay must start with '-u Loopback0'. + dualtor_sample_data = os.path.join(self.test_dir, "dhcp-relay-dualtor-sample.json") + argument = ['-m', self.t0_minigraph_common_dhcp_relay, '-j', dualtor_sample_data, + '-p', self.t0_port_config, '-t', template_path] + output = self.run_script(argument) + self.assertIn('command=/usr/sbin/dhcp6relay -u Loopback0', output) + def test_radv(self): # Test generation of radvd.conf with multiple ipv6 prefixes template_path = os.path.join(self.test_dir, '..', '..', '..', 'dockers', 'docker-router-advertiser', 'radvd.conf.j2')