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

[ARO-5445] DNS checker logging custom DNS server as an error #3453

Merged

Conversation

rolandmkunkel
Copy link
Contributor

Which issue this PR addresses:

Fixes https://issues.redhat.com/browse/ARO-5445
What this PR does / why we need it:

Customer reported ARO operator errors in the logs when custom DNS servers are configured in forward plugin:
https://portal.microsofticm.com/imp/v3/incidents/incident/469705806/summary

This is a supported setup as per our docs and should not be logged as an error.

The DNS checks had been setup initially to be included as cluster conditions for SRE awareness (this did not log the status conditions as errors):
https://github.com/Azure/ARO-RP/blob/3e69b7e742af5aeee471a8d781d518c72fd235dd/pkg/operator/controllers/checker/clusterdns.go

A refactoring was done in this commit. Since then, when the status now needs to be updated, the new logic is returning an Error to the Reconciler, which in turn always returns the error to K8s controller-runtime, which is where it is being logged as an error.

@tiguelu
Copy link
Contributor

tiguelu commented Mar 12, 2024

/azp run e2e

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tiguelu
Copy link
Contributor

tiguelu commented Mar 12, 2024

/azp run e2e

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tiguelu
Copy link
Contributor

tiguelu commented Mar 12, 2024

/azp run e2e

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tiguelu tiguelu merged commit 38ebdae into master Mar 12, 2024
31 checks passed
@tiguelu tiguelu deleted the ARO-5445-DNS-checker-logging-custom-DNS-server-as-an-error branch March 12, 2024 21:04
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.

4 participants