On Friday, August 5, 2011 at 4:33 AM, Avishai Ish-Shalom wrote:
following up on CHEF-1994 (http://tickets.opscode.com/browse/CHEF-1994), i think the notification mechanism deserves some rethinking. The implementation of the notifies and subscribes syntax required late evaluation/resolution along with queuing and thus major changes.
The notification system has already undergone design changes over the years and i think a redesign is in order, perhaps as a more general messaging/hook system:
template “my.cnf” do notifies :reload, “service[mysql]”, :on_update end
template “/etc/php5/apache2/php.ini” do notifies :restart, “service[php5]”, :very_late end
execute “do some important stuff” do notifies :create, “ruby_block[rollback important stuff]”, :on_failure end
The general design i had in mind is of a central message queue class receiving messages from resources and activating pre-registered callbacks on desired delay (immediately, delayed, very_late, last). notifies/subscribes methods will register callbacks in the message queue.
This design pattern will allow complex execution patterns for recipes, which is both and advantage and disadvantage. It will also allow easy extensions of chef-client which various hooks/handlers if we re-implement parts of chef-client to send messages. (why not trigger handlers using the same mechanism? we can have handlers before_chef_run, before_convergence, etc.)
– Regards, Avishai
I agree we need some re-architecting of the notification mechanism as you’ve noticed in implementing your patch. There are a few issues worth untangling:
- Notifications are a property of individual resources, so it’s difficult to implement late resolution for subscribes the same as we have for notifies.
- If you try to trigger a notification on a resource that does not exist (typo, deleted the target resource, etc.) it’s better to fail early and have actionable info in the error message.
- When creating resources during convergence (in a ruby_block), you have to use the #resources method to look up the resource and attach the notification; the string syntax introduced in 0.9.8 doesn’t work.
- More flexible scheduling of notifications is desired.
To these I could also add that LWRPs don’t provide a good interface to marking themselves as updated (when used as a collection of other chef resources, anyway). This is something that might be better to fix in the LWRP DSL, but I think is worth mentioning.
To me, any solution that has a negative impact on #2 is probably a non-starter, which means for the 80%-90% use case of setting up notifications “statically” during compile, the implementation should be able to point out any errors before convergence starts.
I don’t know if this is what you have in mind, but I think the above can be accomplished by storing notifications in a separate data structure, probably “owned” by the ResourceCollection or the Runner. As long as there’s a validation method that can ensure that all the resources exist, it would satisfy #1 and #2. #3 would work, but you’d need to take care that it wouldn’t silently fail if you tried to notify a resource that doesn’t exist (e.g., by toggling a “strict mode” or something like that).
As for having more flexible scheduling of notifications, I’m not sure what you have in mind. Do you just want the ability to add an action to the end of the list of delayed notifications?
As for using the notification mechanism to run handlers and the like, Chef has an internal observer event mechanism already. I don’t know that the two could be combined, since the internal observer system implements events that happen before cookbooks are sync’d.