From f17f19dae941b17a56c1fc83ed3a89712b98c427 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 15 Sep 2007 22:17:20 -0600 Subject: The whole system now uses Configuration objects instead of ever converting the Transportable objects into a tree of components and then converting that into a graph. This is a significant step, and drastically simplifies the model of how to use a configuration. The old code might have looked something like this: file = Puppet::Type.create :path => "/whatever", ... comp = Puppet::Type.create :name => :whatever comp.push file transaction = comp.evaluate transaction.evaluate The new code looks like this: file = Puppet::Type.create :path => "/whatever", ... config = Puppet::Node::Configuration.new config.add_resource file config.apply I did not really intend to do this much refactoring, but I found I could not use a Configuration object to do work without refactoring a lot of the system. The primary problem was that the Client::Master and the Config classes determined how the transactions behaved; when I moved to using a Configuration, this distinction was lost, which meant that configurations were often needing to create other configurations, which resulted in a whole lot of infinite recursion (e.g., Config objects that create directories for Puppet use Configuration objects -- yes, I'm s/Config/Settings/g soon -- and these Configuration objects would need to create directories). Not everything is fixed, but it's very close. I am clearly over the hump, though, so I wanted to get a commit in. --- lib/puppet/node/configuration.rb | 81 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 5 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index be30ddb68..8cb6ed1f9 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -5,9 +5,26 @@ require 'puppet/external/gratr/digraph' # of the information in the configuration, including the resources # and the relationships between them. class Puppet::Node::Configuration < Puppet::PGraph - attr_accessor :name, :version + # The host name this is a configuration for. + attr_accessor :name + + # The configuration version. Used for testing whether a configuration + # is up to date. + attr_accessor :version + + # How long this configuration took to retrieve. Used for reporting stats. + attr_accessor :retrieval_duration + + # How we should extract the configuration for sending to the client. attr_reader :extraction_format + # Whether this is a host configuration, which behaves very differently. + # In particular, reports are sent, graphs are made, and state is + # stored in the state database. If this is set incorrectly, then you often + # end up in infinite loops, because configurations are used to make things + # that the host configuration needs. + attr_accessor :host_config + # Add classes to our class list. def add_class(*classes) classes.each do |klass| @@ -30,10 +47,50 @@ class Puppet::Node::Configuration < Puppet::PGraph else @resource_table[ref] = resource end + add_vertex!(resource) + end + + # Apply our configuration to the local host. + def apply + Puppet::Util::Storage.load if host_config? + transaction = Puppet::Transaction.new(self) + + transaction.addtimes :config_retrieval => @retrieval_duration + + begin + transaction.evaluate + rescue Puppet::Error => detail + Puppet.err "Could not apply complete configuration: %s" % detail + rescue => detail + if Puppet[:trace] + puts detail.backtrace + end + Puppet.err "Got an uncaught exception of type %s: %s" % [detail.class, detail] + ensure + # Don't try to store state unless we're a host config + # too recursive. + Puppet::Util::Storage.store if host_config? + end + + if block_given? + yield transaction + end + + if host_config and (Puppet[:report] or Puppet[:summarize]) + transaction.send_report + end + + return transaction + ensure + if defined? transaction and transaction + transaction.cleanup + end end - def clear - super + def clear(remove_resources = true) + super() + # We have to do this so that the resources clean themselves up. + @resource_table.values.each { |resource| resource.remove } if remove_resources @resource_table.clear end @@ -113,13 +170,23 @@ class Puppet::Node::Configuration < Puppet::PGraph return result end - def initialize(name) + # Make sure all of our resources are "finished". + def finalize + @resource_table.values.each { |resource| resource.finish } + end + + def initialize(name = nil) super() - @name = name + @name = name if name @extraction_format ||= :transportable @tags = [] @classes = [] @resource_table = {} + + if block_given? + yield(self) + finalize() + end end # Look a resource up by its reference (e.g., File[/etc/passwd]). @@ -127,6 +194,10 @@ class Puppet::Node::Configuration < Puppet::PGraph @resource_table[ref] end + def host_config? + host_config || false + end + # Add a tag. def tag(*names) names.each do |name| -- cgit From b3c8cdb67d9a423a1d14764f1e58f677d7ef8d41 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 15 Sep 2007 22:25:54 -0600 Subject: Configurations now set a "configuration" instance variable in resources that are inside a configuration, so the resources can interact with the configuration to get things like relationships. --- lib/puppet/node/configuration.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index 8cb6ed1f9..9cd23926e 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -47,6 +47,7 @@ class Puppet::Node::Configuration < Puppet::PGraph else @resource_table[ref] = resource end + resource.configuration = self add_vertex!(resource) end -- cgit From 9fa2628a844c75b8f554f283dfece01667f20594 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 17 Sep 2007 15:21:44 -0700 Subject: This is basically another intermediate commit. I feel like I've gone too far down the rabbit hole to turn back now, but the code is clearly getting more centralized around the Configuration class, which is the goal. Things are currently a bit muddy between recursion, dynamic resource generation, transactions, and the configuration, and I don't expect to be able to clear it up much until we rewrite all of the tests for the Transaction class, since that is when we'll actually be setting its behaviour. At this point, Files (which are currently the only resources that generate other resources) are responsible for adding their edges to the relationship graph. This puts them knowing more than I would like about how the relationship graph works, but it'll have to do for now. There are still failing tests, but files seem to work again. Now to go through the rest of the tests and make them work. --- lib/puppet/node/configuration.rb | 164 +++++++++++++++++++++++++++++++++++---- 1 file changed, 147 insertions(+), 17 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index 9cd23926e..c882e0ee1 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -35,20 +35,22 @@ class Puppet::Node::Configuration < Puppet::PGraph tag(*classes) end - # Add a resource to our graph and to our resource table. - def add_resource(resource) - unless resource.respond_to?(:ref) - raise ArgumentError, "Can only add objects that respond to :ref" - end + # Add one or more resources to our graph and to our resource table. + def add_resource(*resources) + resources.each do |resource| + unless resource.respond_to?(:ref) + raise ArgumentError, "Can only add objects that respond to :ref" + end - ref = resource.ref - if @resource_table.include?(ref) - raise ArgumentError, "Resource %s is already defined" % ref - else - @resource_table[ref] = resource + ref = resource.ref + if @resource_table.include?(ref) + raise ArgumentError, "Resource %s is already defined" % ref + else + @resource_table[ref] = resource + end + resource.configuration = self + add_vertex!(resource) end - resource.configuration = self - add_vertex!(resource) end # Apply our configuration to the local host. @@ -58,6 +60,8 @@ class Puppet::Node::Configuration < Puppet::PGraph transaction.addtimes :config_retrieval => @retrieval_duration + @applying = true + begin transaction.evaluate rescue Puppet::Error => detail @@ -83,22 +87,62 @@ class Puppet::Node::Configuration < Puppet::PGraph return transaction ensure + @applying = false + cleanup() if defined? transaction and transaction transaction.cleanup end end + + # Are we in the middle of applying the configuration? + def applying? + @applying + end def clear(remove_resources = true) super() # We have to do this so that the resources clean themselves up. @resource_table.values.each { |resource| resource.remove } if remove_resources @resource_table.clear + + if defined?(@relationship_graph) and @relationship_graph + @relationship_graph.clear(false) + @relationship_graph = nil + end end def classes @classes.dup end + # Create an implicit resource, meaning that it will lose out + # to any explicitly defined resources. This method often returns + # nil. + def create_implicit_resource(type, options) + unless options.include?(:implicit) + options[:implicit] = true + end + if resource = create_resource(type, options) + resource.implicit = true + + return resource + else + return nil + end + end + + # Create a new resource and register it in the configuration. + def create_resource(type, options) + unless klass = Puppet::Type.type(type) + raise ArgumentError, "Unknown resource type %s" % type + end + return unless resource = klass.create(options) + + @transient_resources << resource if applying? + add_resource(resource) + resource + end + # Make sure we support the requested extraction format. def extraction_format=(value) unless respond_to?("extract_to_%s" % value) @@ -174,6 +218,12 @@ class Puppet::Node::Configuration < Puppet::PGraph # Make sure all of our resources are "finished". def finalize @resource_table.values.each { |resource| resource.finish } + + write_graph(:resources) + end + + def host_config? + host_config || false end def initialize(name = nil) @@ -183,20 +233,76 @@ class Puppet::Node::Configuration < Puppet::PGraph @tags = [] @classes = [] @resource_table = {} + @transient_resources = [] + @applying = false if block_given? yield(self) finalize() end end + + # Create a graph of all of the relationships in our configuration. + def relationship_graph + unless defined? @relationship_graph and @relationship_graph + relationships = self.class.new + + # First create the dependency graph + self.vertices.each do |vertex| + relationships.add_resource vertex + vertex.builddepends.each do |edge| + relationships.add_edge!(edge) + end + end + + # Lastly, add in any autorequires + relationships.vertices.each do |vertex| + vertex.autorequire.each do |edge| + unless relationships.edge?(edge) + unless relationships.edge?(edge.target, edge.source) + vertex.debug "Autorequiring %s" % [edge.source] + relationships.add_edge!(edge) + else + vertex.debug "Skipping automatic relationship with %s" % (edge.source == vertex ? edge.target : edge.source) + end + end + end + end + + relationships.write_graph(:relationships) + + # Then splice in the container information + relationships.splice!(self, Puppet::Type::Component) + + relationships.write_graph(:expanded_relationships) + @relationship_graph = relationships + end + @relationship_graph + end - # Look a resource up by its reference (e.g., File[/etc/passwd]). - def resource(ref) - @resource_table[ref] + # Remove the resource from our configuration. Notice that we also call + # 'remove' on the resource, at least until resource classes no longer maintain + # references to the resource instances. + def remove_resource(*resources) + resources.each do |resource| + @resource_table.delete(resource.ref) if @resource_table.include?(resource.ref) + remove_vertex!(resource) if vertex?(resource) + resource.remove + end end - def host_config? - host_config || false + # Look a resource up by its reference (e.g., File[/etc/passwd]). + def resource(type, title = nil) + if title + ref = "%s[%s]" % [type.to_s.capitalize, title] + else + ref = type + end + if resource = @resource_table[ref] + return resource + elsif defined?(@relationship_graph) and @relationship_graph + @relationship_graph.resource(ref) + end end # Add a tag. @@ -217,4 +323,28 @@ class Puppet::Node::Configuration < Puppet::PGraph def tags @tags.dup end + + # Produce the graph files if requested. + def write_graph(name) + # We only want to graph the main host configuration. + return unless host_config? + + return unless Puppet[:graph] + + Puppet.config.use(:graphing) + + file = File.join(Puppet[:graphdir], "%s.dot" % name.to_s) + File.open(file, "w") { |f| + f.puts to_dot("name" => name.to_s.capitalize) + } + end + + private + + def cleanup + unless @transient_resources.empty? + remove_resource(*@transient_resources) + @transient_resources.clear + end + end end -- cgit From 46d69068fa7b2f3448294c5d3da21c69cef73d2f Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 20 Sep 2007 12:57:57 -0500 Subject: An intermediate commit so I can start working on a different branch. The file recursion code actually works for the first time in a painful while, but there are still some quirks and design issues to resolve, particularly around creating implicit resources that then fail (i.e., the behaviour of the create_implicit_resource method in Configuration). --- lib/puppet/node/configuration.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index c882e0ee1..de6e776c5 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -55,13 +55,13 @@ class Puppet::Node::Configuration < Puppet::PGraph # Apply our configuration to the local host. def apply + @applying = true + Puppet::Util::Storage.load if host_config? transaction = Puppet::Transaction.new(self) transaction.addtimes :config_retrieval => @retrieval_duration - @applying = true - begin transaction.evaluate rescue Puppet::Error => detail @@ -122,6 +122,7 @@ class Puppet::Node::Configuration < Puppet::PGraph unless options.include?(:implicit) options[:implicit] = true end + # LAK:FIXME catch exceptions here and return nil when problems if resource = create_resource(type, options) resource.implicit = true @@ -235,6 +236,7 @@ class Puppet::Node::Configuration < Puppet::PGraph @resource_table = {} @transient_resources = [] @applying = false + @relationship_graph = nil if block_given? yield(self) @@ -245,11 +247,12 @@ class Puppet::Node::Configuration < Puppet::PGraph # Create a graph of all of the relationships in our configuration. def relationship_graph unless defined? @relationship_graph and @relationship_graph - relationships = self.class.new + relationships = Puppet::Node::Configuration.new + relationships.host_config = host_config? # First create the dependency graph self.vertices.each do |vertex| - relationships.add_resource vertex + relationships.add_vertex! vertex vertex.builddepends.each do |edge| relationships.add_edge!(edge) end @@ -287,6 +290,7 @@ class Puppet::Node::Configuration < Puppet::PGraph resources.each do |resource| @resource_table.delete(resource.ref) if @resource_table.include?(resource.ref) remove_vertex!(resource) if vertex?(resource) + @relationship_graph.remove_vertex!(resource) if @relationship_graph and @relationship_graph.vertex?(resource) resource.remove end end @@ -345,6 +349,7 @@ class Puppet::Node::Configuration < Puppet::PGraph unless @transient_resources.empty? remove_resource(*@transient_resources) @transient_resources.clear + @relationship_graph = nil end end end -- cgit From 60cd6a73b2b0cb7b26b091d4214c66eb5ed3b0ad Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 22 Sep 2007 12:56:32 -0500 Subject: The structure for handling resource generation is now in place, which means I'm over the hump in developing this branch. I have to fix some design flaws I made in the configurations, particularly that the 'runner' handler needs to be able to specify tags and whether to ignore schedules, but otherwise, I think it's straightforward test- and bug-fixing from here out. --- lib/puppet/node/configuration.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index de6e776c5..747ab9ef6 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -118,11 +118,21 @@ class Puppet::Node::Configuration < Puppet::PGraph # Create an implicit resource, meaning that it will lose out # to any explicitly defined resources. This method often returns # nil. + # The quirk of this method is that it's not possible to create + # an implicit resource before an explicit resource of the same name, + # because all explicit resources are created before any generate() + # methods are called on the individual resources. Thus, this + # method can safely just check if an explicit resource already exists + # and toss this implicit resource if so. def create_implicit_resource(type, options) unless options.include?(:implicit) options[:implicit] = true end - # LAK:FIXME catch exceptions here and return nil when problems + + # This will return nil if an equivalent explicit resource already exists. + # When resource classes no longer retain references to resource instances, + # this will need to be modified to catch that conflict and discard + # implicit resources. if resource = create_resource(type, options) resource.implicit = true -- cgit From 86dde63473d29c45d8698ce4edd53c820a621362 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 22 Sep 2007 14:25:17 -0500 Subject: All tests now pass in this configuration branch, which means it's time to merge it back into the indirection branch. Considering that this work was what drove me to create the indirection branch in the first place, i should now be able to merge both back in the master branch. --- lib/puppet/node/configuration.rb | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index 747ab9ef6..360c25fc8 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -53,13 +53,21 @@ class Puppet::Node::Configuration < Puppet::PGraph end end - # Apply our configuration to the local host. - def apply + # Apply our configuration to the local host. Valid options + # are: + # :tags - set the tags that restrict what resources run + # during the transaction + # :ignoreschedules - tell the transaction to ignore schedules + # when determining the resources to run + def apply(options = {}) @applying = true Puppet::Util::Storage.load if host_config? transaction = Puppet::Transaction.new(self) + transaction.tags = options[:tags] if options[:tags] + transaction.ignoreschedules = true if options[:ignoreschedules] + transaction.addtimes :config_retrieval => @retrieval_duration begin @@ -257,24 +265,29 @@ class Puppet::Node::Configuration < Puppet::PGraph # Create a graph of all of the relationships in our configuration. def relationship_graph unless defined? @relationship_graph and @relationship_graph - relationships = Puppet::Node::Configuration.new - relationships.host_config = host_config? + # It's important that we assign the graph immediately, because + # the debug messages below use the relationships in the + # relationship graph to determine the path to the resources + # spitting out the messages. If this is not set, + # then we get into an infinite loop. + @relationship_graph = Puppet::Node::Configuration.new + @relationship_graph.host_config = host_config? # First create the dependency graph self.vertices.each do |vertex| - relationships.add_vertex! vertex + @relationship_graph.add_vertex! vertex vertex.builddepends.each do |edge| - relationships.add_edge!(edge) + @relationship_graph.add_edge!(edge) end end # Lastly, add in any autorequires - relationships.vertices.each do |vertex| + @relationship_graph.vertices.each do |vertex| vertex.autorequire.each do |edge| - unless relationships.edge?(edge) - unless relationships.edge?(edge.target, edge.source) + unless @relationship_graph.edge?(edge) + unless @relationship_graph.edge?(edge.target, edge.source) vertex.debug "Autorequiring %s" % [edge.source] - relationships.add_edge!(edge) + @relationship_graph.add_edge!(edge) else vertex.debug "Skipping automatic relationship with %s" % (edge.source == vertex ? edge.target : edge.source) end @@ -282,13 +295,12 @@ class Puppet::Node::Configuration < Puppet::PGraph end end - relationships.write_graph(:relationships) + @relationship_graph.write_graph(:relationships) # Then splice in the container information - relationships.splice!(self, Puppet::Type::Component) + @relationship_graph.splice!(self, Puppet::Type::Component) - relationships.write_graph(:expanded_relationships) - @relationship_graph = relationships + @relationship_graph.write_graph(:expanded_relationships) end @relationship_graph end -- cgit