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

Make the DMI IDs we whitelist configurable #970

Merged
merged 7 commits into from
May 3, 2018
Merged

Make the DMI IDs we whitelist configurable #970

merged 7 commits into from
May 3, 2018

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Mar 29, 2017

A lot of people want their OEM / IPMI dmi data. It’s super handy to have.

Signed-off-by: Tim Smith tsmith@chef.io

@tas50 tas50 changed the title Allow configuring what DMI IDs we whitelist WIP: Allow configuring what DMI IDs we whitelist Mar 29, 2017
@tas50 tas50 force-pushed the dmi_ids branch 2 times, most recently from 1d83640 to 5b0d752 Compare March 29, 2017 07:44
@tas50 tas50 requested review from lamont-granquist and thommay and removed request for lamont-granquist and thommay March 29, 2017 15:52
ID_TO_CAPTURE = [ 0, 1, 2, 3, 4, 6, 11 ]
# list of IDs to collect from config or default to a sane list that prunes
# away some of the less useful IDs
ID_TO_CAPTURE = Ohai.config[:plugin][:dmi][:ids] || [ 0, 1, 2, 3, 4, 6, 11 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, i don't think we should force any user that wants to add a single DMI ID to have to specify the full list of defaults too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could make this additional IDs pretty easily

@tas50
Copy link
Contributor Author

tas50 commented Apr 4, 2017

@thommay does this sort of thing make sense to allow the user to provide both IDs and the description we use to build the Mash?

@thommay
Copy link
Contributor

thommay commented Apr 4, 2017

This seems good; you might want to think about accepting an array too and just returning the code if there's no description.

A lot of people want their OEM / IPMI dmi data. It’s super handy to have.

Signed-off-by: Tim Smith <tsmith@chef.io>
We need to allow the user to add not only additional IDs, but the value for that ID. If we don’t have a description name it shows up as “unknown”

Signed-off-by: Tim Smith <tsmith@chef.io>
Signed-off-by: Tim Smith <tsmith@chef.io>
This should work since we have the updated list of descriptions. If it doesn't we'll populate the data with a generic description based on the ID.

Signed-off-by: Tim Smith <tsmith@chef.io>
Signed-off-by: Tim Smith <tsmith@chef.io>
Signed-off-by: Tim Smith <tsmith@chef.io>
@plugin.run
expect(@plugin[:dmi][:base_board]).not_to have_key(:error_correction_type)
it "allows capturing additional DMI data" do
Ohai.config[:additional_dmi_ids] = [ 16 ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thommay or @lamont-granquist I'd love a bit of help here. If I set Ohai.config[:additional_dmi_ids] here it fails, but if I stick the same thing way up in the before each it does work here (but fails other tests). What am I missing in rspec land? It seems like this should work.

Copy link
Contributor

Choose a reason for hiding this comment

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

You were shoving additional IDs on to a constant, so that lived forever.

Signed-off-by: Thom May <thom@chef.io>
@thommay thommay changed the title WIP: Allow configuring what DMI IDs we whitelist Make the DMI IDs we whitelist configurable May 3, 2018
@tas50 tas50 merged commit 8227540 into master May 3, 2018
@tas50 tas50 deleted the dmi_ids branch May 3, 2018 15:43
gsreynolds added a commit to gsreynolds/ohai that referenced this pull request May 29, 2018
Backport of chef#970

Signed-off-by: Gavin Reynolds <gavin@chef.io>
@lock
Copy link

lock bot commented Jan 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants