#8433 Globbing imports are now considered undefined behavior

45 views
Skip to first unread message

DEGREMONT Aurelien

unread,
Oct 23, 2013, 9:29:21 AM10/23/13
to puppe...@googlegroups.com
Hi all

I'm currently working on upgrading our old 0.25 puppet infrasctructure
to something more up to date.

I've just faced the issue detailled in:
https://projects.puppetlabs.com/issues/8433

Regarding automatic import and import "*.pp" behavior change.

I'm trying to find an official document explaining how classes should be
organized.


Thanks

Aur�lien

Peter Meier

unread,
Oct 23, 2013, 10:29:19 AM10/23/13
to puppe...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> I'm trying to find an official document explaining how classes
> should be organized.

This should probably help you:

http://docs.puppetlabs.com/puppet/latest/reference/lang_namespaces.html

~pete
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlJn3TkACgkQbwltcAfKi39DwgCglxjMxLaw0BSfUWL3nFwPrerV
S0AAnAi8I7eUGfOKilZXn320liQepbsL
=RhGT
-----END PGP SIGNATURE-----

DEGREMONT Aurelien

unread,
Oct 23, 2013, 10:44:55 AM10/23/13
to puppe...@googlegroups.com, Peter Meier
Thanks a lot.
This was exactly what I was looking for.

It seems that now the recommendation is to have one file per class?
If no, where puppet will look for foo::a::b class, (aside from
module/foo/manifest/a/b.pp)?


Aur�lien


Le 23/10/2013 16:29, Peter Meier a �crit :

R.I.Pienaar

unread,
Oct 23, 2013, 10:46:33 AM10/23/13
to puppe...@googlegroups.com


----- Original Message -----
> From: "DEGREMONT Aurelien" <aurelien....@cea.fr>
> To: puppe...@googlegroups.com
> Cc: "Peter Meier" <peter...@immerda.ch>
> Sent: Wednesday, October 23, 2013 3:44:55 PM
> Subject: Re: [Puppet-dev] #8433 Globbing imports are now considered undefined behavior
>
> Thanks a lot.
> This was exactly what I was looking for.
>
> It seems that now the recommendation is to have one file per class?
> If no, where puppet will look for foo::a::b class, (aside from
> module/foo/manifest/a/b.pp)?

Here's the guide on the module layout, everything should now more or less
go in modules.

http://docs.puppetlabs.com/puppet/2.7/reference/modules_fundamentals.html

Peter Meier

unread,
Oct 23, 2013, 11:41:48 AM10/23/13
to DEGREMONT Aurelien, puppe...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> It seems that now the recommendation is to have one file per
> class?

Yes it is.

> If no, where puppet will look for foo::a::b class, (aside from
> module/foo/manifest/a/b.pp)?

But you can also have this class in

module/foo/manifest/a.pp

module/foo/manifest/init.pp

if I remember that correctly.

But I definitely advice you to put every class/define in its own file.

~pete
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlJn7jkACgkQbwltcAfKi3/2nwCbBZYJX9QAnnsj+OcTpcYbUZRP
+hMAnRcYACKXU/r7Wo7HAirLM0dX1213
=98VY
-----END PGP SIGNATURE-----

DEGREMONT Aurelien

unread,
Oct 24, 2013, 3:54:08 AM10/24/13
to Peter Meier, puppe...@googlegroups.com
Le 23/10/2013 17:41, Peter Meier a �crit :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>> It seems that now the recommendation is to have one file per
>> class?
> Yes it is.
:/
Module manifests will be much more complex with a lot of files in it.

>> If no, where puppet will look for foo::a::b class, (aside from
>> module/foo/manifest/a/b.pp)?
> But you can also have this class in
>
> module/foo/manifest/a.pp
>
> module/foo/manifest/init.pp
>
> if I remember that correctly.
It seems it works that way, at least in 2.7, but I did not find anything
in documentation stating it should work that way.
I'm not confident in this. I'm worried this behaviour will disappeared
in near future and would like to know how long can I rely on this.
At least it will help us a lot in migrating our manifests from
0.25-style to 2.7+ style...



Aur�lien

Adrien Thebo

unread,
Oct 24, 2013, 11:39:22 AM10/24/13
to puppe...@googlegroups.com
On Thu, Oct 24, 2013 at 12:54 AM, DEGREMONT Aurelien <aurelien....@cea.fr> wrote:
Le 23/10/2013 17:41, Peter Meier a écrit :

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

