Merge pull request #268 from Clara12062/fix-ipmi-client-no-close

bitfehler · web-flow · commit 96dc5fff5aa2 · 2025-06-25T12:06:05.000+02:00
FIX: native ipmi client not close
diff --git a/collector.go b/collector.go
@@ -130,7 +130,7 @@ func targetName(target string) string {
 	return target
 }
 
-func NewNativeClient(target ipmiTarget) (*ipmi.Client, error) {
+func NewNativeClient(ctx context.Context, target ipmiTarget) (*ipmi.Client, error) {
 	var client *ipmi.Client
 	var err error
 
@@ -164,9 +164,15 @@ func NewNativeClient(target ipmiTarget) (*ipmi.Client, error) {
 		client = client.WithMaxPrivilegeLevel(priv)
 	}
 	// TODO workaround-flags not used in native client
-	if err := client.Connect(context.TODO()); err != nil {
+	if err := client.Connect(ctx); err != nil {
 		logger.Error("Error connecting to IPMI device", "target", target.host, "error", err)
 		return nil, err
 	}
 	return client, nil
 }
+
+func CloseNativeClient(ctx context.Context, client *ipmi.Client) {
+	if closeErr := client.Close(ctx); closeErr != nil {
+		logger.Warn("Failed to close IPMI client", "target", client.Host, "error", closeErr)
+	}
+}
diff --git a/collector_bmc_native.go b/collector_bmc_native.go
@@ -48,11 +48,13 @@ func (c BMCNativeCollector) Args() []string {
 }
 
 func (c BMCNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetDeviceID(context.TODO())
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetDeviceID(ctx)
 	if err != nil {
 		return 0, err
 	}
@@ -65,7 +67,7 @@ func (c BMCNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metr
 		},
 		SystemFirmwareVersions: make([]*ipmi.SystemInfoParam_SystemFirmwareVersion, 0),
 	}
-	err = client.GetSystemInfoParamsFor(context.TODO(), &systemInfo)
+	err = client.GetSystemInfoParamsFor(ctx, &systemInfo)
 	// This one is not always available
 	systemFirmwareVersion := "N/A"
 	if err != nil {
diff --git a/collector_bmc_watchdog_native.go b/collector_bmc_watchdog_native.go
@@ -95,11 +95,13 @@ func (c BMCWatchdogNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometh
 
 	// TODO this now works remotely
 
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetWatchdogTimer(context.TODO())
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetWatchdogTimer(ctx)
 	if err != nil {
 		return 0, err
 	}
diff --git a/collector_chassis_native.go b/collector_chassis_native.go
@@ -58,11 +58,13 @@ func (c ChassisNativeCollector) Args() []string {
 }
 
 func (c ChassisNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetChassisStatus(context.TODO())
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetChassisStatus(ctx)
 	if err != nil {
 		return 0, err
 	}
diff --git a/collector_dcmi_native.go b/collector_dcmi_native.go
@@ -46,11 +46,13 @@ func (c DCMINativeCollector) Args() []string {
 }
 
 func (c DCMINativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetDCMIPowerReading(context.TODO())
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetDCMIPowerReading(ctx)
 	if err != nil {
 		logger.Error("Failed to collect DCMI data", "target", targetName(target.host), "error", err)
 		return 0, err
diff --git a/collector_ipmi_native.go b/collector_ipmi_native.go
@@ -146,11 +146,13 @@ func (c IPMINativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Met
 		return true
 	}
 
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetSensors(context.TODO(), filter)
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetSensors(ctx, filter)
 	if err != nil {
 		return 0, err
 	}
diff --git a/collector_sel_events_native.go b/collector_sel_events_native.go
@@ -60,11 +60,13 @@ func (c SELEventsNativeCollector) Args() []string {
 func (c SELEventsNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
 	selEventConfigs := target.config.SELEvents
 
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetSELEntries(context.TODO(), 0)
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetSELEntries(ctx, 0)
 	if err != nil {
 		return 0, err
 	}
diff --git a/collector_sel_native.go b/collector_sel_native.go
@@ -53,11 +53,13 @@ func (c SELNativeCollector) Args() []string {
 }
 
 func (c SELNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	res, err := client.GetSELInfo(context.TODO())
+	defer CloseNativeClient(ctx, client)
+	res, err := client.GetSELInfo(ctx)
 	if err != nil {
 		return 0, err
 	}
diff --git a/collector_sm_lan_mode_native.go b/collector_sm_lan_mode_native.go
@@ -48,15 +48,17 @@ func (c SMLANModeNativeCollector) Args() []string {
 }
 
 func (c SMLANModeNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {
-	client, err := NewNativeClient(target)
+	ctx := context.TODO()
+	client, err := NewNativeClient(ctx, target)
 	if err != nil {
 		return 0, err
 	}
-	if _, err := client.SetSessionPrivilegeLevel(context.TODO(), ipmi.PrivilegeLevelAdministrator); err != nil {
+	defer CloseNativeClient(ctx, client)
+	if _, err := client.SetSessionPrivilegeLevel(ctx, ipmi.PrivilegeLevelAdministrator); err != nil {
 		logger.Error("Failed to set privilege level to admin", "target", targetName(target.host))
 		return 0, fmt.Errorf("failed to set privilege level to admin")
 	}
-	res, err := client.RawCommand(context.TODO(), ipmi.NetFnOEMSupermicroRequest, 0x70, []byte{0x0C, 0x00}, "GetSupermicroLanMode")
+	res, err := client.RawCommand(ctx, ipmi.NetFnOEMSupermicroRequest, 0x70, []byte{0x0C, 0x00}, "GetSupermicroLanMode")
 	if err != nil {
 		logger.Error("raw command failed", "error", err)
 		return 0, err

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,7 @@ func targetName(target string) string {`
`130`	`130`	`return target`
`131`	`131`	`}`
`132`	`132`
`133`		`-func NewNativeClient(target ipmiTarget) (*ipmi.Client, error) {`
	`133`	`+func NewNativeClient(ctx context.Context, target ipmiTarget) (*ipmi.Client, error) {`
`134`	`134`	`var client *ipmi.Client`
`135`	`135`	`var err error`
`136`	`136`
`@@ -164,9 +164,15 @@ func NewNativeClient(target ipmiTarget) (*ipmi.Client, error) {`
`164`	`164`	`client = client.WithMaxPrivilegeLevel(priv)`
`165`	`165`	`}`
`166`	`166`	`// TODO workaround-flags not used in native client`
`167`		`- if err := client.Connect(context.TODO()); err != nil {`
	`167`	`+ if err := client.Connect(ctx); err != nil {`
`168`	`168`	`logger.Error("Error connecting to IPMI device", "target", target.host, "error", err)`
`169`	`169`	`return nil, err`
`170`	`170`	`}`
`171`	`171`	`return client, nil`
`172`	`172`	`}`
	`173`	`+`
	`174`	`+func CloseNativeClient(ctx context.Context, client *ipmi.Client) {`
	`175`	`+ if closeErr := client.Close(ctx); closeErr != nil {`
	`176`	`+ logger.Warn("Failed to close IPMI client", "target", client.Host, "error", closeErr)`
	`177`	`+ }`
	`178`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -48,11 +48,13 @@ func (c BMCNativeCollector) Args() []string {`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`func (c BMCNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {`
`51`		`- client, err := NewNativeClient(target)`
	`51`	`+ ctx := context.TODO()`
	`52`	`+ client, err := NewNativeClient(ctx, target)`
`52`	`53`	`if err != nil {`
`53`	`54`	`return 0, err`
`54`	`55`	`}`
`55`		`- res, err := client.GetDeviceID(context.TODO())`
	`56`	`+ defer CloseNativeClient(ctx, client)`
	`57`	`+ res, err := client.GetDeviceID(ctx)`
`56`	`58`	`if err != nil {`
`57`	`59`	`return 0, err`
`58`	`60`	`}`
`@@ -65,7 +67,7 @@ func (c BMCNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metr`
`65`	`67`	`},`
`66`	`68`	`SystemFirmwareVersions: make([]*ipmi.SystemInfoParam_SystemFirmwareVersion, 0),`
`67`	`69`	`}`
`68`		`- err = client.GetSystemInfoParamsFor(context.TODO(), &systemInfo)`
	`70`	`+ err = client.GetSystemInfoParamsFor(ctx, &systemInfo)`
`69`	`71`	`// This one is not always available`
`70`	`72`	`systemFirmwareVersion := "N/A"`
`71`	`73`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -95,11 +95,13 @@ func (c BMCWatchdogNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometh`
`95`	`95`
`96`	`96`	`// TODO this now works remotely`
`97`	`97`
`98`		`- client, err := NewNativeClient(target)`
	`98`	`+ ctx := context.TODO()`
	`99`	`+ client, err := NewNativeClient(ctx, target)`
`99`	`100`	`if err != nil {`
`100`	`101`	`return 0, err`
`101`	`102`	`}`
`102`		`- res, err := client.GetWatchdogTimer(context.TODO())`
	`103`	`+ defer CloseNativeClient(ctx, client)`
	`104`	`+ res, err := client.GetWatchdogTimer(ctx)`
`103`	`105`	`if err != nil {`
`104`	`106`	`return 0, err`
`105`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -58,11 +58,13 @@ func (c ChassisNativeCollector) Args() []string {`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`func (c ChassisNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {`
`61`		`- client, err := NewNativeClient(target)`
	`61`	`+ ctx := context.TODO()`
	`62`	`+ client, err := NewNativeClient(ctx, target)`
`62`	`63`	`if err != nil {`
`63`	`64`	`return 0, err`
`64`	`65`	`}`
`65`		`- res, err := client.GetChassisStatus(context.TODO())`
	`66`	`+ defer CloseNativeClient(ctx, client)`
	`67`	`+ res, err := client.GetChassisStatus(ctx)`
`66`	`68`	`if err != nil {`
`67`	`69`	`return 0, err`
`68`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -146,11 +146,13 @@ func (c IPMINativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Met`
`146`	`146`	`return true`
`147`	`147`	`}`
`148`	`148`
`149`		`- client, err := NewNativeClient(target)`
	`149`	`+ ctx := context.TODO()`
	`150`	`+ client, err := NewNativeClient(ctx, target)`
`150`	`151`	`if err != nil {`
`151`	`152`	`return 0, err`
`152`	`153`	`}`
`153`		`- res, err := client.GetSensors(context.TODO(), filter)`
	`154`	`+ defer CloseNativeClient(ctx, client)`
	`155`	`+ res, err := client.GetSensors(ctx, filter)`
`154`	`156`	`if err != nil {`
`155`	`157`	`return 0, err`
`156`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,11 +53,13 @@ func (c SELNativeCollector) Args() []string {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`func (c SELNativeCollector) Collect(_ freeipmi.Result, ch chan<- prometheus.Metric, target ipmiTarget) (int, error) {`
`56`		`- client, err := NewNativeClient(target)`
	`56`	`+ ctx := context.TODO()`
	`57`	`+ client, err := NewNativeClient(ctx, target)`
`57`	`58`	`if err != nil {`
`58`	`59`	`return 0, err`
`59`	`60`	`}`
`60`		`- res, err := client.GetSELInfo(context.TODO())`
	`61`	`+ defer CloseNativeClient(ctx, client)`
	`62`	`+ res, err := client.GetSELInfo(ctx)`
`61`	`63`	`if err != nil {`
`62`	`64`	`return 0, err`
`63`	`65`	`}`