Loading...

puppet-users@googlegroups.com

[Prev] Thread [Next]  |  [Prev] Date [Next]

[Puppet Users] Re: define function problem jcbollinger Fri Apr 06 08:00:52 2012


On Apr 6, 1:07 am, sam <[EMAIL PROTECTED]> wrote:
> Hi All,
>
> Hope you people are doing good.
>
> I have a manifest file :
>
> lass profile {


That's supposed to be "class" of course.  I guess it's a cut & paste
error.

Anyway, this class (and all of your others) really ought to be in a
module.


> # setup profile parms. We dont handle non Ubuntu OS yet
>     if ("$operatingsystem" == "Ubuntu") {
>         file {
>             "/etc/profile.d":
>                 ensure  => directory,
>                 purge   => true,
>                 force   => true,
>                 recurse => true,
>                 owner   => root,
>                 group   => root,
>                 mode    => 644;
>         }
>     }}
>
> class profile::ulimit inherits profile {


This class really oughtn't to inherit from class "profile".  Puppet
class inheritance is only appropriate where the subclass overrides one
or more properties of one of its superclass's resources (if even
then).  In all other cases, including this one, you should use
composition instead:

    include 'profile'


>     if ("$operatingsystem" == "Ubuntu") {
>         define set_ulimit($limit="null") {
>                 file {
>                     "/etc/profile.d/ulimit.sh":
>                         ensure  => file,
>                         mode    => 644,
>                         owner   => root,
>                         group   => root,
>                         content => template('profile/ulimit.sh.erb');
>                     }
>         }
>
>         set_ulimit { "ulimit.sh": limit  => "$((2 * 1073741824))"; }
>     }}
>
> class profile::ulimit::full inherits profile::ulimit {
>     if ("$operatingsystem" == "Ubuntu") {
>         if defined(Set_ulimit["ulimit.sh"]) {
>             Set_ulimit["ulimit.sh"] {
>                 limit  => "unlimited" #unlimited
>             }
>         }
>     }
>
> }
>
> when i run puppet parser validate it gives me error :
>
> err: Could not parse for environment production: Classes, definitions,
> and nodes may only appear at toplevel or inside other classes;
> expected '%s' at
> /home/vgsuresh/svn/operations/puppet/modules/profile/manifests/init.pp:19
> err: Try 'puppet help parser validate' for usage


Your definition "set_ulimit" can only appear at top level or
(directly) inside a class.  It cannot appear inside a conditional
statement, and you don't need it to do.  Just move the definition
outside the conditional.  It makes no difference to declare it
unconditionally if you only declare instances conditionally.  As a
matter of style, though, it should appear at top level (but in a
module) instead of inside a class.

Furthermore, your naming and manifest design suggest a fundamental
misunderstanding: definitions should not be construed as functions or
(especially) macros.  Rather, they define *resource types*, analogous
to Puppet's built-in resource types in almost every way.  You are
likely to have continual trouble with definitions until you adjust
your mental model.

Note also:

1) Although the 'defined' function can be used to determine whether a
particular resource has been declared, that's poor practice.  If your
manifest structure does not pre-determine the answer (which in your
case it does) then the answer is parse-order dependent (which will
bite you eventually).

2) Since your manifest structure already controls whether resource
Set_ulimit["ulimit.sh"] will have been declared by the time your class
profile::ulimit::full is parsed (it is declared by the superclass),
you don't need to test for it via 'defined':

class profile::ulimit::full inherits profile::ulimit {
    if ("$operatingsystem" == "Ubuntu") {
        Set_ulimit["ulimit.sh"] {
            limit  => "unlimited" #unlimited
        }
    }
}

3) Really, the whole approach is awkward.  It would be much better to
build this around a *data* hierarchy (via Hiera) than around a class
hierarchy.  You could merge 'set_ulimit' into class "profile::ulimit"
and get rid of class "profile::ulimit::full" altogether.  With a bit
more work you could even get rid of the conditional statements, or at
least convert them to be generic instead of OS-specific.


John

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Users" group.
To post to this group, send email to [EMAIL PROTECTED]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/puppet-users?hl=en.