On Fri, 24 Oct 2008 11:48:17 -0500 Luke Kanies <[EMAIL PROTECTED]> wrote:
Ok, first i want to say that i'm very happy with puppet and the way it
works. The require-thing is just one little annoyance which really
bugged me long enough that i decided to look at it closer. So if i
criticize some puppet internals i try to find the most appropriate fair
and clear words. I don't intend to be rough against you luke and all
the other developers who realized this wonderful system.
> [...]
>
> The crux is: What happens if you need to delete a resource in order
> to correctly configure another resource? That is, correct
> configuration of a given resource class requires negation of some
> resource.
>
> In that case, we would incorrectly set order, which would then break
> everything.
I agree, but this is a special case. Let's treat that special and
carefully instead of complicate the standart cases. I argue that it
should be easy to add *and* remove resources which depend on each other
without requiring users to work around dependency-problems for the
ensure => absent case for each and every resource. Additionally with
the "above"/"below" solution "require" and "before" will not be touched
in any way, they continue to behave like before. This two dependency
models may coexist as long as they will not be intermixed heavily -
that's actually a thing which i want to test more in depth.
I've attached POC patches and some test cases to illustrate the
situation. Note the mybind-define. This is one of my most used patterns
and the reason i'm not happy with the dependency system.
> The only real way to do something like this is to have class-level
> 'ensure'-equivalents: We'd need to be able to talk about the whole
> service class being present or running or whatever, and then the
> 'ensure' state of all contained resources would be bit-flipped
> depending on what we did with that class-level state.
I don't thing that this is necessary. I do often something like this:
class apache {
# general defines go here
package { "apache2": ensure => installed }
service { "apache2": enable => false, ensure => stopped }
}
class apache::enable inherits apache {
# config stuff goes here
Service["apache"] { enable => true, ensure => running }
}
class apache::disable inherits apache {
# special clean up stuff (rarely used)
Service["apache"] { enable = false, ensure => stopped }
}
Then i include apache or apache::enable in specialized classes or even
inherit from apache::enable and include them in node definitions. I
think it is better to not try to make puppet smarter than it can be. The
bitflipping approach sounds to me like a very bad solution to a not so
bad problem - at least if i did understand your short description
correctly.
Lorenz
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
above-case.pp
Description: Binary data
require-case-broken.pp
Description: Binary data
require-case-fixed.pp
Description: Binary data
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 85f72f7..88b855a 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1533,9 +1533,18 @@ class Type
self.fail "Could not retrieve dependency '%s' of %s" % [reference, @resource.ref]
end
+ deleting = (@resource[:ensure] == :absent || @resource[:ensure] == :purge)
+ if self.class.direction == :inoncreation
+ direction = deleting ? :out : :in
+ elsif self.class.diroction == :inondeletion
+ direction = deleting ? :in : :out
+ else
+ direction = self.class.direction
+ end
+
# Are we requiring them, or vice versa? See the method docs
# for futher info on this.
- if self.class.direction == :in
+ if direction == :in
source = object
target = @resource
else
@@ -1669,6 +1678,16 @@ class Type
This will restart the sshd service if the sshd config file changes.}
end
+ newmetaparam(:above, :parent => RelationshipMetaparam, :attributes => {:direction => :inoncreation, :events => :NONE}) do
+ desc %{This parameter works like **require** on creation of the
+ ressource (ensure => present) and behaves like **before** on deletion.}
+ end
+
+ newmetaparam(:below, :parent => RelationshipMetaparam, :attributes => {:direction => :inondeletion, :events => :NONE}) do
+ desc %{This parameter works like **require** on deletion of the
+ ressource (ensure => absent) and behaves like **before** on creation.}
+ end
+
###############################
# All of the provider plumbing for the resource types.
require 'puppet/provider'
diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb
index edd5941..37d5e49 100644
--- a/lib/puppet/metatype/metaparams.rb
+++ b/lib/puppet/metatype/metaparams.rb
@@ -284,9 +284,18 @@ class Puppet::Type
self.fail "Could not retrieve dependency '%s' of %s" % [reference, @resource.ref]
end
+ deleting = (@resource[:ensure] == :absent || @resource[:ensure] == :purge)
+ if self.class.direction == :inoncreation
+ direction = deleting ? :out : :in
+ elsif self.class.direction == :inondeletion
+ direction = deleting ? :in : :out
+ else
+ direction = self.class.direction
+ end
+
# Are we requiring them, or vice versa? See the method docs
# for futher info on this.
- if self.class.direction == :in
+ if direction == :in
source = object
target = @resource
else
@@ -363,6 +372,14 @@ class Puppet::Type
"
end
+ newmetaparam(:above, :parent => RelationshipMetaparam, :attributes => {:direction => :inoncreation, :events => :NONE}) do
+ desc %{"**require** on creation **before** on deletion"}
+ end
+
+ newmetaparam(:below, :parent => RelationshipMetaparam, :attributes => {:direction => :inondeletion, :events => :NONE}) do
+ desc %{"**below** on creation **require** on deletion"}
+ end
+
newmetaparam(:subscribe, :parent => RelationshipMetaparam, :attributes => {:direction => :in, :events => :ALL_EVENTS, :callback => :refresh}) do
desc "One or more objects that this object depends on. Changes in the
subscribed to objects result in the dependent objects being
### test with plain types file and mount requiring file resources ### # $ensure = present # puppet require-case-broken.pp notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: created notice: //Node[default]/Mount[/tmp/puppet-testmnt]/ensure: defined 'ensure' as 'mounted' notice: //Node[default]/Mount[/tmp/puppet-testmnt]: Refreshing self # $ensure = absent # puppet require-case-broken.pp notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: removed notice: //Node[default]/Mount[/tmp/puppet-testmnt]/ensure: undefined ensure from 'mounted' # here we see that the mount point is purged before the device is unmounted. ### test with workaround ### # $ensure = present # puppet require-case-fixed.pp notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: created notice: //Node[default]/Mybind[/tmp/puppet-testmnt]/Mount[/tmp/puppet-testmnt]/ensure: defined 'ensure' as 'mounted' notice: //Node[default]/Mybind[/tmp/puppet-testmnt]/Mount[/tmp/puppet-testmnt]: Refreshing self # $ensure = absent # puppet require-case-fixed.pp notice: //Node[default]/Mybind[/tmp/puppet-testmnt]/Mount[/tmp/puppet-testmnt]/ensure: undefined ensure from 'mounted' notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: removed ### test with above-below-puppet, plain types file and mount above file resources ### # $ensure = present # puppet above-case.pp notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: created notice: //Node[default]/Mount[/tmp/puppet-testmnt]/ensure: defined 'ensure' as 'mounted' notice: //Node[default]/Mount[/tmp/puppet-testmnt]: Refreshing self # $ensure = absent # puppet above-case.pp notice: //Node[default]/Mount[/tmp/puppet-testmnt]/ensure: undefined ensure from 'mounted' notice: //Node[default]/File[/tmp/puppet-testmnt]/ensure: removed
