Closed teocns closed 1 month ago
In my opinion, there's no need to perform a nil check on this non-exported function. We have control over it, and we know that cr
and cr.spec
will not be nil. This approach helps keep the code cleaner. :)
Tho if you deploy a RedisCluster resource without specifying the storage parameter you do face an unmanaged nil reference exception which is hard to frame for somebody that is not at hands with the codebase. IMO it should be managed or Storage needs to have defaults
IMO it should be managed or Storage needs to have defaults
I understand you. You could set default value in https://github.com/OT-CONTAINER-KIT/redis-operator/blob/master/api/v1beta2/rediscluster_default.go
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 39.56%. Comparing base (
d121d86
) to head (f2b29b7
). Report is 41 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @drivebyer , I reproduced the same issue on redis standalone, probably there is on other resources as well. was this fixed for every resource or just locally for cluster?
{"level":"info","ts":"2024-05-10T08:46:58Z","msg":"Observed a panic in reconciler: runtime error: invalid memory address or nil pointer dereference","controller":"redis","controllerGroup":"redis.redis.opstreelabs.in","controllerKind":"Redis","Redis":{"name":"redis-standalone","namespace":"integration"},"namespace":"integration","name":"redis-standalone","reconcileID":"7fa7d9ba-823f-4d01-9b79-5de377f5dae9"}
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x17854ba]
The
nil
checks infunc generateRedisClusterParams
are ran beforecr.Spec.Storage.$
is referenced, hence causing a runtime panic when the `storage field isn't defined in the manifest.nil
checks on objects before initializing thestatefulSetParameters
structcr.Spec.Storage
failsnil
check, it initializes it to a new instance of redisv1beta2.Storage{}⚠️ This might not be the ideal place to perform objects initializations. Please carefully review whether this should exist elsewhere according to your design
Type of change
Checklist
Additional Context