From ff9705914570158d1bad3073728a2e94ca4a0060 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Feb 2008 19:06:29 -0500 Subject: Somewhat refactored fileserving so that it no longer caches any objects, nor does it use Puppet's RAL resources. In the process, I fixed #894 (you can now copy links) and refactored other classes as necessary. Mostly it was fixing tests. This is a squashed commit of a temporary branch, fwiw, and it also includes any fixes to the tests that were necessary to get all tests passing again. --- lib/puppet/network/handler/fileserver.rb | 67 +++++++++++--------------------- 1 file changed, 22 insertions(+), 45 deletions(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index e6378bf01..a9a95bcfe 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -5,6 +5,9 @@ require 'cgi' require 'delegate' require 'sync' +require 'puppet/file_serving' +require 'puppet/file_serving/metadata' + class Puppet::Network::Handler AuthStoreError = Puppet::AuthStoreError class FileServerError < Puppet::Error; end @@ -59,40 +62,27 @@ class Puppet::Network::Handler # Describe a given file. This returns all of the manageable aspects # of that file. - def describe(url, links = :ignore, client = nil, clientip = nil) + def describe(url, links = :follow, client = nil, clientip = nil) links = links.intern if links.is_a? String - if links == :manage - raise Puppet::Network::Handler::FileServerError, "Cannot currently copy links" - end - mount, path = convert(url, client, clientip) - if client - mount.debug "Describing %s for %s" % [url, client] - end + mount.debug("Describing %s for %s" % [url, client]) if client + + # Remove any leading slashes, since Metadata doesn't like them, yo. + metadata = Puppet::FileServing::Metadata.new(url, :path => mount.path(client), :relative_path => path.sub(/^\//, ''), :links => links) - obj = nil - unless obj = mount.getfileobject(path, links, client) + return "" unless metadata.exist? + + begin + metadata.collect_attributes + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err detail return "" end - currentvalues = mount.check(obj) - - desc = [] - CHECKPARAMS.each { |check| - if value = currentvalues[check] - desc << value - else - if check == "checksum" and currentvalues[:type] == "file" - mount.notice "File %s does not have data for %s" % - [obj.name, check] - end - desc << nil - end - } - - return desc.join("\t") + return metadata.attributes_with_tabs end # Create a new fileserving module. @@ -140,26 +130,18 @@ class Puppet::Network::Handler def list(url, links = :ignore, recurse = false, ignore = false, client = nil, clientip = nil) mount, path = convert(url, client, clientip) - if client - mount.debug "Listing %s for %s" % [url, client] - end + mount.debug "Listing %s for %s" % [url, client] if client - obj = nil - unless mount.path_exists?(path, client) - return "" - end + return "" unless mount.path_exists?(path, client) desc = mount.list(path, recurse, ignore, client) if desc.length == 0 - mount.notice "Got no information on //%s/%s" % - [mount, path] + mount.notice "Got no information on //%s/%s" % [mount, path] return "" end - - desc.collect { |sub| - sub.join("\t") - }.join("\n") + + desc.collect { |sub| sub.join("\t") }.join("\n") end def local? @@ -213,12 +195,7 @@ class Puppet::Network::Handler return "" end - str = nil - if links == :manage - raise Puppet::Error, "Cannot copy links yet." - else - str = mount.read_file(path, client) - end + str = mount.read_file(path, client) if @local return str -- cgit From ee88c58546c2ca2bf7d77fd36d6443777eb0965f Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 3 Mar 2008 10:29:08 -0600 Subject: Applying patch by DavidS to fix #1083. --- lib/puppet/network/handler/fileserver.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index a9a95bcfe..751a77220 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -69,8 +69,8 @@ class Puppet::Network::Handler mount.debug("Describing %s for %s" % [url, client]) if client - # Remove any leading slashes, since Metadata doesn't like them, yo. - metadata = Puppet::FileServing::Metadata.new(url, :path => mount.path(client), :relative_path => path.sub(/^\//, ''), :links => links) + # use the mount to resolve the path for us. + metadata = Puppet::FileServing::Metadata.new(url, :path => mount.file_path(path, client), :links => links) return "" unless metadata.exist? -- cgit From 388cf7c3df7ce26e953949ed6fe63d76cbbb3691 Mon Sep 17 00:00:00 2001 From: John Ferlito Date: Fri, 14 Mar 2008 13:58:24 +1100 Subject: Regression in :node_name functionality When :node_name="cert" is specified the 'hostname' fact should be set to the SSL certificate common name instead of the results from facter. I've extended this to also set 'domain' and 'fqdn' since that makes a lot of sense to me. This fixes a regression introduced in SVN#1673 --- lib/puppet/network/handler/master.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index 8d84fe8b8..3e004046e 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -81,6 +81,8 @@ class Puppet::Network::Handler clientip = facts["ipaddress"] if Puppet[:node_name] == 'cert' if name + facts["fqdn"] = client + facts["hostname"], facts["domain"] = client.split('.', 2) client = name end if ip -- cgit From 34129d938bcf07d05f6602b7764c688ec4ed226c Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 18 Mar 2008 16:32:53 -0500 Subject: Removing obsolete code from the fileserving handler. This was obsoleted in 0.24.2. --- lib/puppet/network/handler/fileserver.rb | 49 -------------------------------- 1 file changed, 49 deletions(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index 751a77220..3e62cdbd9 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -429,55 +429,6 @@ class Puppet::Network::Handler Puppet::Util.logmethods(self, true) - def getfileobject(dir, links, client = nil) - unless path_exists?(dir, client) - self.debug "File source %s does not exist" % dir - return nil - end - - return fileobj(dir, links, client) - end - - # Run 'retrieve' on a file. This gets the actual parameters, so - # we can pass them to the client. - def check(obj) - # Retrieval is enough here, because we don't want to cache - # any information in the state file, and we don't want to generate - # any state changes or anything. We don't even need to sync - # the checksum, because we're always going to hit the disk - # directly. - - # We're now caching file data, using the LoadedFile to check the - # disk no more frequently than the :filetimeout. - path = obj[:path] - sync = sync(path) - unless data = @@files[path] - data = {} - sync.synchronize(Sync::EX) do - @@files[path] = data - data[:loaded_obj] = Puppet::Util::LoadedFile.new(path) - data[:values] = properties(obj) - return data[:values] - end - end - - changed = nil - sync.synchronize(Sync::SH) do - changed = data[:loaded_obj].changed? - end - - if changed - sync.synchronize(Sync::EX) do - data[:values] = properties(obj) - return data[:values] - end - else - sync.synchronize(Sync::SH) do - return data[:values] - end - end - end - # Create a map for a specific client. def clientmap(client) { -- cgit From 54bedb2bbae2b84fc8f9df8b95e0a904a4e709f7 Mon Sep 17 00:00:00 2001 From: Sam Quigley Date: Tue, 18 Mar 2008 20:33:02 -0700 Subject: tweak the (already applied) patch in 388cf7c3df7ce26e953949ed6fe63d76cbbb3691 to resolve #1137; also, add tests which detect the problem. --- lib/puppet/network/handler/master.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index 3e004046e..dabfaca50 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -81,9 +81,9 @@ class Puppet::Network::Handler clientip = facts["ipaddress"] if Puppet[:node_name] == 'cert' if name + client = name facts["fqdn"] = client facts["hostname"], facts["domain"] = client.split('.', 2) - client = name end if ip clientip = ip -- cgit From 18320b8e3271f7d1d1702907be1ff420acfc8d2b Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 21 Mar 2008 00:39:26 -0500 Subject: Found all instances of methods where split() is used without any local variables and added a local variable -- see http://snurl.com/21zf8. My own testing showed that this caused memory growth to level off at a reasonable level. Note that the link above says the problem is only with class methods, but my own testing showed that it's any method that meets these criteria. This is not a functional change, but should hopefully be the last nail in the coffin of #1131. --- lib/puppet/network/handler/report.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/report.rb b/lib/puppet/network/handler/report.rb index 8ddeed9f6..b92b77ea5 100755 --- a/lib/puppet/network/handler/report.rb +++ b/lib/puppet/network/handler/report.rb @@ -79,7 +79,8 @@ class Puppet::Network::Handler # Handle the parsing of the reports attribute. def reports - Puppet[:reports].gsub(/(^\s+)|(\s+$)/, '').split(/\s*,\s*/) + # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] + x = Puppet[:reports].gsub(/(^\s+)|(\s+$)/, '').split(/\s*,\s*/) end end end -- cgit