On 10/18/2010 05:24 PM, Bruce Richardson wrote:
> On Mon, Oct 18, 2010 at 07:20:10AM -0700, jcbollinger wrote:
>>
>> On Oct 18, 4:38 am, Bruce Richardson <itsbr...@workshy.org> wrote:
>>> No, you can't uninclude a class.
>>
>> Right.
>>
>>>> Is the only way to do this to inherit the base class and override all
>>>> it's resources to do the equivalent of "ensure => absent", then
>>>> something like "if (tagged(class)) { include undo-subclass } " where
>>>> needed?
>>
>> [...]
>>
>>> You *could* do it with overrides but only If you were using a define
>>> within your base class for part of the setup and that define took a
>>> parameter which told it whether to include the ldap class.
>>
>> You could write it that way, but Puppet's subclassing feature is
>> designed precisely so that you don't need to do so.
>
> Um, I completely disagree with you and the rest of your post actually
> backs this up. Class inheritence *only* makes sense if you want to
> override the properties of resources. The OP wants not to include a
> whole class; what you're telling him to do is not uninclude the class,
> but to try and override properties in every resource owned by the class,
> so as to make the class effectively do nothing. There are any number of
> reasons why that's not a smart thing to do, but here are several that
> occur to me immediately.
>
> 1. If the resources in the ldap::client class change, the
> ldap::client::disabled will have to be changed to match. This
> just begs to be a source of error.
>
> 2. The structure of the ldap::client class and it's resources may
> well have to be distorted to fit this arrangement. Some resources
> would have to be set up quite carefully so that they could be
> "negated".
>
> 3. What do you do about virtual resources that are realized in the
> parent class? You can't unrealize them and if you override their
> properties, you may well conflict with other modules or classes which
> use them. #
>
> 4. What do you do about any other classes that are included in the
> parent class? Are you going to include "::disabled" versions of
> those in the ldap::client::disabled class? What if those classes
> are included elsewhere?
>
> Your exmaple cannot achieve the same effect as not including a class;
> the empty files you'd litter the filesystem with is only one example.
> Forcibly negating everything in a class is not the same as not including
> it. Why not just not include it?
>
>> For example, although Bruce's main suggestion could be used if for
>> some reason it were important to avoid subclasses, really in that case
>> something akin to his define-based suggestion (but without subclasses)
>> would be better. That way, if the LDAP client somehow gets turned on
>> on your LDAP server, then Puppet will turn it back off. Unconfigured
>> means "I don't care"; it must not be confused with "off". The
>> subclass usage pattern above achieves the same thing with no
>> conditional inclusions and no define, plus it's safe against inclusion
>> of "ldap::client" by other parts of the manifest.
>
> I'm sorry, but I think you are quite wrong and your recommendations are
> very unwise. I have no objection to class inheritance at all, although
> the way it works in puppet is often misunderstoon and it is often
> overused as a result. Your proposed example is definitely not a good
> use of class inheritance.
>
> "ssh::server::disabled" makes sense, because that's done by overriding
> the properties of an existing service resource to make sure it's
> disabled. ldap::client::disabled does not make sense; the OP wants the
> actions in ldap::client not to be applied, not to be differently
> applied.
>
> By far the simplest and safest way not to include a whole class is the
> "if $ldap_enabled { include ldap::client }" method; it has no bad side
> effects, it works no matter how the internals of the ldap::client
> change, it's a tiny bit of code.
>
Hi Bruce,
I was glad for John's comment on your answer because it mirrored my
sentiment exactly, and I in turn disagree with most of what you've
written here.
Subclasses that effectively disable a class are a sound concept. How
this has to be achieved in any specific case can be somewhat
mind-boggling, but the variable approach has severe limitations of its own.
1. Where is the variable set? You have probably no problem when using
external node definition, but not everybody does. You cannot define the
variable in an arbitrary part of your manifest, because you can be
afflicted by both ordering and scoping issues.
2. The concept of dynamic scoping is going away in future versions (at
least that is what seems to be a community consensus). Again, if you can
globally assign the variable value to your node, this is not a problem.
But if you want to for every node that includes class B to automatically
not include class A, you're out of luck (this is true in many cases
today already, because of the scoping issues).
Example that works:
class default {
include A
}
node X {
include default
include B
}
class B {
include A::disable
...
}
Example which will probably not work:
class default {
if ! $a_disabled { include A }
}
node X {
include default
include B
}
class B {
$a_disabled = true
...
}
This will not work because in the scope of class default, the variable
has not been defined.
Variables are about as far as you can get from a panacea in the puppet
DSL. Granted, class inheritance can be clumsy, awkward and
unmaintainable in many cases. But all that and more applies to variables
as well, and it seems to me that variables are going to become a lot
less useful for such purposes in the future.
Class parameters will probably be an alternative that can be used for
solutions that are superior to both the above, but I cannot yet say much
about that.
Sincerely,
Felix
--
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-us...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.
