Rewrite ohai_plugin.rb to reflect new ohai cookbook version #23

ledgr · 2016-07-04T12:40:45Z

Ohai plugins should be installed ohai_plugin resource: https://github.com/chef-cookbooks/ohai/blob/master/resources/plugin.rb

ton31337 · 2016-07-19T13:09:00Z

ping @zuazo

zuazo · 2016-07-19T13:50:42Z

Hi @ledgr,

Thanks for the PR 😄

Please, could you fix the RuboCop offenses and the Unit tests?

$ rake style
Running RuboCop...
Inspecting 73 files
................................................C........................

Offenses:
recipes/ohai_plugin.rb:34:81: C: Line is too long. [81/80]
  variables enable_build_options: node['dovecot']['ohai_plugin']['build-options']
                                                                                ^
73 files inspected, 1 offense detected
RuboCop failed!

$ rake unit

  1) dovecot::default includes dovecot::user recipe
     Failure/Error: let(:chef_run) { chef_runner.converge(described_recipe) }

     Chef::Exceptions::ResourceNotFound:
       resource apt_package[(core) dovecot-core] is configured to notify resource ohai[reload_dovecot] with action reload, but ohai[reload_dovecot] cannot be found in the resource collection. apt_package[(core) dovecot-core] is defined in recipes/from_package.rb:34:in `block (2 levels) in from_file'
[...]

  2) dovecot::default includes dovecot::service recipe
     Failure/Error: let(:chef_run) { chef_runner.converge(described_recipe) }

     Chef::Exceptions::ResourceNotFound:
       resource apt_package[(core) dovecot-core] is configured to notify resource ohai[reload_dovecot] with action reload, but ohai[reload_dovecot] cannot be found in the resource collection. apt_package[(core) dovecot-core] is defined in recipes/from_package.rb:34:in `block (2 levels) in from_file'
[...]

Finished in 4 minutes 0 seconds (files took 1.53 seconds to load)
169 examples, 81 failures, 2 pending

Also, please avoid updating the cookbook version in the metadata.

If you fix these details, I'm going to merge it and make a release.

Thanks again! And sorry for the delay in my review.

ledgr · 2016-07-19T14:32:57Z

recipes/from_package.rb

@@ -34,9 +34,6 @@
    package "(#{type}) #{pkg}" do
      package_name pkg
      only_if { Dovecot::Conf.require?(type, node['dovecot']) }
-      if type == 'core' || node['dovecot']['ohai_plugin']['build-options']
-        notifies :reload, 'ohai[reload_dovecot]', :immediately
-      end


ohai cookbook does this now using ohai_plugin resource

But we don't need to notify ohai_plugin[dovecot]?

It is done by ohai_plugin
here: https://github.com/chef-cookbooks/ohai/blob/master/resources/plugin.rb#L83
here: https://github.com/chef-cookbooks/ohai/blob/master/resources/plugin.rb#L90

Correct me if I'm wrong but I think that this notification you are referring to only replaces the notification of the old ohai template resource. But not the notification here.

Keep in mind that this change is breaking the integration tests:

$ kitchen test default-debian-7 [...] Recipe: dovecot_test::default * ruby_block[ohai plugin tests] action run ================================================================================ Error executing action `run` on resource 'ruby_block[ohai plugin tests]' ================================================================================ RuntimeError ------------ Ohai plugin cannot get dovecot version. Cookbook Trace: --------------- /tmp/kitchen/cache/cookbooks/dovecot_test/recipes/default.rb:27:in `block (2 levels) in from_file' /tmp/kitchen/cache/cookbooks/compat_resource/files/lib/chef_compat/monkeypatches/chef/runner.rb:41:in `run_action' Resource Declaration: --------------------- # In /tmp/kitchen/cache/cookbooks/dovecot_test/recipes/default.rb 24: ruby_block 'ohai plugin tests' do 25: block do 26: unless node['dovecot']['version'].is_a?(String) 27: raise 'Ohai plugin cannot get dovecot version.' 28: end 29: unless node['dovecot']['build-options'].is_a?(Hash) && 30: !node['dovecot']['build-options'].empty? 31: raise 'Ohai plugin cannot get dovecot build options.' 32: end 33: end 34: end 35: Compiled Resource: ------------------ # Declared in /tmp/kitchen/cache/cookbooks/dovecot_test/recipes/default.rb:24:in `from_file' ruby_block("ohai plugin tests") do action [:run] retries 0 retry_delay 2 default_guard_interpreter :default block_name "ohai plugin tests" declared_type :ruby_block cookbook_name "dovecot_test" recipe_name "default" block #<Proc:0x000000034410d0@/tmp/kitchen/cache/cookbooks/dovecot_test/recipes/default.rb:25> end

