[chef-dev] Re: recipe_eval notifies every time

[Daniel DeLeo < >]

On Sunday, January 27, 2013 at 1:46 PM, Andrea Campi wrote:

tl;dr I think Chef::Provider #set_updated_status should not force updated_by_last_action to true for LWRPs whose action(s) call recipe_eval.

Example: the apt cookbook was recently changed from the traditional, cargo-cult-ish way to recipe_eval (https://github.com/opscode-cookbooks/apt/commit/4b5bfd513deb6aeb9258517243f1474f5cd4667c)

This is a very nice change, as it makes things more readable and more "natural" to anybody who knows Chef.

Unfortunately, now apt_repository will notify every time.

That happens because in https://github.com/opscode/chef/blob/master/lib/chef/provider.rb#L131 converge_actions will not be empty, and thus the resource will be forced to updated=true. There isn't much an LWRP can do to fix that.

So I'm wondering, why does #set_updated_status do that? Is there any reason or is it old cruft? Do people even agree this is a problem? :)

Andrea

1. Recipe eval isn't really meant to be used by LWRPs in this way. I added it as a way for the deploy provider to support inline chef runs (TBH, it was a mistake to define it on the provider base class, IMO). Note that using recipe_eval like this doesn't give you any good options for setting the updated status of a LWRP resource, because the temporary run context is a local variable that you can't easily access. recipe_eval has its own compile/converge phase, so you'd need some ruby_block hackery to access it and check for updated resources at the end of the inline chef run. In the apt repository provider you referenced, it's not too onerous to capture a local variable in the closure to handle this, but this obviously gets less convenient as the number of resources you need to account for increases.

2. In Chef 11, LWRP providers can opt-in to "inline_resource mode" which uses code similar to recipe_eval but specifically handles setting updated/not updated correctly. This isn't much comfort to cookbook authors who will need to support Chef 10.x for a while, but the same quandary occurs every time a cookbook author wants to use a new feature, and there's not much that can be done to make that better.

See: https://github.com/opscode/chef/blob/master/lib/chef/provider/lwrp_base.rb

3. Providers should only create a converge action (e.g., call converge_by) when they have something to do, that is:

    if should_modify_the_system?

      converge_by("change property X") do; # do some stuff; end

    end

By obeying this assumption, why-run mode and output formatters can provide richer information to the user about what a provider is doing. In the specific case of running an arbitrary block of code, Chef doesn't know if the system will be modified, so the only reasonable thing to do is assume it would modify the system.

Considering the above, Chef::Provider should reasonably be able to assume that the system is updated if there are any converge_by blocks.

For the immediate future with LWRPs, I don't see a better solution than backporting inline_resource mode or something like it.

-- 

Daniel DeLeo