Issues with notifies on mongodb_instance definition

[thomas-riccardi]

The mongodb_instance definition accepts the usual notifies parameter, which is great to start services that depend on mongodb for example.

However the current implementation has several issues.

Issue 1:

This seemingly standard and correct usage doesn't work:

mongodb_instance "mongodb" do
  notifies :run, "execute[test_mongodb_notification]"
end

include_recipe "mongodb::replicaset"


execute "test_mongodb_notification" do
  command "echo 'got mongodb notification'"
  action :nothing
end

Error:

==> default: ================================================================================
==> default: Recipe Compile Error in /tmp/vagrant-chef-3/chef-solo-1/cookbooks/ses/recipes/mongodb.rb
==> default: ================================================================================
==> default: 
==> default: 
==> default: Chef::Exceptions::InvalidResourceSpecification
==> default: ----------------------------------------------
==> default: The object `:run' is not valid for resource collection lookup. Use a String like `resource_type[resource_name]' or a Chef::Resource object
==> default: 
==> default: Cookbook Trace:
==> default: ---------------
==> default:   /tmp/vagrant-chef-3/chef-solo-1/cookbooks/mongodb/definitions/mongodb.rb:196:in `block (3 levels) in from_file'
==> default:   /tmp/vagrant-chef-3/chef-solo-1/cookbooks/mongodb/definitions/mongodb.rb:195:in `each'
==> default:   /tmp/vagrant-chef-3/chef-solo-1/cookbooks/mongodb/definitions/mongodb.rb:195:in `block (2 levels) in from_file'
==> default: 
==> default:   /tmp/vagrant-chef-3/chef-solo-1/cookbooks/mongodb/definitions/mongodb.rb:191:in `block in from_file'
==> default:   /tmp/vagrant-chef-3/chef-solo-1/cookbooks/ses/recipes/mongodb.rb:7:in `from_file'
==> default: 
==> default: 
==> default: Relevant File Content:
==> default: ----------------------
==> default: /tmp/vagrant-chef-3/chef-solo-1/cookbooks/mongodb/definitions/mongodb.rb:
==> default: 
==> default: 189:  
==> default: 
==> default: 190:    # service
==> default: 191:    service new_resource.name do
==> default: 
==> default: 192:      provider Chef::Provider::Service::Upstart if node['mongodb']['apt_repo'] == 'ubuntu-upstart'
==> default: 193:      supports :status => true, :restart => true
==> default: 194:      action new_resource.service_action
==> default: 
==> default: 195:      new_resource.service_notifies.each do |service_notify|
==> default: 
==> default: 196>>       notifies :run, service_notify
==> default: 197:      end
==> default: 198:      notifies :create, 'ruby_block[config_replicaset]' if new_resource.is_replicaset && new_resource.auto_configure_replicaset
==> default: 199:      notifies :create, 'ruby_block[config_sharding]', :immediately if new_resource.is_mongos && new_resource.auto_configure_sharding
==> default: 
==> default: 200:        # we don't care about a running mongodb service in these cases, all we need is stopping it
==> default: 201:      ignore_failure true if new_resource.name == 'mongodb'
==> default: 
==> default: 202:    end
==> default: 203:  
==> default: 204:    # replicaset
==> default: 205:    if new_resource.is_replicaset && new_resource.auto_configure_replicaset

This non standard way works though:

mongodb_instance "mongodb" do
  notifies [ "execute[test_mongodb_notification]" ]
end

include_recipe "mongodb::replicaset"


execute "test_mongodb_notification" do
  command "echo 'got mongodb notification'"
  action :nothing
end

Issue 2:

There is no way to specify a different action than :run, and no timer different than the default (:delayed).

Issue 3:

When those notifications are executed, the replicaSet is not ready yet. This is related to #244 and #326.

Code analysis:

All of this is because of these lines: https://github.com/edelight/chef-mongodb/blob/0.16.1/definitions/mongodb.rb#L194-L196

issue 3 is because the config_replset should be done immediately, and both internal notifications should be done before user notifications (in case they are also immediate).

issues 1 and 2 could be fixed by not imposing :run.
However I don't know how multiple identical parameters are handled in definitions, so I don't know how to fix this exactly.

Expected behavior:

All of these should work:

mongodb_instance "mongodb" do
  notifies :run, "execute[some_execution]"
end

mongodb_instance "mongodb" do
  notifies :run, "execute[some_execution]"
  notifies :run, "execute[some_other_execution]"
end

mongodb_instance "mongodb" do
  notifies :run, "execute[test_replSet]", :immediately
  notifies :run, "execute[test_shard]", :immediately
end

mongodb_instance "mongodb" do
  notifies :restart, "service[some_service]"
end

And any combination of them.

[thomas-riccardi]

Issue 3 is easily fixed by using :immediately notification for the ruby_block[config_replicaset]: I tested this for one week now.
Is a release for Issue 3 planned soon? Or at least a commit in master? (Otherwise I'll have to create a local fork)

[jamesonjlee]

for issue 3, you can submit a PR for it, seems simple enough to accept.