FIX: native ipmi client not close

[Clara12062]

Problem Description

In issue #243, the exporter reports an error: can't connect to ipmi collector. Additionally, when using ipmitool, there is a "BMC busy" error. After waiting for 5-10 minutes and running the command:

ipmitool -H <targetIP> -U <user> -P <password> -Ilanplus session info active

it was observed that the number of active sessions continues to increase during the collection process. This suggests that some sessions are not being properly closed.

How to Fix

In the ipmi-native code, the client is not being closed properly. If collection continues without closing the client, it can lead to resource leaks.

After using the NewNativeClient, the client should be explicitly closed.

[Clara12062]

Additionally, regarding the issue mentioned in #243: Unknown sensor state; should the state values for discrete sensors be handled separately?
Currently, my local implementation determines the state based on data.DiscreteActiveEventsString(), but I'm unsure if this approach is reasonable.

[bitfehler]

This is great, thanks! Just a few minor suggestions to make this a bit less verbose:

• We now use context.TODO() twice, which isn't great. Let's create a ctx := context.TODO() once and use that for both the actual work and closing the client; that makes it easier to replace this with a proper context at some point
• Let's move the "close and log on error" into a function CloseNativeClient in collector.go, so we can just call defer CloseNativeClient(client, ctx) everywhere
• Let's make the log a warning, since the sessions piling up does have real consequences
• In the log, I think it's fine to just log Client.Host as target, but if you feel strongly about it the target could also be passed in I guess

[bitfehler]

Additionally, regarding the issue mentioned in #243: Unknown sensor state; should the state values for discrete sensors be handled separately? Currently, my local implementation determines the state based on data.DiscreteActiveEventsString(), but I'm unsure if this approach is reasonable.

Unfortunately, #243 is sort of a collection of various unrelated issues. If you have a proposal for the sensor state issue please open a seperate PR for it, so we can discuss it better.

[Clara12062]

Just a few minor suggestions to make this a bit less verbose

Thanks for your feedback! I have resubmitted it, please review~😄

[bitfehler]

Nice, bonus points for passing the new context to NewNativeClient() as well! 😀

Thanks a lot!