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 #3382

Class require chain thru class with no resources

Added by Alan Harder about 6 years ago. Updated over 5 years ago.

Status:ClosedStart date:03/17/2010
Priority:NormalDue date:
Assignee:Markus Roberts% Done:

0%

Category:functions
Target version:2.6.1
Affected Puppet version:2.6.0 Branch:git@github.com:MarkusQ/puppet.git ticket/master/3382
Keywords:require

We've Moved!

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


Description

This may be working as designed, but wanted to file an issue to make sure. Consider this test case:

class foo {
  notify { 'foo': }
  class bar {
    require foo
#    notify { 'bar': }
  }
}
class baz {
  require foo::bar
  notify { 'baz': }
}

node default {
  include baz
}

When notify ‘bar’ is uncommented, this works as expected.. the notify messages always appear in order: foo, bar, baz. However, with notify ‘bar’ commented the order is sometimes foo,baz and sometimes baz,foo. It seems that some resource must be in the foo::bar class to enforce that foo is evaluated before baz. Is this expected that a class level “require” chain only works when each link in the chain has at least one resource? I use a setup like this sometimes where a subclass sets some variables and then requires the outer class, so it has no resources of its own. If you have another recommended way to do this, let me know :–) I guess I could do “require foo::bar, foo” but that’s less intuitive usage. Thanks.


Related issues

Related to Puppet - Bug #3186: require not working as expected Closed 02/18/2010
Related to Puppet - Bug #3715: require class doesn't do what I expect Duplicate 05/04/2010
Related to Puppet - Bug #8040: Classes should be able to contain other classes to provid... Closed 06/22/2011

History

#1 Updated by Alan Harder about 6 years ago

Additional test case for same issue.. change baz to:

class baz {
  include foo::bar
  notify { 'baz': require => Class['foo::bar'] }
}

Effectively the same thing, just wanted to highlight that it occurs with require parameter as well as the function.

#2 Updated by Brice Figureau about 6 years ago

Alan Harder wrote:

Additional test case for same issue.. change baz to: […] Effectively the same thing, just wanted to highlight that it occurs with require parameter as well as the function.

I actually thinks this is a duplicate of #3186, and that you found the real underlying issue. The require function actually resets the “require” parameter. Can you test the patch in #3186 and report success/failure for this one too?

#3 Updated by Alan Harder about 6 years ago

Tried it out, but still seeing these resources in arbitrary order with the #3186 patch applied to 0.25.4. With or without the patch the —debug output shows both “//baz/require: requires Class[foo::bar]” and “//foo::bar/require: requires Class[foo]”, but the requirement is not enforced for empty foo::bar class.

#4 Updated by Markus Roberts about 6 years ago

  • Status changed from Unreviewed to Investigating
  • Assignee set to Brice Figureau

#5 Updated by Brice Figureau about 6 years ago

Alan Harder wrote:

Tried it out, but still seeing these resources in arbitrary order with the #3186 patch applied to 0.25.4. With or without the patch the —debug output shows both “//baz/require: requires Class[foo::bar]” and “//foo::bar/require: requires Class[foo]”, but the requirement is not enforced for empty foo::bar class.

I had a look to the produced graph and there is indeed a link between foo::bar and foo. I didn’t find the issue, though.

Note that the following manifest doesn’t exhibit the issue:

class foo {
  notify { 'foo': }
}

class foo::bar {
        require ::foo
}

class baz {
  require foo::bar
  notify { 'baz': }
}

node default {
  include baz
}

#6 Updated by Alan Harder about 6 years ago

Is “::foo” correct? With the code above I got:

Could not find dependency Class[::foo] for Class[foo::bar]

I removed the “::” and I see the notify resources run in arbitrary order, just like my original test case.

Solaris 10, ruby 1.8.7, puppet 0.25.4

#7 Updated by Alan Harder about 6 years ago

Any progress on this one? Would be great to see a fix in 0.25.5 ;–)

#8 Updated by Brice Figureau about 6 years ago

Alan Harder wrote:

Any progress on this one? Would be great to see a fix in 0.25.5 ;–)

Sorry, I couldn’t find the issue. It looks like we’re setting the correct “link” between the classes, except that it isn’t enforced when applying the catalog.

#9 Updated by Brice Figureau almost 6 years ago

  • Status changed from Investigating to Accepted

Brice Figureau wrote:

Alan Harder wrote:

Any progress on this one? Would be great to see a fix in 0.25.5 ;–)

Sorry, I couldn’t find the issue. It looks like we’re setting the correct “link” between the classes, except that it isn’t enforced when applying the catalog.

This simpler manifest triggers the problem:

class foo {
  notify { 'foo': }
}

class bar {
  require foo
# notify { "bar" : }
}

class baz {
  require bar
  notify { "baz": }
}

node default {
  include baz
}

OK, after this loooong time, I think I’ve found the issue: During the application of the catalog we call relationship_graph.splice!. This is so that we remove all the containers (ie classes) from the graph and leave only resources. This splice! method removes class vertices and adds edges to the children of the removed node.

Unfortunately for us, the ‘baz’ class has no children (ie no resources), thus the require from baz to bar is not kept, and it can happen bar is applied before foo.

I’m unsure about the fix, except that we might have to walk the graph during splice! until we find a container with some childs.

#10 Updated by Alan Harder almost 6 years ago

  • Affected Puppet version changed from 0.25.4 to 2.6.0rc4

Glad you tracked it down! Hope it can be fixed someday, so we can stop using our “empty_class” define to add a notify resource in such cases. I’m updating the affected version above to 2.6.0rc4.

#11 Updated by Brice Figureau almost 6 years ago

Alan Harder wrote:

Glad you tracked it down! Hope it can be fixed someday, so we can stop using our “empty_class” define to add a notify resource in such cases. I’m updating the affected version above to 2.6.0rc4.

I discussed about this with Luke at Puppetcamp, and he said that 2.6 should have fixed it. Since you changed the affected version, I conclude it is not fixed in 2.6. Is that correct?

#12 Updated by Alan Harder almost 6 years ago

Right.. I’m testing with your simplified case from note-9 above, and seeing “foo” / “baz” in arbitrary order.

#13 Updated by James Turnbull almost 6 years ago

  • Target version set to 2.6.1

#14 Updated by Alan Harder almost 6 years ago

  • Affected Puppet version changed from 2.6.0rc4 to 2.6.0

#15 Updated by Markus Roberts almost 6 years ago

  • Assignee changed from Brice Figureau to Markus Roberts

#16 Updated by Markus Roberts almost 6 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to git@github.com:MarkusQ/puppet.git ticket/master/3382

#17 Updated by Markus Roberts almost 6 years ago

Thanks to Brice for the cogent diagnosis.

#18 Updated by Alan Harder almost 6 years ago

Looks like fix is working for me.. ran 50 times, all had foo/baz in right order. Before I would usually see one out of order within 5 or so tries.

#19 Updated by Nick Lewis almost 6 years ago

  • Status changed from In Topic Branch Pending Review to Closed

Pushed in commit:59a23d69c20a8ab1f7dc9ff1d109305f9027641c in 2.6.x

Also available in: Atom PDF