Add 2crsi-sensors plugin #47
Conversation
Signed-off-by: cecile <cecile@ProjectZero>
Signed-off-by: cecile <cecile@ProjectZero>
Signed-off-by: cecile <cecile@ProjectZero>
AtaxyaNetwork
force-pushed
the
ipmi-sensors
branch
from
ef39d14
to
89ff9ca
Compare
There was a problem hiding this comment.
LGTM :)
Maybe add some simple unit tests like the ones for smartctl.py
| @@ -165,6 +165,20 @@ xe host-call-plugin host-uuid=<uuid> plugin=smartctl.py fn=health | |||
| {"/dev/sdf": "PASSED", "/dev/sdg": "PASSED", "/dev/sdd": "PASSED", "/dev/sde": "PASSED", "/dev/sdb": "PASSED", "/dev/sdc": "PASSED", "/dev/sda": "PASSED"} | |||
| ``` | |||
|
|
|||
| ## IPMI-sensors parser | |||
| A xapi plugin to get information about the host via the BMC | |||
There was a problem hiding this comment.
Maybe add in the README that it requires freeipmi and ipmitool.
What is the error if they are not installed?
My point is that it can be cool, if it is not already the case, that when we run xe host-call-plugin host-uuid=<uuid> plugin=2crsi-sensors.py fn="get_info" the plugin returns a clear message saying "ipmi-sensors not found". So just ensure that in case of error the user will understand that binary is not found.
There was a problem hiding this comment.
I can't remember, don't we have it already installed by default in XCP-ng?
There was a problem hiding this comment.
ipmitool is installed. Not freeipmi.
|
I marked this PR as draft since this is still a work in progress, and the interface might still evolve, so a draft will protect us against an accidental merge and packaging. The review process can continue anyway. |
| A xapi plugin to get information about the host via the BMC | ||
| ### `get_info`: | ||
| ``` | ||
| xe host-call-plugin host-uuid=<uuid> plugin=2crsi-sensors.py fn="get_info" |
There was a problem hiding this comment.
IMHO, it would be better and more robust to have a generic IPMI plugin, no need to have logic both in XO and the plugin that will require synchronization on updates.
Possibly accepting a list of fields to return if we want to fetch only a subset.
There was a problem hiding this comment.
Agree. And I will check if it is easy and safe to parse the output into JSON otherwise the plugin will return raw data and the parsing will be done on XO side.
|
|
||
| @error_wrapped | ||
| def get_data(session, args): | ||
| data = subprocess.check_output(["ipmi-sensors", "-Q", "--ignore-not-available-sensors", "--ignore-unrecognized-events"]) |
There was a problem hiding this comment.
Can't we use ipmitool sensor to get the same info? The thing is that freeipmi is not installed by default while ipmitool is. I tested on one machine and it looks like we have even more information with ipmitool sensor. Can you confirm that you also have all infos with your HW?
This plugin allows XCP-ng to:
Thanks to this plugin, Xen Orchestra can display the BMC's sensors info and the IP address if a 2CRSi Mona server is detected
The plugin require the package
freeipmiandipmitool.Tests requires a Mona server, if you need to test changes, contact me!