DRAFT: RFC for Resource#current_value

[jkeiser]

CC @coderanger @danielsdeleo @someara @lamont-granquist @jaym

Until we've worked through the use cases @coderanger has brought up (I haven't had time this week), this is on hold.

[coderanger]

Probably not intended to commit this.

[coderanger]

I don't know if this should be a global API. I agree the general idea is useful, but I'm not sure this needs to be in core. The examples with core resources feel kind of contrived, this really seems to come up more in super-advanced use cases like chef-provisioning and poise-ruby/python (if we extend this to the general case of "resources having output data". For the example using gem_package to work correctly it would be relying heavily on cloning since you aren't setting gem_binary and that might have been used earlier in a previous definition of the resource. This would basically mean this cannot be used safely in library cookbooks as a resource needs to be defined twice, once for the collection and once for output data.

Basically this feels like a footgun unless we drop down a level and come up with a more general solution for resources having output data, of which current state will be a specific use case.

[coderanger]

Copying from IRC for the future:

1. A way to get a reference version of a resource that does not cause it to also converge
2. A way to get the output of multiple actions that occur on a resource
3. A way to handle the degenerate case of RFC: Windows Package Core Resource/Provider #2 with a single action in a simple way (90% case)

[coderanger]

Also a list of use cases we should map out:

1. Current value of version for a gem_package where the package resource might or might not have a gem_binary (control property) defined earlier in the recipe.
2. Getting the AWS ID for a newly created machine resource.
3. Getting stdout for an execute resource.
4. Getting the ruby_binary path for a ruby_runtime resource.

[lamont-granquist]

I'd like to be able to say:

package package_installable?("git-core") ? "git-core" : "git"

package_installable? would then be some sugar around using the package resource to see if the arg was an available package or not. I want to be able to re-use the package resource so that I don't have to think about yum and apt and everything else myself. I also don't want to trigger the installation and then rescue or epic_fail true. And this is cleaner than platform/platform_family/platform_version case statements and is really the behavior that I want rather than having to think about which debian/ubuntu/linuxmint platform_versions renamed 'git-core' to 'git'.

[martinb3]

Did you decide to call this current_resource instead of current_value after writing this title and the paragraph of text under it? I think these might just need to be updated to:

# Allow Current Resource to be Created from Resource Collection

Resources get a `current_resource` method, which discovers an existing resource in the resource collection and loads a new copy of it

[jkeiser]

The intent is to read the actual value from the system, not from the resource collection, actually.

[martinb3]

But you say current_value here when the examples further down call the new DSL current_resource, right? Or is this proposing both?

[jkeiser]

Just current_value--which corresponds to the new load_current_value API. current_resource might be the right word, but in that case it would still correspond to Provider.load_current_resource, an existing API which lets you load the current value of the resource from the system.

[martinb3]

Just reading the other comments here, and catching up.

I think my use case is a little less elaborate than @coderanger's described one. I'd simply like to see what parameters were set on a resource, by looking it up. For example, if a resource installs some files at a particular path, I'd love to fetch that path later in a recipe in order to some additional work on it.

Regarding @lamont-granquist's example, I couldn't tell if package_installable? happens at converge or compile time. That example feels to me like yet another case I hadn't considered -- drawing on a resource to query some information. Perhaps that would be better suited to something like chef-sugar? Querying state doesn't feel idempotent when used in that example.

Overall, I'd be happy with this RFC being implemented, FWIW. 👍 for writing it @jkeiser!

[lamont-granquist]

So package_installable? clearly would need to be compile-time and the method itself would probably be in chef-sugar, but right now there's no good way to implement that in chef-sugar. This RFC should allow that method to actually get written. And again the problem is that in order to write that you'd want an abstraction over yum/apt/etc packages and then be able to query state without modifying it. That is what is accomplished by being able to grab a resource and inspect the load_current_resource state. Otherwise we have to right some query tools which re-implement a fascade over yum/apt/etc and that's exactly what we already have with the package pseudo-resource, so I want to re-use that.

And querying state like that is perfectly idempotent. But instead of having awful case statements around platform and platform_version (and having to take into account debian, ubuntu and linuxmint and where exactly the pv on all of those uniquely change) you just duck-type the operating system. In pseudo-code it has this kind of shape:

install_git_core if platform.respond_to?(:install_git_core)

For any given platform and platform_version the boolean value it returns is deterministic, and overall its idempotent and convergent, but it allows you to query the operating system to find out how it behaves rather than having to manage a case statement with knowledge about how every platform and platform_version behaves. Because ultimately I really don't care. If I have a platform which lets me install 'git-core' then I know I'm on old-debian where 'git' is really 'gnu interactive toolset' and I need to use 'git-core', otherwise I just use 'git' and that works for every case I'm aware of. If we discover a new platform and platform_versions which have a similar lineage to debian then it'll just work without modification (although the better win is that we already have that platform, which is linuxmint, and i think i'm the only person in the Chef community who ever bothers to convert ubuntu versions to linuxmint versions whenever i find them).

[lamont-granquist]

Also -dev vs -devel vs none for header files gets a lot easier:

package "foo"
package "foo-dev" if package_installable?("foo-dev")
package "foo-devel" if package_installable?("foo-devel")

That'll support fedora, amazon, all the rhels, debian, ubuntu, raspbian, linuxmint and arch at least right out of the box, and has a good shot of solaris, gentoo, etc i think. All without a stupid case statement around the platform.

[lamont-granquist]

There's another use case here, which is minitest-chef-handler. It already implements this kind of behavior using resources and load_current_resource in the provider in order to generate matchers for minitest. So if you're writing a rule that a file should have mode 0644 or whatever, internally minitest will create a file resource and call load_current_resource and then pull the mode out.

If we turn this into a public API it will create less breakage for minitest-chef-handler and we should be able to integrate similar chef resources based matchers with rspec.

[martinb3]

Maybe idempotent is a poor word choice on my part, but something about that example does not feel declarative about the state of the system. In any case, I withdraw the comment since I can't really justify why it strikes me that way.

My use case would be to look at the desired state of a resource that has already been declared, not the current state of the resource, so I'd love to see a way for us to look up both. I would think that if we looked up the resource in the resource collection, it would contain both the result of load_current_resource data as well as the desired state (new_resource.params).

[martinb3]

Perhaps I should just use the existing #resources DSL functionality for my use case, and leave this to the other use cases. There's just something cleaner about package('foo').version over resources(:package, 'foo').version.

[lamont-granquist]

Its still also declarative, but it is giving you a much better if-then language to use: "if you can install git-core then it should be installed" or "if you can install zlib-devel, then it should be installed". Rather than "if you are on ubuntu older than X, or linuxmint older than Y, or debian older than X, then git-core should be installed", which is tedious.

[lamont-granquist]

seems this snuck into the code:

https://github.com/chef/chef/blob/3baa2c1abc07a21acf9dd1430d4b97700c2835a1/lib/chef/resource.rb#L1161-L1176

should probably close this RFC, we could open the RFC again if there were issues with the current_value implementation there.

[thommay]

Agreed.

[lamont-granquist]

We have load_current_value and current_value, and this RFC is inaccurate as to how they've worked now for years.