Limit the DNS responses after getting the NodeRecords (fixes 0 A/AAAA responses)

[42wim]

Fixes the issue when a service has A and AAAA records, the maxServiceResponses (3) records filtering is done at the wrong place.

E.g. asking an A record for a service with 3 A and 3 AAAA records could return 0-3 records depending on how they're shuffled. (Because it would send 0 A records and 3 AAAA to serviceNodeRecords, which only answers the A records. 0 A records in this example)

This patch does the limitation after the correct responses are filled in.

[ryanbreen]

Could we get a test to cover this fix?

[42wim]

Sure, TestDNS_ServiceLookup_MaxResponses added

[jovandeginste]

Tested in our pilot-setup, and verified to work. We have 3 consul servers registered with both ipv4 and ipv6 addresses (service name 'consul'). Before the fix:

• ANY request gives 1, 2 or 3 results (mixed A and AAAA, any combination including 0 - no idea why it does not always return 3 records)
• A request gives 0, 1, 2 or 3 A records - not what we want!
• AAAA request gives 0, 1, 2 or 3 AAAA records - not what we want!

Test:

# for x in A AAAA ANY; do for i in {1..1000}; do echo -n "$x: "; dig $x consul.service.consul  +short | wc -l; done | sort | uniq -c; done
     99 A: 0
    501 A: 1
    361 A: 2
     39 A: 3
    110 AAAA: 0
    532 AAAA: 1
    335 AAAA: 2
     23 AAAA: 3
     18 ANY: 1
    373 ANY: 2
    609 ANY: 3

When 0 records are returned, no SOA record is returned either (fixed with PR #1195) - our central DNS servers then 'forget' about our consul dns servers...

After fix, ANY, A and AAAA always give 3 records (if 3 are available, obviously):

Test:

# for x in A AAAA ANY; do for i in {1..1000}; do echo -n "$x: "; dig $x consul.service.consul  +short | wc -l; done | sort | uniq -c; done
   1000 A: 3
   1000 AAAA: 3
   1000 ANY: 3

[ryanbreen]

I merged GH-1195 and would be happy to merge this, but it has conflicts. Can you update the PR?

[42wim]

Thanks, PR is now rebased on master.

[ryanbreen]

Great, thanks for this!