It seems that now the recommendation is to have one file per
class?
Yes it is.
:/
Module manifests will be much more complex with a lot of files in it.

There will be more files, but you'll still have the same number of classes and defines. In addition, you can locate classes or defines by the filename so it means less hunting around. The vast majority of modules these days are written in this style, so I don't think that it adds that much complexity.
 

If no, where puppet will look for foo::a::b class, (aside from
module/foo/manifest/a/b.pp)?
But you can also have this class in

module/foo/manifest/a.pp

module/foo/manifest/init.pp

if I remember that correctly.
It seems it works that way, at least in 2.7, but I did not find anything in documentation stating it should work that way.
I'm not confident in this. I'm worried this behaviour will disappeared in near future and would like to know how long can I rely on this.
At least it will help us a lot in migrating our manifests from 0.25-style to 2.7+ style...


This behavior is documented at http://docs.puppetlabs.com/puppet/2.7/reference/lang_namespaces.html and is the supported approach to resource namespacing. It's very unlikely that this will change at all, let alone in the near future, and has been the accepted best practice for at least 2 - 3 years.

-- 
Adrien Thebo | Puppet Labs

John Bollinger

unread,
Oct 24, 2013, 3:05:10 PM10/24/13
to puppe...@googlegroups.com


On Thursday, October 24, 2013 10:39:22 AM UTC-5, Adrien Thebo wrote:
On Thu, Oct 24, 2013 at 12:54 AM, DEGREMONT Aurelien <aurelien....@cea.fr> wrote:
Le 23/10/2013 17:41, Peter Meier a écrit :

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

It seems that now the recommendation is to have one file per
class?
Yes it is.
:/
Module manifests will be much more complex with a lot of files in it.

There will be more files, but you'll still have the same number of classes and defines. In addition, you can locate classes or defines by the filename so it means less hunting around. The vast majority of modules these days are written in this style, so I don't think that it adds that much complexity.
 

If no, where puppet will look for foo::a::b class, (aside from
module/foo/manifest/a/b.pp)?
But you can also have this class in

module/foo/manifest/a.pp

module/foo/manifest/init.pp

if I remember that correctly.
It seems it works that way, at least in 2.7, but I did not find anything in documentation stating it should work that way.
I'm not confident in this. I'm worried this behaviour will disappeared in near future and would like to know how long can I rely on this.
At least it will help us a lot in migrating our manifests from 0.25-style to 2.7+ style...




In fairness, it is probably better to characterize the old style as 0.23 or maybe early 0.24.  The current module format and the module autoloader have been available since at least 0.24.8, as I can attest from personal experience.  Outdated literature may have been more prevalent in the 0.25 days, but the current module style wasn't even new any longer then.  Although the 'import' function apparently regressed during the 2.7 series, that's an altogether separate issue, well separated in time.

 
This behavior is documented at http://docs.puppetlabs.com/puppet/2.7/reference/lang_namespaces.html and is the supported approach to resource namespacing. It's very unlikely that this will change at all, let alone in the near future, and has been the accepted best practice for at least 2 - 3 years.



I think the undocumented behavior Aurélien was concerned about is the autoloader falling back from my_module/manifests/a/b.pp to my_module/manifests/a.pp and ultimately to my_module/manifests/init.pp when trying to load class my_module::a::b.  Indeed, it isn't explicitly documented as far as I can tell; certainly the referenced document says nothing about it.

In practice, such behavior is necessary to find and load classes that are lexically nested inside others, so it is at least a matter of quality of implementation (a win) that the autoloader in fact does exhibit that behavior.  Nevertheless, I think Aurélien is right to be wary, especially if class nesting ever moves from disfavor to deprecation, on a path to ultimate removal.  Moreover, I think it is especially risky to rely on that fallback behavior to load classes that are not lexically nested, ala

my_module/manifests/foo.pp:
----
class my_module::foo {
}

class my_module::foo::bar {
}

At present, if the autoloader looks for my_module::foo::bar in my_module/manifests/foo.pp then I don't think it matters whether class my_module::foo::bar is expressed in nested or unnested form.  However, I would account that a parser implementation detail.  For instance, it could be that a future iteration of the parser will attempt to optimize class loading by stopping the parse of my_module/manifests/foo.pp when it reaches the closing brace of class my_module::foo.  If that happened, then class my_module::foo::bar in my example would suddenly become invisible.


