Request for testing: GetZoneRecords() refactor #2214
Comments
|
Note to @vatsalyagoel @tresni @Deraen @riyadhalnur @KaiSchwarz-cnic @jpbede: I've checked your providers off because your provider passed the automated tests. However I would appreciate you testing |
|
Hello. When I run |
|
@riku22 Ah, I typed the wrong git command. Please try again. Thanks! |
|
@tlimoncelli Favour: could you ask testers also to verify whether the new LOC record works for them (update the .Can() and submit Pr if so) 😄 |
|
LuaDNS integration tests passed. Integration test resultDiff2 integration test result |
|
✅ Loopia - One test 🟡 integration✅ Loopia - One test 🟡 diff2 integrationPretty sure this case was weird before your work. ✅ Loopia - preview+push |
Hey folks! This might be a good chance to see if the new LOC record is supported! |
|
Passes all tests for Gcore provider. Tested with diff2 on/off, and with integration test and my own config. |
|
Integration tests pass for oracle (both with and without |
Both of those tests look like the same issue. It is trying to change "foo" to "foo.example.com.". My guess is somewhere there should be a loop that turns all the targets for MX, CNAME, NS (and possibly others) from shortnames to FQDNs. Let me know how I can help! |
|
Integration tests for HEDNS all pass (diff & diff2) except |
PowerDNS works in production. |
|
Works in production. No new failures between master and the new branch. I did find the integration tests failing which seems like a change on transIP there behalve and unrelated to your changes. Do note: the new tag v3.30.0test is not available. I used the branch of the PR. |
|
% git checkout v3.30.0test
error: pathspec 'v3.30.0test' did not match any file(s) known to git
Should I checkout tlim_corrector ?
From: Vincent Hagen ***@***.***>
Reply-To: StackExchange/dnscontrol ***@***.***>
Date: Friday, March 24, 2023 at 5:20 PM
To: StackExchange/dnscontrol ***@***.***>
Cc: "Vernick, Steve" ***@***.***>, Mention ***@***.***>
Subject: Re: [StackExchange/dnscontrol] Request for testing: GetZoneRecords() refactor (Issue #2214)
Works in production. No new failures between master and the new branch. Do note: the new tag v3.30.0test is not available. I used the branch of the PR.
I did find the integration tests failing which seems like a change on transIP there behalve and unrelated to your changes.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/StackExchange/dnscontrol/issues/2214*issuecomment-1483417573__;Iw!!GjvTz_vk!VNPcT0HfCwQWv9LL4V7nLAahWw751_a-MR_nnRdufjr15SelmXllrFn545jCvRxQpSKpBL2gspymqKpBFywt-T8$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AZ7IINFLM47AAM4NEFGLK4DW5YFW3ANCNFSM6AAAAAAWENODGA__;!!GjvTz_vk!VNPcT0HfCwQWv9LL4V7nLAahWw751_a-MR_nnRdufjr15SelmXllrFn545jCvRxQpSKpBL2gspymqKpBjxs3SvY$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Alas it is not looking good for namecheap go test -v -verbose -provider NAMECHEAP resultfull outputgo test -v -verbose -provider NAMECHEAP --diff2 resultfull outputI couldn't see a way of adding a LOC record in the namecheap UI so I presume that is not (yet) possible and did not try enabling it in the provider. I haven't got enough time today to debug but I did run the tests against master and confirm they pass with the previously noted exception of Is it fairly obvious to see what the problem is when I do have some time? |
Thanks for letting me know!
Good. I've updated the doc to recommend doing that instead. For some reason some people are seeing the tag and others aren't. |
|
svernick Yes, please us the branch. (I've updated the OP since the tag isn't working for most people) |
|
willpower232 I think 5fec290 should fix it. Try the tlim_corrector branch and let me know. It that doesn't fix it... Look at line 140 to see some code that I didn't include because it seemed redundant. Maybe it was needed after all? My guess is that |
|
PORKBUN integration tests passed. Integration test resultDiff2 integration test result |
|
I tested the Digitalocean provider with a production-like config, and it worked. I'm no longer using Digitalocean, but I could run my Route53 config after commenting out a few R53-specific records. |
|
Good news that the changes seem to have corrected most of the situation, there are now consistent failures for both with and without diff2 the code you mentioned seems to be missing a |
OVH statusIntegration test results ✅--- PASS: TestDNSProviders (134.96s) `diff2` integration tests ✅--- PASS: TestDNSProviders (135.40s) LOC 🚫 doesn't work as is for OVH (in diff2 and regular modes, I'll have a look in the week) |
I fixed that a few minutes later (oops!). Take a look at the latest tlim_corrector branch and let me know if that worked. |
|
I was referring to these lines missing dnscontrol/providers/namecheap/namecheapProvider.go Lines 153 to 162 in a58fb77 the latest commit of the branch doesn't seem to change anything I'm afraid |
Does the current tlim_corrector branch compile and successfully run the integration tests? If so, then that commented out code can be deleted.
Make sure the branch you are using has |
|
I'm currently at a58fb77 as of yesterday, the setLabel seems to have fixed most of the tests but these ones still fail github is having a bad time right now though so if you have pushed anything since then, it might not have actually pushed 😅 |
This is the 2nd provider to have this issue. I think it might be on the dnscontrol end. Let me poke around. |
I spoke too soon. I don't think that's the issue. Could you try these tests on |
|
alas there is only one fail on the master branch in both with and without diff2, the |
|
@willpower232 Let's move this discussion to a separate issue. I've created #2238 |
|
I did some major surgery to the code of these 3 providers. Please run the tests asap as I don't want to break anything.
This provider had major changes but since I have a Cloudflare account, I was able to test them myself. However I encourage you to do extra testing yourself:
I would appreciate it if people could complete these tests by April 14 or sooner. Thanks! |
|
I started to run the first tests but as usual it takes me a lot of time because of the hourly and daily rate limits of desec. |
Wow! That sounds annoying! I have a similar situation with the CSCGLOBAL provider (12 hours). Here are some things I've done to work around this:
Hope those help! Tom |
|
ClouDNS LGTM. one test failed but also failed in master branch, tracked in #2244 Output: legacy.txt diff2.txt (I'm not ClouDNS maintainer, the maintainer seems busy so I think I can test for him) |
|
@imlonghao Thanks for the assist! |
|
All tests for DNSimple work, including doing production previews and push with the |
|
Hi folks, integration & production (preview/push) tests for |
|
@tlimoncelli thanks for testing us in OT&E. We addressed a little refactoring PR independent of this request for testing. We tested on our end in OT&E as well -> working. Running the integration tests on production isn't useful imho - a domain name is required for doing that (no problem), but the DNS Functionality is not coming with differences for the production environment compared to the OT&E environment. I don't see a benefit in that regard. |
Sure, I've always assumed that testing in OT&E is sufficient. My request to test in production was a request to test any dnsconfig.js configurations you might be using. Tom |
|
The integration tests passed for HETZNER with both diff modes. I'll send you a PR with some cleanup later this weekend. |
|
The integration tests for deSec passed, good to go from my side. |
|
Hey folks! Friendly remind to those that haven't spoken up yet:
I know that Github often doesn't deliver email reliably. I hope these messages are getting through. The changes made to dnsmadeeasy are particularly big, so if anyone knows @vojtad or has an account there and can run the tests, I'd appreciate it. |
|
Hi! We've migrated our DNS from dnsmadeeasy to cloudflare but I will try to do my best to keep the provider up to date. All tests are passing for dnsmadeeasy. However, I am not able to test it in production environment anymore. |
|
akamaiedgedns tests passed. go test -v -verbose -timeout 15m -provider AKAMAIEDGEDNS go test -v -verbose -timeout 15m -provider AKAMAIEDGEDNS --diff2 |
Should I make the change to the 'tlim_corrector' branch? That is, submit a PR to pull my change into 'tlim_corrector'? |
Base the PR against master. I'll merge it up to tlim_corrector afterwords. Thanks for checking! |
|
This is going to be merged tomorrow or Saturday (May 14 or 15th). Last call for testing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
and others
Hi maintainers!
I need your help! Please test this pre-release. I moved a lot of code around. It shouldn't have broken anything but I'd like your testing to be sure. I would appreciate it if your testing could be done by April 13, 2023.
This is a big PR (#2120) and needs extra testing by the provider maintainers and anyone else interested.
Speaking of consistent names, in the future anything that DNSControl corrects will require two functions: If it manages FOO, the functions will be GetFOO() and GetFOOCorrections(). In the future I'll do some global renames related to this.
If you'd like to see the exact change, the PR is #2120
Please test this release:
NOTE: Previously this suggested using the "v3.30.0test" tag. I changed it to the "tlim_corrector" branch because the tag seems to be not updating properly.
Run the integration test (replace NAMEDOTCOM with your provider)
Test it on your production configuration:
If all of that works, please consider making real changes using
--diff2 pushProject status
I would appreciate it if people could complete these tests by April 14 (3+ weeks). Thanks!
The text was updated successfully, but these errors were encountered: