FAQ

On Tuesday, June 4, 2013 6:01:58 PM UTC-5, Tom Lanyon wrote:
Hi John,

Thanks for the reply.
On 05/06/2013, at 12:33 AM, jcbollinger wrote:
On Tuesday, June 4, 2013 1:22:08 AM UTC-5, Tom Lanyon wrote:

Unfortunately, this results in a dependency cycle. It appears that
putting the Package[$title] resource reference in defined() actually
invokes an implicit dependency between my cleanup helper resource in the
cleanup stage and the original Package resource in the main stage.
Augeas[redacted] => Service[iptables] => Class[Iptables] =>
Stage[main] => Stage[cleanup] => Class[App::Package::Cleaner] =>
App::Package::Cleaner::Check_and_remove[package-434] =>
Package[package-434] => Exec[app-graceful-restart] => Class[App] =>
Stage[main]
Does it do that when Package[package-434] is already declared
elsewhere, or only when it is not?

Sorry, I should have been clearer that this occurs when
Package[package-434] IS declared elsewhere.
"!defined(Package[package-434])" therefore is false, so just by referencing
the existing declaration within the defined() call it seems to incite an
implicit dependency.
If that's really what's happening then you should be able to create a
simple test case that demonstrates it. That would be a worthy subject for
a bug report.


Is this implicit dependency expected behaviour or am I doing something
Bad(tm)?
Both.

Supposing that the target package is not declared elsewhere (so that the
!defined() condition is true) the definition will declare the package
itself to ensure it absent, and in that case you would expect a
relationship between the defined-type instance and the resource declared by
it. If elsewhere you have specific references to that package, applicable
resource parameter defaults, or collectors that will match that package,
then you can get relationships with it that are not evident from the
defined type body.
On the other hand, defined() is evil. Do not use it. Ever.
I had this discussion with someone on #puppet IRC earlier and they ended
up with "Oh, in your case, defined() is probably actually what you want."

No. defined() is never what you want. It may at times seem expedient, but
it's bad news every time.


I usually attribute its malignancy to the parse-order dependency it
inherently creates -- which is indeed a serious problem -- but in this case
I think trying to use it to approach your problem it has also obfuscated
your manifests enough to confuse you about the scope and nature of some of
your other declarations.
Instead of using defined(), you can apply logic farther upstream to make
the correct declaration in the first (one) place or to apply resource
parameter overrides to the correct resources. Alternatively, you can
simply determine by other means what packages need to be ensured absent,
such as by filtering a list of possible packages against a list of packages
that are supposed to be installed. Some of those options may still
susceptible to the problem you observed, however, if relevant relationships
spring from declarations elsewhere, as I described they may do.

I've tried this other ways, but here's an example of why farther upstream
logic doesn't work:

define myapp ($requested_package){

package { $requested_package:
ensure => present
}

define package_cleanup {
$installed_package = $title

if $installed_package != $requested_package {
package { $installed_package:
ensure => purged
}
}
}

# assuming a facter fact named 'installed_packages'
package_cleanup { split($::installed_packages, ','): }
}

I don't much like that general approach in the first place on account of
the $requested_package parameter. That you encounter difficulty when you
try something a bit dodgy should not be surprising.


# now in the case of:
# $::installed_packages = 'one,two,three'
# with:
myapp { 'oneA': requested_package => 'one' }
myapp { 'twoA': requested_package => 'two' }
myapp { 'oneB': requested_package => 'one' }

# we'd end up with package conflicts because
# Myapp[oneA] will define Package[one] (present)
# then define Package[two], Package[three] (absent),
# and Myapp[twoA] will try and define Package[two]
# (present) and fail with a non-uniqueness error.

I don't see how this is doable without defined() or some other check of
the catalog to see what packages are "needed" elsewhere. Do you have any
suggestions?
In fact, despite my dissatisfaction with your approach, you can indeed do
this without defined(), and without even disrupting your current structure
very much. Here's one way I think would work:

# This class ensures all known app packages are
# by default purged
class app::packages {
     $apps = split($::app_packages, ',')
     package { $apps:
         ensure => 'purged'
     }
}

# Overrides the requested package to be declared
# present instead of purged.
define app::myapp($requested_package) {
     include 'app::packages'
     Package<| title == $requested_package |> {
         ensure => 'present'
     }
}

# no separate package_cleanup required

For the record, however, no order-of-application relationship should be
implied by the reference itself. Therefore, when the referenced Package is
declared elsewhere (so that the !defined() condition is false), there
should be a relationship between
App::Package::Cleaner::Check_and_remove[foo] and Package[foo] only if that
relationship is declared somewhere else.

I'd hoped that using a Stage to run after everything else would sort this
all out, but I'm now not sure this is correct and also see your earlier
post about never using stages [with this and "defined() is evil", which
pieces of Puppet /can/ we use safely?].
It's not uncommon for stages to cause cycles, but they will not resolve
cycles that are already present without them, because stages only add
relationships to those that are otherwise there. They are not evil in the
same way that defined() is, however. Although I disfavor stages myself and
generally do not recommend them to folks, I do not consider a manifest set
that successfully uses them to be inherently flawed (unlike one that relies
on defined()).

Puppet is broad in scope and rich in features, so it should not be
surprising that a few of its features are of questionable value. Pretty
much any system of comparable size has such. To Puppetlabs's credit, they
do deprecate and eventually remove features that they conclude are obsolete
or irretrievably broken. They have floated the idea of doing this with
defined(), in fact, and that may eventually happen.

Other things I recommend you avoid:

    - most uses of 'import'. Bringing node definitions into site.pp is
    about the only appropriate use of 'import' that comes to mind. Most other
    potential uses are better served by the autoloader.
    - Lexical nesting of classes and/or defined types. This is partly for
    cooperating with the autoloader and partly just a matter of style, but it
    is widely considered a best practice to put each class and definition in
    its own file.
    - parameterized-style class declarations. (That is, declarations of the
    form "class { 'foo': param1 => bar }".) This one is controversial. I
    have written on it frequently in the past, so I will omit the recap unless
    you insist. Parameterized classes themselves are ok in Puppet 3, provided
    that you rely on hiera data bindings or maybe an ENC when you need to
    customize their parameters.

John

"Parameterized classes: the best thing since decaffeinated coffee"

--
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 puppet-users+unsubscribe@googlegroups.com.
To post to this group, send email to puppet-users@googlegroups.com.
Visit this group at http://groups.google.com/group/puppet-users?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

Discussion Posts

Previous

Follow ups

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 4 of 9 | next ›
Discussion Overview
grouppuppet-users @
categoriespuppet
postedJun 4, '13 at 6:23a
activeJun 14, '13 at 2:28p
posts9
users2
websitepuppetlabs.com

2 users in discussion

Jcbollinger: 5 posts Tom Lanyon: 4 posts

People

Translate

site design / logo © 2022 Grokbase