Add hardware plugin for ohai on darwin #839

natewalck · 2016-07-01T15:25:19Z

This adds information about OS X that is basically unusable in the current system_profiler ohai plugin. I made it in such a way that more information could always be parsed out and used.

Apple's formatting for the system_profiler info is really terrible, so even with XML output, it required quite a bit of massaging.

I'm with Facebook and this must be merged by @jaymzh

P.S. I plan on doing rspec tests, but I wanted to make sure I was on the right track before writing them.

chef-supermarket · 2016-07-01T15:26:14Z

Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog.

GitHub Users Who Are Not Authorized To Contribute

The following GitHub users do not appear to have signed a CLA:

@natewalck

Please sign the CLA here.

natewalck · 2016-07-01T16:20:42Z

Revisions planned.....

tas50 · 2016-07-01T16:22:22Z

lib/ohai/plugins/darwin/hardware.rb

+      next
+    end
+
+    hardware Mash.new


We shouldn't create this if hardware already exists. We check first around ohai so we don't globber other plugins that use the same namespace.

Would it be better to make it a different namespace? I plan on adding similar data for Windows and Linux as well (to be used mainly for client machines).

hardware is a good high level namespace. The subkeys seem to be pretty clean here. If someone later on wanted to add something like HP iLO on the Linux side there'd be room and it wouldn't be horribly cluttered.

I've looked in the code base and poked the Ohai object. What is the best way to look for an existing namespace? I assume I can return at the beginning of the collect_data function before anything happens.

tas50 · 2016-07-01T16:45:41Z

Works btw. Also I'm going to need a new battery soon

 "hardware": {
    "SMC_version_system": "2.19f12",
    "boot_rom_version": "MBP112.0138.B17",
    "cpu_type": "Intel Core i7",
    "current_processor_speed": "2.8 GHz",
    "l2_cache_core": "256 KB",
    "l3_cache": "6 MB",
    "machine_model": "MacBookPro11,3",
    "machine_name": "MacBook Pro",
    "number_processors": 4,
    "packages": 1,
    "physical_memory": "16 GB",
    "platform_UUID": "08CD3042-4DB0-550F-A936-0BB809D15B7E",
    "serial_number": "C02N63D0G3QP",
    "operating_system": "Mac OS X",
    "operating_system_version": "10.11.5",
    "build_version": "15F34",
    "architecture": "x86_64",
    "storage": [
      {
        "name": "Macintosh HD",
        "bsd_name": "disk1",
        "capacity": 499046809600,
        "drive_type": "ssd",
        "smart_status": "Verified",
        "partitions": 1
      }
    ],
    "battery": {
      "current_capacity": 6872,
      "max_capacity": 7728,
      "fully_charged": false,
      "is_charging": true,
      "charge_cycle_count": 474,
      "health": "Good",
      "serial": "D864315Y5LRF9CPAD",
      "remaining": 88
    }
  },

natewalck · 2016-07-01T18:11:51Z

I'm not seeing any other Ohai plugins that check the namespace before using it (unless I am missing the few that do). What is the best way to do this?

tas50 · 2016-07-01T18:17:49Z

This sort of thing

ohai/lib/ohai/plugins/freebsd/network.rb

Line 24 in 80c91e7

network Mash.new unless network

natewalck · 2016-07-01T18:35:49Z

Aha! I didn't realize all the Ohai data wasn't scoped in any way. I kept trying ohai['cpu'], node['cpu'], etc. I knew CPU existed, but wasn't sure where on the namespace. I'll do this check for hardware now that I know how its scoped.

Thanks!

natewalck · 2016-07-01T21:13:49Z

Ready for feedback now. Is this sufficient? Should I get started on the specs?

I'm not sure why the CI is failing....it looked like a bundle install error. I'll check it out again after this run.

tas50 · 2016-07-01T21:42:18Z

I'll take a look at what's there now. CI is failing due to rack 2.0.1 getting released. We're waiting on a new chef gem to fix that.

