Skip to content

logging: Try all upstream targets before logging failure #248#715

Open
hartmark wants to merge 1 commit into
containers:mainfrom
hartmark:bug/248_dns_request_failed
Open

logging: Try all upstream targets before logging failure #248#715
hartmark wants to merge 1 commit into
containers:mainfrom
hartmark:bug/248_dns_request_failed

Conversation

@hartmark

@hartmark hartmark commented Jul 7, 2026

Copy link
Copy Markdown

I had AI help me fix this but it seems that with this fix aardvark doesn't spam log about the failures as the second upstream resolver try worked.

@Luap99

Luap99 commented Jul 8, 2026

Copy link
Copy Markdown
Member

https://github.com/containers/common/blob/main/CONTRIBUTING.md#sign-your-prs
we cannot accept contributions with unclear copyright status

Signed-off-by: Markus Hartung <mail@hartmark.se>
@hartmark
hartmark force-pushed the bug/248_dns_request_failed branch from 0871a11 to 599f8b8 Compare July 8, 2026 11:19
@hartmark

hartmark commented Jul 8, 2026

Copy link
Copy Markdown
Author

https://github.com/containers/common/blob/main/CONTRIBUTING.md#sign-your-prs we cannot accept contributions with unclear copyright status

I have attributed the change to myself as the change is quite trivial. And any random developer would come up with this fix with or without AI

Comment thread src/dns/coredns.rs
proto: Protocol,
) {
let mut timeout = DEFAULT_TIMEOUT;
// Remember do not divide by 0.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is removing a unrelated comment

Comment thread src/dns/coredns.rs
if !nameservers.is_empty() {
timeout = Duration::from_secs(5) / nameservers.len() as u32
}
// forward dns request to hosts's /etc/resolv.conf

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here the comment is deleted for no reason?

Comment thread src/dns/coredns.rs
Comment on lines -314 to -315
// request resolved from following resolver so
// break and don't try other resolvers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and again here the comment is removed

Comment thread src/dns/coredns.rs
"dns request {} failed: all nameservers exhausted",
req.id
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see the point in this. this will still spam if you have just one server all or all fail...

I think having the failed requests on debug is fine, no need to spam the journal with the same errors over and over again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants