-
Notifications
You must be signed in to change notification settings - Fork 448
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
Nil pointer dereference in metrics exporter #1187
Comments
zcross
added a commit
to zcross/clickhouse-operator
that referenced
this issue
Jul 7, 2023
…ureStatus() access Signed-off-by: Zach Cross <zcross@chronosphere.io>
3 tasks
sunsingerus
added a commit
that referenced
this issue
Jul 7, 2023
Fix nil pointer deref in metrics exporter (#1187)
@zcross , this is fixed in 0.21.3 |
Thank you! |
@alex-zaitsev |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I recently observed this on
0.21.2
while bootstrapping a brand new installation. FWIW, it was "self healing" after a few minutes: ongoing reconciliation operations seemed to bring the installation(s) into a state that caused the status to no longer be nil.clickhouse-operator/pkg/apis/metrics/exporter.go
Line 295 in 32ef0fa
My guess is that we can fix this by changing the access of
chi.Status.NormalizedCHICompleted
tochi.EnsureStatus().GetNormalizedCHICompleted()
, or something like that. I can't do it right this moment, but I'll circle back to this issue when I get some bandwidth.To generalize: maybe we want to use Golang visibility to make raw/unsynchronized fields on
Status
package-internal to more strongly encourage usage of safe public getter/setter APIs. I considered this in #1119 but IIRC, this resulted in problems when trying to deserialize/serialize the state ofStatus
(because it relied on field visibility)... I think?The text was updated successfully, but these errors were encountered: