Is ensure_resource() evil?

[Atom Powers]

Over the years I've heard a lot of people declare one function/method/implementation of something to be "evil". I've found that, more often than not, the person declaring it to be bad has simply been burned by trying to use it for something the function was never intended to be used for. (Usually an easy trap due to incomplete documentation.)

So I wonder, are ensure_resource and create_resources really evil, or just too easy to abuse in evil ways? 

On Fri, Mar 28, 2014 at 9:06 AM, jcbollinger <John.Bo...@stjude.org> wrote:

Ensure_resource() is evil.  Do not use it.  

 

--
Perfection is just a word I use occasionally with mustard.
--Atom Powers--

[jcbollinger]

On Sunday, March 30, 2014 11:24:30 PM UTC-5, Atom Powers wrote:

Over the years I've heard a lot of people declare one function/method/implementation of something to be "evil". I've found that, more often than not, the person declaring it to be bad has simply been burned by trying to use it for something the function was never intended to be used for. (Usually an easy trap due to incomplete documentation.)

So I wonder, are ensure_resource and create_resources really evil, or just too easy to abuse in evil ways? 

On Fri, Mar 28, 2014 at 9:06 AM, jcbollinger <John.Bo...@stjude.org> wrote:

Ensure_resource() is evil.  Do not use it.  

