Committing most of the scheduling stuff. There is still a bit of work to do in terms of how puppetd interacts with scheduling, but the bulk of the work is done.

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

10 files changed, 749 insertions, 162 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index 5dacb15bf..b092dd7e2 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -94,13 +94,16 @@ PUPPETVERSION = '0.11.2'
         :httplogfile    => [:logdir,         "http.log"],
         :masterlog      => [:logdir,         "puppetmaster.log"],
         :masterhttplog  => [:logdir,         "masterhttp.log"],
-        :checksumfile   => [:statedir,       "checksums"],
+        :statefile      => [:statedir,       "state.yaml"],
+        :checksumfile   => [:statedir,       "state.yaml"],
         :ssldir         => [:puppetconf,     "ssl"],
 
         # and finally the simple answers,
         :server         => "puppet",
         :user           => "puppet",
         :group          => "puppet",
+        :schedule       => "puppet",
+        :ignoreschedules => false,
         :rrdgraph       => false,
         :noop           => false,
         :parseonly      => false,
diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index 9023a2529..27c7cef72 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -264,6 +264,11 @@ module Puppet
                         detail
                 rescue => detail
                     Puppet.err "Found a bug: %s" % detail
+                    if Puppet[:debug]
+                        puts detail.backtrace
+                    end
+                ensure
+                    Puppet::Storage.store
                 end
                 Puppet::Metric.gather
                 Puppet::Metric.tally
@@ -271,7 +276,6 @@ module Puppet
                     Metric.store
                     Metric.graph
                 end
-                Puppet::Storage.store
 
                 return transaction
             end
@@ -416,6 +420,9 @@ module Puppet
                 end
                 @objects = nil
 
+                # First create the default scheduling objects
+                Puppet.type(:schedule).mkdefaultschedules
+
                 # Now convert the objects to real Puppet objects
                 @objects = objects.to_type
 
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index a4fe00b7b..dfb259909 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -4,7 +4,7 @@ module Puppet
             attr_reader :validater, :munger, :name, :default
             attr_accessor :ismetaparameter, :element
 
-            # Define the default value for a given state or parameter.  This
+            # Define the default value for a given parameter or parameter.  This
             # means that 'nil' is an invalid default value.  This defines
             # the 'default' instance method.
             def defaultto(value = nil, &block)
@@ -29,7 +29,7 @@ module Puppet
             # This is how we munge the value.  Basically, this is our
             # opportunity to convert the value from one form into another.
             def munge(&block)
-                # I need to wrap the unsafe version in begin/rescue statements,
+                # I need to wrap the unsafe version in begin/rescue parameterments,
                 # but if I directly call the block then it gets bound to the
                 # class's context, not the instance's, thus the two methods,
                 # instead of just one.
@@ -107,6 +107,41 @@ module Puppet
                     end
                 end
             end
+
+            # Define a new value for our parameter.
+            def newvalues(*names)
+                @parametervalues ||= []
+
+                names.each { |name|
+                    if @parametervalues.include?(name)
+                        Puppet.warning "%s already has a value for %s" % [name, name]
+                    end
+                    @parametervalues << name
+                }
+            end
+
+            def aliasvalue(name, other)
+                @parametervalues ||= []
+                unless @parametervalues.include?(other)
+                    raise Puppet::DevError, "Cannot alias nonexistent value %s" % other
+                end
+
+                @aliasvalues ||= {}
+                @aliasvalues[name] = other
+            end
+
+            def alias(name)
+                @aliasvalues[name]
+            end
+
+            # Return the list of valid values.
+            def values
+                @parametervalues ||= []
+                @aliasvalues ||= {}
+
+                #[@aliasvalues.keys, @parametervalues.keys].flatten
+                @parametervalues.dup
+            end
         end
 
         # Just a simple method to proxy instance methods to class methods
@@ -177,6 +212,78 @@ module Puppet
             raise error
         end
 
+        # Log a message using the parent's log level.
+        def log(msg)
+            unless @parent[:loglevel]
+                p @parent
+                self.devfail "Parent %s has no loglevel" %
+                    @parent.name
+            end
+            Puppet::Log.create(
+                :level => @parent[:loglevel],
+                :message => msg,
+                :source => self
+            )
+        end
+
+        # each parameter class must define the name() method, and parameter instances
+        # do not change that name
+        # this implicitly means that a given object can only have one parameter
+        # instance of a given parameter class
+        def name
+            return self.class.name
+        end
+
+        # for testing whether we should actually do anything
+        def noop
+            unless defined? @noop
+                @noop = false
+            end
+            tmp = @noop || self.parent.noop || Puppet[:noop] || false
+            #debug "noop is %s" % tmp
+            return tmp
+        end
+
+        # return the full path to us, for logging and rollback; not currently
+        # used
+        def path
+            return [@parent.path, self.name].join("/")
+        end
+
+        # If the specified value is allowed, then munge appropriately.
+        munge do |value|
+            if self.class.values.empty?
+                # This parameter isn't using defined values to do its work.
+                return value
+            end
+            intern = value.to_s.intern
+            # If it's a valid value, always return it as a symbol.
+            if self.class.values.include?(intern)
+                retval = intern
+            elsif other = self.class.alias(intern)
+                self.info "returning alias %s for %s" % [other, intern]
+                retval = other
+            else
+                retval = value
+            end
+            retval
+        end
+
+        # Verify that the passed value is valid.
+        validate do |value|
+            if self.class.values.empty?
+                # This parameter isn't using defined values to do its work.
+                return 
+            end
+            unless value.is_a?(Symbol)
+                value = value.to_s.intern
+            end
+            unless self.class.values.include?(value) or self.class.alias(value)
+                self.fail "Invalid '%s' value '%s'.  Valid values are '%s'" %
+                        [self.class.name, value, self.class.values.join(", ")]
+            end
+        end
+
         # This should only be called for parameters, but go ahead and make
         # it possible to call for states, too.
         def value
