Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auth api: flush all caches when flushing #13514

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Conversation

zeha
Copy link
Collaborator

@zeha zeha commented Nov 20, 2023

Short description

So far we never flushed the DNSSEC caches, except when DELETEing a domain. However clearly some operations can affect the DNSSEC settings, and then the caches should go.

Also do this for the flush API, to be consistent, and for users writing to the DNSSEC settings/data externally.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@coveralls
Copy link

coveralls commented Nov 20, 2023

Pull Request Test Coverage Report for Build 7226248045

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 31 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.3%) to 57.993%

Files with Coverage Reduction New Missed Lines %
pdns/pollmplexer.cc 1 82.61%
modules/godbcbackend/sodbc.cc 2 70.8%
pdns/misc.cc 2 63.04%
pdns/signingpipe.cc 5 83.33%
pdns/packethandler.cc 8 73.07%
pdns/recursordist/test-syncres_cc1.cc 13 88.75%
Totals Coverage Status
Change from base Build 7222412548: 0.3%
Covered Lines: 108173
Relevant Lines: 155130

💛 - Coveralls

@klaus-nicat
Copy link
Contributor

Does this also clear the domainmetadata-cache?

@zeha
Copy link
Collaborator Author

zeha commented Nov 20, 2023

It should. Maybe having a test for it would be nice, but I'm not sure how to go about that.

@Habbie Habbie added this to the auth-4.9.0 milestone Nov 20, 2023
@zeha
Copy link
Collaborator Author

zeha commented Dec 6, 2023

Rebased this in the hope of getting an all green CI.

@zeha
Copy link
Collaborator Author

zeha commented Dec 15, 2023

  FAILED test_Lua.py::LuaHooksRecursorTest::testPreResolve - TypeError: msg is not a dns.message.Message but a <class 'NoneType'>
  FAILED test_Lua.py::LuaHooksRecursorTest::testVanillaNXD - ConnectionRefusedError: [Errno 111] Connection refused
  FAILED test_Lua.py::LuaHooksRecursorDistributesTest::testPreResolve - TypeError: msg is not a dns.message.Message but a <class 'NoneType'>
  FAILED test_Lua.py::LuaHooksRecursorDistributesTest::testVanillaNXD - ConnectionRefusedError: [Errno 111] Connection refused

these don't seem to be introduced by my PR?

@rgacogne
Copy link
Member

  FAILED test_Lua.py::LuaHooksRecursorTest::testPreResolve - TypeError: msg is not a dns.message.Message but a <class 'NoneType'>
  FAILED test_Lua.py::LuaHooksRecursorTest::testVanillaNXD - ConnectionRefusedError: [Errno 111] Connection refused
  FAILED test_Lua.py::LuaHooksRecursorDistributesTest::testPreResolve - TypeError: msg is not a dns.message.Message but a <class 'NoneType'>
  FAILED test_Lua.py::LuaHooksRecursorDistributesTest::testVanillaNXD - ConnectionRefusedError: [Errno 111] Connection refused

these don't seem to be introduced by my PR?

Looks like #13587 so a rebase should help.

@zeha
Copy link
Collaborator Author

zeha commented Dec 15, 2023

rebases for everyone! err, for this branch!

@rgacogne
Copy link
Member

FAILED test_SimpleForwardOverDoT.py::testSimpleForwardOverDoT::testA - AssertionError: False is not true : No AD flag found in the message for dns.google.

This one is indeed flaky, restarted.

@zeha
Copy link
Collaborator Author

zeha commented Dec 15, 2023

This one is indeed flaky, restarted.

green \o/

@Habbie Habbie modified the milestones: auth-4.9.0, auth-4.9.0-alpha2 Dec 15, 2023
So far we never flushed the DNSSEC caches, except when DELETEing a domain.
However clearly some operations can affect the DNSSEC settings, and then the
caches should go.

Also do this for the flush API, to be consistent, and for users writing to the
DNSSEC settings/data externally.
@zeha
Copy link
Collaborator Author

zeha commented Dec 15, 2023

I fixed the rebase and this has also resulted in #13641

@Habbie Habbie merged commit cecf970 into PowerDNS:master Dec 18, 2023
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants