From be4c0e7fbe5e652ec1d49eab2fdc7a5fbbc486f3 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 26 Aug 2008 23:12:03 -0700 Subject: The file source is now refactored and uses REST. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 7 ++++ lib/puppet/type/file/source.rb | 54 +++++++++++++++------------- spec/unit/type/file.rb | 48 +++++-------------------- spec/unit/type/file/source.rb | 81 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 65 deletions(-) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index ef010efda..bc2e53c9f 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -332,6 +332,13 @@ module Puppet recurse() end + def flush + # We want to make sure we retrieve metadata anew on each transaction. + @parameters.each do |name, param| + param.flush if param.respond_to?(:flush) + end + end + # Deal with backups. def handlebackup(file = nil) # let the path be specified diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index e8db13cfa..5eefb1dcb 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -83,9 +83,9 @@ module Puppet def change_to_s(currentvalue, newvalue) # newvalue = "{md5}" + @metadata.checksum if @resource.property(:ensure).retrieve == :absent - return "creating from source %s with contents %s" % [@source, @metadata.checksum] + return "creating from source %s with contents %s" % [metadata.source, @metadata.checksum] else - return "replacing from source %s with contents %s" % [@source, @metadata.checksum] + return "replacing from source %s with contents %s" % [metadata.source, @metadata.checksum] end end @@ -97,6 +97,19 @@ module Puppet end end + # Look up (if necessary) and return remote content. + def content + raise Puppet::DevError, "No source for content was stored with the metadata" unless metadata.source + + unless defined?(@content) and @content + unless tmp = Puppet::FileServing::Content.find(@metadata.source) + fail "Could not find any content at %s" % @metadata.source + end + @content = tmp.content + end + @content + end + # Copy the values from the source to the resource. Yay. def copy_source_values devfail "Somehow got asked to copy source values without any metadata" unless metadata @@ -116,21 +129,15 @@ module Puppet end end - # Ask the file server to describe our file. - def describe(source) - begin - Puppet::FileServing::Metadata.find source - rescue => detail - fail detail, "Could not retrieve file metadata for %s: %s" % [path, detail] - end + # Remove any temporary attributes we manage. + def flush + @metadata = nil + @content = nil end - - # Use the info we get from describe() to check if we're in sync. + + # Use the remote metadata to see if we're in sync. + # LAK:NOTE This method should still get refactored. def insync?(currentvalue) - if currentvalue == :nocopy - return true - end - # the only thing this actual state can do is copy files around. Therefore, # only pay attention if the remote is a file. unless @metadata.ftype == "file" @@ -149,10 +156,8 @@ module Puppet # Diff the contents if they ask it. This is quite annoying -- we need to do this in # 'insync?' because they might be in noop mode, but we don't want to do the file # retrieval twice, so we cache the value. - if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) and ! @metadata._diffed - @metadata._remote_content = get_remote_content - string_file_diff(@resource[:path], @metadata._remote_content) - @metadata._diffed = true + if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) + string_file_diff(@resource[:path], content) end return result end @@ -188,10 +193,13 @@ module Puppet return @metadata end + # Just call out to our copy method. Hopefully we'll refactor 'source' to + # be a parameter soon, in which case 'retrieve' is unnecessary. def retrieve copy_source_values end + # Return the whole array, rather than the first item. def should @should end @@ -208,13 +216,9 @@ module Puppet end def sync - exists = File.exists?(@resource[:path]) + exists = FileTest.exist?(@resource[:path]) - if content = Puppet::FileServing::Content.find(@metadata.source) - @resource.write(content.content, :source, @metadata.checksum) - else - raise "Could not retrieve content" - end + @resource.write(content, :source, @metadata.checksum) if exists return :file_changed diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb index 7f9688f0b..8b7bedee6 100755 --- a/spec/unit/type/file.rb +++ b/spec/unit/type/file.rb @@ -31,46 +31,6 @@ describe Puppet::Type.type(:file) do end end - describe "when specifying a source" do - before do - @file[:source] = "/bar" - end - - it "should raise if source doesn't exist" do - @file.property(:source).expects(:found?).returns(false) - lambda { @file.retrieve }.should raise_error(Puppet::Error) - end - - end - - describe "when retrieving remote files" do - before do - @filesource = Puppet::Type::File::FileSource.new - @filesource.server = mock 'fileserver' - - @file.stubs(:uri2obj).returns(@filesource) - - @file[:source] = "puppet:///test" - end - - it "should fail without writing if it cannot retrieve remote contents" do - # create the file, because we only get the problem when it starts - # out absent. - File.open(@file[:path], "w") { |f| f.puts "a" } - @file.expects(:write).never - - @filesource.server.stubs(:describe).returns("493\tfile\t100\t0\t{md5}3f5fef3bddbc4398c46a7bd7ba7b3af7") - @filesource.server.stubs(:retrieve).raises(RuntimeError) - @file.property(:source).retrieve - lambda { @file.property(:source).sync }.should raise_error(Puppet::Error) - end - - it "should fail if it cannot describe remote contents" do - @filesource.server.stubs(:describe).raises(Puppet::Network::XMLRPCClientError.new("Testing")) - lambda { @file.retrieve }.should raise_error(Puppet::Error) - end - end - describe "when managing links" do require 'puppettest/support/assertions' include PuppetTest @@ -110,4 +70,12 @@ describe Puppet::Type.type(:file) do ("%o" % (File.stat(@file).mode & 007777)).should == "%o" % 0755 end end + + describe "when flushing" do + it "should flush all properties that respond to :flush" do + @resource = Puppet.type(:file).create(:path => "/foo/bar", :source => "/bar/foo") + @resource.property(:source).expects(:flush) + @resource.flush + end + end end diff --git a/spec/unit/type/file/source.rb b/spec/unit/type/file/source.rb index bb689ce76..d6aa25fe7 100755 --- a/spec/unit/type/file/source.rb +++ b/spec/unit/type/file/source.rb @@ -173,4 +173,85 @@ describe Puppet::Type.type(:file).attrclass(:source) do @source.retrieve end end + + describe "when flushing" do + it "should set its metadata to nil" do + @source = source.new(:resource => @resource) + @source.metadata = "foo" + @source.flush + @source.instance_variable_get("@metadata").should be_nil + end + + it "should reset its content" do + @source = source.new(:resource => @resource) + @source.instance_variable_set("@content", "foo") + @source.flush + @source.instance_variable_get("@content").should be_nil + end + end + + it "should have a method for returning the content" do + source.new(:resource => @resource).must respond_to(:content) + end + + describe "when looking up the content" do + before do + @source = source.new(:resource => @resource) + @metadata = stub 'metadata', :source => "/my/source" + @source.metadata = @metadata + + @content = stub 'content', :content => "foobar" + end + + it "should fail if the metadata does not have a source set" do + @metadata.stubs(:source).returns nil + lambda { @source.content }.should raise_error(Puppet::DevError) + end + + it "should look the content up from the Content class using the metadata source if no content is set" do + Puppet::FileServing::Content.expects(:find).with("/my/source").returns @content + @source.content.should == "foobar" + end + + it "should return previously found content" do + Puppet::FileServing::Content.expects(:find).with("/my/source").returns @content + @source.content.should == "foobar" + @source.content.should == "foobar" + end + + it "should fail if no content can be retrieved" do + Puppet::FileServing::Content.expects(:find).with("/my/source").returns nil + @source.expects(:fail).raises RuntimeError + lambda { @source.content }.should raise_error(RuntimeError) + end + end + + describe "when changing the content" do + before do + @source = source.new(:resource => @resource) + @source.stubs(:content).returns "foobar" + + @metadata = stub 'metadata', :checksum => 123 + @source.metadata = @metadata + @resource.stubs(:[]).with(:path).returns "/boo" + end + + it "should use the file's :write method to write the content" do + @resource.expects(:write).with("foobar", :source, 123) + + @source.sync + end + + it "should return :file_changed if the file already existed" do + @resource.stubs(:write) + FileTest.expects(:exist?).with("/boo").returns true + @source.sync.should == :file_changed + end + + it "should return :file_created if the file already existed" do + @resource.stubs(:write) + FileTest.expects(:exist?).with("/boo").returns false + @source.sync.should == :file_created + end + end end -- cgit