On 2/28/25 9:19 AM, Ales Musil wrote:
> On Wed, Feb 19, 2025 at 6:28 PM Xavier Simonart <[email protected]> wrote:
> 
>> Default lease file for dhcp varies between OSes. For instance,
>> /var/lib/dhcp/dhcpd6.leases or /var/lib/dhcpd/dhcpd6.leases might be used.
>> Before this patch, lease file was not deleted on some OSes, resulting in
>> potential test failures (e.g. when multiple prefix delgation tests were
>> executed within a certain time).
>>
>> Signed-off-by: Xavier Simonart <[email protected]>
>>
>> ---
>> v2: Updated based on Ilya's feedback i.e. use subdir of test instead of
>> /tmp.
>> ---
>>  tests/system-common-macros.at | 32 +++++++++++++++++++++++---------
>>  tests/system-ovn.at           | 12 +++---------
>>  2 files changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
>> index 72ff6bdfc..c0dfcc8f0 100644
>> --- a/tests/system-common-macros.at
>> +++ b/tests/system-common-macros.at
>> @@ -293,6 +293,25 @@ m4_define([NETNS_START_TCPDUMP],
>>      ]
>>  )
>>
>> +# NETNS_START_DHCPD([namespace], [iface], [dhcpd.conf])
>> +#
>> +# Helper to properly start dhcpd
>> +m4_define([NETNS_START_DHCPD],
>> +    [
>> +        DHCP_TEST_DIR="$ovs_base/dhcp-test"
>> +        mkdir $DHCP_TEST_DIR

Just in case we run the tests with TESTSUITEFLAGS="-d" I changed this to
mkdir -p.  It wasn't a huge problem without the "-p" but I'd like to
avoid an annoying (harmless) error message in the logs.

>> +
>> +        mv $3 $DHCP_TEST_DIR/dhcpd.conf
>> +
>> +        touch $DHCP_TEST_DIR/dhcpd.leases
>> +        chown root:dhcpd $DHCP_TEST_DIR $DHCP_TEST_DIR/dhcpd.leases
>> +        chmod 775 $DHCP_TEST_DIR
>> +        chmod 664 $DHCP_TEST_DIR/dhcpd.leases
>> +
>> +        NETNS_START_TCPDUMP([$1], [-nni $2], [$1])
>> +        NETNS_DAEMONIZE([$1], [dhcpd -6 -f -lf
>> $DHCP_TEST_DIR/dhcpd.leases -cf $DHCP_TEST_DIR/dhcpd.conf $2 > dhcpd.log
>> 2>&1], [dhcpd.pid])
>> +    ]
>> +)
>>
>>  # OVS_CHECK_VXLAN()
>>  #
>> @@ -446,21 +465,16 @@ OVN_POPULATE_ARP
>>
>>  check ovn-nbctl --wait=hv sync
>>
>> -cat > /etc/dhcp/dhcpd.conf <<EOF
>> +cat > dhcpd.conf <<EOF
>>  option dhcp-rebinding-time 10;
>>  option dhcp-renewal-time 5;
>>  subnet6 2001:db8:3333::/56 {
>>      prefix6 2001:db8:3333:100:: 2001:db8:3333:111:: /64;
>>  }
>>  EOF
>> -rm -f /var/lib/dhcp/dhcpd6.leases
>> -touch /var/lib/dhcp/dhcpd6.leases
>> -chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases
>> -chmod 775 /var/lib/dhcp
>> -chmod 664 /var/lib/dhcp/dhcpd6.leases
>> -
>> -NETNS_START_TCPDUMP([server], [-nni s1], [server])
>> -NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], [dhcpd.pid])
>> +
>> +NETNS_START_DHCPD([server], [s1], [dhcpd.conf])
>> +
>>  check ovn-nbctl --wait=hv sync
>>
>>  OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-public
>> ipv6_prefix | cut -c4-15)" = ""])
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index 9982da7fe..59ba58b1f 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>> @@ -14605,21 +14605,15 @@ OVN_POPULATE_ARP
>>
>>  check ovn-nbctl --wait=hv sync
>>
>> -cat > /etc/dhcp/dhcpd.conf <<EOF
>> +cat > dhcpd.conf <<EOF
>>  option dhcp-rebinding-time 10;
>>  option dhcp-renewal-time 5;
>>  subnet6 2001:db8:3333::/56 {
>>      prefix6 2001:db8:3333:100:: 2001:db8:3333:111:: /64;
>>  }
>>  EOF
>> -rm -f /var/lib/dhcp/dhcpd6.leases
>> -touch /var/lib/dhcp/dhcpd6.leases
>> -chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases
>> -chmod 775 /var/lib/dhcp
>> -chmod 664 /var/lib/dhcp/dhcpd6.leases
>> -
>> -NETNS_START_TCPDUMP([server], [-nni s1], [server])
>> -NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], [dhcpd.pid])
>> +
>> +NETNS_START_DHCPD([server], [s1], [dhcpd.conf])
>>  check ovn-nbctl --wait=hv sync
>>
>>  AT_CHECK([ovn-appctl debug/dump-peer-ports | sort], [0], [dnl
>> --
>> 2.47.1
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Looks good to me, thanks.
> Acked-by: Ales Musil <[email protected]>

Thanks, Xavier, Ales and Ilya!  Applied to main and 25.03.  It didn't
apply cleanly to older branches.

Also, we still have one system test that starts dhcpd (for IPv4), "DHCP
RELAY", and uses /tmp to store things.  It would be nice to follow up
and clean that one up too.

Thanks,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to