Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/ipmi status codes #5816

Merged
merged 2 commits into from
May 7, 2019
Merged

Fix/ipmi status codes #5816

merged 2 commits into from
May 7, 2019

Conversation

AlirieGray
Copy link
Contributor

@AlirieGray AlirieGray commented May 7, 2019

Closes: #5755

This PR adds support for hex values in IPMI messages. It also adds unit tests for both version 1 & 2 parsing of IPMI messages.

  • Has appropriate unit tests.

@glinton glinton added the fix pr to fix corresponding bug label May 7, 2019
@glinton
Copy link
Contributor

glinton commented May 7, 2019

Also, there's no need to force push on telegraf, we squash and merge so it ends up being one commit in the end.

description := ipmiFields["description"]

// handle hex description field
if description[:2] == "0x" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check length of description first to avoid panic


// handle hex description field
if description[:2] == "0x" {
descriptionInt, err := strconv.ParseInt(description, 0, 64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check error before using descriptionInt

@danielnelson danielnelson added this to the 1.10.4 milestone May 7, 2019
@danielnelson danielnelson merged commit b22bf01 into master May 7, 2019
@danielnelson danielnelson deleted the fix/ipmi-status-codes branch May 7, 2019 22:42
danielnelson pushed a commit that referenced this pull request May 7, 2019
hwaastad pushed a commit to hwaastad/telegraf that referenced this pull request Jun 13, 2019
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
@safadig
Copy link

safadig commented Jan 5, 2020

can you please update documentation for where to add hex value? Thanks

@danielnelson
Copy link
Contributor

@safadig This PR should allow hex values to be parsed from the output of ipmitool, but it should be transparent and there is no configuration, values are reported in decimal. Let me know if I am misunderstanding or you can open a new issue.

@safadig
Copy link

safadig commented Jan 7, 2020

@safadig This PR should allow hex values to be parsed from the output of ipmitool, but it should be transparent and there is no configuration, values are reported in decimal. Let me know if I am misunderstanding or you can open a new issue.

@danielnelson I am trying to use the ipmi from a Dell iDRAC that has user; password, as well as a hex based encryption key to allow access.
I can get access using ipmitool:
sudo ipmitool -I lanplus -H 192.168.1.1 -U user -P password -y 0000000000000000000000000000000000000000 sdr -v
However, this is how the documentation lists the variables in telegraf.conf. I tried inputing the encrypted hex key before root and other areas but was not successful:
[[inputs.ipmi_sensor]] servers =["root:Password@lan(192.168.1.1)"] interval = "30s" timeout = "20s"

@danielnelson
Copy link
Contributor

Got it. It won't be possible to pass this option right now, but I see your feature request #6858.

@Raynok
Copy link

Raynok commented Mar 30, 2020

@danielnelson, also need this feature. I believe it can be a set variable that can be commented or uncommented in the config file (similar to the privilege level variable)

encryption Key = "number"

athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin: ipmi_sensor: fails to detect psu failure
5 participants