Fixing a bunch of small bugs, mostly found by testing on solaris, and added a check to the test system that points out memory growth

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1113 980ebf18-57e1-0310-9a29-db15c13687c0

9 files changed, 68 insertions, 49 deletions

diff --git a/lib/puppet/client/dipper.rb b/lib/puppet/client/dipper.rb
index 0ad1926ab..bc85236a3 100644
--- a/lib/puppet/client/dipper.rb
+++ b/lib/puppet/client/dipper.rb
@@ -6,6 +6,7 @@ module Puppet
             
             attr_accessor :name
 
+            # Create our bucket client
             def initialize(hash = {})
                 if hash.include?(:Path)
                     bucket = Puppet::Server::FileBucket.new(
@@ -18,26 +19,25 @@ module Puppet
                 super(hash)
             end
 
+            # Back up a file to our bucket
             def backup(file)
                 unless FileTest.exists?(file)
-                    raise(BucketError, "File %s does not exist" % file, caller)
+                    raise(BucketError, "File %s does not exist" % file)
                 end
-                contents = File.open(file) { |of| of.read }
-
+                contents = File.read(file)
                 string = Base64.encode64(contents)
-                #puts "string is created"
 
                 sum = @driver.addfile(string,file)
-                #puts "file %s is added" % file
+                string = ""
+                contents = ""
                 return sum
             end
 
+            # Restore the file
             def restore(file,sum)
                 restore = true
                 if FileTest.exists?(file)
-                    contents = File.open(file) { |of| of.read }
-
-                    cursum = Digest::MD5.hexdigest(contents)
+                    cursum = Digest::MD5.hexdigest(File.read(file))
 
                     # if the checksum has changed...
                     # this might be extra effort
@@ -50,6 +50,7 @@ module Puppet
                     #puts "Restoring %s" % file
                     if tmp = @driver.getfile(sum)
                         newcontents = Base64.decode64(tmp)
+                        tmp = ""
                         newsum = Digest::MD5.hexdigest(newcontents)
                         changed = nil
                         unless FileTest.writable?(file)
@@ -71,7 +72,6 @@ module Puppet
                 else
                     return nil
                 end
-
             end
         end
     end
diff --git a/lib/puppet/parser/ast/tag.rb b/lib/puppet/parser/ast/tag.rb
index ce59558f3..0807d207e 100644
--- a/lib/puppet/parser/ast/tag.rb
+++ b/lib/puppet/parser/ast/tag.rb
@@ -2,7 +2,7 @@ class Puppet::Parser::AST
     # The code associated with a class.  This is different from components
     # in that each class is a singleton -- only one will exist for a given
     # node.
-    class Set < AST::Branch
+    class Tag < AST::Branch
         @name = :class
         attr_accessor :type
 
diff --git a/lib/puppet/server/filebucket.rb b/lib/puppet/server/filebucket.rb
index 704d1c76d..8a4d1a0a4 100755
--- a/lib/puppet/server/filebucket.rb
+++ b/lib/puppet/server/filebucket.rb
@@ -40,8 +40,6 @@ class Server
         end
 
         def initialize(hash)
-            # build our AST
-
             if hash.include?(:ConflictCheck)
                 @conflictchk = hash[:ConflictCheck]
                 hash.delete(:ConflictCheck)
@@ -67,12 +65,8 @@ class Server
 
         # accept a file from a client
         def addfile(string,path, client = nil, clientip = nil)
-            #puts "entering addfile"
             contents = Base64.decode64(string)
-            #puts "string is decoded"
-
             md5 = Digest::MD5.hexdigest(contents)
-            #puts "md5 is made"
 
             bpath, bfile, pathpath = FileBucket.paths(@bucket,md5)
 
@@ -82,11 +76,9 @@ class Server
                 msg += " from #{client}" if client
                 self.info msg
                 # ...then just create the file
-                #puts "creating file"
                 File.open(bfile, File::WRONLY|File::CREAT, 0440) { |of|
                     of.print contents
                 }
-                #puts "File is created"
             else # if the dir already existed...
                 # ...we need to verify that the contents match the existing file
                 if @conflictchk
@@ -95,13 +87,10 @@ class Server
                             "No file at %s for sum %s" % [bfile,md5], caller)
                     end
 
-                    curfile = ""
-                    File.open(bfile) { |of|
-                        curfile = of.read
-                    }
+                    curfile = File.read(bfile)
 
-                    # if the contents don't match, then we've found a conflict
-                    # unlikely, but quite bad
+                    # If the contents don't match, then we've found a conflict.
+                    # Unlikely, but quite bad.
                     if curfile != contents
                         raise(BucketError,
                             "Got passed new contents for sum %s" % md5, caller)
@@ -111,9 +100,10 @@ class Server
                         self.info msg
                     end
                 end
-                #puts "Conflict check is done"
             end
 
+            contents = ""
+
             # in either case, add the passed path to the list of paths
             paths = nil
             addpath = false
@@ -129,14 +119,12 @@ class Server
             else
                 addpath = true
             end
-            #puts "Path is checked"
 
             # if it's a new file, or if our path isn't in the file yet, add it
             if addpath
                 File.open(pathpath, File::WRONLY|File::CREAT|File::APPEND) { |of|
                     of.puts path
                 }
-                #puts "Path is added"
             end
 
             return md5
