On Fri, Aug 30, 2013 at 1:05 AM, R.I.Pienaar <r...@devco.net> wrote:

>
>
> ----- Original Message -----
> > From: "Luke Kanies" <l...@puppetlabs.com>
> > To: puppet-dev@googlegroups.com
> > Sent: Thursday, August 29, 2013 11:27:00 PM
> > Subject: Re: Anchor pattern (was Re: [Puppet-dev] Puppet 4 discussions)
> >
> > On Aug 29, 2013, at 12:24 PM, John Bollinger <john.bollin...@stjude.org>
> > wrote:
> >
> > >
> > >
> > > On Wednesday, August 28, 2013 5:56:45 PM UTC-5, Andy Parker wrote:
> > > On Wed, Aug 28, 2013 at 3:22 PM, Luke Kanies <lu...@puppetlabs.com>
> wrote:
> > > On Aug 28, 2013, at 12:38 PM, Andy Parker <an...@puppetlabs.com>
> wrote:
> > >> On Wed, Aug 28, 2013 at 10:20 AM, Luke Kanies <lu...@puppetlabs.com>
> > >> wrote:
> > >> On Aug 28, 2013, at 8:45 AM, Andy Parker <an...@puppetlabs.com>
> wrote:
> > >>
> > >> >   * #8040 - anchor pattern. I think a solution is in sight, but it
> > >> >   didn't make 3.3.0 and it is looking like it might be backwards
> > >> >   incompatible.
> > >>
> > >> Why would it be incompatible?
> > >>
> > >> That implies that we can't ship it until 4.0, which would be a tragedy
> > >> worth fighting hard to avoid.
> > >>
> > >>
> > >> The only possible problem, that I know of, would be that it would
> change
> > >> the evaluation order. Once things get contained correctly that might
> > >> cause problems. We never give very strong guarantees between versions
> of
> > >> puppet, but given the concern with manifest order, I thought that I
> would
> > >> call this out as well.
> > >
> > > Do you mean, for 2 classes that should have a relationship but
> currently
> > > don't because of the bug (and the lack of someone using an anchor
> pattern
> > > to work around the bug), fixing that bug would cause them to have a
> > > relationship and thus change the order?
> > >
> > >
> > > No that shouldn't be a problem. I think we will be using making the
> > > resource syntax for classes ( class { foo: } ) create the containment
> > > relationship. That doesn't allow multiple declarations and so we
> shouldn't
> > > encounter the problem of the class being in two places.
> > >
> > >
> > > But it does allow multiple declarations, so long as only the first one
> > > parsed uses the parameterized syntax.  There can be any number of other
> > > places where class foo is declared via the include() function or
> require()
> > > function.
> > >
> > >
> > > That is, you're concerned that the bug has been around so long it's
> > > considered a feature, and thus we can't change it except in a major
> > > release?
> > >
> > >
> > > More of just that the class will start being contained in another
> class and
> > > so it will change where it is evaluated in an agent run. That could
> cause
> > > something that worked before to stop working (it only worked before
> > > because of random luck). I'm also, right now, wondering if there are
> > > possible dependency cycles that might show up. I haven't thought that
> one
> > > through.
> > >
> > >
> > > Yes, it is possible that dependency cycles could be created where none
> > > existed before.  About a week ago I added an example to the comments
> > > thread on this issue; it is part of a larger objection to the proposed
> > > solution: http://projects.puppetlabs.com/issues/8040#note-35.  I also
> > > included a proposed alternative solution that could go into Puppet 3.
> >
> > As mentioned in my other email, the solution to this problem should not
> in
> > any way require changes to containment semantics, and certainly shouldn't
> > require class evaluation to indicate class containment.  As I said, it
> used
> > to do that for the first instance (but not for second, which led to some
> > inconsistencies and surprises, which is why I removed it).  These days,
> > though, in general classes only contain resources, not other classes.
>  What
>
> I am not sure I follow and have missed some of this thread while on hols
> but
>  here is why people use the anchor pattern:
>
> class one {
>   include two
>
>   notify{$name: }
> }
>
> class two {
>   notify{$name: }
> }
>
> class three {
>    notify{$name: require => Class["one"]}
> }
>
> include one, three
>
> $ puppet apply test.pp
> Notice: /Stage[main]/One/Notify[one]/message: defined 'message' as 'one'
> Notice: /Stage[main]/Three/Notify[three]/message: defined 'message' as
> 'three'
> Notice: /Stage[main]/Two/Notify[two]/message: defined 'message' as 'two'
> Notice: Finished catalog run in 0.11 seconds
>
> The desired outcome is that Notify[two] is before Notify[three]
>
>
Thank you, RI. I was starting to wonder if Patrick and I were completely
misunderstanding the problem. We have the same understanding as you.

Luke, the example you gave in which you claim that an empty class
disappears doesn't happen. I put together a test and reviewed the code,
just to make sure, and it works exactly as expected. I think your dislike
of whits is confusing you about what the problem actually is and how it
shows up.


> So unless I am reading you wrong, the anchor pattern is used specifically
> because
> today many people have classes contained in other classes and it does not
> work
> as desired.
>
>
Right, and it is a behavior that was explicitly removed (see
https://projects.puppetlabs.com/issues/2423). The catalog simply does not
track the information about a class being inside another class.


> Also I think fixing this only for the new resource like syntax and not for
> include would be a mistake - though i can see why that would be the chosen
> path as doing it otherwise would easily create loops etc.
>
>
The reason for using the resource syntax to enforce containment is because
something can only be contained in one place. If containment is allowed in
multiple places you end up with inexplicable ordering and loops and are
back at #2423. Include has come to mean "I want this in the catalog with no
dependencies", and so adding containment to that would be a problem. There
is also the possibility of some new syntax to allow a "contained" class (a
function could be done, but then parameters would need to be passed in a
hash and it would be a bit odd).

Here are the possibilities:

  * resource like syntax for classes expresses containment:

      class container { class { contained: parameter => value } }

  * a function declares the class *and* expresses containment

      class container { contain(contained, { parameter => value }) }

  * a function expresses containment, but does not declare the class

      class container { class { contained: parameter => value }
contain(contained) }

  * a new "resource type" expresses containment

     class container { contain { contained: parameter => value } }

The two functions could actually be written right now and distributed in a
module, they just need to make sure that when the class is added to the
catalog that it also adds an edge from the container to the contained class
(Patrick spiked this and verified that it works).

Now, the reason that I think that the existing resource syntax is the right
way is that it already allows the class to appear in one place, which is
what we need for reasonable containment. This also gives classes
containment of the exact same form as resources (because they currently get
contained in their class), which is a good symmetry to have. In addition,
there has been talk about allowing duplicate resources in the catalog,
which will open up the whole containment question for resources (what does
it mean for the resource to be in two places?). To solve that we will
probably need some way of "declaring without containment" for resources,
and having classes and resources agree on how to "declare *with*
containment" will make any solution there transfer better to both classes
and resources.

--
> 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+unsubscr...@googlegroups.com.
> To post to this group, send email to puppet-dev@googlegroups.com.
> Visit this group at http://groups.google.com/group/puppet-dev.
> For more options, visit https://groups.google.com/groups/opt_out.
>



-- 
Andrew Parker
a...@puppetlabs.com
Freenode: zaphod42
Twitter: @aparker42
Software Developer

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

-- 
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+unsubscr...@googlegroups.com.
To post to this group, send email to puppet-dev@googlegroups.com.
Visit this group at http://groups.google.com/group/puppet-dev.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to