So, we still need to notify the ohai plugin to reload here.

The problem maybe is that we cannot notify the internal ohai[dovecot] resource and the ohai_plugin resource has no :reload action defined. A possible solution would be to add a dummy ohai[dovecot] resource:

# recipes/ohai_plugin.rb # dummy resource to be able to notify reload action to the ohai plugin ohai 'dovecot' do action :nothing end

# recipes/from_package.rb package "(#{type}) #{pkg}" do package_name pkg only_if { Dovecot::Conf.require?(type, node['dovecot']) } if type == 'core' || node['dovecot']['ohai_plugin']['build-options'] notifies :reload, 'ohai[dovecot]', :immediately end end # package

Some integration tests also need to be updated:

-----> Verifying <default-debian-7>... Preparing files for transfer Suite path directory /tmp/verifier/suites does not exist, skipping. Transferring files to <default-debian-7> -----> Running bats test suite ✗ ohai gets dovecot version (in test file /tmp/verifier/suites/bats/ohai.bats, line 5) `ohai -d /etc/chef/ohai_plugins | tr -d $'\n' | grep -q '"dovecot":\s*{\s*"version"'' failed [2016-07-20T08:22:29+00:00] WARN: The plugin path /etc/chef/ohai_plugins does not exist. Skipping... ✗ ohai prints nothing to stderr (in test file /tmp/verifier/suites/bats/ohai.bats, line 10) `[ -z "`ohai -d /etc/chef/ohai_plugins 2>&1 > /dev/null`" ]' failed 5 tests, 2 failures !!!!!! Command [/tmp/verifier/vendor/bats/bin/bats /tmp/verifier/suites/bats] exit code was 1

You can see the full output in travis. Also, see TESTING if you need help with the tests.

ledgr · 2016-07-19T14:35:05Z

Updated. I have dropped few tests, hope not too much. @zuazo

coveralls · 2016-07-19T14:38:47Z

Coverage remained the same at 100.0% when pulling 509fd70 on ledgr:fix/ohai_plugin_changes into 1e9f840 on zuazo:master.

zuazo · 2016-07-19T19:28:41Z

test/unit/recipes/ohai_plugin_spec.rb

-  it 'dovecot plugin installation notifies ohai reload' do
-    resource = chef_run.template('/etc/chef/ohai_plugins/dovecot.rb')
-    expect(resource).to notify('ohai[reload_dovecot]').to(:reload).immediately
-  end


I think all these tests should be replaced by a create_ohai_plugin matcher. Something like the following:

expect(chef_run).to create_ohai_plugin('dovecot')

zuazo · 2016-07-19T19:32:27Z

OK, thanks @ledgr. I have reviewed your code. Please, read my comments.

zuazo · 2016-07-19T19:35:09Z

Also there are two unit test errors:

  1) dovecot::ohai_plugin with Ohai 6 uses the template from plugins/
     Failure/Error:
       expect(chef_run).to create_template('/etc/chef/ohai/plugins/dovecot.rb')
         .with_source('ohai_plugins/dovecot.rb.erb')
       expected "template[/etc/chef/ohai/plugins/dovecot.rb]" with action :create to be in Chef run. Other template resources:
     # ./test/unit/recipes/ohai_plugin_spec.rb:29:in `block (3 levels) in <top (required)>'

  2) dovecot::ohai_plugin with Ohai 7 uses the template from plugins7/
     Failure/Error:
       expect(chef_run).to create_template('/etc/chef/ohai/plugins/dovecot.rb')
         .with_source('ohai7_plugins/dovecot.rb.erb')
       expected "template[/etc/chef/ohai/plugins/dovecot.rb]" with action :create to be in Chef run. Other template resources:
     # ./test/unit/recipes/ohai_plugin_spec.rb:38:in `block (3 levels) in <top (required)>'