diff --git a/lib/puppet/sslcertificates/ca.rb b/lib/puppet/sslcertificates/ca.rb
index 1d9823ed1..04d950e21 100644
--- a/lib/puppet/sslcertificates/ca.rb
+++ b/lib/puppet/sslcertificates/ca.rb
@@ -168,8 +168,8 @@ class Puppet::SSLCertificates::CA
 
     # List certificates waiting to be signed.
     def list
-        return Dir.entries(Puppet[:csrdir]).reject { |file|
-            file =~ /^\.+$/
+        return Dir.entries(Puppet[:csrdir]).find_all { |file|
+            file =~ /\.pem$/
         }.collect { |file|
             file.sub(/\.pem$/, '')
         }
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index c10dd72ec..95bc17f35 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -11,7 +11,7 @@ module Puppet
         @doc = "Manage packages.  There is a basic dichotomy in package
             support right now:  Some package types (e.g., yum and apt) can
             retrieve their own package files, while others (e.g., rpm and
-            sunpkg) cannot.  For those package formats that cannot retrieve
+            sun) cannot.  For those package formats that cannot retrieve
             their own files, you can use the ``source`` parameter to point to
             the correct file.
 
@@ -388,7 +388,7 @@ module Puppet
                 )
             end
             case @platform
-            when "solaris": @default = :sunpkg
+            when "solaris": @default = :sun
             when "gentoo":
                 Puppet.notice "No support for gentoo yet"
                 @default = nil
diff --git a/lib/puppet/type/package/sun.rb b/lib/puppet/type/package/sun.rb
index e58980228..b4b563461 100755
--- a/lib/puppet/type/package/sun.rb
+++ b/lib/puppet/type/package/sun.rb
@@ -1,5 +1,5 @@
 module Puppet
-    Puppet.type(:package).newpkgtype(:sunpkg) do
+    Puppet.type(:package).newpkgtype(:sun) do
         # Get info on a package, optionally specifying a device.
         def info2hash(device = nil)
             names = {
diff --git a/lib/puppet/type/parsedtype/mount.rb b/lib/puppet/type/parsedtype/mount.rb
index e6d233933..69254e1b6 100755
--- a/lib/puppet/type/parsedtype/mount.rb
+++ b/lib/puppet/type/parsedtype/mount.rb
@@ -5,7 +5,6 @@ require 'puppet/type/state'
 
 module Puppet
     newtype(:mount, Puppet::Type::ParsedType) do
-
         ensurable do
             desc "Create, remove, or mount a filesystem mount."
 
@@ -218,7 +217,12 @@ module Puppet
         # Is the mount currently mounted?
         def mounted?
             platform = Facter["operatingsystem"].value
-            %x{df}.split("\n").find do |line|
+            df = "df"
+            case Facter["operatingsystem"].value
+            # Solaris's df prints in a very weird format
+            when "Solaris": df = "df -k"
+            end
+            %x{#{df}}.split("\n").find do |line|
                 fs = line.split(/\s+/)[-1]
                 if platform == "Darwin"
                     fs == "/private/var/automount" + self[:path] or
diff --git a/lib/puppet/type/service/smf.rb b/lib/puppet/type/service/smf.rb
index 3304b0bbc..0f72479ae 100755
--- a/lib/puppet/type/service/smf.rb
+++ b/lib/puppet/type/service/smf.rb
@@ -60,8 +60,9 @@ Puppet.type(:service).newsvctype(:smf) do
         }
 
         if $? != 0
-            raise Puppet::Error,
-                "Could not get status on service %s" % self.name
+            #raise Puppet::Error,
+            warning "Could not get status on service %s" % self.name
+            return :stopped
         end
     end
 
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 7da32e280..cedb64264 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -36,13 +36,17 @@ module Util
                     gid = self.gid(group)
                 end
 
-                if Process.gid != gid
-                    oldgid = Process.gid
-                    begin
-                        Process.egid = gid
-                    rescue => detail
-                        raise Puppet::Error, "Could not change GID: %s" % detail
+                if gid
+                    if Process.gid != gid
+                        oldgid = Process.gid
+                        begin
+                            Process.egid = gid
+                        rescue => detail
+                            raise Puppet::Error, "Could not change GID: %s" % detail
+                        end
                     end
+                else
+                    Puppet.warning "Could not retrieve GID for %s" % group
                 end
             end
 
@@ -53,14 +57,19 @@ module Util
                     uid = self.uid(user)
                 end
                 uid = self.uid(user)
-                # Now change the uid
-                if Process.uid != uid
-                    olduid = Process.uid
-                    begin
-                        Process.euid = uid
-                    rescue => detail
-                        raise Puppet::Error, "Could not change UID: %s" % detail
+
+                if uid
+                    # Now change the uid
+                    if Process.uid != uid
+                        olduid = Process.uid
+                        begin
+                            Process.euid = uid
+                        rescue => detail
+                            raise Puppet::Error, "Could not change UID: %s" % detail
+                        end
                     end
+                else
+                    Puppet.warning "Could not retrieve UID for %s" % user
                 end
             end
 
@@ -338,6 +347,23 @@ module Util
     end
 
     module_function :benchmark
+
+    def memory
+        unless defined? @pmap
+            pmap = %x{which pmap 2>/dev/null}.chomp
+            if pmap == ""
+                @pmap = nil
+            else
+                @pmap = pmap
+            end
+        end
+        if @pmap
+            return %x{pmap #{Process.pid}| grep total}.chomp.sub(/^\s*total\s+/, '').sub(/K$/, '').to_i
+        else
+            0
+        end
+    end
+    module_function :memory
 end
 end