-
Notifications
You must be signed in to change notification settings - Fork 1.4k
🌱 Reduce noisy logs #12606
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
🌱 Reduce noisy logs #12606
Conversation
Signed-off-by: Stefan Büringer [email protected]
|
/cherry-pick release-1.11 |
|
@sbueringer: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| // if error is not nil, so setting result here to empty to avoid noisy warnings. | ||
| if rerr != nil { | ||
| retRes = ctrl.Result{} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this we get these logs
I0811 08:55:52.637042 1 controller.go:351] "Warning: Reconciler returned both a non-zero result and a non-nil error. The result will always be ignored if the error is non-nil and the non-nil error causes requeuing with exponential backoff. For more details, see: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/reconcile#Reconciler" controller="kubeadmconfig" controllerGroup="bootstrap.cluster.x-k8s.io" controllerKind="KubeadmConfig" KubeadmConfig="node-drain-cg4f22/node-drain-yrt3jt-mp-0-dd2dp" namespace="node-drain-cg4f22" name="node-drain-yrt3jt-mp-0-dd2dp" reconcileID="00abc693-0f18-4f80-8e3a-29c083cde356"
https://storage.googleapis.com/kubernetes-ci-logs/logs/periodic-cluster-api-e2e-main/1954807928472997888/artifacts/clusters/bootstrap/logs/capi-kubeadm-bootstrap-system/capi-kubeadm-bootstrap-controller-manager/capi-kubeadm-bootstrap-controller-manager-f4cf5f747-m76js/manager.log
|
|
||
| // DeprecationWarning is a regular expression that matches a | ||
| // warning that the API server returns when using a type that is deprecated. | ||
| func DeprecationWarning() regexp.Regexp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get a lot of warnings in our controllers if our controllers use deprecated APIs because our users use deprecated APIs, e.g.
"infrastructure.cluster.x-k8s.io/v1beta1 DockerClusterTemplate is deprecated; use infrastructure.cluster.x-k8s.io/v1beta2 DockerClusterTemplate" logger="API Server Warning"
https://storage.googleapis.com/kubernetes-ci-logs/logs/periodic-cluster-api-e2e-main/1954807928472997888/artifacts/clusters/bootstrap/logs/capi-system/capi-controller-manager/capi-controller-manager-655d6cb6cc-4vgbr/manager.log
I think it's enough if the users get deprecation warnings from the apiserver. I think it's not worth the log spam that we also get them in controller logs.
| func DomainQualifiedFinalizerWarning(domain string) regexp.Regexp { | ||
| return *regexp.MustCompile( | ||
| fmt.Sprintf("^metadata.finalizers:.*%s.*prefer a domain-qualified finalizer name to avoid accidental conflicts with other finalizer writers$", domain), | ||
| fmt.Sprintf("^metadata.finalizers:.*%s.*prefer a domain-qualified finalizer name (including a path \\(\\/\\) )?to avoid accidental conflicts with other finalizer writers$", domain), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two versions of this warning. The regexp now matches both, xref: https://github.com/kubernetes/kubernetes/blob/091fa293908f8e9c132da315449f0c24014db478/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go#L90-L92
|
/assign @sivchari @JoelSpeed @mboersma |
|
/test pull-cluster-api-e2e-main |
|
/lgtm |
|
LGTM label has been added. Git tree hash: fcc81012bff8513f7cc723045e0537f1e10970ba
|
mboersma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
/approve Given lgtm's above |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@sbueringer: new pull request created: #12626 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: Stefan Büringer [email protected]
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #