The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #7709

expanded_relationship.dot should not have both containment and relationship edges

Added by Anonymous almost 5 years ago. Updated over 2 years ago.

Status:AcceptedStart date:05/27/2011
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-
Affected Puppet version: Branch:
Keywords:cycle graph containment edge relationship frontier

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This ticket is now tracked at: https://tickets.puppetlabs.com/browse/PUP-914


Description

Overview

Running against master (2.7.0rc3-135-g520cbc0) I notice that using —graph has obvious cycles in the graph when the catalog itself does not actually have cycles and applies cleanly.

Please see the attached screenshot.

Talking with Daniel, the suspicion is that the graphing output is not drawing a distinction between containment edges and relationship edges and is displaying both in the graph output.

Desired Behavior

The graph output should only display relationship edges between resources as it does with Puppet 2.6 and earlier.

Actual Behavior

Visible cycles are displayed in the graph output when there are no relationship edge cycles.

Steps to reproduce

A simple catalog should show some resources looping against themselves. I can provide example manifests upon request.

Containment_Edges_in_Relatonship_Edges_in_2.7.png (2.16 MB) Anonymous, 05/27/2011 04:11 pm


Related issues

Related to Puppet - Feature #11253: Provide useful console/terminal views of the graph when r... Accepted 12/07/2011
Related to Puppet - Bug #8040: Classes should be able to contain other classes to provid... Closed 06/22/2011
Duplicated by Puppet - Bug #11265: Dependency cycle graph unclear Duplicate 12/07/2011

History

#2 Updated by Anonymous almost 5 years ago

Image

Visible Cycles

#3 Updated by Markus Roberts almost 5 years ago

I believe the problem description is a little off here; not factually incorrect, just framed in a way that is less actionable than it could be. The difference is subtle and much easier to explain at a whiteboard, but I’ll give it a shot here.

The behavior we’re seeing is the consequence of two changes. One part was intentional and the other is an unintended side effect of a change which cleaned up log messages by removing duplication/confusing information. This second change had the unintended consequence of giving pairs of node in the graph the same name, causing them to by “unified” into one node when then graph was produced.

The change in question was:

--- a/lib/puppet/type/whit.rb
+++ b/lib/puppet/type/whit.rb
@@ -5,8 +5,14 @@ Puppet::Type.newtype(:whit) do
desc "The name of the whit, because it must have one."
end
+
+  # Hide the fact that we're a whit from logs
def to_s
-    "(#{name})"
+    name.sub(/^completed_|^admissible_/, "")
+  end
+
+  def path
+    to_s
end
def refresh

from commit:5c245418115396df655f86065d2d1d3af62e39ee and the proper fix is probably to move the name-simplification somewhere where it will only affect the log messages and not other things (such as the graphs).

As for the first (intentional) part, the key thing to realize is that neither the old or the new graph contains containment edges per se, but both contain additional dependency edges induced by the containment edges (this is the subtlety I mentioned above). In the old code would take a two containers full of resources with a dependency between the containers:

( x x x x x ) --> ( o o o o o o )

and induce a K(n,m) graph of dependencies between them:

          x x x x x
      // a rats nest of arrows
      // connecting each x
      // to each o (30 in all)
         o o o o o o 

but now we introduce sentinels (which act like cable ties) to reduce this complexity:

                  start_A
            // one edge from start_A to each x
                 x x x x x
            // one edge from each x to A_done
                  A_done
                     |
                     V
                  start_B
            // one edge from start_B to each o
                 o o o o o o
            // one edge from each o to B_done
                  B_done

in this case it reduces 30 edges to 22, but in large graphs the savings is considerably greater.

The code above, however, apparently causes both the opening and closing sentinels to be given the same name (that of the container that they were induced by) which makes the logs prettier but visually “joins” the entry and exit nodes in a way that appears to represent loops.

#4 Updated by Ben Hughes almost 5 years ago

  • Status changed from Unreviewed to Accepted

#5 Updated by Nigel Kersten almost 5 years ago

  • Priority changed from Normal to High
  • Target version set to 2.7.x

#6 Updated by Nigel Kersten over 4 years ago

  • Assignee set to Anonymous
  • Priority changed from High to Urgent
  • Affected Puppet version deleted (2.7.0rc1)

Daniel, I’m assigning over to you, and I consider this to be an urgent fix. It’s making it difficult to actually use graphs. :(

#7 Updated by Anonymous over 4 years ago

Related Info

Just as an FYI, with the request in #8844 to document the Anchor Pattern I added information about this to http://projects.puppetlabs.com/projects/puppet/wiki/Anchor_Pattern.

I mention this here because the containment mentioned in #8040 plays heavily into visualizing the graph which relates to this ticket. Perhaps resolving this issue will also resolve #8040 but I’m not sure.

-Jeff

#8 Updated by Anonymous over 4 years ago

This is still a problem as of 2.7.10rc1

#9 Updated by eric sorenson almost 4 years ago

  • Priority changed from Urgent to Normal

#10 Updated by Anonymous over 3 years ago

  • Target version deleted (2.7.x)

#11 Updated by Anonymous almost 3 years ago

  • Assignee deleted (Anonymous)

#12 Updated by Reid Vandewiele over 2 years ago

A fix has been submitted for #8040 but I don’t see that it impacts this issue at all. Nigel’s statement “it’s making it difficult to actually use graphs” still holds.

Also available in: Atom PDF