Splitting classes into separate files

[Douglas Garstang]

The module standards page at
http://projects.puppetlabs.com/projects/puppet/wiki/Module_Standards
advises than when there are multiple classes in a module, you should
split them into separate files.

"Each (externally visible) define be in a separate file, named after
the class to facilitate autoloading. For groups of small and/or
tightly related defines, putting them together into a single file
might make sense. The module has to import that in the init.pp to
facilitate autoloading."

So, when I go and split the init.pp into seperate files in the
manifests directory, puppet doesn't automatically load them. What's
the best way to do this? I went and put and moved them to a classes
directory at the same level as the manifests directory (not sure why I
did this except I've seen others doing it. I'd rather leave it in
manifests), and put an import "classes/*.pp" at the top of init.pp.
Puppet then tells me:

Could not retrieve catalog from remote server: Error 400 on SERVER: No
file(s) found for import of 'classes/*.pp' at
/etc/puppet/modules/repo/manifests/init.pp:2 on node
log01.sjc.xxx.com.

However, the classes directory has files in it:

prov01 /etc/puppet/modules/repo/classes:# ls -l
total 8
-rw-r--r-- 1 puppet puppet 4538 Jul 1 16:22 repolist.pp

Doug.

[Chuck]

Move the classes directory to

/etc/puppet/modules/repo/manifests/classes



On Jul 1, 6:46 pm, Douglas Garstang <doug.garst...@gmail.com> wrote:
> The module standards page athttp://projects.puppetlabs.com/projects/puppet/wiki/Module_Standards

[Jeff McCune]

On Thu, Jul 1, 2010 at 6:03 PM, Chuck <css...@gmail.com> wrote:
> Move the classes directory to
>
>  /etc/puppet/modules/repo/manifests/classes
>

With puppet 0.25.x, you should no longer need to use any import
statements at all. The autoloader will import the correct manifest
file so long as you follow the standard.

A typical module for apache contains:
a class named apache
a class named apache::disable to disable the service
a defined type named apache::virtualhost to model a virtual host.

In this module, if you use the following orginization puppet will
autoload everything:

manifests/init.pp contains class apache { }
manifests/disable.pp contains class apache::disable inherits apache {}
manifests/virtualhost.pp contains define apache::virtualhost(){}

If you want additional namespaces, they go in directories.
class apache::service::disable would go in manifests/service/disable.pp

I highly recommend against using import today and in the future.

Hope this helps,
--
Jeff McCune
http://www.puppetlabs.com/

[Thomas Bellman]

On 2010-07-03, Jeff McCune wrote:

> In this module, if you use the following orginization puppet will
> autoload everything:
>
> manifests/init.pp contains class apache { }
> manifests/disable.pp contains class apache::disable inherits apache {}
> manifests/virtualhost.pp contains define apache::virtualhost(){}

Yuck! Forcing each class or define into its own file sucks royally.
No sane person wants their code organized like that.

> If you want additional namespaces, they go in directories.
> class apache::service::disable would go in manifests/service/disable.pp
>
> I highly recommend against using import today and in the future.

The cure is in this case worse, *much* worse, than the illness of having
to do explicit imports. I'd rather have two dozen import lines in site.pp
(which is what I have now) than having to split my classes and defines into
almost 200 files in two dozen directories.

/Thomas Bellman

[Patrick Mohr]

I'll try to post an example soon, but you don't have to split it up into "200 files" to take advantage of autoloading. You would need to split it into "two dozen directories" though.

On the other hand, you can put everything into site.pp. I'm sure you'll agree that this is a mistake too. I use modules but I don't split a module's manifest (init.pp) into different files until the file starts to get large.

