diff mbox series

RFC: auto-t: fix netconfig to handle resolvconf values out of order

Message ID 20240228124018.334892-1-prestwoj@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series RFC: auto-t: fix netconfig to handle resolvconf values out of order | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-alpine-ci-fetch success Fetch PR
prestwoj/iwd-ci-gitlint success GitLint
prestwoj/iwd-ci-fetch success Fetch PR
prestwoj/iwd-ci-makedistcheck success Make Distcheck
prestwoj/iwd-alpine-ci-makedistcheck success Make Distcheck
prestwoj/iwd-ci-incremental_build success Incremental build not run PASS
prestwoj/iwd-alpine-ci-incremental_build success Incremental build not run PASS
prestwoj/iwd-ci-build success Build - Configure
prestwoj/iwd-alpine-ci-build success Build - Configure
prestwoj/iwd-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-ci-clang success clang PASS
prestwoj/iwd-ci-makecheck success Make Check
prestwoj/iwd-alpine-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-alpine-ci-makecheck success Make Check
prestwoj/iwd-ci-testrunner success test-runner PASS

Commit Message

James Prestwood Feb. 28, 2024, 12:40 p.m. UTC 
The slaac_test was one that would occationally fail, but very rarely,
due to the resolvconf log values appearing in an unexpected order.

This appears to be related to a typo in netconfig-commit which would
not set netconfig-domains and instead set dns_list. This was fixed
with a pending patch:

https://lore.kernel.org/iwd/20240227204242.1509980-1-denkenz@gmail.com/T/#u

But applying this now leads to testNetconfig failing slaac_test
100% of the time.

I'm not familiar enough with resolveconf to know if this test change
is ok, but based on the test behavior the expected log and disk logs
are the same, just in the incorrect order. I'm not sure if this the
log order is deterministic so instead the check now iterates the
expected log and verifies each value appears once in the resolvconf
log.

Here is an example of the expected vs disk logs after running the
test:

Expected:

-a wlan1.dns
nameserver 192.168.1.2
nameserver 3ffe:501:ffff:100::10
nameserver 3ffe:501:ffff:100::50
-a wlan1.domain
search test1
search test2

Resolvconf log:

-a wlan1.domain
search test1
search test2
-a wlan1.dns
nameserver 192.168.1.2
nameserver 3ffe:501:ffff:100::10
nameserver 3ffe:501:ffff:100::50
---
 autotests/testNetconfig/slaac_test.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

KeithG Feb. 28, 2024, 3:11 p.m. UTC | #1 
James,

Does this work if the system is using systemd-resolved instead of resolvconf?

Keith

On Wed, Feb 28, 2024 at 6:52 AM James Prestwood <prestwoj@gmail.com> wrote:
>
> The slaac_test was one that would occationally fail, but very rarely,
> due to the resolvconf log values appearing in an unexpected order.
>
> This appears to be related to a typo in netconfig-commit which would
> not set netconfig-domains and instead set dns_list. This was fixed
> with a pending patch:
>
> https://lore.kernel.org/iwd/20240227204242.1509980-1-denkenz@gmail.com/T/#u
>
> But applying this now leads to testNetconfig failing slaac_test
> 100% of the time.
>
> I'm not familiar enough with resolveconf to know if this test change
> is ok, but based on the test behavior the expected log and disk logs
> are the same, just in the incorrect order. I'm not sure if this the
> log order is deterministic so instead the check now iterates the
> expected log and verifies each value appears once in the resolvconf
> log.
>
> Here is an example of the expected vs disk logs after running the
> test:
>
> Expected:
>
> -a wlan1.dns
> nameserver 192.168.1.2
> nameserver 3ffe:501:ffff:100::10
> nameserver 3ffe:501:ffff:100::50
> -a wlan1.domain
> search test1
> search test2
>
> Resolvconf log:
>
> -a wlan1.domain
> search test1
> search test2
> -a wlan1.dns
> nameserver 192.168.1.2
> nameserver 3ffe:501:ffff:100::10
> nameserver 3ffe:501:ffff:100::50
> ---
>  autotests/testNetconfig/slaac_test.py | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/autotests/testNetconfig/slaac_test.py b/autotests/testNetconfig/slaac_test.py
> index 26ae0e46..5aeb730e 100644
> --- a/autotests/testNetconfig/slaac_test.py
> +++ b/autotests/testNetconfig/slaac_test.py
> @@ -81,14 +81,21 @@ class Test(unittest.TestCase):
>          self.assertEqual(expected_routes6, set(testutil.get_routes6(ifname)))
>
>          rclog = open('/tmp/resolvconf.log', 'r')
> -        entries = rclog.readlines()
> +        entries = [l.strip() for l in rclog.readlines()[-7:]]
>          rclog.close()
> -        expected_rclog = ['-a %s.dns\n' % (ifname,), 'nameserver 192.168.1.2\n',
> -                'nameserver 3ffe:501:ffff:100::10\n', 'nameserver 3ffe:501:ffff:100::50\n',
> -                '-a %s.domain\n' % (ifname,), 'search test1\n', 'search test2\n']
> -        # Every resolvconf -a run overwrites the previous settings.  Check the last seven lines
> -        # of our log since we care about the end result here.
> -        self.assertEqual(expected_rclog, entries[-7:])
> +        expected_rclog = [
> +            '-a %s.dns' % (ifname,),
> +            'nameserver 192.168.1.2',
> +            'nameserver 3ffe:501:ffff:100::10',
> +            'nameserver 3ffe:501:ffff:100::50',
> +            '-a %s.domain' % (ifname,),
> +            'search test1',
> +            'search test2'
> +        ]
> +
> +        for line in expected_rclog:
> +            self.assertIn(line, entries)
> +            expected_rclog.remove(line)
>
>          device.disconnect()
>          condition = 'not obj.connected'
> --
> 2.34.1
>
>
James Prestwood Feb. 28, 2024, 3:12 p.m. UTC | #2 
Hi Keith,

