LightBitsLabs / los-csi

Lightbits LightOS CSI Plugin
Other
0 stars 2 forks source link

WIP: remove init func, usage of init should be avoided #13

Closed majst01 closed 2 years ago

majst01 commented 2 years ago

This is the first PR extracted from #11

Init should be avoided if not strictly necessary.

lev-lb commented 2 years ago

@majst01 : so far this sounds like a very personal preference to me. can you point out a specific problem that this patch solves?

capsCache should have been a compile time constant, but due to the Go type system limitations that's impossible. the next closest thing is to do this one-time initialisation in init(). i don't see how recreating the caps on every poll from K8s helps or prevents anything untoward.

to avoid superfluous debates on the matter of taste i will push a .golangci-lint configuration used with this project, and will try to integrate the check with the PR check.

majst01 commented 2 years ago

Problem with init() is that order of execution is not guaranteed nor predictable. The content of the capsCache is never changing, if you are afraid of performance impacts of the ControllerGetCapabilities func, i doubt this will take longer than some ns. Can even be measured with a benchmark.

lev-lb commented 2 years ago

@majst01 : the order of execution of init() relatively to all the users of capsCache in the LB CSI plugin is perfectly predictable: init() is guaranteed to run before them (gRPC server for CSI in only created from main.main(), by which time driver.init() is guaranteed to have finished).

this is exactly what init() was designed for: "initializations that cannot be expressed as declarations" (see above on const vs Go).