Repository navigation
Nil pointer dereference in metrics exporter #1187
Description
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.
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x198 pc=0x1370d8e] goroutine 1 [running]:
github.com/altinity/clickhouse-operator/pkg/apis/metrics.(*Exporter).DiscoveryWatchedCHIs(0x16c6661?, {0x18f6338, 0xc00002e6e0}, 0xc0000317f0) /clickhouse-operator/pkg/apis/metrics/exporter.go:295 +0x3ee
github.com/altinity/clickhouse-operator/cmd/metrics_exporter/app.Run() /clickhouse-operator/cmd/metrics_exporter/app/metrics_exporter.go:108 +0x36b
main.main() /clickhouse-operator/cmd/metrics_exporter/main.go:23 +0x17
My guess is that we can fix this by changing the access of chi.Status.NormalizedCHICompleted to chi.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 of Status (because it relied on field visibility)... I think?
Activity
Merge pull request #1188 from zcross/zcross/bugfix-npe-issue-1187
zcross commentedon Jul 7, 2023
Let's close this if the build with #1188 passes? I'm not sure how deterministic it is to repro, but one thing worth trying is just bringing up a fresh installation and seeing if that reproduces on
0.21.2– then trying again with0.21.3Edit: #1189 fixes this w/ compiling code :)
alex-zaitsev commentedon Jul 11, 2023
@zcross , this is fixed in 0.21.3
zcross commentedon Jul 11, 2023
Thank you!
zcross commentedon Jul 11, 2023
@alex-zaitsev
Would it be possible to cut a release tag for
0.21.3to distribute this fix?