On 2/28/24 7:11 AM, KeithG wrote:
> James,
>
> Does this work if the system is using systemd-resolved instead of resolvconf?
This is only a test change and doesn't effect the core code.
>
> Keith
>
> On Wed, Feb 28, 2024 at 6:52 AM James Prestwood <prestwoj@gmail.com> wrote:
>> The slaac_test was one that would occationally fail, but very rarely,
>> due to the resolvconf log values appearing in an unexpected order.
>>
>> This appears to be related to a typo in netconfig-commit which would
>> not set netconfig-domains and instead set dns_list. This was fixed
>> with a pending patch:
>>
>> https://lore.kernel.org/iwd/20240227204242.1509980-1-denkenz@gmail.com/T/#u
>>
>> But applying this now leads to testNetconfig failing slaac_test
>> 100% of the time.
>>
>> I'm not familiar enough with resolveconf to know if this test change
>> is ok, but based on the test behavior the expected log and disk logs
>> are the same, just in the incorrect order. I'm not sure if this the
>> log order is deterministic so instead the check now iterates the
>> expected log and verifies each value appears once in the resolvconf
>> log.
>>
>> Here is an example of the expected vs disk logs after running the
>> test:
>>
>> Expected:
>>
>> -a wlan1.dns
>> nameserver 192.168.1.2
>> nameserver 3ffe:501:ffff:100::10
>> nameserver 3ffe:501:ffff:100::50
>> -a wlan1.domain
>> search test1
>> search test2
>>
>> Resolvconf log:
>>
>> -a wlan1.domain
>> search test1
>> search test2
>> -a wlan1.dns
>> nameserver 192.168.1.2
>> nameserver 3ffe:501:ffff:100::10
>> nameserver 3ffe:501:ffff:100::50
>> ---
>>   autotests/testNetconfig/slaac_test.py | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/autotests/testNetconfig/slaac_test.py b/autotests/testNetconfig/slaac_test.py
>> index 26ae0e46..5aeb730e 100644
>> --- a/autotests/testNetconfig/slaac_test.py
>> +++ b/autotests/testNetconfig/slaac_test.py
>> @@ -81,14 +81,21 @@ class Test(unittest.TestCase):
>>           self.assertEqual(expected_routes6, set(testutil.get_routes6(ifname)))
>>
>>           rclog = open('/tmp/resolvconf.log', 'r')
>> -        entries = rclog.readlines()
>> +        entries = [l.strip() for l in rclog.readlines()[-7:]]
>>           rclog.close()
>> -        expected_rclog = ['-a %s.dns\n' % (ifname,), 'nameserver 192.168.1.2\n',
>> -                'nameserver 3ffe:501:ffff:100::10\n', 'nameserver 3ffe:501:ffff:100::50\n',
>> -                '-a %s.domain\n' % (ifname,), 'search test1\n', 'search test2\n']
>> -        # Every resolvconf -a run overwrites the previous settings.  Check the last seven lines
>> -        # of our log since we care about the end result here.
>> -        self.assertEqual(expected_rclog, entries[-7:])
>> +        expected_rclog = [
>> +            '-a %s.dns' % (ifname,),
>> +            'nameserver 192.168.1.2',
>> +            'nameserver 3ffe:501:ffff:100::10',
>> +            'nameserver 3ffe:501:ffff:100::50',
>> +            '-a %s.domain' % (ifname,),
>> +            'search test1',
>> +            'search test2'
>> +        ]
>> +
>> +        for line in expected_rclog:
>> +            self.assertIn(line, entries)
>> +            expected_rclog.remove(line)
>>
>>           device.disconnect()
>>           condition = 'not obj.connected'
>> --
>> 2.34.1
>>
>>
Denis Kenzior Feb. 28, 2024, 3:23 p.m. UTC | #3 
Hi James,