Ensure_resource() is evil.  Create_resources() is not.   The post from which you excerpted my comment (https://groups.google.com/forum/#!searchin/puppet-users/ensure_resource/puppet-users/dOCIZ8-Gfgw/VhlBbrSRpb8J) explains why ensure_resource() is evil: in short, because it looks like it does something useful, but in fact hands you a gun loaded with foot-seeking bullets.  It takes at least as much work to use ensure_resource() safely as it does to avoid any need for it in the first place.


John

[Dan Bode]

I disagree that ensure_resource is evil and should not be used. It is however, potentially problematic, and it's issues are worth discussing. It was written (full disclosure by me) as an improvement over defined(). While it is not perfect (it suffers from parse-order issues), it is intended as an improvement of defined, a convenience method which exists in Puppet core. 

Specifically, it is intended to solve the limitation in Puppet that duplicate resource declarations are treated as failures in Puppet, even if the resources they describe are identical.

That being said, there are two reasons it may be problematic.

1. Behavior depends on parse order - this is probably the best reason not to use it. The behavior of how it will fail depends on the order in which code is parsed in Puppet. That is why this method (as well as defined(), and resource collection overrides) should only be used by users who understand this behavior, it's limitations, and how to debug it when they run into issues. It's usage and the fact that it depends on the order in which your code is parsed makes it problematic to have in modules that are intended to be shared. That being said, it looks like Puppet is moving to a more procedural language,which may make this less of an issue going forward.

2. It is not part of Puppet core and may need access to unsupported APIs to work as expected. I was not aware until I read that email thread that it was not working correctly with parameter aliases, it's pretty reasonable to open this up as an issue and see if someone volunteers to fix it. That being said, I think it will require access to some undocumented unsupported APIs in order to implement correctly. 

--
You received this message because you are subscribed to the Google Groups "Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/f5d5c0aa-2355-4dc8-91da-2311fc3f679a%40googlegroups.com.

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

[Henrik Lindberg]

On 2014-31-03 20:08, Dan Bode wrote:
> I disagree that ensure_resource is evil and should not be used. It is
> however, potentially problematic, and it's issues are worth discussing.
> It was written (full disclosure by me) as an improvement over defined().
> While it is not perfect (it suffers from parse-order issues), it is
> intended as an improvement of defined, a convenience method which exists
> in Puppet core.
>
> Specifically, it is intended to solve the limitation in Puppet that
> duplicate resource declarations are treated as failures in Puppet, even
> if the resources they describe are identical.
>
> That being said, there are two reasons it may be problematic.
>
> 1. Behavior depends on parse order - this is probably the best reason
> not to use it. The behavior of how it will fail depends on the order in
> which code is parsed in Puppet. That is why this method (as well as
> defined(), and resource collection overrides) should only be used by
> users who understand this behavior, it's limitations, and how to debug
> it when they run into issues. It's usage and the fact that it depends on
> the order in which your code is parsed makes it problematic to have in
> modules that are intended to be shared. That being said, it looks like
> Puppet is moving to a more procedural language,which may make this less
> of an issue going forward.
>

Going forward, I think the Puppet Language should handle the situation
where more than one resource definition / class definition is made and
where the two are considered compatible (they describe the "same
state"). We are discussing solutions for this with the idea to solve
this during Puppet 4x.

The construction of the catalog is lazy (or call it declarative /
functional if you will), and will remain so even if there are other
things done to the language that are imperative.

(Side note: it is *evaluation order* not *parse order*, files are always
parsed in the order the text is written from top to bottom, every line.
I know the term "parse order" is used in the community to be synonym
with evaluation order, but it throws me every time).

Regards
- henrik

> 2. It is not part of Puppet core and may need access to unsupported APIs
> to work as expected. I was not aware until I read that email thread that
> it was not working correctly with parameter aliases, it's pretty
> reasonable to open this up as an issue and see if someone volunteers to
> fix it. That being said, I think it will require access to some
> undocumented unsupported APIs in order to implement correctly.
>
>
>
>
> On Mon, Mar 31, 2014 at 7:08 AM, jcbollinger <John.Bo...@stjude.org
> <mailto:John.Bo...@stjude.org>> wrote:
>
>
>
> On Sunday, March 30, 2014 11:24:30 PM UTC-5, Atom Powers wrote:
>
> Over the years I've heard a lot of people declare one
> function/method/implementation of something to be "evil". I've
> found that, more often than not, the person declaring it to be
> bad has simply been burned by trying to use it for something the
> function was never intended to be used for. (Usually an easy
> trap due to incomplete documentation.)
>
> So I wonder, are ensure_resource and create_resources really
> evil, or just too easy to abuse in evil ways?
>
>
> On Fri, Mar 28, 2014 at 9:06 AM, jcbollinger
> <John.Bo...@stjude.org> wrote:
>
>
> Ensure_resource() is evil. Do not use it.
>
>
>
> Ensure_resource() is evil. Create_resources() is not. The post
> from which you excerpted my comment
> (https://groups.google.com/forum/#!searchin/puppet-users/ensure_resource/puppet-users/dOCIZ8-Gfgw/VhlBbrSRpb8J

> <https://groups..google.com/forum/#!searchin/puppet-users/ensure_resource/puppet-users/dOCIZ8-Gfgw/VhlBbrSRpb8J>)

> explains why ensure_resource() is evil: in short, because it looks
> like it does something useful, but in fact hands you a gun loaded

> with foot-seeking bullets. It takes /at least/ as much work to use

> ensure_resource() safely as it does to avoid any need for it in the
> first place.
>
>
> John
>
> --
> You received this message because you are subscribed to the Google
> Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to puppet-users...@googlegroups.com

> <mailto:puppet-users...@googlegroups.com>.

> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-users/f5d5c0aa-2355-4dc8-91da-2311fc3f679a%40googlegroups.com

> <https://groups.google.com/d/msgid/puppet-users/f5d5c0aa-2355-4dc8-91da-2311fc3f679a%40googlegroups..com?utm_medium=email&utm_source=footer>.

>
> For more options, visit https://groups.google.com/d/optout.
>
>

> --
> You received this message because you are subscribed to the Google
> Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to puppet-users...@googlegroups.com

> <mailto:puppet-users...@googlegroups.com>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/puppet-users/CA%2B0t2LxOkQOosGsGuDRSWc24jq8t%2BUa2CXuV10SkSQK8DMCREg%40mail.gmail.com
> <https://groups.google.com/d/msgid/puppet-users/CA%2B0t2LxOkQOosGsGuDRSWc24jq8t%2BUa2CXuV10SkSQK8DMCREg%40mail.gmail.com?utm_medium=email&utm_source=footer>.

[jcbollinger]

On Monday, March 31, 2014 3:55:03 PM UTC-5, Henrik Lindberg wrote:

Going forward, I think the Puppet Language should handle the situation
where more than one resource definition / class definition is made and
where the two are considered compatible (they describe the "same
state"). We are discussing solutions for this with the idea to solve
this during Puppet 4x.

I'd really be interested in hearing more about that.  In particular, I'm interested in how one might judge whether resource declarations are compatible when they are not identical (after defaults, overrides, and any other relevant modifications are applied).  I still have yet to hear any approach to this problem that I like better than my "constraints" idea, now more than two years old: https://groups.google.com/forum/#!searchin/puppet-users/module$20compatibility$20constraints/puppet-users/Fvl0aOe4RPE/XpoI1oKpTF0J.  On the other hand, I haven't heard much of anything new on the topic since that particular thread went cold.

John

[jcbollinger]

On Monday, March 31, 2014 1:08:25 PM UTC-5, Dan Bode wrote:

That being said, there are two reasons it may be problematic.

1. Behavior depends on parse order - this is probably the best reason not to use it.

And that is indeed an excellent reason not to use it.

 

The behavior of how it will fail depends on the order in which code is parsed in Puppet. That is why this method (as well as defined(), and resource collection overrides) should only be used by users who understand this behavior, it's limitations, and how to debug it when they run into issues. It's usage and the fact that it depends on the order in which your code is parsed makes it problematic to have in modules that are intended to be shared.

So ensure_resource() appears to be a solution to module compatibility issues, and sometimes is purported to be such, yet in practice it is "problematic" to use it for that purpose.  Let me expand a bit on "problematic":

• Ensure_resource() is only reliable for avoiding duplicate declaration of a given resource if every declaration of that resource anywhere in the manifest set is via ensure_resource().  It clashes with ordinary resource declarations and also with declarations via create_resources().
  
• The details of a resource declared via ensure_resource() are stable and predictable only if all declarations of that resource (via ensure_resource() and/or any other means) are consistent.
• When working as designed, ensure_resource() actively suppresses any diagnostic information Puppet might otherwise emit regarding the existence of a real or potential compatibility issue.

Now consider: is it wise to duplicate resource details at several places throughout a manifest set, and to undertake the maintenance burden of keeping them all synchronized, especially when you cannot rely on any signal other than possible (but not certain) misconfiguration of your production systems should you fail to meet your maintenance requirements?  Do you really want to risk sudden failure of a Puppet-managed system triggered by modification of the manifests for something completely unrelated?

Ensure_resource() is intended to ease construction and maintenance of complex manifest sets, especially those involving many modules, but using it safely is actually more work than avoiding it.  Moreover, if you use ensure_resource() in a shared module, then you saddle all users of the module with those maintenance issues -- usually unbeknownst to them.

And here's the icing on the cake: the ensure_resource() function creates more work for me simply by existing.  If I want to avoid all of the maintenance issues it presents then I need to audit every third-party module I use.  Without it I may still need to modify third-party modules, but Puppet will tell me when I need to do so.  If nothing else does so, then that puts ensure_resource() over the top into the "evil" category.

In case it was not clear, this is not about unintended uses of ensure_resource(); it is about the function's core use case.  The whole concept of the function is wrongheaded.  If you don't like "evil" then choose another adjective that suits you better, but whatever you do, DO NOT USE ensure_resource().

John

[Henrik Lindberg]

One major problem is containment, if something is declared more than
once, and with different constraints, that needs to be recorded
somewhere - the 3x catalog format does not lend itself easily to
something like that. In the first cut of the 4x catalog model we added
the idea of a proxy resource; if the same resource is required from more
than one place, those places become proxies for the resource that has
the aggregated constraints.

We have not yet drilled any deeper into how the constraints work, what
is considered compatible etc.

When we get to it, there are lots of things to deal with wrt.
containment and dependencies of the proxies (dependencies needs to also
be joined, and may need to propagate to their containers, etc. We will
probably start with solving those issues (with the idea that there is at
least one set of compatibility when resources are identical which gives
us a starting point.


- henrik

[Felix Frank]

I'm torn. While the duplication of resources across modules is a huge
issue, I'm not sure I'd be comfortable with opening this particular box
of Pandora's.

Imagining a world where users can (and will, trust me) declare the same
resource all over the place, I see much potential for bugs, unintended
behavior and strange edge cases.

I have no idea how to safely explore that brave new world (if indeed it
comes to pass), but I'd feel safer with the addition of John's
constraints, honestly.

Pardon the ominous rambling,
Felix

[Ken Miller]

Is this really still true?  From the looks of the stdlib source of ensure_resource it seems like it just parses parameters and calls create_resources .

Trying to understand if this is 3 year old lore or still valid in 2017.

Regards,

== k+ ==

[jcbollinger]

On Monday, October 16, 2017 at 5:33:12 PM UTC-5, Ken Miller wrote:

Is this really still true?  From the looks of the stdlib source of ensure_resource it seems like it just parses parameters and calls create_resources .

Trying to understand if this is 3 year old lore or still valid in 2017.

Nothing has changed with `ensure_resource()`.  The problem is not its implementation details, but rather the nature of the usage modes the function is designed to support.  The whole idea of it is flawed.  We can reprise that discussion if you like, but you already have this very thread and others to which it links from which you can review some of the discussions that we have had before.

John

[Johan Fleury]

Le 17/10/2017 à 08:59, jcbollinger a écrit :
> Nothing has changed with `ensure_resource()`. The problem is not its
> implementation details, but rather the nature of the usage modes the

> function is *designed* to support. The whole idea of it is flawed. We can

> reprise that discussion if you like, but you already have this very thread
> and others to which it links from which you can review some of the
> discussions that we have had before.
>

What is the problem with ensure_resource?

I use it every time I have to write something like:

```
if !defined(Foo['bar']) {
foo { 'bar': }
}
```

Am I doing it wrong?

--
Johan Fleury
PGP Key ID : 0x5D404386805E56E6

[Rob Nelson]

It’s not wrong, but it’s order dependent. Assuming the two or more resource definitions have some variance, you cannot guarantee the resource will be realized as you intend. This you have to be very careful to not have conflicting definitions that could flip flop over time or outright conflict. 

--
You received this message because you are subscribed to the Google Groups "Puppet Users" group.

To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/e7ecdc15-49bc-b6a6-ed64-795c71cca811%40arcaik.net.

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

--

Rob Nelson

[jcbollinger]

On Wednesday, October 18, 2017 at 6:38:51 PM UTC-5, Johan Fleury wrote:

Le 17/10/2017 à 08:59, jcbollinger a écrit :
> Nothing has changed with `ensure_resource()`.  The problem is not its
> implementation details, but rather the nature of the usage modes the
> function is *designed* to support.  The whole idea of it is flawed.  We can
> reprise that discussion if you like, but you already have this very thread
> and others to which it links from which you can review some of the
> discussions that we have had before.
>

What is the problem with ensure_resource? 

So we'll have the reprise, then.

The job of ensure_resource() is to declare a resource in the event that it is not declared.  This is primarily meant help address of module compatibility concerns.  That sounds just peachy, but it presents both conceptual and practical problems.

On the practical side,

• the function doesn't do quite what you would expect.  All other considerations aside, you would like it to declare the specified resource if otherwise it would not have been declared, but what the function actually does is declare the resource if it has not been declared yet when the function is evaluated.  Evaluation-order dependencies such as that produce all kinds of trouble, and in particular,
  
• if another declaration of the same resource is evaluated after the ensure_resource() call, then catalog building will fail with a duplicate resource declaration -- the very thing that using ensure_resource() was supposed to avoid.  In practice, then, ensure_resource() is reliable only if it is used consistently on a per-resource basis.  Also,
• even if all declarations of a given resource were made via ensure_resource(), if in fact there is more than one then there is significant risk that the resource attributes they specify are inconsistent.  This is the very thing that Puppet is designed to avoid by forbidding duplicate resource declarations in the first place, and it can produce the result that the target machine is silently configured differently than you expect.  As an extreme case, consider conflicting  ensure_resource() calls for some resource -- a Package, say -- in which one specifies ensure => 'present' and the other specifies ensure => 'absent'.  Whichever happens to be evaluated first will win, foiling the intent of the other.
• All of that is exacerbated by the fact that Puppet manifest evaluation order is difficult to predict, and somewhat chaotic.  That is, small manifest changes in one place can produce widespread evaluation-order changes across the manifest set.  As a result, your ensure_resource()-dependent manifest set that works today may break tomorrow when some unrelated change is committed, or may not work for the new machine you're about to add that has a different configuration from any previous one.
• I am also inclined to suspect, though I have not seen or worked out the details, that evaluation-order dependencies are prone to play poorly with Puppet's environment caching.

On the conceptual side, the mere existence of ensure_resource() and ensure_resources() influences manifest writers away from best practices.  It's like a creepy guy in a panel van rolling up next to impressionable young Puppeteers and offering them candy.  Or "candy".

The best solution for avoiding duplicate resource declarations is to ensure that every resource is the responsibility of exactly one class (though classes may be responsible for multiple resources).  That naturally avoids any possibility of duplicate declarations via classes' singleton nature, while nevertheless allowing the requirement for that resource, and perhaps related ones, to be expressed independently at multiple points in the manifest set, in the form of include-like class declarations.  Furthermore, if there are conflicting requirements for the resource then those will tend to come quickly to developers attention, and site data can be DRYer.  But doing this properly is more work than just writing resource declarations as ensure_resource() calls, and ensure_resource() makes the immediate pain go away often enough to be tempting.

Oh, and note that almost every word of that applies equally to wrapping ordinary resource declarations in `if !defined()` checks.  `defined()` is evil, too, as I have previously said on more than one occasion.

John

[jcbollinger]

On Tuesday, October 17, 2017 at 7:59:16 AM UTC-5, jcbollinger wrote:

Nothing has changed with `ensure_resource()`.  The problem is not its implementation details, but rather the nature of the usage modes the function is designed to support.  The whole idea of it is flawed.

Funny story:

I have recently been working with an "approved" Forge module, which will go unnamed.  It has a long history, and overall is of high quality.  But it happens to use ensure_resource(), and sure enough, that bit me in the ass: it is the cause of a module bug that crops up under relatively unusual circumstances.  Then when I tried to fix the bug, ensure_resource() did it again.

The failure mode isn't even among those I usually harp on.  Maybe the function's behavior has changed over time, or maybe I never knew this about it, but in Stdlib 4, the function only protects against declaration of identical resources -- it will happily generate a resource with the same title as another that has already been declared, as long as there is a difference between their declared parameters.  This of course elicits a duplicate resource declaration error from Puppet (and this is intentional: function's docs disclose it).

And here's the kicker: after my attempted bugfix, the duplicate declarations in fact are identical, at least as expressed to ensure_resource().  They are rendered non-identical externally by application of a resource override.  Apparently, the override is applied either together with application of the first ensure_resource() call in some way that messes up the resource comparison in the second, or between the effects of the first call and the effects of the second call.  Maybe that's a bug.

To be sure, I would rather have the function fail loudly than break quietly, but I would even more have liked to have a module that doesn't depend on ensure_resource() in the first place.


John
"ensure_resource() is evil"

[Martin Alfke]

I prefer the each lambda over ensure_resource and create_resource.
my 5pc.

> --
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/46001944-4feb-4721-bf6c-b2a1b9daa9b7%40googlegroups.com.

[jcbollinger]

On Wednesday, March 21, 2018 at 4:03:28 AM UTC-5, Martin Alfke wrote:

I prefer the each lambda over ensure_resource and create_resource.
my 5pc. 

In this particular case, the ensure_resource() call is inside a defined type that is used both internally and externally, so I don't think the each() function is going to help me.  But I think I can probably solve the problem (as much as I care to do) by overriding parameters of the defined type instances instead of the objects of ensure_resource().  The cost of that will be that the defined type needs to expose such parameters in the first place, which it otherwise would not need to do, and those parameters will serve an internal purpose rather than an external one.

John

#ensureresourceisevil

[jcbollinger]

... and, silly me, of course that doesn't work.  Overriding the parameters of the defined type lands too late to affect the resource it (maybe) declares indirectly via ensure_resource() -- which is yet another reason why that function is an evil demon toad, lurking in the shadows and waiting to pounce on you and jump up and down on your head.  With nasty, big, pointy teeth.

John

#evildemontoad