For instance, you could put all your classes into /modules/module-name/init.pp. This works well if all but one or two class are really small. This is what I do by default because if I put "include cups::client" in site.pp, puppet will auto import:
/modules/cups/init.pp
/modules/cups/client.pp
/modules/cups/client/init.pp (I think this last one is true, but I don't know)

I like the different folders because it keeps the files and templates with the manifests, and t makes it easier to tell which files can safely be deleted.

For me, the key to keeping things easy was to remember that I didn't need to break a module into more than one file, but I could if they got too big.

[TomTom]

Hey Fellows,
What an interesting discussion. Have you guys ever seen Example42's
Apache Module?
http://www.example42.com/puppet/browsemodules.php

It is an an excellent model of what everyone is talking about.

I always import my defines/ and classes subdirectories with the
following in my module's init.pp:

#apache class init.pp
import "defines/*.pp"
import "classes/*.pp"

class apache {
#stub code
}

The one caveat, is that your defines/ and classes/ subdirectories will
have to have a "naked" .pp file in both directories,
or puppet will error out. You could also comment out the imports, but
it is up to your preference.

I handle it by starting with a puppet_module SKEL directory, and it
has a defines/BLANK.pp and a classes/BLANK.pp
file in each subdirectory.

Have a great July 5th!
-Tom

[Dan Carley]

On 5 July 2010 15:11, Thomas Bellman <bel...@nsc.liu.se> wrote:

On 2010-07-03, Jeff McCune wrote:

> In this module, if you use the following orginization puppet will
> autoload everything:
>
> manifests/init.pp contains class apache { }
> manifests/disable.pp contains class apache::disable inherits apache {}
> manifests/virtualhost.pp contains define apache::virtualhost(){}

Yuck!  Forcing each class or define into its own file sucks royally.
No sane person wants their code organized like that.

Really? I find it a much more manageable way to work, rather than wading through lines of `init.pp` to find the relevant class or maintaining lists of slightly artificially named `import` statements. It also provides some more reliable behaviour with regards to caching of compilation failures.

[yatesco]

Coming from a Java background, this makes perfect sense to me.

[Patrick Mohr]

I usually put some of the really small classes in init.pp.  For instance, do you put your packages classes in their own file?

class apache::package {

package { apache2: ensure => present }

}

Would you put this in it's own file?

[Dan Carley]

On 5 July 2010 16:27, Patrick Mohr <kc7...@gmail.com> wrote:

I usually put some of the really small classes in init.pp.  For instance, do you put your packages classes in their own file?

class apache::package {

package { apache2: ensure => present }

}

Would you put this in it's own file?

Yeah. I uniformly split all classes and definitions into their own files, big or small.

I wouldn't typically put that snippet into it's own class - but that's probably another topic.

[Peter Meier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi

+1 this is just small, clean and easy understandable. I got rid of every
import in my manifests and I would consider them as code smell.

cheers pete
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwyNNYACgkQbwltcAfKi3+xSACfT+V4HTPHKXhdOf62ofGuI7Ng
PfUAn3T5eFjKJtvk0WyJWwMOZpwZzAaQ
=eQgf
-----END PGP SIGNATURE-----

[Jeff McCune]

On Mon, Jul 5, 2010 at 7:11 AM, Thomas Bellman <bel...@nsc.liu.se> wrote:
> On 2010-07-03, Jeff McCune wrote:
>
>> In this module, if you use the following orginization puppet will
>> autoload everything:
>>
>> manifests/init.pp contains class apache { }
>> manifests/disable.pp contains class apache::disable inherits apache {}
>> manifests/virtualhost.pp contains define apache::virtualhost(){}
>
> Yuck!  Forcing each class or define into its own file sucks royally.
> No sane person wants their code organized like that.

What I have described is the recommended method of organizing
manifests. This recommendation comes from years of experience of many
in the puppet community, myself included. There is relatively healthy
adoption of this standard within the community.

I'm working from the assumption you're organizing your manifests in
modules. If you're not, I recommend re-factoring into modules for
maintainability and reuse.

There are a number of advantages to adopting this organization standard:

Most important in my opinion; with no other information than the
resource or class name you know where it lives in the file-system. If
you and I both follow the standard and I give you something named
jeff::shirt::red, you know it lives in the jeff module in the
manifests/shirt/red.pp file. Without the standard, I'm forcing you to
spend time and effort hunting for my code. This quickly becomes a
heavy burden at scale with multiple people involved.

Second, puppetdoc works nicely with this standard to create
self-documented manifests.

Third, this standard is common and familiar to those who have used
other high level languages like ruby, python, etc...

Fourth, this is the standard we are recommending for modules posted
publicly to the module forge.

Ultimately, any standard is preferable to no standard.

>> If you want additional namespaces, they go in directories.
>> class apache::service::disable would go in manifests/service/disable.pp
>>
>> I highly recommend against using import today and in the future.
>
> The cure is in this case worse, *much* worse, than the illness of having
> to do explicit imports.  I'd rather have two dozen import lines in site.pp
> (which is what I have now) than having to split my classes and defines into
> almost 200 files in two dozen directories.

Why is splitting classes and definitions into discrete files a problem?

[Julian Simpson]

Why is splitting classes and definitions into discrete files a problem?

Personally, I think any convention is for the better, and it seems to gel with most languages to do things this way.  You'll always get people who don't like the convention, however (often for valid reasons).  Just google 'Maven' or 'Ruby on Rails' :)

J.

--
Julian Simpson
Software Build and Deployment
http://www.build-doctor.com
http://twitter.com/builddoctor

[Douglas Garstang]

On Fri, Jul 2, 2010 at 6:31 PM, Jeff McCune <je...@puppetlabs.com> wrote:
> On Thu, Jul 1, 2010 at 6:03 PM, Chuck <css...@gmail.com> wrote:
>> Move the classes directory to
>>
>>  /etc/puppet/modules/repo/manifests/classes
>>
>
> With puppet 0.25.x, you should no longer need to use any import
> statements at all.  The autoloader will import the correct manifest
> file so long as you follow the standard.
>
> A typical module for apache contains:
> a class named apache
> a class named apache::disable to disable the service
> a defined type named apache::virtualhost to model a virtual host.
>
> In this module, if you use the following orginization puppet will
> autoload everything:
>
> manifests/init.pp contains class apache { }
> manifests/disable.pp contains class apache::disable inherits apache {}
> manifests/virtualhost.pp contains define apache::virtualhost(){}

Really? Are you serious???. Why is puppet dictating to me that it will
only autoload classes if I have a class named module::disable,
module::virtualhost? Who's idea was that and why? This seems awfully
arbitrary.

Doug.

[Peter Meier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>> In this module, if you use the following orginization puppet will

>> autoload everything:
>>
>> manifests/init.pp contains class apache { }
>> manifests/disable.pp contains class apache::disable inherits apache {}
>> manifests/virtualhost.pp contains define apache::virtualhost(){}
>
> Really? Are you serious???. Why is puppet dictating to me that it will
> only autoload classes if I have a class named module::disable,
> module::virtualhost? Who's idea was that and why? This seems awfully
> arbitrary.

on what other basis should puppet then autoload classes/defines?

I suspect that you didn't understand what Jeff have written...

cheers pete
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwyY/UACgkQbwltcAfKi3+gGQCeJ3jQqmHI9Yw5xbaSRv332urK
PPAAn2nO1r3vWFkAhm4Sl95ABi6DqH7t
=KDat
-----END PGP SIGNATURE-----

[James Turnbull]

Douglas Garstang wrote:
>
> Really? Are you serious???. Why is puppet dictating to me that it will

Quite serious.

> only autoload classes if I have a class named module::disable,
> module::virtualhost? Who's idea was that and why? This seems awfully
> arbitrary.

Why shouldn't it be arbitrary? The autoloader is there to make using
modules easier. The current model represents a standard loading model
that controls what is loaded and operates by the principle of least
surprise. That's not to say it can't be improved or changed and if you
have a suggestion about how it should autoload or an alterative scheme
please feel free to log a ticket.

I'd also like to address the tone of your emails to the list - I find
requests will generally dictate the tone of replies. I've noted that you
obviously find Puppet quite frustrating on occasion and this seems to be
reflected in the tone of your emails to the list.

We, both Puppet Labs and the community, recognize Puppet isn't perfect -
no solution is - and we're working to improve it every day. I
personally don't think a constant stream of "Puppet is stupid, I hate
Puppet, Puppet sucks, I can't believe Puppet does it this way and that's
stupid" emails help that process.

What is an awesome contribution to making Puppet better is reasoned
feedback, ideas, alternatives, changes ... oh and especially patches ...
like every open source project we love patches. You can send these to
the list or even better in the form of tickets:

http://projects.puppetlabs.com/

We welcome the input - it's what drives Puppet forward.

Regards

James Turnbull

--
Puppet Labs - http://www.puppetlabs.com
C: 503-734-8571

[Douglas Garstang]

On Mon, Jul 5, 2010 at 4:00 PM, Peter Meier <peter...@immerda.ch> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>>> In this module, if you use the following orginization puppet will
>>> autoload everything:
>>>
>>> manifests/init.pp contains class apache { }
>>> manifests/disable.pp contains class apache::disable inherits apache {}
>>> manifests/virtualhost.pp contains define apache::virtualhost(){}
>>
>> Really? Are you serious???. Why is puppet dictating to me that it will
>> only autoload classes if I have a class named module::disable,
>> module::virtualhost? Who's idea was that and why? This seems awfully
>> arbitrary.
>
> on what other basis should puppet then autoload classes/defines?

Let me try this again.

Your saying that unless my module contains a class specifically called
$module::disable.pp (and therefore a filed called 'disable.pp' in
manifests), then puppet will not autoload anything? I just got it to
work without such a class in a module. All I had to do was throw a
bunch of files in $module/manifests/, include $module::something and
it worked.

Where is this documented?

Doug.

[James Turnbull]

Douglas Garstang wrote:

> Your saying that unless my module contains a class specifically called
> $module::disable.pp (and therefore a filed called 'disable.pp' in
> manifests), then puppet will not autoload anything? I just got it to
> work without such a class in a module. All I had to do was throw a
> bunch of files in $module/manifests/, include $module::something and
> it worked.

I think you've misunderstood as Peter indicated.

>
> Where is this documented?

http://docs.puppetlabs.com/guides/modules.html

[Jeff McCune]

On Mon, Jul 5, 2010 at 4:00 PM, Peter Meier <peter...@immerda.ch> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>>> In this module, if you use the following orginization puppet will
>>> autoload everything:
>>>
>>> manifests/init.pp contains class apache { }
>>> manifests/disable.pp contains class apache::disable inherits apache {}
>>> manifests/virtualhost.pp contains define apache::virtualhost(){}
>>
>> Really? Are you serious???. Why is puppet dictating to me that it will
>> only autoload classes if I have a class named module::disable,
>> module::virtualhost? Who's idea was that and why? This seems awfully
>> arbitrary.
>
> on what other basis should puppet then autoload classes/defines?
>
> I suspect that you didn't understand what Jeff have written...

To clarify what I've written:

Clearly the autoloader must follow some deterministic set of rules.
Puppet is not dictating anything, but the autoloader does need to
operate in a predictable manner.

To clarify the history of the autoloader:

I take quite a bit of responsibility for the current behavior, I made
a number of suggestions and contributions to the current design when I
faced numerous issues with the autoloader in the past. Puppet is an
open source project and the ability to provide constructive feedback
is invaluable to me and a huge part of the reason I've been so
involved in the project.

The community is open and incredibly receptive to constructive
feedback which is why my suggestions and contributions helped produce
the autoloader we have today. I'm certainly not the only one who
contributed to the design, I filed tickets and suggested viable
alternatives along with other members of the community which is why
the autoloader improved to the point of imports not being necessary
any more. It's not perfect, but it's an improvement and it satisfies
my needs and the needs of many in the community.

None of these recommendations or standards are enforced. Technically
by the tools or by the community. You're free to structure your code
any way you choose. My thesis statement is; you will receive a number
of benefits "for free" if you structure your manifests in the way the
autoloader expects them to be structured.

If the autoloader behavior is an issue for you, please help us improve it.

Thanks,

[Luke Kanies]

Note that you can have all of the classes etc. for a given module just
in that module's init.pp and autoload will still work correctly.

So you can get some benefit from autoload without going crazy.

--
Real freedom lies in wildness, not in civilization.
-- Charles Lindbergh
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199

[Luke Kanies]

Just to be more clear:

Nowhere does Puppet require that you have those files specifically.

However, *if* you want Puppet to autoload a class named
'$module::foo', then that class must either be in the module's init.pp
file or in a class named 'foo.pp' in that module's 'manifests'
directory.

There was clearly some confusion on specificity here - an example was
being used, but it wasn't meant to say that all classes had to look
like that.

--
Brand's Asymmetry:
The past can only be known, not changed. The future can only be
changed, not known.

[Douglas Garstang]

*whew* Thanks.