From 2a640adbc1c02f1e9bd3a105d572a7bf49505d9f Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 5 Nov 2025 14:57:33 +0100 Subject: [PATCH 1/4] remove dns_zone_expire dead code Removing `dns_zone_expire` function which is never called (the zone expiration is detected internally in `lib/dns/zone.c`). --- lib/dns/include/dns/zone.h | 11 ----------- lib/dns/zone.c | 9 --------- 2 files changed, 20 deletions(-) diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 70b79471a4..e6cbb76372 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -593,17 +593,6 @@ dns_zone_markdirty(dns_zone_t *zone); *\li 'zone' to be a valid zone. */ -void -dns_zone_expire(dns_zone_t *zone); -/*%< - * Mark the zone as expired. If the zone requires dumping cause it to - * be initiated. Set the refresh and retry intervals to there default - * values and unload the zone. - * - * Require - *\li 'zone' to be a valid zone. - */ - void dns_zone_refresh(dns_zone_t *zone); /*%< diff --git a/lib/dns/zone.c b/lib/dns/zone.c index a9acedda42..546de131e3 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -11762,15 +11762,6 @@ again: UNLOCK_ZONE(zone); } -void -dns_zone_expire(dns_zone_t *zone) { - REQUIRE(DNS_ZONE_VALID(zone)); - - LOCK_ZONE(zone); - zone_expire(zone); - UNLOCK_ZONE(zone); -} - static void zone_expire(dns_zone_t *zone) { dns_db_t *db = NULL; From 0b93d5725b6c34028a1ffe4471ff973eafca4516 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 5 Nov 2025 15:06:03 +0100 Subject: [PATCH 2/4] add query ID to the query trace message Adding the query ID to the query trace message. The log is now as the following (id is at the end): query client=0x7f75c5017000 thread=0x7f75c6dfe680(foo.fr/A): \ client attr:0x22300, query attr:0x700, restarts:0, \ origqname:foo.fr, timer:0, authdb:0, referral:0, id:21338 This should help debugging tests, in particular to quickly get a specific query from the logs. --- lib/ns/query.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 8733e0938d..3e7e37d56e 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -5283,12 +5283,13 @@ query_trace(query_ctx_t *qctx) { snprintf(mbuf, sizeof(mbuf) - 1, "client attr:0x%x, query attr:0x%X, restarts:%u, " - "origqname:%s, timer:%d, authdb:%d, referral:%d", + "origqname:%s, timer:%d, authdb:%d, referral:%d, id:%hu", qctx->client->inner.attributes, qctx->client->query.attributes, qctx->client->query.restarts, qbuf, (int)qctx->client->query.timerset, (int)qctx->client->query.authdbset, - (int)qctx->client->query.isreferral); + (int)qctx->client->query.isreferral, + qctx->client->message->id); CCTRACE(ISC_LOG_DEBUG(3), mbuf); #else /* ifdef WANT_QUERYTRACE */ UNUSED(qctx); From 611a556a6cc688afd88f36ff13b3283c8c937503 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 5 Nov 2025 15:08:51 +0100 Subject: [PATCH 3/4] harden ede24 system test There was a random failure of ede24 system test. While this is still a bit speculative, the two reasons were: - in the case of `test_ede24_noloaded` the test might attempt to early (before the zone actually transfered on the secondary server) to query ns2. - still in the case of `test_ede24_noloaded`, even after waiting for transfer succeed logs, if the CI machine is slow, the zone could be expired before the request checking the secondary zone works because the expiration time of the zone was very short (1s). Moving this expiration time to 3 seconds should be enough (while not making the test execution too much longer when waiting for the zone expiration). - in the case of `test_ede24_expired`, the zone expired flag is flipped and the log message is printed immediately after. However, it is possible that because the flag is set using a relaxed atomic operation, another thread process the query and gets the previous (non-expired) value of the flag. In order to workaround this, the test now also expects another log written after the zone expiration (stop timers) on the next UV tick. --- bin/tests/system/ede24/ns1/foo.fr.db | 2 +- bin/tests/system/ede24/tests_ede24.py | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/ede24/ns1/foo.fr.db b/bin/tests/system/ede24/ns1/foo.fr.db index f3937c043c..262895e1a9 100644 --- a/bin/tests/system/ede24/ns1/foo.fr.db +++ b/bin/tests/system/ede24/ns1/foo.fr.db @@ -14,7 +14,7 @@ foo.fr. IN SOA ns.foo.fr. op.foo.fr. ( 3 ; serial 1 ; refresh 1 ; retry - 1 ; expire + 3 ; expire 60 ; minimum ) foo.fr. NS ns.foo.fr. diff --git a/bin/tests/system/ede24/tests_ede24.py b/bin/tests/system/ede24/tests_ede24.py index 7c5771715e..a492ff7457 100644 --- a/bin/tests/system/ede24/tests_ede24.py +++ b/bin/tests/system/ede24/tests_ede24.py @@ -34,7 +34,10 @@ def check_soa_servfail_ede24(edemsg): def test_ede24_noloaded(ns1, ns2): - # Sanity check that everything works first + # Sanity check that everything works first, once we're sure the foo.fr zone + # has transfered to ns2. + with ns2.watch_log_from_start() as watcher: + watcher.wait_for_line("Transfer status: success") check_soa_noerror() # Stop all servers, and we'll restart only ns2. @@ -59,7 +62,11 @@ def test_ede24_expired(ns1, ns2): # Stop the primary and wait for expiration of the zone in the secondary. with ns2.watch_log_from_here() as watcher: ns1.stop() - watcher.wait_for_line(" zone foo.fr/IN: expired") + log_sequence = [ + " zone foo.fr/IN: expired", + " zone foo.fr/IN: stop zone timer", + ] + watcher.wait_for_sequence(log_sequence) # ns2 can't answer anymore. check_soa_servfail_ede24("zone expired") From 11a4df7ec5af1ac9197d0870f28fe83cf7b4ff48 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 6 Nov 2025 14:35:33 +0100 Subject: [PATCH 4/4] split ede24 system test into separate modules Because ede24 system tests require stopping/restarting server, there is always the risk that the test ends (with a failure) with server in an wrong and impredictible state. This would make the other tests to fail in a strange way as well. To avoid this problem, split the test into different modules, so if a module fails, the other module is not impacted as it uses separate server instances. --- bin/tests/system/ede24/common.py | 39 ++++++++++ bin/tests/system/ede24/tests_ede24.py | 78 ------------------- bin/tests/system/ede24/tests_ede24_expired.py | 36 +++++++++ .../system/ede24/tests_ede24_noloaded.py | 28 +++++++ 4 files changed, 103 insertions(+), 78 deletions(-) create mode 100644 bin/tests/system/ede24/common.py delete mode 100644 bin/tests/system/ede24/tests_ede24.py create mode 100644 bin/tests/system/ede24/tests_ede24_expired.py create mode 100644 bin/tests/system/ede24/tests_ede24_noloaded.py diff --git a/bin/tests/system/ede24/common.py b/bin/tests/system/ede24/common.py new file mode 100644 index 0000000000..89b37b2993 --- /dev/null +++ b/bin/tests/system/ede24/common.py @@ -0,0 +1,39 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import isctest + + +def check_soa_noerror(): + msg = isctest.query.create("foo.fr", "SOA") + res = isctest.query.udp(msg, "10.53.0.2") + isctest.check.noerror(res) + + +def check_soa_servfail_ede24(edemsg): + msg = isctest.query.create("foo.fr", "SOA") + res = isctest.query.udp(msg, "10.53.0.2") + isctest.check.servfail(res) + + # Few CI machines uses old version of dnspython which doesn't supports + # EDNS, so we effectively bypass the check for those one. (It's fine, a + # bunch of other CI machines _does_ have recent version of dnspython). + if hasattr(res, "extended_errors"): + assert len(res.extended_errors()) == 1 + assert res.extended_errors()[0].to_text() == f"EDE 24 (Invalid Data): {edemsg}" + + +def check_ns2_ready(ns2): + # Sanity check that everything works first, once we're sure the foo.fr zone + # has transfered to ns2. + with ns2.watch_log_from_start() as watcher: + watcher.wait_for_line("Transfer status: success") + check_soa_noerror() diff --git a/bin/tests/system/ede24/tests_ede24.py b/bin/tests/system/ede24/tests_ede24.py deleted file mode 100644 index a492ff7457..0000000000 --- a/bin/tests/system/ede24/tests_ede24.py +++ /dev/null @@ -1,78 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import os - -import isctest - - -def check_soa_noerror(): - msg = isctest.query.create("foo.fr", "SOA") - res = isctest.query.udp(msg, "10.53.0.2") - isctest.check.noerror(res) - - -def check_soa_servfail_ede24(edemsg): - msg = isctest.query.create("foo.fr", "SOA") - res = isctest.query.udp(msg, "10.53.0.2") - isctest.check.servfail(res) - - # Few CI machines uses old version of dnspython which doesn't supports - # EDNS, so we effectively bypass the check for those one. (It's fine, a - # bunch of other CI machines _does_ have recent version of dnspython). - if hasattr(res, "extended_errors"): - assert len(res.extended_errors()) == 1 - assert res.extended_errors()[0].to_text() == f"EDE 24 (Invalid Data): {edemsg}" - - -def test_ede24_noloaded(ns1, ns2): - # Sanity check that everything works first, once we're sure the foo.fr zone - # has transfered to ns2. - with ns2.watch_log_from_start() as watcher: - watcher.wait_for_line("Transfer status: success") - check_soa_noerror() - - # Stop all servers, and we'll restart only ns2. - ns1.stop() - ns2.stop() - with ns2.watch_log_from_here() as watcher: - ns2.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) - watcher.wait_for_line("failure trying primary 10.53.0.1") - - # ns2 attempts an XFR but ns1 since is off the zone DB can't be loaded. - check_soa_servfail_ede24("zone not loaded") - - -def test_ede24_expired(ns1, ns2): - # Restart ns1 then checks the server notify the zone in ns2 and ns2 serves - # the zone again. - with ns2.watch_log_from_here() as watcher: - ns1.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) - watcher.wait_for_line("Transfer status: success") - check_soa_noerror() - - # Stop the primary and wait for expiration of the zone in the secondary. - with ns2.watch_log_from_here() as watcher: - ns1.stop() - log_sequence = [ - " zone foo.fr/IN: expired", - " zone foo.fr/IN: stop zone timer", - ] - watcher.wait_for_sequence(log_sequence) - - # ns2 can't answer anymore. - check_soa_servfail_ede24("zone expired") - - # Restart the primary and wait for the zone to be back up again. - with ns2.watch_log_from_here() as watcher: - ns1.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) - watcher.wait_for_line("Transfer status: success") - check_soa_noerror() diff --git a/bin/tests/system/ede24/tests_ede24_expired.py b/bin/tests/system/ede24/tests_ede24_expired.py new file mode 100644 index 0000000000..02e129f4ba --- /dev/null +++ b/bin/tests/system/ede24/tests_ede24_expired.py @@ -0,0 +1,36 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import os + +from ede24.common import check_ns2_ready, check_soa_noerror, check_soa_servfail_ede24 + + +def test_ede24_expired(ns1, ns2): + check_ns2_ready(ns2) + + # Stop the primary and wait for expiration of the zone in the secondary. + with ns2.watch_log_from_here() as watcher: + ns1.stop() + log_sequence = [ + " zone foo.fr/IN: expired", + " zone foo.fr/IN: stop zone timer", + ] + watcher.wait_for_sequence(log_sequence) + + # ns2 can't answer anymore. + check_soa_servfail_ede24("zone expired") + + # Restart the primary and wait for the zone to be back up again. + with ns2.watch_log_from_here() as watcher: + ns1.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) + watcher.wait_for_line("Transfer status: success") + check_soa_noerror() diff --git a/bin/tests/system/ede24/tests_ede24_noloaded.py b/bin/tests/system/ede24/tests_ede24_noloaded.py new file mode 100644 index 0000000000..90b77b8a55 --- /dev/null +++ b/bin/tests/system/ede24/tests_ede24_noloaded.py @@ -0,0 +1,28 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import os + +from ede24.common import check_ns2_ready, check_soa_servfail_ede24 + + +def test_ede24_noloaded(ns1, ns2): + check_ns2_ready(ns2) + + # Stop all servers, and we'll restart only ns2. + ns1.stop() + ns2.stop() + with ns2.watch_log_from_here() as watcher: + ns2.start(["--noclean", "--restart", "--port", os.environ["PORT"]]) + watcher.wait_for_line("failure trying primary 10.53.0.1") + + # ns2 attempts an XFR but ns1 since is off the zone DB can't be loaded. + check_soa_servfail_ede24("zone not loaded")