ROUTE53: make caching of zones thread-safe
Hi @tresni!
While reviewing all the ZoneCreator implementations, I noticed that the ROUTE53 provider has an unsafe caching implementation for zones and EnsureZoneExists has a race-condition bug. EnsureZoneExists resets the cache before making the API call for creating a given zone. This might run concurrently to the processing of other zones and in turn could result in unexpected behavior: the cache could get re-populated before the creation of the zone completes and the new zone could be missing from the newly populated cache. This PR is making all access to the zone cache thread-safe and fixing the aforementioned race-condition. Would you mind giving this a try and let me know how it goes? Thanks!
Bonus: The zone cache is no longer reset when creating zones. Instead, the cache is populated with the zone that is included in the API response from creating the zone.
Part of https://github.com/StackExchange/dnscontrol/issues/3007
This passes integration tests. However those tests don't use any concurrency.
...
--- PASS: TestDNSProviders/dnscontroltest-r53.com/81:structured_TXT_as_native_records_***SKIPPED(disabled_by_only)***:Empty (0.04s)
--- PASS: TestDNSProviders/dnscontroltest-r53.com/82:PORKBUN_URLFWD_tests_***SKIPPED(disabled_by_only)***:Empty (0.07s)
--- PASS: TestDNSProviders/dnscontroltest-r53.com/83:GCORE_metadata_tests_***SKIPPED(disabled_by_only)***:Empty (0.04s)
--- PASS: TestDNSProviders/dnscontroltest-r53.com/Clean_Slate:Empty#52 (0.07s)
--- PASS: TestDNSProviders/dnscontroltest-r53.com/84:final:final (0.71s)
=== RUN TestDualProviders
Testing Profile="ROUTE53" TYPE="ROUTE53"
integration_test.go:414: Clearing everything
integration_test.go:408: #1:
- DELETE final.dnscontroltest-r53.com TXT "TestDNSProviders was successful!" ttl=300
integration_test.go:421: Adding test nameservers
integration_test.go:408: #1:
+ CREATE dnscontroltest-r53.com NS ns1.example.com. ttl=300
+ CREATE dnscontroltest-r53.com NS ns2.example.com. ttl=300
integration_test.go:424: Running again to ensure stability
integration_test.go:440: Removing test nameservers
integration_test.go:408: #1:
- DELETE dnscontroltest-r53.com NS ns1.example.com. ttl=300
- DELETE dnscontroltest-r53.com NS ns2.example.com. ttl=300
--- PASS: TestDualProviders (0.88s)
=== RUN TestNameserverDots
Testing Profile="ROUTE53" TYPE="ROUTE53"
=== RUN TestNameserverDots/No_trailing_dot_in_nameserver
--- PASS: TestNameserverDots (0.12s)
--- PASS: TestNameserverDots/No_trailing_dot_in_nameserver (0.00s)
PASS
ok github.com/StackExchange/dnscontrol/v4/integrationTest 158.090s
@tresni Can you try running this branch on a dnsconfig.js with a lot of ROUTE53 domains? The concurrency should work a lot better now.
Friendly reminder to rebase so that this can be merged. Thanks!
Thanks!