Finished in 2 minutes 9.2 seconds (files took 1.15 seconds to load)
123 examples, 2 failures, 2 pending

ledgr · 2016-07-20T07:55:13Z

@zuazo: Should be good to go. I have replaced all tests with your proposed expect(chef_run).to create_ohai_plugin('dovecot')

coveralls · 2016-07-20T07:55:22Z

Coverage remained the same at 100.0% when pulling 7b5ec3b on ledgr:fix/ohai_plugin_changes into 1e9f840 on zuazo:master.

coveralls · 2016-07-20T08:16:54Z

Coverage remained the same at 100.0% when pulling 7b5ec3b on ledgr:fix/ohai_plugin_changes into 1e9f840 on zuazo:master.

zuazo · 2016-07-20T12:32:35Z

@ledgr first of all, thanks for your patience with this issue and excuse my "impertinence" but I think we still have to polish this a bit before merging it. Let me know if you need help with the tests.

zuazo · 2016-07-27T12:58:39Z

Hi @ledgr. I completed your PR in 45846dd and merged it manually. Thanks for your work!

ledgr · 2016-07-27T13:02:55Z

Thanks, @zuazo!

zuazo · 2016-10-09T21:27:07Z

Released in 3.0.0.

zuazo added the bug label Jul 9, 2016

ledgr reviewed Jul 19, 2016
View reviewed changes

zuazo reviewed Jul 19, 2016
View reviewed changes

Rewrite ohai_plugin.rb to reflect new ohai cookbook version

7b5ec3b

zuazo added a commit that referenced this pull request Jul 27, 2016

Complete ohai_plugin.rb rewrite (issue #23)

45846dd

zuazo merged commit 7b5ec3b into zuazo:master Jul 27, 2016

zuazo mentioned this pull request Jul 27, 2016

Recipe Compile Error with ohai 4.0.0 #22

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite ohai_plugin.rb to reflect new ohai cookbook version #23

Rewrite ohai_plugin.rb to reflect new ohai cookbook version #23

ledgr commented Jul 4, 2016

ton31337 commented Jul 19, 2016

zuazo commented Jul 19, 2016

ledgr Jul 19, 2016

zuazo Jul 19, 2016

ledgr Jul 20, 2016

zuazo Jul 20, 2016

ledgr commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

zuazo Jul 19, 2016

zuazo commented Jul 19, 2016

zuazo commented Jul 19, 2016 •

edited

Loading

ledgr commented Jul 20, 2016

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

zuazo commented Jul 20, 2016

zuazo commented Jul 27, 2016

ledgr commented Jul 27, 2016

zuazo commented Oct 9, 2016

Rewrite ohai_plugin.rb to reflect new ohai cookbook version #23

Rewrite ohai_plugin.rb to reflect new ohai cookbook version #23

Conversation

ledgr commented Jul 4, 2016

ton31337 commented Jul 19, 2016

zuazo commented Jul 19, 2016

ledgr Jul 19, 2016

Choose a reason for hiding this comment

zuazo Jul 19, 2016

Choose a reason for hiding this comment

ledgr Jul 20, 2016

Choose a reason for hiding this comment

zuazo Jul 20, 2016

Choose a reason for hiding this comment

ledgr commented Jul 19, 2016 • edited Loading

coveralls commented Jul 19, 2016 • edited Loading

zuazo Jul 19, 2016

Choose a reason for hiding this comment

zuazo commented Jul 19, 2016

zuazo commented Jul 19, 2016 • edited Loading

ledgr commented Jul 20, 2016

coveralls commented Jul 20, 2016 • edited Loading

coveralls commented Jul 20, 2016 • edited Loading

zuazo commented Jul 20, 2016

zuazo commented Jul 27, 2016

ledgr commented Jul 27, 2016

zuazo commented Oct 9, 2016

ledgr commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 19, 2016 •

edited

Loading

zuazo commented Jul 19, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading

coveralls commented Jul 20, 2016 •

edited

Loading