@@ -195,7 +302,7 @@ module Puppet
         # late-binding (e.g., users might not exist when the value is assigned
         # but might when it is asked for).
         def value=(value)
-            # If we're a state, just hand the processing off to the should method.
+            # If we're a parameter, just hand the processing off to the should method.
             if self.is_a?(Puppet::State)
                 return self.should = value
             end
diff --git a/lib/puppet/storage.rb b/lib/puppet/storage.rb
index b174e0183..a12a94178 100644
--- a/lib/puppet/storage.rb
+++ b/lib/puppet/storage.rb
@@ -9,6 +9,18 @@ module Puppet
             self.class.load
         end
 
+        # Return a hash that will be stored to disk.  It's worth noting
+        # here that we use the object's full path, not just the name/type
+        # combination.  At the least, this is useful for those non-isomorphic
+        # types like exec, but it also means that if an object changes locations
+        # in the configuration it will lose its cache.
+        def self.cache(object)
+            unless object.is_a? Puppet::Type
+                raise Puppet::DevFail, "Must pass a Type instance to Storage.cache"
+            end
+            return @@state[object.path] ||= {}
+        end
+
         def self.clear
             @@state.clear
             Storage.init
@@ -23,32 +35,32 @@ module Puppet
         self.init
 
         def self.load
-            if Puppet[:checksumfile].nil?
+            if Puppet[:statefile].nil?
                 raise Puppet::DevError, "Somehow the statefile is nil"
             end
 
-            unless File.exists?(Puppet[:checksumfile])
-                Puppet.info "Statefile %s does not exist" % Puppet[:checksumfile]
+            unless File.exists?(Puppet[:statefile])
+                Puppet.info "Statefile %s does not exist" % Puppet[:statefile]
                 unless defined? @@state and ! @@state.nil?
                     self.init
                 end
                 return
             end