John

DEGREMONT Aurelien

unread,
Oct 29, 2013, 12:05:39 PM10/29/13
to puppe...@googlegroups.com, John Bollinger
You totally catch my concerns!
What could we expect from that in the future?


Aurélien

John Bollinger

unread,
Oct 29, 2013, 3:11:08 PM10/29/13
to puppe...@googlegroups.com


On Tuesday, October 29, 2013 11:05:39 AM UTC-5, Aurélien Degrémont wrote:
Le 24/10/2013 21:05, John Bollinger a écrit :

[...]
 


I think the undocumented behavior Aurélien was concerned about is the autoloader falling back from my_module/manifests/a/b.pp to my_module/manifests/a.pp and ultimately to my_module/manifests/init.pp when trying to load class my_module::a::b.  Indeed, it isn't explicitly documented as far as I can tell; certainly the referenced document says nothing about it.

In practice, such behavior is necessary to find and load classes that are lexically nested inside others, so it is at least a matter of quality of implementation (a win) that the autoloader in fact does exhibit that behavior.  Nevertheless, I think Aurélien is right to be wary, especially if class nesting ever moves from disfavor to deprecation, on a path to ultimate removal.  Moreover, I think it is especially risky to rely on that fallback behavior to load classes that are not lexically nested, ala

my_module/manifests/foo.pp:
----
class my_module::foo {
}

class my_module::foo::bar {
}

At present, if the autoloader looks for my_module::foo::bar in my_module/manifests/foo.pp then I don't think it matters whether class my_module::foo::bar is expressed in nested or unnested form.  However, I would account that a parser implementation detail.  For instance, it could be that a future iteration of the parser will attempt to optimize class loading by stopping the parse of my_module/manifests/foo.pp when it reaches the closing brace of class my_module::foo.  If that happened, then class my_module::foo::bar in my example would suddenly become invisible.

You totally catch my concerns!
What could we expect from that in the future?



I do not represent PuppetLabs, but I predict that the autoloader's current fallback behavior will be retained more or less as-is at least into the mid-term future.  Nevertheless, I recommend that you do plan to transition to the recommended form of one class or definition per file, because the future for that arrangement seems a lot more certain.  Completing such a transition is not urgent, but perhaps it would be wise to establish maintenance and development policies that will drive you through it over time.  For example, insist that new manifests adhere to the recommended form, and that existing manifests be transitioned if they are modified.

If class nesting is ever deprecated in a future version of Puppet, then I think that will be a signal to prioritize completing the transition (regardless of whether your manifests actually use nesting).


John

Andy Parker

unread,
Oct 29, 2013, 3:18:46 PM10/29/13
to puppe...@googlegroups.com
Yes, we know that many manifests rely on this behavior, so it would not be going away without a deprecation period. At the moment I don't see us changing the behavior soon, but as John says, conforming to the suggested layouts is the best way to protect yourself from those kinds of changes.
 

If class nesting is ever deprecated in a future version of Puppet, then I think that will be a signal to prioritize completing the transition (regardless of whether your manifests actually use nesting).


John

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/0050745a-d864-4b2c-b372-b58a66138ce7%40googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.



--
Andrew Parker
Freenode: zaphod42
Twitter: @aparker42
Software Developer

Join us at PuppetConf 2014September 23-24 in San Francisco

DEGREMONT Aurelien

unread,
Oct 30, 2013, 1:08:56 PM10/30/13
to puppe...@googlegroups.com
Le 24/10/2013 21:05, John Bollinger a �crit :
>
> At present, if the autoloader looks for my_module::foo::bar in
> my_module/manifests/foo.pp then I don't think it matters whether class
> my_module::foo::bar is expressed in nested or unnested form.

I could not reproduce this behaviour with 2.6
I mean the "fallback" thing.

If I got "foo::bar::quick" in "modules/foo/manifests/bar.pp", Puppet 2.6
does not find it. I need to put it in modules/foo/manifests/bar/quick.pp

Does it make sense to you that this fallback behavior is specific to 2.7
or it should work in 2.6 and possible 0.25 ?

This would help to ease our migration phase.



Aur�lien
Reply all
Reply to author
Forward
0 new messages