Dependency cycle using tidy with puppet 3

[Chris Lee]

Hi all,

We are busy migrating out puppet 2.7 code to puppet 3 and have run into a problem where we are getting dependency cycles when using tidy.
This mostly happens with stages (and yes, I completely understand why we should avoid them).

As an example we have a defined class that we use to create our cron jobs, and then a tidy to clean up anything that isn't defined.

class crond::cleanup {
    tidy {
        "/etc/cron.d":
        age => 0, recurse => true,
        matches => "*.puppet.cron"
    }
}

define crond::job($jobs,$comment,$mail="root") {
    include crond::cleanup
    file {
        "/etc/cron.d/${name}.puppet.cron":
        owner=>root,group=>root,
        content=>template("crond/job.erb")
    }
}

and we get a dependency as follows:

(File[/etc/cron.d/puppetcheck.puppet.cron] => Tidy[/etc/cron.d] => Class[Crond::Cleanup] => Stage[main] => Stage[apps] => Stage[apps] => Stage[post] => Class[Puppet::Service] => Crond::Job[puppetcheck] => File[/etc/cron.d/puppetcheck.puppet.cron])

Does anyone know of a workaround?

Thanks
Chris

[jcbollinger]

Yes.  Stop using run stages.

That's somewhat tongue-in-cheek, but it is certainly the case that your use of run stages is a significant factor in this problem, and that use of run stages is prone to this kind of problem in general.  That doesn't necessarily mean you shouldn't use run stages at all, but I usually discourage it.  There is nothing you can express via stages that you cannot also express (albeit more verbosely) via ordinary resource relationships, and although stages are not inherently bad, but they are trickier to use and more dangerous than their documentation might lead you to believe.

If you feel compelled to use stages, then I recommend that you at least tread lightly: as much as possible should go in Stage[main], and there should be no more than three stages altogether (up to one preceding main and up to one following it).

As for the specifics of your case, not all of the relationships in that cycle are coming from the code you presented, and the reasons for some of them are unclear to me.  In addition to the run stage assignments, it is unclear

• whether Class[Puppet::Service] actually needs the specified relationship with Crond::Job[puppetcheck];
• where the relationship File[/etc/cron.d/puppetcheck.puppet.cron] -> Tidy[/etc/cron.d] is coming from; it could in principle be an autorequire, but no such autorequire is documented in the type reference, and neither the code for the File type nor the code for the Tidy type appears to have such an autorequire
  
• why Crond::Job[puppetcheck] needs to be applied in Stage[post], or after Class[Puppet::Service] specifically

If any of those is wrong or unneeded, then getting rid of it will probably help.

I cannot explain why this issue did not appear under Puppet 2.7.  Is it possible that a change applied as part of your migration process is contributing?


John

[Chris Lee]

We try not to use stages at all, unless absolutely necessary and we would love to avoid them if we could.

Our puppet code is used on around ~3000 machines in a heterogeneous environment of both various hardwares, os boot systems and applications, so at times its an necessary evil

It is my understanding that tidy now now creates a dependency on the file, but since this is run in the beginning of the catalog, the tidy on the files which are created in post creates a loop. For all the other times we use tidy I was able to get around this by splitting the tidy from the classes and simple including them, this is the last one I cant seem to get around. I did find another post about this and puppet 3, but no solution https://ask.puppetlabs.com/question/13431/how-do-i-work-around-puppet-3-semantics-change/

Unfortunately we have way to many variables, and while direct requires might be possible it would mean an entire rewrite of ~ 4 years of code, and many times we have run into conflicts anyway which finally forced us into this.
Mostly the idea is that our (systems admin) configuration is run in main, specific user requirements are configured in apps, and only after all of that, do we declare the machine usable (remove nologin, etc). Finally, we then ensure that things like puppet are running, where prior to that on a broken system it wouldn't be required anyway,  and then apply certain safety checks, as with this cron which does a daily check to ensure puppet hasn't been disabled and mails a report which we can compare with puppet dashboards output.

With out the stages this would literally require hundreds of "if defined" statements due to all the various configurations available.

The "File[/etc/cron.d/puppetcheck.

puppet.cron] -> Tidy[/etc/cron.d]" relationship is from the new design  of tidy as explained in the post I linked above. The code I posted is simple called in a class that is set to stage post?

[Chris Lee]

hmmm, I've created a new class trying to replicated the exact same thing with different files, and I cant seem to..
Will have to do a lot more digging to see where this is coming from

[Chris Lee]

The first error was being thrown out and I didn't see the second one. Its easy to replicate using puppet 3.3.2

node test {
    class {"crond::test":stage=>post,}


}

define crond::job($jobs,$comment,$mail="root") {
    include crond::cleanup
    file {
        "/etc/cron.d/${name}.puppet.cron":
        owner=>root,group=>root,
        content=>template("crond/job.erb")
    }
}
class crond::cleanup {
    tidy {
        "/etc/cron.d":
        age => 0, recurse => true,


        matches => "*.puppet.cron",
    }
    service {
    "crond": ensure=>running,enable=>true
    }
}
class crond::test  {
    crond::job {"tidytest":
  mail=>"root",
  comment=>"testing tidy",
  jobs=>"6 6 * * * root nice /bin/true",
    }
}

