diff options
| -rw-r--r-- | chef/REPORTING_TODO.md | 131 | ||||
| -rw-r--r-- | chef/lib/chef/diff.rb | 24 | ||||
| -rw-r--r-- | chef/lib/chef/mixin/enforce_ownership_and_permissions.rb | 2 | ||||
| -rw-r--r-- | chef/lib/chef/provider.rb | 28 | ||||
| -rw-r--r-- | chef/lib/chef/provider/file.rb | 24 | ||||
| -rw-r--r-- | chef/lib/chef/provider/resource_update.rb | 54 | ||||
| -rw-r--r-- | chef/lib/chef/provider/template.rb | 11 | ||||
| -rw-r--r-- | chef/lib/chef/resource/file.rb | 2 | ||||
| -rw-r--r-- | chef/lib/chef/resource/template.rb | 2 | ||||
| -rw-r--r-- | chef/lib/chef/run_context.rb | 3 | ||||
| -rw-r--r-- | chef/spec/unit/provider/file_spec.rb | 23 |
11 files changed, 274 insertions, 30 deletions
diff --git a/chef/REPORTING_TODO.md b/chef/REPORTING_TODO.md new file mode 100644 index 0000000000..e8a1918d43 --- /dev/null +++ b/chef/REPORTING_TODO.md @@ -0,0 +1,131 @@ +## Resource Status ## +Want to do: capture the state (and state transition) for something like +"file doesn't exist" -> "file created" + +Example: File +exists, doesn't exist + +Example: Service +two axes: enabled/disabled, running/not running. + +Possible implementations: +1. Expose a "raw" array, providers modify it +2. zero or more boolean "attributes" (how service resource currently + implemented) +3. zero or more pairs of methods, e.g., + + @current_resource.enabled ; @current_resource.disabled + @current_resource.running ; @current_resource.stopped + +## Update Resources: Annotate State Attributes +Annotating Chef::Resource classes so we know which attributes describe +system state and which ones don't. + +### TEST TODO ### +* scan_access_control + +### Resources to Annotate ### + * apt_package + * bash + * breakpoint + * cookbook_file + * cron + * csh + * deploy + * deploy_revision + * directory + * dpkg_package + * easy_install_package + * env + * erl_call + * execute + * file + * freebsd_package + * gem_package + * git + * group + * http_request + * ifconfig + * link + * log + * macports_package + * mdadm + * mount + * ohai + * package + * pacman_package + * perl + * portage_package + * python + * remote_directory + * remote_file + * route + * rpm_package + * ruby + * ruby_block + * scm + * script + * service + * solaris_package + * subversion + * template + * timestamped_deploy + * user + * yum_package + + +# NOTES + +## Example Designs for Resource Attribute Definition + + class FileResource2 < Chef::Resource + + # Totally DSL + + state_attribute(:mode) do |a| + a.desc = "Desired unix permissions of the file" + a.validate do |value| + unless value.kind_of?(String) or value.kind_of?(Integer) + invalid!("invalid type blah blah") + end + # other stuff + end + end + + + # Standard Rubyisms using method_added: + + state_attr + desc "Desired unix permissions" + validate do |value| + # stuff + end + + def mode(value=NULL) + @mode = value unless value.equal?(NULL) + end + + # Same as above, but for undocumented attributes: + + state_attr + + def mode(value=NULL) + # blah blah + end + + + # Kinds of Resource Attributes + # * id_attr: part of the resource's identity; usually the "name attribute" + # (always?); uniquely identifies the resource (given the type); ex: file's + # path, package's name, etc. + # * state_attr: a modifiable property of the resource, like a file's permissions/owner/group/checksum + # * others -- "modifier_attr" (?) + # ex: + # retry, ignore_failure, response_file (package), options (package) + + # Contrarily... what is the worse-is-better way to accomplish this? + + + + end + diff --git a/chef/lib/chef/diff.rb b/chef/lib/chef/diff.rb new file mode 100644 index 0000000000..571cc4557f --- /dev/null +++ b/chef/lib/chef/diff.rb @@ -0,0 +1,24 @@ +require 'chef/mixin/shell_out' + +class Chef + + # Generates file diffs. + #-- + # TODO/BUG: + # This is a quick hack. The final implementation should do the following: + # * Support a max file size above which diffs won't be attempted. + # * Support a max diff size above which results will be thrown out. + # * implement diffs in a way that will be performant and not depend on the diff(1) program. + # Xdiff looks pretty hot for this: http://XMailServer.Org/xdiff.html + class Diff + include Mixin::ShellOut + + def initialize(old_file_path, new_file_path) + @old_file_path, @new_file_path = old_file_path, new_file_path + end + + def diff + shell_out!("diff -N -U 3 #{@old_file_path} #{@new_file_path}", :returns => [0,1]).stdout + end + end +end diff --git a/chef/lib/chef/mixin/enforce_ownership_and_permissions.rb b/chef/lib/chef/mixin/enforce_ownership_and_permissions.rb index 1dd219b2ba..e72595c2c8 100644 --- a/chef/lib/chef/mixin/enforce_ownership_and_permissions.rb +++ b/chef/lib/chef/mixin/enforce_ownership_and_permissions.rb @@ -31,7 +31,7 @@ class Chef end access_controls = Chef::FileAccessControl.new(new_resource, path) access_controls.set_all - new_resource.updated_by_last_action(access_controls.modified?) + new_resource.updated_by_last_action(true) if access_controls.modified? end end diff --git a/chef/lib/chef/provider.rb b/chef/lib/chef/provider.rb index 0d0b45935f..32e83ba9c4 100644 --- a/chef/lib/chef/provider.rb +++ b/chef/lib/chef/provider.rb @@ -42,7 +42,7 @@ class Chef @new_resource = new_resource @current_resource = nil @run_context = run_context - @resource_update = ResourceUpdate.new + @resource_update = nil end def node @@ -60,26 +60,18 @@ class Chef def run_action(action) @executing_action = action - load_current_resource - record_current_state - # assume updated state will be as specified in new_resource - record_updated_state - send("action_#{action}") - end + @resource_update = ResourceUpdate.new(@new_resource) - # Add a snapshot of the current (before change) state of the resource to - # the +resource_update+ - def record_current_state - # LWRPs or other hacky providers may not have a current resource - unless current_resource.nil? - resource_update.initial_state_from_resource(current_resource) + @resource_update.run_action do + load_current_resource + @resource_update.initial_state_from_resource(@current_resource) + send("action_#{action}") + end + + if @new_resource.updated_by_last_action? + @run_context.resource_updated(@resource_update) end - end - # Add a snapshot of the updated (after change) state of the resource to the - # +resource_update+ - def record_updated_state - resource_update.updated_state_from_resource(new_resource) end def load_current_resource diff --git a/chef/lib/chef/provider/file.rb b/chef/lib/chef/provider/file.rb index 24418590d5..e956822e1c 100644 --- a/chef/lib/chef/provider/file.rb +++ b/chef/lib/chef/provider/file.rb @@ -22,6 +22,7 @@ require 'chef/resource/file' require 'chef/mixin/checksum' require 'chef/scan_access_control' require 'chef/provider' +require 'chef/diff' require 'etc' require 'fileutils' @@ -30,10 +31,16 @@ class Chef class File < Chef::Provider include Chef::Mixin::Checksum + def initialize(*args) + super + @backup_path = nil + end + def load_current_resource @current_resource = Chef::Resource::File.new(@new_resource.name) @new_resource.path.gsub!(/\\/, "/") # for Windows @current_resource.path(@new_resource.path) + @current_resource.status = ::File.exist?(@new_resource.path) ? :exists : :nonexistent ScanAccessControl.new(@new_resource, @current_resource).set_all! @current_resource end @@ -48,7 +55,12 @@ class Chef unless compare_content backup @new_resource.path if ::File.exists?(@new_resource.path) ::File.open(@new_resource.path, "w") {|f| f.write @new_resource.content } + unless @backup_path.nil? + differ = Diff.new(@backup_path, @new_resource.path) + resource_update.event_data = differ.diff + end Chef::Log.info("#{@new_resource} contents updated") + resource_update.status = :updated @new_resource.updated_by_last_action(true) end end @@ -59,9 +71,11 @@ class Chef ::File.open(@new_resource.path, "w+") {|f| f.write @new_resource.content } @new_resource.updated_by_last_action(true) Chef::Log.info("#{@new_resource} created file #{@new_resource.path}") + resource_update.status = :created else set_content unless @new_resource.content.nil? end + @new_resource.status = :exists enforce_ownership_and_permissions end @@ -84,6 +98,8 @@ class Chef raise "Cannot delete #{@new_resource} at #{@new_resource_path}!" end end + resource_update.status = :deleted + @new_resource.status = :nonexistent end def action_touch @@ -105,10 +121,10 @@ class Chef # keeping the backup in the same directory. We also need to to_s it # so we don't get a type error around implicit to_str conversions. prefix = Chef::Config[:file_backup_path].to_s - backup_path = ::File.join(prefix, backup_filename) - FileUtils.mkdir_p(::File.dirname(backup_path)) if Chef::Config[:file_backup_path] - FileUtils.cp(file, backup_path, :preserve => true) - Chef::Log.info("#{@new_resource} backed up to #{backup_path}") + @backup_path = ::File.join(prefix, backup_filename) + FileUtils.mkdir_p(::File.dirname(@backup_path)) if Chef::Config[:file_backup_path] + FileUtils.cp(file, @backup_path, :preserve => true) + Chef::Log.info("#{@new_resource} backed up to #{@backup_path}") # Clean up after the number of backups slice_number = @new_resource.backup diff --git a/chef/lib/chef/provider/resource_update.rb b/chef/lib/chef/provider/resource_update.rb index 6423e8f291..1f3ac7aa2f 100644 --- a/chef/lib/chef/provider/resource_update.rb +++ b/chef/lib/chef/provider/resource_update.rb @@ -29,24 +29,68 @@ class Chef class ResourceUpdate - attr_accessor :type - attr_accessor :name attr_accessor :duration #ms attr_accessor :status attr_accessor :initial_state attr_accessor :final_state attr_accessor :initial_properties - attr_accessor :final_properties + #attr_accessor :final_properties attr_accessor :event_data # e.g., a diff. + def initialize(new_resource) + @new_resource = new_resource + @current_resource = nil + @start_time, @end_time = nil, nil + @result = nil + end + + def type + @new_resource.type + end + + def name + @new_resource.name + end + + def run_action + @start_time = Time.now + yield + rescue Exception => e + @event_data = e + @result = :failed + raise + else + @result = :success + ensure + @end_time = Time.now + end + def initial_state_from_resource(current_resource) + @current_resource = current_resource @initial_properties = current_resource.state end - def updated_state_from_resource(new_resource) - @final_properties = new_resource.state + def final_properties + @new_resource.state + end + + def duration + @end_time.to_f - @start_time.to_f end + def display + s=(<<-EOH) +#{@new_resource} #{status} in #{duration.round(4)}s + status: (#{@current_resource.status} -> #{@new_resource.status}) +initial state: #{initial_properties} + final state: #{final_properties} +EOH + s << "Change:\n#@event_data" if @event_data + s + end + + + end end end diff --git a/chef/lib/chef/provider/template.rb b/chef/lib/chef/provider/template.rb index b2f3f4899e..440d9fc774 100644 --- a/chef/lib/chef/provider/template.rb +++ b/chef/lib/chef/provider/template.rb @@ -20,6 +20,7 @@ require 'chef/provider/file' require 'chef/mixin/template' require 'chef/mixin/checksum' +require 'chef/diff' require 'chef/file_access_control' class Chef @@ -38,16 +39,24 @@ class Chef def action_create render_with_context(template_location) do |rendered_template| rendered(rendered_template) - if ::File.exist?(@new_resource.path) && content_matches? + file_exists = ::File.exist?(@new_resource.path) + if file_exists && content_matches? Chef::Log.debug("#{@new_resource} content has not changed.") set_all_access_controls(@new_resource.path) + resource.status = :updated if @new_resource.updated_by_last_action? else backup set_all_access_controls(rendered_template.path) + + differ = Diff.new(@new_resource.path, rendered_template.path) + resource_update.event_data = differ.diff + FileUtils.mv(rendered_template.path, @new_resource.path) Chef::Log.info("#{@new_resource} updated content") + resource_update.status = file_exists ? :created : :updated @new_resource.updated_by_last_action(true) end + @new_resource.status = :exists end end diff --git a/chef/lib/chef/resource/file.rb b/chef/lib/chef/resource/file.rb index 64135906b1..b25d2eaf18 100644 --- a/chef/lib/chef/resource/file.rb +++ b/chef/lib/chef/resource/file.rb @@ -31,6 +31,8 @@ class Chef # TODO: fix for windows :/ state_attrs :checksum, :owner, :group, :mode + valid_status_names :exists, :nonexistent + provides :file, :on_platforms => :all def initialize(name, run_context=nil) diff --git a/chef/lib/chef/resource/template.rb b/chef/lib/chef/resource/template.rb index 26472bfad1..8f1535d448 100644 --- a/chef/lib/chef/resource/template.rb +++ b/chef/lib/chef/resource/template.rb @@ -26,6 +26,8 @@ class Chef class Template < Chef::Resource::File include Chef::Mixin::Securable + valid_status_names :exists, :nonexistent + provides :template, :on_platforms => :all def initialize(name, run_context=nil) diff --git a/chef/lib/chef/run_context.rb b/chef/lib/chef/run_context.rb index f9ff4c088f..5831f3b7c9 100644 --- a/chef/lib/chef/run_context.rb +++ b/chef/lib/chef/run_context.rb @@ -88,6 +88,9 @@ class Chef end end + def resource_updated(resource_update) + @resource_updates << resource_update + end private diff --git a/chef/spec/unit/provider/file_spec.rb b/chef/spec/unit/provider/file_spec.rb index 65e59a5fa1..531ae2dc1b 100644 --- a/chef/spec/unit/provider/file_spec.rb +++ b/chef/spec/unit/provider/file_spec.rb @@ -43,6 +43,22 @@ describe Chef::Provider::File do @provider.node.should eql(@node) end + describe "when the specified file does not yet exist" do + before do + @resource.path("/etc/foo.conf") + @resource.user(1000) + @resource.group(1000) + @resource.mode("0644") + File.should_receive(:exist?).with("/etc/foo.conf").twice.and_return(false) + end + + it "sets the status of current_resource to nonexistent" do + @provider.load_current_resource + @provider.current_resource.status.should == :nonexistent + end + + end + describe "when the specified file exists" do before do @resource.path("/etc/foo.conf") @@ -53,7 +69,7 @@ describe Chef::Provider::File do # Would prefer not to have to stub all instances but we don't have # a good avenue to use DI instead :( Chef::ScanAccessControl.any_instance.stub(:stat).and_return(@file_stat) - File.should_receive(:exist?).with("/etc/foo.conf").and_return(true) + File.should_receive(:exist?).with("/etc/foo.conf").twice.and_return(true) end it "updates current_resource with the access control settings for the current file" do @@ -63,6 +79,11 @@ describe Chef::Provider::File do cr.group.should == 0 cr.mode.should == "755" end + + it "sets the status of the current resource to :exists" do + @provider.load_current_resource + @provider.current_resource.status.should == :exists + end end it "should load a current resource based on the one specified at construction" do |