mcquin · 2016-07-05T18:25:32Z

lib/ohai/plugins/darwin/hardware.rb

+    if sp_std.exitstatus == 0
+      sp_hash = Plist.parse_xml(sp_std.stdout)
+    else
+      Ohai::Log.debug("system_profiler failed to run")


As a note, the shell_out method has some debugging which may duplicate this message: https://github.com/chef/ohai/blob/master/lib/ohai/mixin/command.rb#L39. It's your decision.

natewalck · 2016-07-05T21:44:36Z

Rebased and changed a few things based on feedback.

mcquin · 2016-07-07T19:54:46Z

It looks like you have some ChefStyle* offenses. You can fix most of these automatically by running bundle exec chefstyle -a lib/ohai/plugins/darwin/hardware.rb (leave off the -a to only detect offenses).

*Original message cited RuboCop. It's actually ChefStyle we're adhering to.

natewalck · 2016-07-07T21:24:26Z

If this looks good now, I can start on the rspec. Let me know!

mcquin · 2016-07-08T16:53:05Z

Go forth and write tests (though there's really no need to wait ;) ).

natewalck · 2016-07-08T18:25:30Z

reminder, this must be merged by @jaymzh . I'll get on the rspecs.

natewalck · 2016-07-08T22:03:46Z

There is a fail on the build, but it is a chefstyle error that I am uncertain on how to fix. when I add the 2 space indent, it causes the output blocks to get messed up ;(

mcquin · 2016-07-09T01:09:25Z

@natewalck You don't have to indent the entire string block, just the lines beginning with Hardware, Storage, and Power. It'll look something like:

module HardwareSystemProfilerOutput
  Hardware = <<hardware_output
<?xml version="1.0" encoding="UTF-8"?>
# ...
hardware_output

# ...
end

If you don't like the way that looks, which is a fine choice, you may want to explore copying the command outputs to spec/data/plugins. For example, if you saved hardware output to spec/data/plugins/system_profiler_hardware.output, you could replace HardwareSystemProfilerOutput::Hardware with File.read(File.join(SPEC_PLUGIN_PATH, "system_profiler_hardware.output"))

mcquin · 2016-07-09T13:50:14Z

spec/unit/plugins/darwin/hardware_spec.rb

+    )
+  end
+
+  it "should parse hardware data correctly" do


(Minor style comment) In alignment with BetterSpecs we prefer to not use "should" in test descriptions.

natewalck · 2016-07-12T21:39:49Z

I guess I don't understand why this is failing. It runs great on my own machine(OS X), but since it is failing on *nix, it seems like the shell_out mocks are not working quite as I expect them to.

I threw some pry binds in and I confirmed plugin contains the expected (mocked) data. Any idea where this could be breaking down? I basically copied the allow commands from system_profiler_spec, which isn't failing.

mcquin · 2016-07-13T01:17:26Z

Ohai only runs the default and operating system-specific plugins. The tests pass on your machine because Ohai detects OS X (:darwin), but fail on Travis because it detects another operating system (:linux).

You can trick Ohai into running this plugin during testing by adding this line to your before(:each) block:

allow(plugin).to receive(:collect_os).and_return(:darwin)

natewalck · 2016-07-13T15:43:53Z

That did it! Let me know if there are any other changes. If not, I'll squash and tag Phild.

mcquin · 2016-07-13T18:36:53Z

This looks great. Squash and summon Phil.

natewalck · 2016-07-13T18:42:14Z

@jaymzh, I choose you! (please don't kill me...)

natewalck · 2016-07-13T18:44:34Z

@mcquin Thanks a ton for working through this diff with me! It was my first PR for ohai, which was a little different to test than providers, etc. Should be more complete for the initial PR next time.

jaymzh · 2016-07-20T18:07:27Z

👍

jaymzh · 2016-07-20T18:07:54Z

Sorry for the delay. Was in TX. :)

tas50 reviewed Jul 1, 2016
View reviewed changes

mcquin reviewed Jul 5, 2016
View reviewed changes

natewalck force-pushed the ohai_osx branch from 3a65edf to f43db92 Compare July 5, 2016 21:44

mcquin reviewed Jul 9, 2016
View reviewed changes

Added hardware plugin for ohai on darwin

238c283

natewalck force-pushed the ohai_osx branch from a712e92 to 238c283 Compare July 13, 2016 18:40

tas50 added the Enhancement label Jul 17, 2016

jaymzh merged commit 618c449 into chef:master Jul 20, 2016

thommay added Type: Enhancement Adds new functionality. and removed Enhancement labels Jan 24, 2017

chef locked and limited conversation to collaborators Nov 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hardware plugin for ohai on darwin #839

Add hardware plugin for ohai on darwin #839

natewalck commented Jul 1, 2016 •

edited

Loading

chef-supermarket commented Jul 1, 2016

natewalck commented Jul 1, 2016

tas50 Jul 1, 2016

natewalck Jul 1, 2016

tas50 Jul 1, 2016

natewalck Jul 1, 2016

tas50 commented Jul 1, 2016 •

edited

Loading

natewalck commented Jul 1, 2016

tas50 commented Jul 1, 2016

natewalck commented Jul 1, 2016

natewalck commented Jul 1, 2016

tas50 commented Jul 1, 2016

mcquin Jul 5, 2016

natewalck commented Jul 5, 2016

mcquin commented Jul 7, 2016 •

edited

Loading

natewalck commented Jul 7, 2016

mcquin commented Jul 8, 2016

natewalck commented Jul 8, 2016

natewalck commented Jul 8, 2016

mcquin commented Jul 9, 2016

mcquin Jul 9, 2016

natewalck commented Jul 12, 2016

mcquin commented Jul 13, 2016

natewalck commented Jul 13, 2016

mcquin commented Jul 13, 2016

natewalck commented Jul 13, 2016

natewalck commented Jul 13, 2016

jaymzh commented Jul 20, 2016

jaymzh commented Jul 20, 2016

Add hardware plugin for ohai on darwin #839

Add hardware plugin for ohai on darwin #839

Conversation

natewalck commented Jul 1, 2016 • edited Loading

chef-supermarket commented Jul 1, 2016

GitHub Users Who Are Not Authorized To Contribute

natewalck commented Jul 1, 2016

tas50 Jul 1, 2016

Choose a reason for hiding this comment

natewalck Jul 1, 2016

Choose a reason for hiding this comment

tas50 Jul 1, 2016

Choose a reason for hiding this comment

natewalck Jul 1, 2016

Choose a reason for hiding this comment

tas50 commented Jul 1, 2016 • edited Loading

natewalck commented Jul 1, 2016

tas50 commented Jul 1, 2016

natewalck commented Jul 1, 2016

natewalck commented Jul 1, 2016

tas50 commented Jul 1, 2016

mcquin Jul 5, 2016

Choose a reason for hiding this comment

natewalck commented Jul 5, 2016

mcquin commented Jul 7, 2016 • edited Loading

natewalck commented Jul 7, 2016

mcquin commented Jul 8, 2016

natewalck commented Jul 8, 2016

natewalck commented Jul 8, 2016

mcquin commented Jul 9, 2016

mcquin Jul 9, 2016

Choose a reason for hiding this comment

natewalck commented Jul 12, 2016

mcquin commented Jul 13, 2016

natewalck commented Jul 13, 2016

mcquin commented Jul 13, 2016

natewalck commented Jul 13, 2016

natewalck commented Jul 13, 2016

jaymzh commented Jul 20, 2016

jaymzh commented Jul 20, 2016

natewalck commented Jul 1, 2016 •

edited

Loading

tas50 commented Jul 1, 2016 •

edited

Loading

mcquin commented Jul 7, 2016 •

edited

Loading