Check if class has been included?

[Ryan Bowlby]

Hi All,

I recently added the puppet-concat module in order to implement the
example motd use case. Now our motd includes a list of modules being
used on the server, which is awesome.

All the modules define an motd::register so they expect that the motd
module was included. When a node does not include the module all those
dependent modules fail. I would like to have each module first check
to see if the motd class has been included.

Current example:

class pulp (..
.....

# list this module/class in motd
os::motd::register {"pulp": }
}

Future example:

class pulp (..
.....
if defined(os::motd) {
# list this module/class in motd
os::motd::register {"pulp": }
}
}

The question is whether this is the right way to go about it? Does the
define wait for all class declarations before checking (does order
matter)?

Thanks,
Ryan

[jcbollinger]

On May 2, 3:14 pm, Ryan Bowlby <rbowlb...@gmail.com> wrote:
> Hi All,
>
> I recently added the puppet-concat module in order to implement the
> example motd use case. Now our motd includes a list of modules being
> used on the server, which is awesome.
>
> All the modules define an motd::register so they expect that the motd
> module was included. When a node does not include the module all those
> dependent modules fail. I would like to have each module first check
> to see if the motd class has been included.
>
> Current example:
>
> class pulp (..
> .....
>
>   # list this module/class in motd
>   os::motd::register {"pulp": }
>
> }

> Future example:
>
> class pulp (..
>     .....
>   if defined(os::motd) {
>     # list this module/class in motd
>     os::motd::register {"pulp": }
>   }
>
> }

You are confusing me with your terminology. An entity referred to in
a Puppet manifest as 'os::motd' is either a class or a defined type in
module 'os', not a module itself. You can "include" or "declare" a
class, but not a module. You can "instantiate" or sometimes "declare
[an instance of]" a defined type, or some people "call" one. Your
manifests cannot directly refer to a module at all, only to its
contents.

I'm not familiar with the motd module, but os::motd::register must be
a defined type. It should be the responsibility of the definition to
include any classes it depends on. It can and should do so unless one
or more of those classes is parameterized, so that your own classes
that use it Just Work. If the definition needs a parameterized class
to have been declared, however, then it thereby leaves you hanging.

> The question is whether this is the right way to go about it? Does the
> define wait for all class declarations before checking (does order
> matter)?

No and no (and yes). Parse-order dependency is a prime reason why
such useage of the 'defined' function is poor practice. There has
even been some discussion of deprecating it.

If 'os::motd' is an unparameterized class, then your best options are:
1) put "include 'os::motd'" at the begining of the body of
os::motd::register, OR
2) put "include 'os::motd'" at the begining of every class that
instantiates os::motd::register, OR
3) put "include 'os::motd'" at the begining of every node definition
(or make your ENC output that class first in its class list)

If 'os::motd' is a parameterized class then your best bet is to adjust
your node declarations / ENC so that os::motd is declared first for
every node.


John

[Ryan Bowlby]

My apologies on terminology and thank you for the list of best
options. My problem is this, I want to have this os::motd::register
defined type exist in the main class of every module. Yet I don't want
to force a host to have to include os::motd. I want it as an option.
The fact that I don't want to include os::motd on each node means all
three options don't work.

I'm starting to think using the os::motd::register defined type in
every module is not the way to go. Perhaps using exported resources is
a more flexible method of achieving this?

Thanks,
Ryan

[jcbollinger]

On May 3, 2:53 pm, Ryan Bowlby <rbowlb...@gmail.com> wrote:
> My apologies on terminology and thank you for the list of best
> options. My problem is this, I want to have this os::motd::register
> defined type exist in the main class of every module. Yet I don't want
> to force a host to have to include os::motd. I want it as an option.
> The fact that I don't want to include os::motd on each node means all
> three options don't work.
>
> I'm starting to think using the os::motd::register defined type in
> every module is not the way to go. Perhaps using exported resources is
> a more flexible method of achieving this?

Hmm. I guess I misunderstood your objective. It is still true that
'defined' is not a good approach, however, and also that
os::motd::register is a bit rude to not take care of declaring its
dependencies itself.

It might work to declare all your os::motd::register instances
virtually, right where they now are, and then collect them at the end
of each node definition. I suspect, though, that you would end up
with the same problem you already have.

Generally, I recommend replacing "is foo defined?" conditions with "is
foo *supposed to be* defined?". The latter can be evaluated based on
global or class variables, or (better) external data.


John

[Dan Carley]

On 3 May 2012 23:32, jcbollinger <John.Bo...@stjude.org> wrote:

Hmm.  I guess I misunderstood your objective.  It is still true that
'defined' is not a good approach, however, and also that
os::motd::register is a bit rude to not take care of declaring its
dependencies itself.

It might work to declare all your os::motd::register instances
virtually, right where they now are, and then collect them at the end
of each node definition.  I suspect, though, that you would end up
with the same problem you already have.

Generally, I recommend replacing "is foo defined?" conditions with "is
foo *supposed to be* defined?".  The latter can be evaluated based on
global or class variables, or (better) external data.

It's a fine use case for virtual resources. Each class would declare the virtual:

class pulp {

    @os::motd::register { $name: }

    ...

}

Then os::motd would, when included in the catalog, collect/realize everything available:

class os::motd {

    Os::Motd::Register <| |>

    ...

}

[jcbollinger]

On May 4, 1:54 am, Dan Carley <dan.car...@gmail.com> wrote:

Yes, that's similar to what I suggested. I suspect, however, that it
will not solve the OP's problem of wanting to avoid declaring class
os::motd on some nodes, given that os::motd::register depends on that
class having been declared. Under those circumstances, I expect that
even a virtual declaration of os::motd::register will fail if os::motd
is not (already) declared, regardless of whether the virtual resources
are ever collected.

Also, I strongly suspect that collecting os::motd::register instances
in os::motd would not work, because no such instances can have been
declared at the time that the collection declaration is parsed.

That all does suggest another possible approach, though: the situation
would be much improved if os::motd::register's dependency on os::motd
could be removed. Without seeing the code I can only speculate, but
it may be that os::motd could be refactored to make that possible.
For instance, if os::motd::register's need is merely for class
variables from os::motd, then perhaps those can be extracted into a
separate os::motd::params class that os::motd::register would use
instead.


John

[Ryan Bowlby]

Dead on John. I ended up going simple and making os::motd::register a
simple define that creates a file in /var/lib/puppet. If os::motd
happens to be declared then it just concatenates those individual
files into one that is further added to motd using puppet-concat.
Worked like a gem, K.I.S.S.

Thanks Guys.