From 0eef7753802ce3739bf63060e822f98657ef056a Mon Sep 17 00:00:00 2001 From: John Keiser Date: Tue, 12 May 2015 09:56:31 -0700 Subject: [PATCH 1/7] Add load and converge proposal --- new/load-and-converge.md | 249 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 new/load-and-converge.md diff --git a/new/load-and-converge.md b/new/load-and-converge.md new file mode 100644 index 00000000..965fe265 --- /dev/null +++ b/new/load-and-converge.md @@ -0,0 +1,249 @@ +--- +RFC: unassigned +Author: John Keiser +Status: Draft +Type: Standards Track +--- + +# Easy Resource Load And Converge + +With the introduction of `action` on resources, it becomes useful to have a +blessed way to get the actual value of the resource. This proposal makes it +work with a "lazy read," making two benefits possible at once: + +- A super easy converge model that automatically compares current vs. desired + values and prints green text +- A familiar read API for users (who currently either use raw API code or give up) + +## Motivation + + As a Chef resource writer, + I want to be able to read the current value of my resource at converge time, + so that it is easy to tell the difference between current and desired value. + + As a Chef resource writer, + I want a converge model that compares current and desired values for me, + So that the easiest converge to write is the most correct one. + + As a Chef resource writer, + I want to write my resource's read API code on my resource, + So that my users can use it as well as me. + + As a Chef user, + I want to be able to read resources with the resource interface, + So that I don't have to learn two systems to make decisions. + +## Specification + +Three very closely related features are here, relating to attribute load and +converge. They make each other stronger, which is why they are all included in +this RFC. + +1. **`load`**. We introduce this syntax: + + ```ruby + class File < Chef::Resource + load do + if File.exist?(path) + mode File.stat(path).mode + content IO.read(path) + end + end + end + ``` + +2. `converge`. This syntax enables simple, efficient, why-run-safe test-and-set: + + ```ruby + class File < Chef::Resource + action :create do + converge do + File.chmod(mode, path) + IO.write(content) + end + end + end + ``` + +3. Attribute current value: This exposes a very simple read API: + + ```ruby + puts file('/x.txt').content + ``` + +### `load do`: in-place resource load + +The user `load do ` is entered in the class, a `current_resource` method +is created which creates a new resource, calls the block to load it, caches +and returns the resource. + +The block is run inside the newly created resource, allowing for a natural +syntax without dots and without repetitive ceremony. + +```ruby + load do + if File.exist?(path) + # Sets "mode" on the current resource. + mode File.stat(path).mode + end + end +``` + +The block will also be passed the original resource as a parameter, in case it +is needed. + +#### Existence Test + +The `load` block may return `:nonexistent` to signal that the resource does not +exist. + +#### Inheritance + +The `current_resource` method does not call the superclass's `current_resource` +method by itself, but you may call `super()` from the block to invoke it. + +#### Handling Multi-Key Resources + +The new resource is created with identity values copied over (and non-identity +values left clear). Any attributes tagged with `identity: true` will be copied. +`name_attribute` implies `identity: true`, and `name` automatically has `identity: true`. + +```ruby +class DataBagItem < Chef::Resource + attribute :item_name, name_attribute: true + attribute :data_bag_name, index: true + attribute :data + load do + data Chef::DataBagItem.new(data_bag_name, item_name).data + end +end +``` + +### `converge`: automatic test-and-set + +The new `converge do ... end` syntax is added to actions, which enables a *lot* +of help for resource writers to make safe, effective resources. It performs +several key tasks common to nearly every resource (which are often not done +correctly): + +- Goes through all attributes on the resource and checks whether the desired + value is different from the current value. +- If any attributes are different, prints appropriate green text. +- Honors why-run (and does not call the `converge` block if why-run is enabled). + +#### Compound Resource Convergence + +Some resources perform several different operations depending on what is set. +`converge :attribute1, :attribute2, ... do` allows the user to target different +groups of changes based on exactly which attributes have changed: + +```ruby +class File < Chef::Resource + action :create do + converge :mode do + File.chmod(mode, path) + end + converge :content do + IO.write(path, content) + end + end +end +``` + +### Attribute Current Value + +To enable the read API, the following rule is introduced: + +> By default, resources make no change to the actual value: the desired value of +> an attribute is its *current value*, if any. + +In practical terms, this means that if you don't set an attribute to a value, +then when you ask for the attribute's value, you get the *current* value, not +`nil`. + +#### Default Value + +The `default` value is reserved for cases where there is no current value for a +resource. It is the value that will be placed on the attribute if the resource +is *created*. It does not come into play if the resource already has a value. + +#### Use Case: Safer Resources + +This means it's a bit harder to make an unsafe resource--if a property is +not set, its value will be the current value, so even if you ham-fistedly set +the value of the real resource, it'll just replace the current value. + +#### Use Case: Patchy Resources + +This rule allows for "patchy resources," which are common but hard to write +currently: + +```ruby +# Must leave content alone! +file '/x.txt' do + mode 0666 +end +# Must leave mode alone! +file '/y.txt' do + content "Hello World" +end +``` + +#### Use Case: Attribute Append + +This also allows for appends, something that has been difficult up to this point. +This means you can say "please make sure this attribute contains X" without the +resource author having to make a specific method for you to do that: + +```ruby +node 'mynode' do + normal_attributes[:foo] = 'bar' + run_list << 'apache2' +end +``` + +Even though the resource author didn't specifically mention it: + +```ruby +class Node < Chef::Resource + attribute :normal_attributes, default: {} + attribute :run_list, default: [] + load do + node = Chef::Node.get(name) + normal_attributes node.normal + run_list node.run_list + end + action :create do + converge do + node = Chef::Node.new(name) + node.run_list = run_list.uniq + node.normal = normal_attributes + node.save + end + end +end +``` + +#### Use Case: Read API + +Resource authors have always written code to read the current value of a +resource (in `load_current_reosurce`). What has been frustrating (for both +resource writers and users) is that this code is not available to the user, who +really does need it from time to time. + +`current_resource` would be sufficient for this; but since we're doing lazy load +for these other reasons, the API gets even stronger: + +```ruby +puts file('/x.txt').content +``` + +Since the `content` attribute is not set on this resource, it returns +`current_value.content` (or the default value, if the file does not exist). + +## Copyright + +This work is in the public domain. In jurisdictions that do not allow for this, +this work is available under CC0. To the extent possible under law, the person +who associated CC0 with this work has waived all copyright and related or +neighboring rights to this work. From 31fe261fd9568afe898a0ca3505bfd23de52aaac Mon Sep 17 00:00:00 2001 From: John Keiser Date: Tue, 12 May 2015 10:08:06 -0700 Subject: [PATCH 2/7] Update load-and-converge.md --- new/load-and-converge.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index 965fe265..14febc4f 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -8,9 +8,10 @@ Type: Standards Track # Easy Resource Load And Converge With the introduction of `action` on resources, it becomes useful to have a -blessed way to get the actual value of the resource. This proposal makes it -work with a "lazy read," making two benefits possible at once: +blessed way to get the actual value of the resource. This proposal adds a +`load` DSL enabling: +- Low-ceremony load methods (as easy to write as we can make it) - A super easy converge model that automatically compares current vs. desired values and prints green text - A familiar read API for users (who currently either use raw API code or give up) From 1b2d45e9a58c5763f8a55609d0b671522b5f0b49 Mon Sep 17 00:00:00 2001 From: John Keiser Date: Tue, 12 May 2015 15:37:16 -0700 Subject: [PATCH 3/7] Explain unspecified attribute behavior better --- new/load-and-converge.md | 92 ++++++++++++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 22 deletions(-) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index 14febc4f..607f7cd1 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -93,11 +93,6 @@ syntax without dots and without repetitive ceremony. The block will also be passed the original resource as a parameter, in case it is needed. -#### Existence Test - -The `load` block may return `:nonexistent` to signal that the resource does not -exist. - #### Inheritance The `current_resource` method does not call the superclass's `current_resource` @@ -151,33 +146,86 @@ class File < Chef::Resource end ``` -### Attribute Current Value +### Unspecified Attributes + +The value of an attribute in a Chef resource represents a *desired value* for +the attribute. When an attribute is unspecified, the user is simply not stating +their desire. It's therefore our job to do something reasonable and sane. + +To accomodate this, we add a second step to the attribute getter: + +1. If `attribute` has been set to a desired value, that is returned. +2. If `current_resource.attribute` is loaded with a current value, that is returned. +3. Otherwise, the attribute's default value is returned. + +This means that an unspecified attribute's desired value is its current value, +if one exists. The current attribute system (with no load method) says that an +unspecified attribute's desired value is its *default* value. + +This system is biased towards these outcomes: + +- When *updating* a resource with an unspecified attribute, we make no change. +- When *creating* a resource with an unspecified attribute, we choose a reasonable + default (specified by the resource's default value). -To enable the read API, the following rule is introduced: +#### Backcompat -> By default, resources make no change to the actual value: the desired value of -> an attribute is its *current value*, if any. +This is 100% backwards compatible with the current system, because it depends on +`load` filling in attributes, and existing resources do not implement `load`. + +## Use Cases + +The below is not prescriptive, but is a discussion of some of the supported +use cases: + +### Updating a resource + +When you update a resource and leave an attribute unspecified, the converge +method is easy to mess up, and harder to get right. + +```ruby +class File < Chef::Resource + attribute :path, name_attribute: true + attribute :content -In practical terms, this means that if you don't set an attribute to a value, -then when you ask for the attribute's value, you get the *current* value, not -`nil`. + load do + if File.exist?(path) + content IO.read(path) + end + end + + action :create do + if content != current_resource.content + IO.write(path, content) + end + end +end + +file '/x.txt' do + # Note: content was not specified! +end +``` -#### Default Value +Given this (perfectly reasonable looking) code, the current system would +*truncate the file* if you didn't specify `content`. You need to check if +`content.nil?` before you compare. This means that the most obvious, +least-ceremony thing to do is the wrong thing. This is also not an error you are +likely to catch early: *not* specifying attributes is not among the first things +people test. -The `default` value is reserved for cases where there is no current value for a -resource. It is the value that will be placed on the attribute if the resource -is *created*. It does not come into play if the resource already has a value. +In the proposed system, When an attribute defaults to the *current* value, the +code above will not overwrite the file at all. -#### Use Case: Safer Resources +#### Use Case: creating a resource -This means it's a bit harder to make an unsafe resource--if a property is -not set, its value will be the current value, so even if you ham-fistedly set -the value of the real resource, it'll just replace the current value. +When creating a resource, the above code will do the same thing in both cases: +use the default value and create an empty file. This seems like the right thing +in either case. #### Use Case: Patchy Resources -This rule allows for "patchy resources," which are common but hard to write -currently: +This rule allows for "patchy resources," which are commonly desired and +intuitive but hard to write currently: ```ruby # Must leave content alone! From fd0338042be41aaff9b59030fcb288e14ae57dfd Mon Sep 17 00:00:00 2001 From: John Keiser Date: Wed, 13 May 2015 16:13:12 -0700 Subject: [PATCH 4/7] Add is_set? --- new/load-and-converge.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index 607f7cd1..a3e6943e 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -168,6 +168,17 @@ This system is biased towards these outcomes: - When *creating* a resource with an unspecified attribute, we choose a reasonable default (specified by the resource's default value). +#### is_set?(:attribute) + +While you will end up wanting the desired value most of the time, there are times +where you need to know whether the value of the attribute was explicitly set by +the user. For these times, `is_set?(:attribute)` will tell you whether an attribute +was explicitly set or not. + +This also handles the existing case where the attribute returns the *default +value*, allowing you to distinguish between the default value and a user setting +the value explicitly to its default. + #### Backcompat This is 100% backwards compatible with the current system, because it depends on @@ -213,7 +224,7 @@ least-ceremony thing to do is the wrong thing. This is also not an error you are likely to catch early: *not* specifying attributes is not among the first things people test. -In the proposed system, When an attribute defaults to the *current* value, the +In the proposed system, when an attribute defaults to the *current* value, the code above will not overwrite the file at all. #### Use Case: creating a resource @@ -224,8 +235,8 @@ in either case. #### Use Case: Patchy Resources -This rule allows for "patchy resources," which are commonly desired and -intuitive but hard to write currently: +This rule works very well for "patchy resources," which are commonly desired and +intuitive to use, but hard to write currently: ```ruby # Must leave content alone! From b5e198ed07fc8ba64f31ce8b0fd8337e48120d1e Mon Sep 17 00:00:00 2001 From: John Keiser Date: Tue, 14 Jul 2015 14:05:58 -0600 Subject: [PATCH 5/7] Remove compile-time read from proposal, rename load -> load_actual_value --- new/load-and-converge.md | 277 ++++++++++----------------------------- 1 file changed, 71 insertions(+), 206 deletions(-) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index a3e6943e..09ffd6a1 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -8,13 +8,11 @@ Type: Standards Track # Easy Resource Load And Converge With the introduction of `action` on resources, it becomes useful to have a -blessed way to get the actual value of the resource. This proposal adds a -`load` DSL enabling: +blessed way to get the actual value of the resource. This proposal adds `load` and `converge` to help with this purpose, enabling: - Low-ceremony load methods (as easy to write as we can make it) - A super easy converge model that automatically compares current vs. desired values and prints green text -- A familiar read API for users (who currently either use raw API code or give up) ## Motivation @@ -26,90 +24,53 @@ blessed way to get the actual value of the resource. This proposal adds a I want a converge model that compares current and desired values for me, So that the easiest converge to write is the most correct one. - As a Chef resource writer, - I want to write my resource's read API code on my resource, - So that my users can use it as well as me. - - As a Chef user, - I want to be able to read resources with the resource interface, - So that I don't have to learn two systems to make decisions. - ## Specification -Three very closely related features are here, relating to attribute load and -converge. They make each other stronger, which is why they are all included in -this RFC. - -1. **`load`**. We introduce this syntax: - - ```ruby - class File < Chef::Resource - load do - if File.exist?(path) - mode File.stat(path).mode - content IO.read(path) - end - end - end - ``` +### `load_actual_value`: in-place resource load -2. `converge`. This syntax enables simple, efficient, why-run-safe test-and-set: +When using `action`, one needs a way to load the *actual* system value of the resource, so that it can be compared to the desired value and a decision made as to whether to change anything. - ```ruby - class File < Chef::Resource - action :create do - converge do - File.chmod(mode, path) - IO.write(content) - end - end - end - ``` +When the resource writer defines `load_actual_value` on the resource class, it can be called to load the real system value into the resource. Before any action runs, this will be used by `load_current_resource` to load the resource. `action` will do some important work before calling the new method: -3. Attribute current value: This exposes a very simple read API: +1. Create a new instance of the resource with the same name. +2. Copy all non-desired-state values from the desired resource into the new instance. +3. Call `load_actual_value` on the new instance. - ```ruby - puts file('/x.txt').content - ``` +#### Non-existence -### `load do`: in-place resource load +To appropriately handle actual value loading, the user needs a way to specify that the actual value legitimately does not exist (rather than simply not filling in the object and getting `nil`s in it). If `load_actual_value` raises `Chef::Exceptions::ActualValueDoesNotExist`, the new resource will be discarded and `current_resource` becomes `nil`. The `actual_value_does_not_exist!` method can be called to raise this. -The user `load do ` is entered in the class, a `current_resource` method -is created which creates a new resource, calls the block to load it, caches -and returns the resource. - -The block is run inside the newly created resource, allowing for a natural -syntax without dots and without repetitive ceremony. +NOTE: The alternative was to have users return `false` if the resource does not exist; but I didn't want users to be forced into the ceremony of a trailing `true` line. ```ruby - load do - if File.exist?(path) - # Sets "mode" on the current resource. - mode File.stat(path).mode - end + load_actual_value do + # Check for existence before doing anything else. + actual_value_does_not_exist! if !File.exist?(path) + + # Set "mode" on the resource. + mode File.stat(path).mode end ``` -The block will also be passed the original resource as a parameter, in case it -is needed. +The block will also be passed the original (desired) resource as a parameter, in case it is needed. #### Inheritance -The `current_resource` method does not call the superclass's `current_resource` -method by itself, but you may call `super()` from the block to invoke it. +`super` in `load_actual_value!` will call the superclass's `load_actual_value!` method. #### Handling Multi-Key Resources -The new resource is created with identity values copied over (and non-identity -values left clear). Any attributes tagged with `identity: true` will be copied. -`name_attribute` implies `identity: true`, and `name` automatically has `identity: true`. +The new resource is created with all properties copied over *except* desired state properties (properties in `ResourceClass.state_properties`). This means `name`, and properties with `identity: true` or `desired_state: false` are copied over. Normal `property` and `attribute` are not. ```ruby class DataBagItem < Chef::Resource + # Copied attribute :item_name, name_attribute: true - attribute :data_bag_name, index: true + attribute :data_bag_name, identity: true + attribute :recursively_delete, desired_state: false + # Not copied: attribute :data - load do + def load_actual_value! data Chef::DataBagItem.new(data_bag_name, item_name).data end end @@ -117,193 +78,97 @@ end ### `converge`: automatic test-and-set -The new `converge do ... end` syntax is added to actions, which enables a *lot* -of help for resource writers to make safe, effective resources. It performs -several key tasks common to nearly every resource (which are often not done -correctly): +The new `converge do ... end` syntax is added to actions, which enables a *lot* of help for resource writers to make safe, effective resources. It performs several key tasks common to nearly every resource (which are often not done correctly): - Goes through all attributes on the resource and checks whether the desired value is different from the current value. - If any attributes are different, prints appropriate green text. - Honors why-run (and does not call the `converge` block if why-run is enabled). -#### Compound Resource Convergence - -Some resources perform several different operations depending on what is set. -`converge :attribute1, :attribute2, ... do` allows the user to target different -groups of changes based on exactly which attributes have changed: - ```ruby class File < Chef::Resource + property :path, name_attribute: true + property :content + + load_actual_value do + actual_value_does_not_exist! unless File.exist?(path) + content IO.read(path) + end + action :create do - converge :mode do - File.chmod(mode, path) - end - converge :content do + converge do IO.write(path, content) end end end ``` -### Unspecified Attributes - -The value of an attribute in a Chef resource represents a *desired value* for -the attribute. When an attribute is unspecified, the user is simply not stating -their desire. It's therefore our job to do something reasonable and sane. - -To accomodate this, we add a second step to the attribute getter: - -1. If `attribute` has been set to a desired value, that is returned. -2. If `current_resource.attribute` is loaded with a current value, that is returned. -3. Otherwise, the attribute's default value is returned. - -This means that an unspecified attribute's desired value is its current value, -if one exists. The current attribute system (with no load method) says that an -unspecified attribute's desired value is its *default* value. - -This system is biased towards these outcomes: - -- When *updating* a resource with an unspecified attribute, we make no change. -- When *creating* a resource with an unspecified attribute, we choose a reasonable - default (specified by the resource's default value). +#### Desired value = actual value -#### is_set?(:attribute) +> The easiest way to write a resource must be the most correct one. -While you will end up wanting the desired value most of the time, there are times -where you need to know whether the value of the attribute was explicitly set by -the user. For these times, `is_set?(:attribute)` will tell you whether an attribute -was explicitly set or not. +There is a subtle pitfall when updating a resource, where the user has set *some* values, but not all. One can easily end up writing a resource which will overwrite perfectly good system properties with their defaults, which can cause instability. If the user does not specify a property, it is generally preferable to preserve its existing value rather than overwrite it. -This also handles the existing case where the attribute returns the *default -value*, allowing you to distinguish between the default value and a user setting -the value explicitly to its default. - -#### Backcompat - -This is 100% backwards compatible with the current system, because it depends on -`load` filling in attributes, and existing resources do not implement `load`. - -## Use Cases - -The below is not prescriptive, but is a discussion of some of the supported -use cases: - -### Updating a resource - -When you update a resource and leave an attribute unspecified, the converge -method is easy to mess up, and harder to get right. +To prevent this, referencing the bare property in an `action` will now yield the *actual* value if load_actual_value succeeded, and the *default* value if we are creating a new resource (if `load_actual_value` raised `ActualValueDoesNotExist`). ```ruby class File < Chef::Resource - attribute :path, name_attribute: true - attribute :content - - load do - if File.exist?(path) - content IO.read(path) - end + property :path, name_attribute: true + property :mode, default: 0666 + property :content + + load_actual_value do + actual_value_does_not_exist! unless File.exist?(path) + mode File.stat(path).mode + content IO.read(path) end action :create do - if content != current_resource.content + converge do + File.chmod(mode, path) IO.write(path, content) end end end file '/x.txt' do - # Note: content was not specified! + # Before the change, the above code would have modified `mode` to be `0666`. + # After, it leaves `mode` alone. + content 'Hello World' end ``` -Given this (perfectly reasonable looking) code, the current system would -*truncate the file* if you didn't specify `content`. You need to check if -`content.nil?` before you compare. This means that the most obvious, -least-ceremony thing to do is the wrong thing. This is also not an error you are -likely to catch early: *not* specifying attributes is not among the first things -people test. - -In the proposed system, when an attribute defaults to the *current* value, the -code above will not overwrite the file at all. - -#### Use Case: creating a resource - -When creating a resource, the above code will do the same thing in both cases: -use the default value and create an empty file. This seems like the right thing -in either case. +There will be times when the old behavior of overwriting with defaults is desired. The resource writer can still find out whether `mode` was set with `property_is_set?(:mode)`, and can still access the default value with `new_resource.mode` if it is not set. -#### Use Case: Patchy Resources +There are no backwards-compatibility issues with this because it only applies to `action`, which has not been released yet. -This rule works very well for "patchy resources," which are commonly desired and -intuitive to use, but hard to write currently: - -```ruby -# Must leave content alone! -file '/x.txt' do - mode 0666 -end -# Must leave mode alone! -file '/y.txt' do - content "Hello World" -end -``` - -#### Use Case: Attribute Append - -This also allows for appends, something that has been difficult up to this point. -This means you can say "please make sure this attribute contains X" without the -resource author having to make a specific method for you to do that: - -```ruby -node 'mynode' do - normal_attributes[:foo] = 'bar' - run_list << 'apache2' -end -``` +#### Compound Resource Convergence -Even though the resource author didn't specifically mention it: +Some resources perform several different (possibly expensive) operations depending on what is set. `converge :attribute1, :attribute2, ... do` allows the user to target different groups of changes based on exactly which attributes have changed: ```ruby -class Node < Chef::Resource - attribute :normal_attributes, default: {} - attribute :run_list, default: [] - load do - node = Chef::Node.get(name) - normal_attributes node.normal - run_list node.run_list +class File < Chef::Resource + property :path, name_attribute: true + property :mode + property :content + + load_actual_value do + actual_value_does_not_exist! unless File.exist?(path) + mode File.stat(path).mode + content IO.read(path) end + action :create do - converge do - node = Chef::Node.new(name) - node.run_list = run_list.uniq - node.normal = normal_attributes - node.save + converge :mode do + File.chmod(mode, path) + end + converge :content do + IO.write(path, content) end end end ``` -#### Use Case: Read API - -Resource authors have always written code to read the current value of a -resource (in `load_current_reosurce`). What has been frustrating (for both -resource writers and users) is that this code is not available to the user, who -really does need it from time to time. - -`current_resource` would be sufficient for this; but since we're doing lazy load -for these other reasons, the API gets even stronger: - -```ruby -puts file('/x.txt').content -``` - -Since the `content` attribute is not set on this resource, it returns -`current_value.content` (or the default value, if the file does not exist). - ## Copyright -This work is in the public domain. In jurisdictions that do not allow for this, -this work is available under CC0. To the extent possible under law, the person -who associated CC0 with this work has waived all copyright and related or -neighboring rights to this work. +This work is in the public domain. In jurisdictions that do not allow for this, this work is available under CC0. To the extent possible under law, the person who associated CC0 with this work has waived all copyright and related or neighboring rights to this work. From 71d2a029ad980d2365a457f7981b91aa2a6786ee Mon Sep 17 00:00:00 2001 From: John Keiser Date: Thu, 23 Jul 2015 09:37:54 -0600 Subject: [PATCH 6/7] Rename load_actual_value to load_current_value --- new/load-and-converge.md | 72 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index 09ffd6a1..6924aaf9 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -36,6 +36,34 @@ When the resource writer defines `load_actual_value` on the resource class, it c 2. Copy all non-desired-state values from the desired resource into the new instance. 3. Call `load_actual_value` on the new instance. + +```ruby +class File < Chef::Resource + property :path, name_attribute: true + property :mode, default: 0666 + property :content + + load_actual_value do + actual_value_does_not_exist! unless File.exist?(path) + mode File.stat(path).mode + content IO.read(path) + end + + action :create do + converge do + File.chmod(mode, path) + IO.write(path, content) + end + end +end + +file '/x.txt' do + # Before the change, the above code would have modified `mode` to be `0666`. + # After, it leaves `mode` alone. + content 'Hello World' +end +``` + #### Non-existence To appropriately handle actual value loading, the user needs a way to specify that the actual value legitimately does not exist (rather than simply not filling in the object and getting `nil`s in it). If `load_actual_value` raises `Chef::Exceptions::ActualValueDoesNotExist`, the new resource will be discarded and `current_resource` becomes `nil`. The `actual_value_does_not_exist!` method can be called to raise this. @@ -103,6 +131,50 @@ class File < Chef::Resource end ``` +#### Side-by-side: new and old + +Here is a sample `converge` statement from a hypothetical FooBarBaz resource with properties `foo`, `bar` and `baz`: + +```ruby +converge do + if current_resource + FooBarBaz.update(new_resource.id, new_resource.foo, new_resource.bar, new_resource.baz) + else + FooBarBaz.create(new_resource.id, new_resource.foo, new_resource.bar, new_resource.baz) + end +end +``` + +This is what you would have to write to do the equivalent: + +```ruby +if current_resource + # We're updating; look for properties that the user wants to change (do the "test" part of test-and-set) + differences = [] + if (new_resource.property_is_set?(:foo) && new_resource.foo != current_resource.foo) + differences << "foo = #{new_resource.foo}" + end + if (new_resource.property_is_set?(:bar) && new_resource.bar != current_resource.bar) + differences << "bar = #{new_resource.bar}" + end + if (new_resource.property_is_set?(:baz) && new_resource.baz != current_resource.baz) + differences << "baz = #{new_resource.baz}" + end + + if !differences.empty? + converge_by "updating FooBarBaz #{new_resource.id}, setting #{differences.join(", ")}" do + FooBarBaz.create(new_resource.id, new_resource.foo, new_resource.bar, new_resource.baz) + end + end + +else + # If the current resource doesn't exist, we're definitely creating it + converge_by "creating FooBarBaz #{new_resource.id} with foo = #{new_resource.foo}, bar = #{new_resource.bar}, baz = #{new_resource.baz}" do + FooBarBaz.update(new_resource.id, new_resource.foo, new_resource.bar, new_resource.baz) + end +end +``` + #### Desired value = actual value > The easiest way to write a resource must be the most correct one. From adc622f82275b989890cf0750b64508b22d519ab Mon Sep 17 00:00:00 2001 From: John Keiser Date: Thu, 23 Jul 2015 09:39:11 -0600 Subject: [PATCH 7/7] Rename "converge" to "converge_if_changed" --- new/load-and-converge.md | 59 ++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/new/load-and-converge.md b/new/load-and-converge.md index 6924aaf9..904bc398 100644 --- a/new/load-and-converge.md +++ b/new/load-and-converge.md @@ -8,7 +8,8 @@ Type: Standards Track # Easy Resource Load And Converge With the introduction of `action` on resources, it becomes useful to have a -blessed way to get the actual value of the resource. This proposal adds `load` and `converge` to help with this purpose, enabling: +blessed way to get the actual value of the resource. This proposal adds +`load_current_value` and `converge_if_changed` to help with this purpose, enabling: - Low-ceremony load methods (as easy to write as we can make it) - A super easy converge model that automatically compares current vs. desired @@ -26,15 +27,15 @@ blessed way to get the actual value of the resource. This proposal adds `load` a ## Specification -### `load_actual_value`: in-place resource load +### `load_current_value`: in-place resource load When using `action`, one needs a way to load the *actual* system value of the resource, so that it can be compared to the desired value and a decision made as to whether to change anything. -When the resource writer defines `load_actual_value` on the resource class, it can be called to load the real system value into the resource. Before any action runs, this will be used by `load_current_resource` to load the resource. `action` will do some important work before calling the new method: +When the resource writer defines `load_current_value` on the resource class, it can be called to load the real system value into the resource. Before any action runs, this will be used by `load_current_resource` to load the resource. `action` will do some important work before calling the new method: 1. Create a new instance of the resource with the same name. 2. Copy all non-desired-state values from the desired resource into the new instance. -3. Call `load_actual_value` on the new instance. +3. Call `load_current_value` on the new instance. ```ruby @@ -43,14 +44,14 @@ class File < Chef::Resource property :mode, default: 0666 property :content - load_actual_value do - actual_value_does_not_exist! unless File.exist?(path) + load_current_value do + current_value_does_not_exist! unless File.exist?(path) mode File.stat(path).mode content IO.read(path) end action :create do - converge do + converge_if_changed do File.chmod(mode, path) IO.write(path, content) end @@ -66,14 +67,14 @@ end #### Non-existence -To appropriately handle actual value loading, the user needs a way to specify that the actual value legitimately does not exist (rather than simply not filling in the object and getting `nil`s in it). If `load_actual_value` raises `Chef::Exceptions::ActualValueDoesNotExist`, the new resource will be discarded and `current_resource` becomes `nil`. The `actual_value_does_not_exist!` method can be called to raise this. +To appropriately handle actual value loading, the user needs a way to specify that the actual value legitimately does not exist (rather than simply not filling in the object and getting `nil`s in it). If `load_current_value` raises `Chef::Exceptions::ActualValueDoesNotExist`, the new resource will be discarded and `current_resource` becomes `nil`. The `current_value_does_not_exist!` method can be called to raise this. NOTE: The alternative was to have users return `false` if the resource does not exist; but I didn't want users to be forced into the ceremony of a trailing `true` line. ```ruby - load_actual_value do + load_current_value do # Check for existence before doing anything else. - actual_value_does_not_exist! if !File.exist?(path) + current_value_does_not_exist! if !File.exist?(path) # Set "mode" on the resource. mode File.stat(path).mode @@ -84,7 +85,7 @@ The block will also be passed the original (desired) resource as a parameter, in #### Inheritance -`super` in `load_actual_value!` will call the superclass's `load_actual_value!` method. +`super` in `load_current_value!` will call the superclass's `load_current_value!` method. #### Handling Multi-Key Resources @@ -98,33 +99,33 @@ class DataBagItem < Chef::Resource attribute :recursively_delete, desired_state: false # Not copied: attribute :data - def load_actual_value! + def load_current_value! data Chef::DataBagItem.new(data_bag_name, item_name).data end end ``` -### `converge`: automatic test-and-set +### `converge_if_changed`: automatic test-and-set -The new `converge do ... end` syntax is added to actions, which enables a *lot* of help for resource writers to make safe, effective resources. It performs several key tasks common to nearly every resource (which are often not done correctly): +The new `converge_if_changed do ... end` syntax is added to actions, which enables a *lot* of help for resource writers to make safe, effective resources. It performs several key tasks common to nearly every resource (which are often not done correctly): - Goes through all attributes on the resource and checks whether the desired value is different from the current value. - If any attributes are different, prints appropriate green text. -- Honors why-run (and does not call the `converge` block if why-run is enabled). +- Honors why-run (and does not call the `converge_if_changed` block if why-run is enabled). ```ruby class File < Chef::Resource property :path, name_attribute: true property :content - load_actual_value do - actual_value_does_not_exist! unless File.exist?(path) + load_current_value do + current_value_does_not_exist! unless File.exist?(path) content IO.read(path) end action :create do - converge do + converge_if_changed do IO.write(path, content) end end @@ -133,10 +134,10 @@ end #### Side-by-side: new and old -Here is a sample `converge` statement from a hypothetical FooBarBaz resource with properties `foo`, `bar` and `baz`: +Here is a sample `converge_if_changed` statement from a hypothetical FooBarBaz resource with properties `foo`, `bar` and `baz`: ```ruby -converge do +converge_if_changed do if current_resource FooBarBaz.update(new_resource.id, new_resource.foo, new_resource.bar, new_resource.baz) else @@ -181,7 +182,7 @@ end There is a subtle pitfall when updating a resource, where the user has set *some* values, but not all. One can easily end up writing a resource which will overwrite perfectly good system properties with their defaults, which can cause instability. If the user does not specify a property, it is generally preferable to preserve its existing value rather than overwrite it. -To prevent this, referencing the bare property in an `action` will now yield the *actual* value if load_actual_value succeeded, and the *default* value if we are creating a new resource (if `load_actual_value` raised `ActualValueDoesNotExist`). +To prevent this, referencing the bare property in an `action` will now yield the *actual* value if load_current_value succeeded, and the *default* value if we are creating a new resource (if `load_current_value` raised `ActualValueDoesNotExist`). ```ruby class File < Chef::Resource @@ -189,14 +190,14 @@ class File < Chef::Resource property :mode, default: 0666 property :content - load_actual_value do - actual_value_does_not_exist! unless File.exist?(path) + load_current_value do + current_value_does_not_exist! unless File.exist?(path) mode File.stat(path).mode content IO.read(path) end action :create do - converge do + converge_if_changed do File.chmod(mode, path) IO.write(path, content) end @@ -216,7 +217,7 @@ There are no backwards-compatibility issues with this because it only applies to #### Compound Resource Convergence -Some resources perform several different (possibly expensive) operations depending on what is set. `converge :attribute1, :attribute2, ... do` allows the user to target different groups of changes based on exactly which attributes have changed: +Some resources perform several different (possibly expensive) operations depending on what is set. `converge_if_changed :attribute1, :attribute2, ... do` allows the user to target different groups of changes based on exactly which attributes have changed: ```ruby class File < Chef::Resource @@ -224,17 +225,17 @@ class File < Chef::Resource property :mode property :content - load_actual_value do - actual_value_does_not_exist! unless File.exist?(path) + load_current_value do + current_value_does_not_exist! unless File.exist?(path) mode File.stat(path).mode content IO.read(path) end action :create do - converge :mode do + converge_if_changed :mode do File.chmod(mode, path) end - converge :content do + converge_if_changed :content do IO.write(path, content) end end