allow querying IPMI sensors via the open interface #2244
Conversation
|
I've also updated the README so that you can evaluate the changes better. |
|
@sparrc, any chance of having this in the next release? :( |
|
All tests are passing now, I've re-factored the code a little bit more. |
m4ce
changed the title
plugins/inputs/ipmi_sensor/README.md
Outdated
| @@ -4,23 +4,32 @@ Get bare metal metrics using the command line utility `ipmitool` | |||
|
|
|||
| see ipmitool(https://sourceforge.net/projects/ipmitool/files/ipmitool/) | |||
|
|
|||
| The plugin will use the following command to collect remote host sensor stats: | |||
| The plugin supports two modes. When 'servers' is not specified, the plugin will utilize the OpenIPMI kernel device driver to retrieve the sensor stats. | |||
There was a problem hiding this comment.
what does this mean? does it still run an ipmitool command? can you provide it here?
| * Fields: | ||
| - status | ||
| - value | ||
|
|
||
| The `server` tag will be made available when retrieving stats from remote server(s). | ||
|
|
||
| ## Configuration | ||
|
|
||
| ```toml | ||
| [[inputs.ipmi_sensor]] |
There was a problem hiding this comment.
you should change the SampleConfig to indicate that servers is optional
plugins/inputs/ipmi_sensor/ipmi.go
Outdated
| func (m *Ipmi) SampleConfig() string { | ||
| return sampleConfig | ||
| } | ||
|
|
||
| func (m *Ipmi) Description() string { | ||
| return "Read metrics from one or many bare metal servers" | ||
| return "Read metrics from the local machines (via the open interface) or one or many bare metal servers (via the lan interface)" |
plugins/inputs/ipmi_sensor/ipmi.go
Outdated
| if m.runner == nil { | ||
| m.runner = CommandRunner{} | ||
| if len(m.path) == 0 { | ||
| return errors.New("ipmitool not found: verify that ipmitool is installed and that ipmitool is in your PATH") |
There was a problem hiding this comment.
use fmt.Errorf instead of errors.New
|
@m4ce I'm not sure I understand the distinction you're making in documentation with local vs. external hosts using the "OpenIPMI device driver"? Don't they both use OpenIPMI? |
|
@sparrc, when running |
|
but in both cases we are querying the OpenIPMI device driver, it's just that in the I think you should change the documentation to only specify that if a server is not given, then we will query locally. |
|
@sparrc, I've improved the documentation and amended the code based on your reviews. |
plugins/inputs/ipmi_sensor/ipmi.go
Outdated
| ## optionally specify the path to the ipmitool executable | ||
| ## path = "/usr/bin/ipmitool" | ||
|
|
||
| ## optionally specify one or more servers via a url matching. If servers is omitted, only the local machine sensor stats will be retrieved. |
There was a problem hiding this comment.
config file line length should be limited to 80 characters
|
thanks @m4ce |
Hi,
this solves #1547.
I have not adjuted the tests yet. I wanted to first confirm with you that my changes are okay and then update the tests accordingly.
Cheers,
Matteo