-            #Puppet.debug "Loading statefile %s" % Puppet[:checksumfile]
-            Puppet::Util.lock(Puppet[:checksumfile]) { |file|
+            #Puppet.debug "Loading statefile %s" % Puppet[:statefile]
+            Puppet::Util.lock(Puppet[:statefile]) { |file|
                 #@@state = Marshal.load(file)
                 begin
                     @@state = YAML.load(file)
                 rescue => detail
                     Puppet.err "Checksumfile %s is corrupt; replacing" %
-                        Puppet[:checksumfile]
+                        Puppet[:statefile]
                     begin
-                        File.rename(Puppet[:checksumfile],
-                            Puppet[:checksumfile] + ".bad")
+                        File.rename(Puppet[:statefile],
+                            Puppet[:statefile] + ".bad")
                     rescue
                         raise Puppet::Error,
                             "Could not rename corrupt %s; remove manually" %
-                            Puppet[:checksumfile]
+                            Puppet[:statefile]
                     end
                 end
             }
@@ -60,44 +72,26 @@ module Puppet
             @@state.inspect
         end
 
-        def self.state(myclass)
-            unless myclass.is_a? Class
-                myclass = myclass.class
-            end
-
-            @@state[myclass.to_s] ||= {}
-            return @@state[myclass.to_s]
-        end
-
         def self.store
-            unless FileTest.directory?(File.dirname(Puppet[:checksumfile]))
+            unless FileTest.directory?(File.dirname(Puppet[:statefile]))
                 begin
-                    Puppet.recmkdir(File.dirname(Puppet[:checksumfile]))
+                    Puppet.recmkdir(File.dirname(Puppet[:statefile]))
                     Puppet.info "Creating state directory %s" %
-                        File.dirname(Puppet[:checksumfile])
+                        File.dirname(Puppet[:statefile])
                 rescue => detail
                     Puppet.err "Could not create state file: %s" % detail
                     return
                 end
             end
 
-            unless FileTest.exist?(Puppet[:checksumfile])
-                Puppet.info "Creating state file %s" % Puppet[:checksumfile]
+            unless FileTest.exist?(Puppet[:statefile])
+                Puppet.info "Creating state file %s" % Puppet[:statefile]
             end
 
-            Puppet::Util.lock(Puppet[:checksumfile], File::CREAT|File::WRONLY, 0600) { |file|
+            Puppet::Util.lock(
+                Puppet[:statefile], File::CREAT|File::WRONLY, 0600
+            ) { |file|
                 file.print YAML.dump(@@state)
-                #file.puts(Marshal::dump(@@state))
-                #File.open(Puppet[:checksumfile], File::CREAT|File::WRONLY, 0600) { |file|
-                #    @@state.each { |klass, thash|
-                #        thash.each { |key,value|
-                #            Puppet.warning "Storing: %s %s %s" %
-                #                [klass, key.inspect, value.inspect]
-                #            mvalue = Marshal::dump(value)
-                #            file.puts([klass,key,mvalue].join(@@splitchar))
-                #        }
-                #    }
-                #}
             }
         end
     end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 612cee87e..39aa27858 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -44,6 +44,7 @@ class Transaction
         Puppet.debug "Beginning transaction %s with %s changes" %
             [self.object_id, @changes.length]
 
+        now = Time.now.to_i
         events = @changes.collect { |change|
             if change.is_a?(Puppet::StateChange)
                 change.transaction = self
@@ -66,13 +67,19 @@ class Transaction
                     # should do so
                 end
 
+                # This is kinda lame, because it can result in the same
+                # object being modified multiple times, but that's difficult
+                # to avoid as long as we're syncing each state individually.
+                change.state.parent.cache(:synced, now)
+
                 unless events.nil? or (events.is_a?(Array) and events.empty?)
                     change.changed = true
                 end
                 events
             else
+                puts caller
                 raise Puppet::DevError,
-                    "Transactions cannot handle objects of type %s" % child.class
+                    "Transactions cannot handle objects of type %s" % change.class
             end
         }.flatten.reject { |event|
             event.nil?
@@ -107,11 +114,16 @@ class Transaction
         end
         # change collection is in-band, and message generation is out-of-band
         # of course, exception raising is also out-of-band
-        @changes = @objects.collect { |child|
+        now = Time.now.to_i
+        @changes = @objects.find_all { |child|
+            child.scheduled?
+        }.collect { |child|
             # these children are all Puppet::Type instances
             # not all of the children will return a change, and Containers
             # return transactions
-            child.evaluate
+            ary = child.evaluate
+            child.cache(:checked, now)
+            ary
         }.flatten.reject { |child|
             child.nil? # remove empties
         }
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 6a7dcacc1..5c9bcf027 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -54,23 +54,6 @@ class Type < Puppet::Element
 
     public
 
-    # these objects are used for mapping type names (e.g., 'file')
-    # to actual object classes; because Type.inherited is
-    # called before the <subclass>.name method is defined, we need
-    # to store each class in an array, and then later actually iterate
-    # across that array and make a map
-    #@@typeary = [self] # so that the allowedmethods stuff works
-    #@@typehash = Hash.new { |hash,key|
-    #    if key.is_a?(String)
-    #        key = key.intern
-    #    end
-    #    if hash.include?(key)
-    #        hash[key]
-    #    else
-    #        raise TypeError.new("Object type %s not found" % key)
-    #    end
-    #}
-
     # the Type class attribute accessors
     class << self
         attr_reader :name, :states
@@ -463,6 +446,7 @@ class Type < Puppet::Element
         end
         param.ismetaparameter
         param.class_eval(&block)
+        const_set("MetaParam" + name.to_s.capitalize,param)
         @@metaparams ||= []
         @@metaparams << param
 
@@ -483,6 +467,7 @@ class Type < Puppet::Element
         end
         param.element = self
         param.class_eval(&block)
+        const_set("Parameter" + name.to_s.capitalize,param)
         @parameters ||= []
         @parameters << param
 
@@ -756,8 +741,12 @@ class Type < Puppet::Element
         else
             if @states.has_key?(attr)
                 @states.delete(attr)
+            elsif @parameters.has_key?(attr)
+                @parameters.delete(attr)
+            elsif @metaparams.has_key?(attr)
+                @metaparams.delete(attr)
             else
-                raise Puppet::DevError.new("Undefined state '#{attr}' in #{self}")
+                raise Puppet::DevError.new("Undefined attribute '#{attr}' in #{self}")
             end
         end
     end
@@ -809,11 +798,11 @@ class Type < Puppet::Element
 
         error = type.new(args.join(" "))
 
-        if @line
+        if defined? @line and @line
             error.line = @line
         end
 
-        if @file
+        if defined? @file and @file
             error.file = @file
         end
 
@@ -1142,8 +1131,8 @@ class Type < Puppet::Element
         @noop = false
         # keeping stats for the total number of changes, and how many were
         # completely sync'ed
-        # this isn't really sufficient either, because it adds lots of special cases
-        # such as failed changes
+        # this isn't really sufficient either, because it adds lots of special
+        # cases such as failed changes
         # it also doesn't distinguish between changes from the current transaction
         # vs. changes over the process lifetime
         @totalchanges = 0
@@ -1180,15 +1169,42 @@ class Type < Puppet::Element
             hash.delete(:parent)
         end
 
+        # Convert all args to symbols
         hash = self.argclean(hash)
 
-        self.class.allattrs.each { |name|
+        # Let's do the name first, because some things need to happen once
+        # we have the name but before anything else
+
+        attrs = self.class.allattrs
+        namevar = self.class.namevar
+
+        if hash.include?(namevar)
+            self[namevar] = hash[namevar]
+            hash.delete(namevar)
+            if attrs.include?(namevar)
+                attrs.delete(namevar)
+            else
+                self.devfail "My namevar isn't a valid attribute...?"
+            end
+        else
+            self.devfail "I was not passed a namevar"
+        end
+
+        # The information to cache to disk.  We have to do this after
+        # the name is set because it uses the name and/or path, but before
+        # everything else is set because the states need to be able to
+        # retrieve their stored info.
+        #@cache = Puppet::Storage.cache(self)
+
+        # This is all of our attributes except the namevar.
+        attrs.each { |name|
             if hash.include?(name)
                 begin
                     self[name] = hash[name]
                 rescue => detail
                     self.devfail(
-                        "Could not set %s on %s: %s" % [name, self.class.name, detail]
+                        "Could not set %s on %s: %s" %
+                            [name, self.class.name, detail]
                     )
                 end
                 hash.delete name
@@ -1253,18 +1269,66 @@ class Type < Puppet::Element
         self.schedule
     end
 
+    # Return a cached value
+    def cached(name)
+        Puppet::Storage.cache(self)[name]
+        #@cache[name] ||= nil
+    end
+
+    # Cache a value
+    def cache(name, value)
+        Puppet::Storage.cache(self)[name] = value
+        #@cache[name] = value
+    end
+
+    # Look up the schedule and set it appropriately.  This is done after
+    # the instantiation phase, so that the schedule can be anywhere in the
+    # file.
     def schedule
-        unless self[:schedule]
-            return
-        end
 
-        if sched = Puppet.type(:schedule)[self[:schedule]]
-            self[:schedule] = sched
+        # If we've already set the schedule, then just move on
+        return if self[:schedule].is_a?(Puppet.type(:schedule))
+
+        # Schedules don't need to be scheduled
+        return if self.is_a?(Puppet.type(:schedule))
+
+        # Nor do components
+        return if self.is_a?(Puppet.type(:component))
+
+        if self[:schedule]
+            if sched = Puppet.type(:schedule)[self[:schedule]]
+                self[:schedule] = sched
+            else
+                self.fail "Could not find schedule %s" % self[:schedule]
+            end
+        elsif Puppet[:schedule] and ! Puppet[:ignoreschedules]
+            # We handle schedule defaults here mostly because otherwise things
+            # will behave very very erratically during testing.
+            if sched = Puppet.type(:schedule)[Puppet[:schedule]]
+                self[:schedule] = sched
+            else
+                self.fail "Could not find default schedule %s" % Puppet[:schedule]
+            end
         else
-            self.fail "Could not find schedule %s" % self[:schedule]
+            # While it's unlikely we won't have any schedule (since there's a
+            # default), it's at least possible during testing
+            return true
         end
     end
 
+    # Check whether we are scheduled to run right now or not.
+    def scheduled?
+        return true if Puppet[:ignoreschedules]
+        return true unless schedule = self[:schedule]
+
+        # We use 'checked' here instead of 'synced' because otherwise we'll
+        # end up checking most elements most times, because they will generally
+        # have been synced a long time ago (e.g., a file only gets updated
+        # once a month on the server and its schedule is daily; the last sync time
+        # will have been a month ago, so we'd end up checking every run).
+        return schedule.match?(self.cached(:checked))
+    end
+
     # Is the specified parameter set?
     def attrset?(type, attr)
         case type
@@ -1776,8 +1840,29 @@ class Type < Puppet::Element
     end
 
     newmetaparam(:schedule) do
-        desc "On what schedule the object should be managed.
-            Currently non-functional."
+        desc "On what schedule the object should be managed.  You must create a
+            schedule_ object, and then reference the name of that object to use
+            that for your schedule:
+
+                schedule { daily:
+                    period => daily,
+                    range => \"2-4\"
+                }
+
+                exec { \"/usr/bin/apt-get update\":
+                    schedule => daily
+                }
+
+            The creation of the schedule object does not need to appear in the
+            configuration before objects that use it."
+
+        munge do |name|
+            if schedule = Puppet.type(:schedule)[name]
+                return schedule
+            else
+                return name
+            end
+        end
     end
 
     newmetaparam(:check) do
diff --git a/lib/puppet/type/parsedtype.rb b/lib/puppet/type/parsedtype.rb
index 21d0299f1..268b7ad56 100755
--- a/lib/puppet/type/parsedtype.rb
+++ b/lib/puppet/type/parsedtype.rb
@@ -238,7 +238,7 @@ module Puppet
 
                     return str
                 else
-                    Puppet.notice "No host instances for %s" % user
+                    Puppet.notice "No host instances"
                     return ""
                 end
             end
diff --git a/lib/puppet/type/pfile/checksum.rb b/lib/puppet/type/pfile/checksum.rb
index 61d8fea80..5d936f91e 100755
--- a/lib/puppet/type/pfile/checksum.rb
+++ b/lib/puppet/type/pfile/checksum.rb
@@ -20,6 +20,27 @@ module Puppet
             @checktypes[0]
         end
 
+        # Checksums need to invert how changes are printed.
+        def change_to_s
+            begin
+                if @is == :absent
+                    return "defined '%s' as '%s'" %
+                        [self.name, self.should_to_s]
+                elsif self.should == :absent
+                    return "undefined %s from '%s'" %
+                        [self.name, self.is_to_s]
+                else
+                    return "%s changed '%s' to '%s'" %
+                        [self.name, self.should_to_s, self.is_to_s]
+                end
+            rescue Puppet::Error, Puppet::DevError
+                raise
+            rescue => detail
+                raise Puppet::DevError, "Could not convert change %s to string: %s" %
+                    [self.name, detail]
+            end
+        end
+
         def getsum(checktype)
             sum = ""
             case checktype
@@ -85,27 +106,45 @@ module Puppet
             end
 
             @checktypes << value
-            state = Puppet::Storage.state(self)
 
-            unless state
-                self.devfail "Did not get state back from Storage"
+            hash = nil
+            unless hash = @parent.cached(:checksums) 
+                hash = {}
+                @parent.cache(:checksums, hash)
             end
-            if hash = state[@parent[:path]]
-                if hash.include?(value)
-                    #self.notice "Found checksum %s for %s" %
-                    #    [hash[value] ,@parent[:path]]
-                    return hash[value]
-                else
-                    #self.notice "Found checksum for %s but not of type %s" %
-                    #    [@parent[:path],@checktypes[0]]
-                    return :nosum
-                end
-            else
-                # We can't use :absent here, because then it'll match on
-                # non-existent files
+
+            #unless state
+            #    self.devfail "Did not get state back from Storage"
+            #end
+
+            if hash.include?(value)
+                #self.notice "Found checksum %s for %s" %
+                #    [hash[value] ,@parent[:path]]
+                return hash[value]
+            elsif hash.empty?
                 #self.notice "Could not find sum of type %s" % @checktypes[0]
                 return :nosum
+            else
+                #self.notice "Found checksum for %s but not of type %s" %
+                #    [@parent[:path],@checktypes[0]]
+                return :nosum
             end
+#            if hash = state[@parent[:path]]
+#                if hash.include?(value)
+#                    #self.notice "Found checksum %s for %s" %
+#                    #    [hash[value] ,@parent[:path]]
+#                    return hash[value]
+#                else
+#                    #self.notice "Found checksum for %s but not of type %s" %
+#                    #    [@parent[:path],@checktypes[0]]
+#                    return :nosum
+#                end
+#            else
+#                # We can't use :absent here, because then it'll match on
+#                # non-existent files
+#                #self.notice "Could not find sum of type %s" % @checktypes[0]
+#                return :nosum
+#            end
         end
 
         # Even though they can specify multiple checksums, the insync?
@@ -188,11 +227,11 @@ module Puppet
         # Store the new sum to the state db.
         def updatesum
             result = false
-            state = Puppet::Storage.state(self)
-            unless state.include?(@parent.name)
-                self.debug "Initializing state hash for %s" % @parent.name
-
-                state[@parent.name] = Hash.new
+            state = nil
+            unless state = @parent.cached(:checksums) 
+                self.debug "Initializing checksum hash for %s" % @parent.name
+                state = {}
+                @parent.cache(:checksums, state)
             end
 
             if @is.is_a?(Symbol)
@@ -206,7 +245,7 @@ module Puppet
             end
 
             # if we're replacing, vs. updating
-            if state[@parent.name].include?(@checktypes[0])
+            if state.include?(@checktypes[0])
                 unless defined? @should
                     raise Puppet::Error.new(
                         ("@should is not initialized for %s, even though we " +
@@ -214,7 +253,7 @@ module Puppet
                     )
                 end
                 self.debug "Replacing %s checksum %s with %s" %
-                    [@parent.name, state[@parent.name][@checktypes[0]],@is]
+                    [@parent.name, state[@checktypes[0]],@is]
                 #@parent.debug "@is: %s; @should: %s" % [@is,@should]
                 result = true
             else
@@ -222,7 +261,7 @@ module Puppet
                     [@is,@checktypes[0]]
                 result = false
             end
-            state[@parent.name][@checktypes[0]] = @is
+            state[@checktypes[0]] = @is
             return result
         end
     end
diff --git a/lib/puppet/type/schedule.rb b/lib/puppet/type/schedule.rb
new file mode 100755
index 000000000..515ae0194
--- /dev/null
+++ b/lib/puppet/type/schedule.rb
@@ -0,0 +1,341 @@
+module Puppet
+    newtype(:schedule) do
+        @doc = "Defined schedules for Puppet.  The important thing to understand
+            about how schedules are currently implemented in Puppet is that they
+            can only be used to stop an element from being applied, they never
+            guarantee that it is applied.
+            
+            Every time Puppet applies its configuration, it will collect the
+            list of elements whose schedule does not eliminate them from
+            running right then, but there is currently no system in place to
+            guarantee that a given element runs at a given time.  If you
+            specify a very  restrictive schedule and Puppet happens to run at a
+            time within that schedule, then the elements will get applied;
+            otherwise, that work may never get done.
+            
+            Thus, it behooves you to use wider scheduling (e.g., over a couple of
+            hours) combined with periods and repetitions.  For instance, if you
+            wanted to restrict certain elements to only running once, between
+            the hours of two and 4 AM, then you would use this schedule::
+                
+                schedule { maint:
+                    range => \"2 - 4\",
+                    period => daily,
+                    repeat => 1
+                }
+
+            With this schedule, the first time that Puppet runs between 2 and 4 AM,
+            all elements with this schedule will get applied, but they won't
+            get applied again between 2 and 4 because they will have already
+            run once that day, and they won't get applied outside that schedule
+            because they will be outside the scheduled range.
+            
+            Puppet automatically creates a schedule for each valid period with the
+            same name as that period (e.g., hourly and daily).  Additionally,
+            a schedule named *puppet* is created and used as the default,
+            with the following attributes:
+
+                schedule { puppet:
+                    period => hourly,
+                    repeat => 2
+                }
+
+            This will cause elements to be applied every 30 minutes by default.
+            "
+
+        @states = []
+
+        newparam(:name) do
+            desc "The name of the schedule.  This name is used to retrieve the
+                schedule when assigning it to an object::
+                    
+                    schedule { daily:
+                        period => daily,
+                        range => [2, 4]
+                    }
+
+                    exec { \"/usr/bin/apt-get update\":
+                        schedule => daily
+                    }
+                
+                "
+            isnamevar
+        end
+
+        newparam(:range) do
+            desc "The earliest and latest that an element can be applied.  This
+                is always a range within a 24 hour period, and hours must be
+                specified in numbers between 0 and 23, inclusive.  Minutes and
+                seconds can be provided, using the normal colon as a separator.
+                For instance::
+
+                    schedule { maintenance:
+                        range => \"1:30 - 4:30\"
+                    }
+                
+                This is mostly useful for restricting certain elements to being
+                applied in maintenance windows or during off-peak hours."
+
+            # This is lame; states all use arrays as values, but parameters don't.
+            # That's going to hurt eventually.
+            validate do |values|
+                values = [values] unless values.is_a?(Array)
+                values.each { |value|
+                    unless  value.is_a?(String) and
+                            value =~ /\d+(:\d+){0,2}\s*-\s*\d+(:\d+){0,2}/
+                        self.fail "Invalid range value '%s'" % value
+                    end
+                }
+            end
+
+            munge do |values|
+                values = [values] unless values.is_a?(Array)
+                ret = []
+
+                values.each { |value|
+                    range = []
+                    # Split each range value into a hour, minute, second triad
+                    value.split(/\s*-\s*/).each { |val|
+                        # Add the values as an array.
+                        range << val.split(":").collect { |n| n.to_i }
+                    }
+
+                    if range.length != 2
+                        self.fail "Invalid range %s" % value
+                    end
+
+                    if range[0][0] > range[1][0]
+                        self.fail(("Invalid range %s; " % value) +
+                            "ranges cannot span days."
+                        )
+                    end
+                    ret << range
+                }
+
+                # Now our array of arrays
+                ret
+            end
+
+            def match?(previous, now)
+                # The lowest-level array is of the hour, minute, second triad
+                # then it's an array of two of those, to present the limits
+                # then it's array of those ranges
+                unless @value[0][0].is_a?(Array)
+                    @value = [@value]
+                end
+
+                @value.each do |value|
+                    limits = value.collect do |range|
+                        ary = [now.year, now.month, now.day, range[0]]
+                        if range[1]
+                            ary << range[1]
+                        else
+                            ary << now.min
+                        end
+
+                        if range[2]
+                            ary << range[2]
+                        else
+                            ary << now.sec
+                        end
+
+                        time = Time.local(*ary)
+
+                        unless time.hour == range[0]
+                            self.devfail(
+                                "Incorrectly converted time: %s: %s vs %s" %
+                                    [time, time.hour, range[0]]
+                            )
+                        end
+
+                        time
+                    end
+
+                    unless limits[0] < limits[1]
+                        self.info(
+                        "Assuming upper limit should be that time the next day"
+                        )
+
+                        ary = limits[1].to_a
+                        ary[3] += 1
+                        limits[1] = Time.local(*ary)
+                            
+                        #self.devfail("Lower limit is above higher limit: %s" %
+                        #    limits.inspect
+                        #)
+                    end
+
+                    #self.info limits.inspect
+                    #self.notice now
+                    return now.between?(*limits)
+                end
+
+                # Else, return false, since our current time isn't between
+                # any valid times
+                return false
+            end
+        end
+
+        newparam(:periodmatch) do
+            desc "Whether periods should be matched by number (e.g., the two times
+                are in the same hour) or by distance (e.g., the two times are
+                60 minutes apart). *number*/**distance**"
+
+            newvalues(:number, :distance)
+
+            defaultto :distance
+        end
+
+        newparam(:period) do
+            desc "The period of repetition for an element.  Choose from among
+                a fixed list of *hourly*, *daily*, *weekly*, and *monthly*.
+                The default is for an element to get applied every time that
+                Puppet runs, whatever that period is.
+
+                Note that the period defines how often a given element will get
+                applied but not when; if you would like to restrict the hours
+                that a given element can be applied (e.g., only at night during
+                a maintenance window) then use the ``range`` attribute.
+
+                If the provided periods are not sufficient, you can provide a
+                value to the *repeat* attribute, which will cause Puppet to
+                schedule the affected elements evenly in the period the
+                specified number of times.  Take this schedule::
+
+                    schedule { veryoften:
+                        period => hourly,
+                        repeat => 6
+                    }
+
+                This can cause Puppet to apply that element up to every 10 minutes.
+
+                At the moment, Puppet cannot guarantee that level of
+                repetition; that is, it can run up to every 10 minutes, but
+                internal factors might prevent it from actually running that
+                often (e.g., long-running Puppet runs will squash conflictingly
+                scheduled runs).
+                
+                See the ``periodmatch`` attribute for tuning whether to match
+                times by their distance apart or by their specific value."
+
+            newvalues(:hourly, :daily, :weekly, :monthly)
+
+            @@scale = {
+                :hourly => 3600,
+                :daily => 86400,
+                :weekly => 604800,
+                :monthly => 2592000
+            }
+            @@methods = {
+                :hourly => :hour,
+                :daily => :day,
+                :monthly => :month,
+                :weekly => proc do |prev, now|
+                    prev.strftime("%U") == now.strftime("%U")
+                end
+            }
+
+            def match?(previous, now)
+                value = self.value
+                case @parent[:periodmatch]
+                when :number
+                    method = @@methods[value]
+                    if method.is_a?(Proc)
+                        return method.call(previous, now)
+                    else
+                        # We negate it, because if they're equal we don't run
+                        val = now.send(method) != previous.send(method)
+                        return val
+                    end
+                when :distance
+                    scale = @@scale[value]
+
+                    # If the number of seconds between the two times is greater
+                    # than the unit of time, we match.  We divide the scale
+                    # by the repeat, so that we'll repeat that often within
+                    # the scale.
+                    return (now.to_i - previous.to_i) >= (scale / @parent[:repeat])
+                end
+            end
+        end
+
+        newparam(:repeat) do
+            desc "How often the application gets repeated in a given period.
+                Defaults to 1."
+
+            defaultto 1
+
+            validate do |value|
+                unless value.is_a?(Integer) or value =~ /^\d+$/
+                    raise Puppet::Error,
+                        "Repeat must be a number"
+                end
+
+                # This implicitly assumes that 'periodmatch' is distance -- that
+                # is, if there's no value, we assume it's a valid value.
+                return unless @parent[:periodmatch]
+
+                if value != 1 and @parent[:periodmatch] != :distance
+                    raise Puppet::Error,
+                        "Repeat must be 1 unless periodmatch is 'distance', not '%s'" %
+                            @parent[:periodmatch]
+                end
+            end
+
+            munge do |value|
+                unless value.is_a?(Integer)
+                    value = Integer(value)
+                end
+
+                value
+            end
+
+            def match?(previous, now)
+                true
+            end
+        end
+
+        def self.mkdefaultschedules
+            Puppet.info "Creating default schedules"
+            # Create our default schedule
+            self.create(
+                :name => "puppet",
+                :period => :hourly,
+                :repeat => "2"
+            )
+
+            # And then one for every period
+            @parameters.find { |p| p.name == :period }.values.each { |value|
+                self.create(
+                    :name => value.to_s,
+                    :period => value
+                )
+            }
+        end
+
+        def match?(previous = nil, now = nil)
+
+            # If we've got a value, then convert it to a Time instance
+            if previous
+                previous = Time.at(previous)
+            end
+
+            now ||= Time.now
+
+            # Pull them in order
+            self.class.allattrs.each { |param|
+                if @parameters.include?(param) and
+                    @parameters[param].respond_to?(:match?)
+                    #self.notice "Trying to match %s" % param
+                    return false unless @parameters[param].match?(previous, now)
+                end
+            }
+
+            # If we haven't returned false, then return true; in other words,
+            # any provided schedules need to all match
+            return true
+        end
+    end
+end
+
+# $Id$
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index 6e1600083..2a5f18a89 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -28,32 +28,32 @@ class State < Puppet::Parameter
         end
     end
 
-    # Define a new value for our state.
+    # Parameters just use 'newvalues', since there's no work associated with them.
     def self.newvalue(name, &block)
-        @statevalues ||= {}
+        @parametervalues ||= {}
 
-        if @statevalues.include?(name)
-            Puppet.warning "%s already has a value for %s" % [self.name, name]
+        if @parametervalues.include?(name)
+            Puppet.warning "%s already has a value for %s" % [name, name]
         end
-        @statevalues[name] = block
+        @parametervalues[name] = block
 
         define_method("set_" + name.to_s, &block)
     end
-
-    def self.aliasvalue(name, other)
-        @statevalues ||= {}
-        unless @statevalues.include?(other)
-            raise Puppet::DevError, "Cannot alias nonexistent value %s" % other
-        end
-
-        @aliasvalues ||= {}
-        @aliasvalues[name] = other
-    end
-
-    def self.alias(name)
-        @aliasvalues[name]
-    end
-
+#
+#    def self.aliasvalue(name, other)
+#        @statevalues ||= {}
+#        unless @statevalues.include?(other)
+#            raise Puppet::DevError, "Cannot alias nonexistent value %s" % other
+#        end
+#
+#        @aliasvalues ||= {}
+#        @aliasvalues[name] = other
+#    end
+#
+#    def self.alias(name)
+#        @aliasvalues[name]
+#    end
+#
     def self.defaultvalues
         newvalue(:present) do
             @parent.create
@@ -66,15 +66,15 @@ class State < Puppet::Parameter
         # This doc will probably get overridden
         @doc ||= "The basic state that the object should be in."
     end
-
-    # Return the list of valid values.
-    def self.values
-        @statevalues ||= {}
-        @aliasvalues ||= {}
-
-        #[@aliasvalues.keys, @statevalues.keys].flatten
-        @statevalues.keys
-    end
+#
+#    # Return the list of valid values.
+#    def self.values
+#        @statevalues ||= {}
+#        @aliasvalues ||= {}
+#
+#        #[@aliasvalues.keys, @statevalues.keys].flatten
+#        @statevalues.keys
+#    end
 
     # Call the method associated with a given value.
     def set
@@ -278,38 +278,38 @@ class State < Puppet::Parameter
         self.set
     end
 
-    munge do |value|
-        if self.class.values.empty?
-            # This state isn't using defined values to do its work.
-            return value
-        end
-        intern = value.to_s.intern
-        # If it's a valid value, always return it as a symbol.
-        if self.class.values.include?(intern)
-            retval = intern
-        elsif other = self.class.alias(intern)
-            self.info "returning alias %s for %s" % [other, intern]
-            retval = other
-        else
-            retval = value
-        end
-        retval
-    end
-
-    # Verify that the passed value is valid.
-    validate do |value|
-        if self.class.values.empty?
-            # This state isn't using defined values to do its work.
-            return 
-        end
-        unless value.is_a?(Symbol)
-            value = value.to_s.intern
-        end
-        unless self.class.values.include?(value) or self.class.alias(value)
-            self.fail "Invalid '%s' value '%s'.  Valid values are '%s'" %
-                    [self.class.name, value, self.class.values.join(", ")]
-        end
-    end
+#    munge do |value|
+#        if self.class.values.empty?
+#            # This state isn't using defined values to do its work.
+#            return value
+#        end
+#        intern = value.to_s.intern
+#        # If it's a valid value, always return it as a symbol.
+#        if self.class.values.include?(intern)
+#            retval = intern
+#        elsif other = self.class.alias(intern)
+#            self.info "returning alias %s for %s" % [other, intern]
+#            retval = other
+#        else
+#            retval = value
+#        end
+#        retval
+#    end
+#
+#    # Verify that the passed value is valid.
+#    validate do |value|
+#        if self.class.values.empty?
+#            # This state isn't using defined values to do its work.
+#            return 
+#        end
+#        unless value.is_a?(Symbol)
+#            value = value.to_s.intern
+#        end
+#        unless self.class.values.include?(value) or self.class.alias(value)
+#            self.fail "Invalid '%s' value '%s'.  Valid values are '%s'" %
+#                    [self.class.name, value, self.class.values.join(", ")]
+#        end
+#    end
 
     # How should a state change be printed as a string?
     def change_to_s
@@ -378,7 +378,6 @@ class State < Puppet::Parameter
         end
 
         def retrieve
-            self.warning "retrieving"
             if @parent.exists?
                 @is = :present
             else