[jcbollinger]

It's not clear to me why the autorequire was added to the Tidy resource type, and in fact I am having trouble even locating it in the Puppet code.  I suppose the objective is probably to deal with recursively tidying directories that are wholly or partially managed via recursive File resources, as such Files may also add resources to the catalog, and you (presumably) want the Tidy to defer to the File in such cases.

Ideally, you would move the Tidy (and all others) someplace that would cause it to be applied in the last run stage.  If it is applied any time sooner then you risk a cycle.  For instance, perhaps you could add a declaration of class crond::cleanup with an explicit assignment to Stage['post'], some place where it will be evaluated before any of the cron::job declarations.  Or if you're using Hiera, you may be able to just put

crond::cleanup::stage: "Stage['post']"

in your data.

Alternatively, you should be able to overcome any autorequire by declaring a contradictory explicit relationship.  For example, you could try this:

define crond::job($jobs,$comment,$mail="root") {
    include crond::cleanup
    file {
        "/etc/cron.d/${name}.puppet.cron":
        owner=>root,group=>root,
        content=>template("crond/job.erb")

    } <- Tidy['/etc/cron.d']
}


In that case, however, do check carefully to be sure that the Tidy is not removing any files you want to retain.


John



[Christopher Wood]

On Thu, Oct 02, 2014 at 01:39:09AM -0700, Chris Lee wrote:
> We try not to use stages at all, unless absolutely necessary and we would
> love to avoid them if we could.

Have you tried collectors+tagging and chaining? We've had good results with those.

https://docs.puppetlabs.com/puppet/latest/reference/lang_collectors.html
https://docs.puppetlabs.com/puppet/latest/reference/lang_relationships.html#chaining-arrows
https://docs.puppetlabs.com/puppet/latest/reference/lang_tags.html

When dependency cycles come up it's mostly because we're trying to manage related resources in separate modules and it's harder to keep track. The infamous example is the loop created by managing a service in a servicename module, but then having a security module fiddling with service-related items. (Rather than having the security requirements built right into the original servicename module.)

> Our puppet code is used on around ~3000 machines in a heterogeneous
> environment of both various hardwares, os boot systems and applications,

> so at times its an unnecessary evil

>
> It is my understanding that tidy now now creates a dependency on the file,

> but since this is run in the beginning, the tidy on the files which are

> created in post creates a loop. For all the other times we use tidy I was

> able to get around by splitting the tidy from the classes and simple

> including them, this is the last one I cant seem to get around. I did find
> another post about this and puppet 3, but no solution
> https://ask.puppetlabs.com/question/13431/how-do-i-work-around-puppet-3-semantics-change/
>
> Unfortunately we have way to many variables, and while direct requires
> might be possible it would mean an entire rewrite of ~ 4 years of code,
> and many times we have run into conflicts anyway which finally forced us
> into this.
> Mostly the idea is that our (systems admin) configuration is run in main,
> specific user requirements are configured in apps, and only after all of

> that, do we declare the machine usable. Finally we ensure that things like

> puppet are running, where prior to that on a broken system it wouldn't be
> required anyway,  and then apply certain safety checks, as with this cron
> which does a daily check to ensure puppet hasn't been disabled and mails a
> report which we can compare with puppet dashboards output.
>
> With out the stages this would literally require hundreds of "if defined"
> statements due to all the various configurations available.
>

> The "File[/etc/cron.d/puppetcheck.puppet.cron] -> Tidy[/etc/cron.d]"

> relationship is from the new design  of tidy as explained in the post I
> linked above. The code I posted is simple called in a class that is set to

> stage post.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [1]puppet-users...@googlegroups.com.
> To view this discussion on the web visit
> [2]https://groups.google.com/d/msgid/puppet-users/8972ab55-6b62-499c-bd58-eb82d5593192%40googlegroups.com.
> For more options, visit [3]https://groups.google.com/d/optout.
>
> References
>
> Visible links
> 1. mailto:puppet-users...@googlegroups.com
> 2. https://groups.google.com/d/msgid/puppet-users/8972ab55-6b62-499c-bd58-eb82d5593192%40googlegroups.com?utm_medium=email&utm_source=footer
> 3. https://groups.google.com/d/optout

[Chris Lee]

Alternatively, you should be able to overcome any autorequire by declaring a contradictory explicit relationship.  For example, you could try this:

define crond::job($jobs,$comment,$mail="root") {
    include crond::cleanup
    file {
        "/etc/cron.d/${name}.puppet.cron":
        owner=>root,group=>root,
        content=>template("crond/job.erb")
    } <- Tidy['/etc/cron.d']
}


That worked perfectly, and it doesn't appear to be removing any of the other files.
The next part of our migration will obviously be moving what we can from extdata to hiera, etc and then a massive code clean up to conform with all the "new" best practises. Getting around this now though makes it much easier to get the system working with Puppet 3.0+ and start the big project of cleaning.

Thank you so much

Chris