On 2/28/24 06:40, James Prestwood wrote:
> The slaac_test was one that would occationally fail, but very rarely,
> due to the resolvconf log values appearing in an unexpected order.
> 
> This appears to be related to a typo in netconfig-commit which would
> not set netconfig-domains and instead set dns_list. This was fixed
> with a pending patch:
> 
> https://lore.kernel.org/iwd/20240227204242.1509980-1-denkenz@gmail.com/T/#u
> 
> But applying this now leads to testNetconfig failing slaac_test
> 100% of the time.
> 
> I'm not familiar enough with resolveconf to know if this test change
> is ok, but based on the test behavior the expected log and disk logs
> are the same, just in the incorrect order. I'm not sure if this the
> log order is deterministic so instead the check now iterates the
> expected log and verifies each value appears once in the resolvconf
> log.
> 
> Here is an example of the expected vs disk logs after running the
> test:
> 
> Expected:
> 
> -a wlan1.dns
> nameserver 192.168.1.2
> nameserver 3ffe:501:ffff:100::10
> nameserver 3ffe:501:ffff:100::50
> -a wlan1.domain
> search test1
> search test2
> 
> Resolvconf log:
> 
> -a wlan1.domain
> search test1
> search test2
> -a wlan1.dns
> nameserver 192.168.1.2
> nameserver 3ffe:501:ffff:100::10
> nameserver 3ffe:501:ffff:100::50
> ---
>   autotests/testNetconfig/slaac_test.py | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
> 

Looks reasonable to me.  I'm okay applying this even as an RFC, unless you want 
to resend?

Regards,
-Denis
diff mbox series

Patch

diff --git a/autotests/testNetconfig/slaac_test.py b/autotests/testNetconfig/slaac_test.py
index 26ae0e46..5aeb730e 100644
--- a/autotests/testNetconfig/slaac_test.py
+++ b/autotests/testNetconfig/slaac_test.py
@@ -81,14 +81,21 @@  class Test(unittest.TestCase):
         self.assertEqual(expected_routes6, set(testutil.get_routes6(ifname)))
 
         rclog = open('/tmp/resolvconf.log', 'r')
-        entries = rclog.readlines()
+        entries = [l.strip() for l in rclog.readlines()[-7:]]
         rclog.close()
-        expected_rclog = ['-a %s.dns\n' % (ifname,), 'nameserver 192.168.1.2\n',
-                'nameserver 3ffe:501:ffff:100::10\n', 'nameserver 3ffe:501:ffff:100::50\n',
-                '-a %s.domain\n' % (ifname,), 'search test1\n', 'search test2\n']
-        # Every resolvconf -a run overwrites the previous settings.  Check the last seven lines
-        # of our log since we care about the end result here.
-        self.assertEqual(expected_rclog, entries[-7:])
+        expected_rclog = [
+            '-a %s.dns' % (ifname,),
+            'nameserver 192.168.1.2',
+            'nameserver 3ffe:501:ffff:100::10',
+            'nameserver 3ffe:501:ffff:100::50',
+            '-a %s.domain' % (ifname,),
+            'search test1',
+            'search test2'
+        ]
+
+        for line in expected_rclog:
+            self.assertIn(line, entries)
+            expected_rclog.remove(line)
 
         device.disconnect()
         condition = 'not obj.connected'