From fc14b81f99adc9c9308a26d322adaa59a7b7716d Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 28 Jan 2009 17:11:19 -0600 Subject: Splitting the Agent class into Agent and Configurer Once I went to add runinterval support to the Agent class, I realized it's really two classes: One that handles starting, stopping, running, et al (still called Agent), and one that handles downloading the catalog, running it, etc. (now called Configurer). This commit includes some additional code, but 95% of it is just moving code around. Signed-off-by: Luke Kanies --- lib/puppet/agent.rb | 249 +++++++++++--------------------- lib/puppet/agent/downloader.rb | 79 ---------- lib/puppet/agent/fact_handler.rb | 48 ------ lib/puppet/agent/locker.rb | 7 +- lib/puppet/agent/plugin_handler.rb | 25 ---- lib/puppet/configurer.rb | 185 ++++++++++++++++++++++++ lib/puppet/configurer/downloader.rb | 79 ++++++++++ lib/puppet/configurer/fact_handler.rb | 48 ++++++ lib/puppet/configurer/plugin_handler.rb | 25 ++++ lib/puppet/daemon.rb | 2 - 10 files changed, 424 insertions(+), 323 deletions(-) delete mode 100644 lib/puppet/agent/downloader.rb delete mode 100644 lib/puppet/agent/fact_handler.rb delete mode 100644 lib/puppet/agent/plugin_handler.rb create mode 100644 lib/puppet/configurer.rb create mode 100644 lib/puppet/configurer/downloader.rb create mode 100644 lib/puppet/configurer/fact_handler.rb create mode 100644 lib/puppet/configurer/plugin_handler.rb (limited to 'lib') diff --git a/lib/puppet/agent.rb b/lib/puppet/agent.rb index e2b4ebdc7..c91552b16 100644 --- a/lib/puppet/agent.rb +++ b/lib/puppet/agent.rb @@ -1,201 +1,74 @@ -# The client for interacting with the puppetmaster config server. require 'sync' -require 'timeout' -require 'puppet/network/http_pool' -require 'puppet/util' +require 'puppet/daemon' +# A general class for triggering a run of another +# class. class Puppet::Agent - require 'puppet/agent/fact_handler' - require 'puppet/agent/plugin_handler' - require 'puppet/agent/locker' + include Puppet::Daemon - include Puppet::Agent::FactHandler - include Puppet::Agent::PluginHandler + require 'puppet/agent/locker' include Puppet::Agent::Locker - # For benchmarking - include Puppet::Util - - unless defined? @@sync - @@sync = Sync.new - end - - attr_accessor :catalog - attr_reader :compile_time - - class << self - # Puppetd should only have one instance running, and we need a way - # to retrieve it. - attr_accessor :instance - include Puppet::Util - end - - def clear - @catalog.clear(true) if @catalog - @catalog = nil - end + attr_reader :client_class, :client + attr_accessor :stopping - # Initialize and load storage - def dostorage - begin - Puppet::Util::Storage.load - @compile_time ||= Puppet::Util::Storage.cache(:configuration)[:compile_time] - rescue => detail - if Puppet[:trace] - puts detail.backtrace - end - Puppet.err "Corrupt state file %s: %s" % [Puppet[:statefile], detail] - begin - ::File.unlink(Puppet[:statefile]) - retry - rescue => detail - raise Puppet::Error.new("Cannot remove %s: %s" % - [Puppet[:statefile], detail]) - end - end - end - # Just so we can specify that we are "the" instance. - def initialize - Puppet.settings.use(:main, :ssl, :puppetd) - - self.class.instance = self - @running = false + def initialize(client_class) @splayed = false - end - - # Prepare for catalog retrieval. Downloads everything necessary, etc. - def prepare - dostorage() - - download_plugins() - download_fact_plugins() - - upload_facts() + @client_class = client_class end - # Mark that we should restart. The Puppet module checks whether we're running, - # so this only gets called if we're in the middle of a run. - def restart - # If we're currently running, then just mark for later - Puppet.notice "Received signal to restart; waiting until run is complete" - @restart = true + def lockfile_path + client_class.lockfile_path end - # Should we restart? - def restart? - if defined? @restart - @restart - else - false + # Perform a run with our client. + def run + if client + Puppet.notice "Run of %s already in progress; skipping" % client_class + return end - end - - # Get the remote catalog, yo. Returns nil if no catalog can be found. - def retrieve_catalog - name = Facter.value("hostname") - catalog_class = Puppet::Resource::Catalog - - # First try it with no cache, then with the cache. - result = nil - begin - duration = thinmark do - result = catalog_class.find(name, :use_cache => false) - end - rescue => detail - puts detail.backtrace if Puppet[:trace] - Puppet.err "Could not retrieve catalog from remote server: %s" % detail + if stopping? + Puppet.notice "In shutdown progress; skipping run" + return end - - unless result + splay + with_client do |client| begin - duration = thinmark do - result = catalog_class.find(name, :use_cache => true) - end + sync.synchronize { lock { client.run } } rescue => detail puts detail.backtrace if Puppet[:trace] - Puppet.err "Could not retrieve catalog from cache: %s" % detail + Puppet.err "Could not run %s: %s" % [client_class, detail] end end - - return nil unless result - - convert_catalog(result, duration) end - # Convert a plain resource catalog into our full host catalog. - def convert_catalog(result, duration) - catalog = result.to_ral - catalog.retrieval_duration = duration - catalog.host_config = true - catalog.write_class_file - return catalog - end - - # The code that actually runs the catalog. - # This just passes any options on to the catalog, - # which accepts :tags and :ignoreschedules. - def run(options = {}) - got_lock = false - splay - Puppet::Util.sync(:puppetrun).synchronize(Sync::EX) do - got_lock = lock do - unless catalog = retrieve_catalog - Puppet.err "Could not retrieve catalog; skipping run" - return - end - - begin - benchmark(:notice, "Finished catalog run") do - catalog.apply(options) - end - rescue => detail - puts detail.backtrace if Puppet[:trace] - Puppet.err "Failed to apply catalog: %s" % detail - end - end - - unless got_lock - Puppet.notice "Lock file %s exists; skipping catalog run" % lockfile.lockfile - return + def shutdown + if self.stopping? + Puppet.notice "Already in shutdown" + return + end + self.stopping = true + if client and client.respond_to?(:stop) + begin + client.stop + rescue + puts detail.backtrace if Puppet[:trace] + Puppet.err "Could not stop %s: %s" % [client_class, detail] end - - # Now close all of our existing http connections, since there's no - # reason to leave them lying open. - Puppet::Network::HttpPool.clear_http_instances - - lockfile.unlock - - # Did we get HUPped during the run? If so, then restart now that we're - # done with the run. - Process.kill(:HUP, $$) if self.restart? end - end - def running? - lockfile.locked? + super + ensure + self.stopping = false end - private - - def self.timeout - timeout = Puppet[:configtimeout] - case timeout - when String: - if timeout =~ /^\d+$/ - timeout = Integer(timeout) - else - raise ArgumentError, "Configuration timeout must be an integer" - end - when Integer: # nothing - else - raise ArgumentError, "Configuration timeout must be an integer" - end - - return timeout + def stopping? + stopping end + # Have we splayed already? def splayed? @splayed end @@ -210,4 +83,44 @@ class Puppet::Agent sleep(time) @splayed = true end + + # Start listening for events. We're pretty much just listening for + # timer events here. + def start + # Create our timer. Puppet will handle observing it and such. + Puppet.newtimer( + :interval => Puppet[:runinterval], + :tolerance => 1, + :start? => true + ) do + run() + end + + # Run once before we start following the timer + run() + end + + def sync + unless defined?(@sync) and @sync + @sync = Sync.new + end + @sync + end + + private + + # Create and yield a client instance, keeping a reference + # to it during the yield. + def with_client + begin + @client = client_class.new + rescue => details + puts detail.backtrace if Puppet[:trace] + Puppet.err "Could not create instance of %s: %s" % [client_class, details] + return + end + yield @client + ensure + @client = nil + end end diff --git a/lib/puppet/agent/downloader.rb b/lib/puppet/agent/downloader.rb deleted file mode 100644 index edc5931c3..000000000 --- a/lib/puppet/agent/downloader.rb +++ /dev/null @@ -1,79 +0,0 @@ -require 'puppet/agent' -require 'puppet/resource/catalog' - -class Puppet::Agent::Downloader - attr_reader :name, :path, :source, :ignore - - # Determine the timeout value to use. - def self.timeout - timeout = Puppet[:configtimeout] - case timeout - when String: - if timeout =~ /^\d+$/ - timeout = Integer(timeout) - else - raise ArgumentError, "Configuration timeout must be an integer" - end - when Integer: # nothing - else - raise ArgumentError, "Configuration timeout must be an integer" - end - - return timeout - end - - # Evaluate our download, returning the list of changed values. - def evaluate - Puppet.info "Retrieving #{name}" - - files = [] - begin - Timeout.timeout(self.class.timeout) do - catalog.apply do |trans| - trans.changed?.find_all do |resource| - yield resource if block_given? - files << resource[:path] - end - end - end - rescue Puppet::Error, Timeout::Error => detail - puts detail.backtrace if Puppet[:debug] - Puppet.err "Could not retrieve #{name}: %s" % detail - end - - return files - end - - def initialize(name, path, source, ignore = nil) - @name, @path, @source, @ignore = name, path, source, ignore - end - - def catalog - catalog = Puppet::Resource::Catalog.new - catalog.add_resource(file) - catalog - end - - def file - args = default_arguments.merge(:path => path, :source => source) - args[:ignore] = ignore if ignore - Puppet::Type.type(:file).create(args) - end - - private - - def default_arguments - { - :path => path, - :recurse => true, - :source => source, - :tag => name, - :owner => Process.uid, - :group => Process.gid, - :purge => true, - :force => true, - :backup => false, - :noop => false - } - end -end diff --git a/lib/puppet/agent/fact_handler.rb b/lib/puppet/agent/fact_handler.rb deleted file mode 100644 index 266ae1815..000000000 --- a/lib/puppet/agent/fact_handler.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'puppet/indirector/facts/facter' - -# Break out the code related to facts. This module is -# just included into the agent, but having it here makes it -# easier to test. -module Puppet::Agent::FactHandler - def download_fact_plugins? - Puppet[:factsync] - end - - def upload_facts - # XXX down = Puppet[:downcasefacts] - - reload_facter() - - # This works because puppetd configures Facts to use 'facter' for - # finding facts and the 'rest' terminus for caching them. Thus, we'll - # compile them and then "cache" them on the server. - Puppet::Node::Facts.find(Puppet[:certname]) - end - - # Retrieve facts from the central server. - def download_fact_plugins - return unless download_fact_plugins? - - Puppet::Agent::Downloader.new("fact", Puppet[:factsource], Puppet[:factdest], Puppet[:factsignore]).evaluate - end - - # Clear out all of the loaded facts and reload them from disk. - # NOTE: This is clumsy and shouldn't be required for later (1.5.x) versions - # of Facter. - def reload_facter - Facter.clear - - # Reload everything. - if Facter.respond_to? :loadfacts - Facter.loadfacts - elsif Facter.respond_to? :load - Facter.load - else - Puppet.warning "You should upgrade your version of Facter to at least 1.3.8" - end - - # This loads all existing facts and any new ones. We have to remove and - # reload because there's no way to unload specific facts. - Puppet::Node::Facts::Facter.load_fact_plugins() - end -end diff --git a/lib/puppet/agent/locker.rb b/lib/puppet/agent/locker.rb index 03736b278..c24fdad64 100644 --- a/lib/puppet/agent/locker.rb +++ b/lib/puppet/agent/locker.rb @@ -30,9 +30,14 @@ module Puppet::Agent::Locker def lockfile unless defined?(@lockfile) - @lockfile = Puppet::Util::Pidlock.new(Puppet[:puppetdlockfile]) + #@lockfile = Puppet::Util::Pidlock.new(Puppet[:puppetdlockfile]) + @lockfile = Puppet::Util::Pidlock.new(lockfile_path) end @lockfile end + + def running? + lockfile.locked? + end end diff --git a/lib/puppet/agent/plugin_handler.rb b/lib/puppet/agent/plugin_handler.rb deleted file mode 100644 index 306b8b6df..000000000 --- a/lib/puppet/agent/plugin_handler.rb +++ /dev/null @@ -1,25 +0,0 @@ -# Break out the code related to plugins. This module is -# just included into the agent, but having it here makes it -# easier to test. -module Puppet::Agent::PluginHandler - def download_plugins? - Puppet[:pluginsync] - end - - # Retrieve facts from the central server. - def download_plugins - return nil unless download_plugins? - Puppet::Agent::Downloader.new("plugin", Puppet[:pluginsource], Puppet[:plugindest], Puppet[:pluginsignore]).evaluate.each { |file| load_plugin(file) } - end - - def load_plugin(file) - return if FileTest.directory?(file) - - begin - Puppet.info "Loading downloaded plugin %s" % file - load file - rescue Exception => detail - Puppet.err "Could not load downloaded file %s: %s" % [file, detail] - end - end -end diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb new file mode 100644 index 000000000..df531f494 --- /dev/null +++ b/lib/puppet/configurer.rb @@ -0,0 +1,185 @@ +# The client for interacting with the puppetmaster config server. +require 'sync' +require 'timeout' +require 'puppet/network/http_pool' +require 'puppet/util' + +class Puppet::Configurer + require 'puppet/configurer/fact_handler' + require 'puppet/configurer/plugin_handler' + + include Puppet::Configurer::FactHandler + include Puppet::Configurer::PluginHandler + + # For benchmarking + include Puppet::Util + + attr_accessor :catalog + attr_reader :compile_time + + # Provide more helpful strings to the logging that the Agent does + def self.to_s + "Puppet configuration client" + end + + class << self + # Puppetd should only have one instance running, and we need a way + # to retrieve it. + attr_accessor :instance + include Puppet::Util + end + + # How to lock instances of this class. + def self.lockfile_path + Puppet[:puppetdlockfile] + end + + def clear + @catalog.clear(true) if @catalog + @catalog = nil + end + + # Initialize and load storage + def dostorage + begin + Puppet::Util::Storage.load + @compile_time ||= Puppet::Util::Storage.cache(:configuration)[:compile_time] + rescue => detail + if Puppet[:trace] + puts detail.backtrace + end + Puppet.err "Corrupt state file %s: %s" % [Puppet[:statefile], detail] + begin + ::File.unlink(Puppet[:statefile]) + retry + rescue => detail + raise Puppet::Error.new("Cannot remove %s: %s" % + [Puppet[:statefile], detail]) + end + end + end + + # Just so we can specify that we are "the" instance. + def initialize + Puppet.settings.use(:main, :ssl, :puppetd) + + self.class.instance = self + @running = false + @splayed = false + end + + # Prepare for catalog retrieval. Downloads everything necessary, etc. + def prepare + dostorage() + + download_plugins() + + download_fact_plugins() + + upload_facts() + end + + # Mark that we should restart. The Puppet module checks whether we're running, + # so this only gets called if we're in the middle of a run. + def restart + # If we're currently running, then just mark for later + Puppet.notice "Received signal to restart; waiting until run is complete" + @restart = true + end + + # Should we restart? + def restart? + if defined? @restart + @restart + else + false + end + end + + # Get the remote catalog, yo. Returns nil if no catalog can be found. + def retrieve_catalog + name = Facter.value("hostname") + catalog_class = Puppet::Resource::Catalog + + # First try it with no cache, then with the cache. + result = nil + begin + duration = thinmark do + result = catalog_class.find(name, :use_cache => false) + end + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Could not retrieve catalog from remote server: %s" % detail + end + + unless result + begin + duration = thinmark do + result = catalog_class.find(name, :use_cache => true) + end + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Could not retrieve catalog from cache: %s" % detail + end + end + + return nil unless result + + convert_catalog(result, duration) + end + + # Convert a plain resource catalog into our full host catalog. + def convert_catalog(result, duration) + catalog = result.to_ral + catalog.retrieval_duration = duration + catalog.host_config = true + catalog.write_class_file + return catalog + end + + # The code that actually runs the catalog. + # This just passes any options on to the catalog, + # which accepts :tags and :ignoreschedules. + def run(options = {}) + unless catalog = retrieve_catalog + Puppet.err "Could not retrieve catalog; skipping run" + return + end + + begin + benchmark(:notice, "Finished catalog run") do + catalog.apply(options) + end + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Failed to apply catalog: %s" % detail + end + + # Now close all of our existing http connections, since there's no + # reason to leave them lying open. + Puppet::Network::HttpPool.clear_http_instances + + # Did we get HUPped during the run? If so, then restart now that we're + # done with the run. + Process.kill(:HUP, $$) if self.restart? + end + + private + + def self.timeout + timeout = Puppet[:configtimeout] + case timeout + when String: + if timeout =~ /^\d+$/ + timeout = Integer(timeout) + else + raise ArgumentError, "Configuration timeout must be an integer" + end + when Integer: # nothing + else + raise ArgumentError, "Configuration timeout must be an integer" + end + + return timeout + end +end diff --git a/lib/puppet/configurer/downloader.rb b/lib/puppet/configurer/downloader.rb new file mode 100644 index 000000000..f1c4c03b1 --- /dev/null +++ b/lib/puppet/configurer/downloader.rb @@ -0,0 +1,79 @@ +require 'puppet/agent' +require 'puppet/resource/catalog' + +class Puppet::Configurer::Downloader + attr_reader :name, :path, :source, :ignore + + # Determine the timeout value to use. + def self.timeout + timeout = Puppet[:configtimeout] + case timeout + when String: + if timeout =~ /^\d+$/ + timeout = Integer(timeout) + else + raise ArgumentError, "Configuration timeout must be an integer" + end + when Integer: # nothing + else + raise ArgumentError, "Configuration timeout must be an integer" + end + + return timeout + end + + # Evaluate our download, returning the list of changed values. + def evaluate + Puppet.info "Retrieving #{name}" + + files = [] + begin + Timeout.timeout(self.class.timeout) do + catalog.apply do |trans| + trans.changed?.find_all do |resource| + yield resource if block_given? + files << resource[:path] + end + end + end + rescue Puppet::Error, Timeout::Error => detail + puts detail.backtrace if Puppet[:debug] + Puppet.err "Could not retrieve #{name}: %s" % detail + end + + return files + end + + def initialize(name, path, source, ignore = nil) + @name, @path, @source, @ignore = name, path, source, ignore + end + + def catalog + catalog = Puppet::Resource::Catalog.new + catalog.add_resource(file) + catalog + end + + def file + args = default_arguments.merge(:path => path, :source => source) + args[:ignore] = ignore if ignore + Puppet::Type.type(:file).create(args) + end + + private + + def default_arguments + { + :path => path, + :recurse => true, + :source => source, + :tag => name, + :owner => Process.uid, + :group => Process.gid, + :purge => true, + :force => true, + :backup => false, + :noop => false + } + end +end diff --git a/lib/puppet/configurer/fact_handler.rb b/lib/puppet/configurer/fact_handler.rb new file mode 100644 index 000000000..9435cb22a --- /dev/null +++ b/lib/puppet/configurer/fact_handler.rb @@ -0,0 +1,48 @@ +require 'puppet/indirector/facts/facter' + +# Break out the code related to facts. This module is +# just included into the agent, but having it here makes it +# easier to test. +module Puppet::Configurer::FactHandler + def download_fact_plugins? + Puppet[:factsync] + end + + def upload_facts + # XXX down = Puppet[:downcasefacts] + + reload_facter() + + # This works because puppetd configures Facts to use 'facter' for + # finding facts and the 'rest' terminus for caching them. Thus, we'll + # compile them and then "cache" them on the server. + Puppet::Node::Facts.find(Puppet[:certname]) + end + + # Retrieve facts from the central server. + def download_fact_plugins + return unless download_fact_plugins? + + Puppet::Configurer::Downloader.new("fact", Puppet[:factsource], Puppet[:factdest], Puppet[:factsignore]).evaluate + end + + # Clear out all of the loaded facts and reload them from disk. + # NOTE: This is clumsy and shouldn't be required for later (1.5.x) versions + # of Facter. + def reload_facter + Facter.clear + + # Reload everything. + if Facter.respond_to? :loadfacts + Facter.loadfacts + elsif Facter.respond_to? :load + Facter.load + else + Puppet.warning "You should upgrade your version of Facter to at least 1.3.8" + end + + # This loads all existing facts and any new ones. We have to remove and + # reload because there's no way to unload specific facts. + Puppet::Node::Facts::Facter.load_fact_plugins() + end +end diff --git a/lib/puppet/configurer/plugin_handler.rb b/lib/puppet/configurer/plugin_handler.rb new file mode 100644 index 000000000..cadf300fd --- /dev/null +++ b/lib/puppet/configurer/plugin_handler.rb @@ -0,0 +1,25 @@ +# Break out the code related to plugins. This module is +# just included into the agent, but having it here makes it +# easier to test. +module Puppet::Configurer::PluginHandler + def download_plugins? + Puppet[:pluginsync] + end + + # Retrieve facts from the central server. + def download_plugins + return nil unless download_plugins? + Puppet::Configurer::Downloader.new("plugin", Puppet[:pluginsource], Puppet[:plugindest], Puppet[:pluginsignore]).evaluate.each { |file| load_plugin(file) } + end + + def load_plugin(file) + return if FileTest.directory?(file) + + begin + Puppet.info "Loading downloaded plugin %s" % file + load file + rescue Exception => detail + Puppet.err "Could not load downloaded file %s: %s" % [file, detail] + end + end +end diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb index 24d743764..b0576124e 100755 --- a/lib/puppet/daemon.rb +++ b/lib/puppet/daemon.rb @@ -76,8 +76,6 @@ module Puppet::Daemon Puppet::Util::Log.destinations.reject { |d| d == :console }.each do |dest| Puppet::Util::Log.close(dest) end - - super end end -- cgit