Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-08 Thread Dan Prince
On Fri, 2015-05-08 at 11:41 -0400, James Slagle wrote:
> On Thu, May 7, 2015 at 5:46 PM, Giulio Fidente  wrote:
> > On 05/07/2015 07:35 PM, Dan Prince wrote:
> >>
> >> On Thu, 2015-05-07 at 17:36 +0200, Giulio Fidente wrote:
> >>>
> >>> On 05/07/2015 03:31 PM, Dan Prince wrote:
> 
>  On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:
> >
> >
> > [...]
> >
> >>> on the other hand, we can very well get rid of the ifs today by
> >>> deploying *with* pacemaker in single node scenario as well! we already
> >>> have EnablePacemaker always set to true for dev purposes, even on single
> >>> node
> >>
> >>
> >> EnablePacemaker is set to 'false' by default. IMO it should be opt-in:
> >>
> >>
> >> http://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=1f7426a014f0f83ace4d2b3531014e22f7778b4d
> >
> >
> > sure that param is false by default, but one can enable it and deploy with
> > pacemaker on single node, and in fact many people do this for dev purposes
> >
> > before that change, we were even running CI on single node with pacemaker so
> > as a matter of fact, one could get rid of the conditionals in the manifest
> > today by just assuming there will be pacemaker
> 
> This is the direction I thought we were moving. When you deploy a
> single controller, it is an HA cluster of 1. As opposed to just not
> using pacemaker entirely. This is the model we did previously for HA
> and I thought it worked well in that it got everyone testing and using
> the same code path.
> 
> I thought the EnablePacemaker parameter was more or less a temporary
> thing to get us over the initial disruption of moving things over to
> pacemaker.

I personally think there may be value in having an option to deploy
without Pacemaker. I would very much like to see TripleO maintain an
option that supports that. The initial goal of EnablePacemaker (as I
understood it) was just this. To support deploying with and without
Pacemaker.

The talk in this thread is largely about the stylistic concerns of using
the $enable_pacemaker boolean within the same manifest and the
maintenance burden this is going to cause. Simply stated it seems
cleaner to just split things out into separate templates based upon the
pacemaker and non-pacemaker version. Given that our goal is to strive
towards minimal manifests (with just includes) this should be a nice
middle ground.

With regards to TripleO upstream defaults I suppose we need more
discussion about this. My understanding was that traditionally TripleO
has been in the keepalived/HAProxy camp for VIP management. Use of
EnablePacemaker would (currently) disable this.

>From a product prospective a company could take either approach and run
w/ it as their default implementation. I think it is also fine for one
approach to evolve ahead of the other.

Dan

> 
> >
> > this said, I prefer myself to leave some "air" for a (future?) non-pacemaker
> > scenario, but I still wanted to point out the reason why the conditionals
> > are there in the first place
> 



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-08 Thread Giulio Fidente

On 05/08/2015 05:41 PM, James Slagle wrote:

On Thu, May 7, 2015 at 5:46 PM, Giulio Fidente  wrote:

On 05/07/2015 07:35 PM, Dan Prince wrote:


On Thu, 2015-05-07 at 17:36 +0200, Giulio Fidente wrote:


On 05/07/2015 03:31 PM, Dan Prince wrote:


On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:



EnablePacemaker is set to 'false' by default. IMO it should be opt-in:


http://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=1f7426a014f0f83ace4d2b3531014e22f7778b4d



sure that param is false by default, but one can enable it and deploy with
pacemaker on single node, and in fact many people do this for dev purposes

before that change, we were even running CI on single node with pacemaker so
as a matter of fact, one could get rid of the conditionals in the manifest
today by just assuming there will be pacemaker


This is the direction I thought we were moving. When you deploy a
single controller, it is an HA cluster of 1. As opposed to just not
using pacemaker entirely. This is the model we did previously for HA
and I thought it worked well in that it got everyone testing and using
the same code path.


indeed this holds true

today if EnablePacemaker is true and ControllerCount is 1 you do get a 
working overcloud, with Pacemaker and 1 controller


the very same overcloud config applies to ControllerCount >= 3


I thought the EnablePacemaker parameter was more or less a temporary
thing to get us over the initial disruption of moving things over to
pacemaker.


not really, purpose of that boolean is to support the deployment of an 
overcloud without using Pacemaker


this is possible today as well, but only with ControllerCount 1

probably, in the future, we will work on the non-pacemaker scenario with 
ControllerCount >= 3 as well, in which case a split of the manifests 
would really be useful


my only concerns on the topics are:

1. where and when to move the parts which are shared amongst the manifests
2. if it is urgent or not to do the split today given the shared parts 
would be duplicated

--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-08 Thread James Slagle
On Thu, May 7, 2015 at 5:46 PM, Giulio Fidente  wrote:
> On 05/07/2015 07:35 PM, Dan Prince wrote:
>>
>> On Thu, 2015-05-07 at 17:36 +0200, Giulio Fidente wrote:
>>>
>>> On 05/07/2015 03:31 PM, Dan Prince wrote:

 On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:
>
>
> [...]
>
>>> on the other hand, we can very well get rid of the ifs today by
>>> deploying *with* pacemaker in single node scenario as well! we already
>>> have EnablePacemaker always set to true for dev purposes, even on single
>>> node
>>
>>
>> EnablePacemaker is set to 'false' by default. IMO it should be opt-in:
>>
>>
>> http://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=1f7426a014f0f83ace4d2b3531014e22f7778b4d
>
>
> sure that param is false by default, but one can enable it and deploy with
> pacemaker on single node, and in fact many people do this for dev purposes
>
> before that change, we were even running CI on single node with pacemaker so
> as a matter of fact, one could get rid of the conditionals in the manifest
> today by just assuming there will be pacemaker

This is the direction I thought we were moving. When you deploy a
single controller, it is an HA cluster of 1. As opposed to just not
using pacemaker entirely. This is the model we did previously for HA
and I thought it worked well in that it got everyone testing and using
the same code path.

I thought the EnablePacemaker parameter was more or less a temporary
thing to get us over the initial disruption of moving things over to
pacemaker.

>
> this said, I prefer myself to leave some "air" for a (future?) non-pacemaker
> scenario, but I still wanted to point out the reason why the conditionals
> are there in the first place

-- 
-- James Slagle
--

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-08 Thread Dan Prince
On Thu, 2015-05-07 at 09:10 -0400, Jay Dobies wrote:
> 
> On 05/07/2015 06:01 AM, Giulio Fidente wrote:
> > On 05/07/2015 11:15 AM, marios wrote:
> >> On 07/05/15 05:32, Dan Prince wrote:
> >
> > [..]
> >
> >>> Something like this:
> >>>
> >>> https://review.openstack.org/#/c/180833/
> >>
> >> +1 I like this as an idea. Given we've already got quite a few reviews
> >> in flight making changes to overcloud_controller.pp (we're still working
> >> out how to, and enabling services) I'd be happier to let those land and
> >> have the tidy up once it settles (early next week at the latest) -
> >> especially since there's some working out+refactoring to do still,
> >
> > +1 on not block ongoing work
> >
> > as of today a split would cause the two .pp to have a lot of duplicated
> > data, making them not better than one with the ifs IMHO
> 
> I'm with Giulio here. I'm not as strong on my puppet as everyone else, 
> but I don't see the current approach as duplication, it's just passing 
> in different configurations.

What about this isn't duplication?


if $enable_pacemaker {

class { 'neutron::agents::metering':
  manage_service => false,
  enabled => false,
}
  pacemaker::resource::service
{ $::neutron::params::metering_agent_service:
clone   => true,
require => Class['::neutron::agents::metering'],
  }
} else {

  include ::neutron::agents::metering

}

---

We'll have this for basically all the pacemaker enabled services. It is
already messy. It will get worse.

Again, One of the goals of our current TripleO puppet manifests
architecture was that our role scripts would be just 'include'
statements for the related puppet classes. Using Hiera as much as
possible, etc. Having two controller templates which have the same
include statements isn't ideal, but I think it is better than the mess
we've got now.

Actually seeing the pacemaker stuff implemented we have a lot of bioler
plate stuff now going into the templates. All the conditional
manage_service => false, enabled => false could be simply stashed into a
new Hiera data file that only gets included w/ that implementation. This
makes the pacemaker version cleaner... something you just can't do with
the current implementation.

Unfortunately I don't know of a way to do any of this without the
resource registry. If it is concerning it would for example be quite
easy for someone developing a product to simply make this resource
registry by default (thus leaving pacemaker on, always). No tuskar or
template changes would be required for this.

Dan

> 
> > we should probably move out of the existing .pp the duplicated parts
> > first (see my other email on the matter)
> 
> My bigger concern is Tuskar. It has the ability to set parameters. It's 
> hasn't moved to a model where you're configuring the overcloud through 
> selecting entries in the resource registry. I can see that making sense 
> in the future, but that's going to require API changes.
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Giulio Fidente

On 05/07/2015 07:35 PM, Dan Prince wrote:

On Thu, 2015-05-07 at 17:36 +0200, Giulio Fidente wrote:

On 05/07/2015 03:31 PM, Dan Prince wrote:

On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:


[...]


on the other hand, we can very well get rid of the ifs today by
deploying *with* pacemaker in single node scenario as well! we already
have EnablePacemaker always set to true for dev purposes, even on single
node


EnablePacemaker is set to 'false' by default. IMO it should be opt-in:

http://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=1f7426a014f0f83ace4d2b3531014e22f7778b4d


sure that param is false by default, but one can enable it and deploy 
with pacemaker on single node, and in fact many people do this for dev 
purposes


before that change, we were even running CI on single node with 
pacemaker so as a matter of fact, one could get rid of the conditionals 
in the manifest today by just assuming there will be pacemaker


this said, I prefer myself to leave some "air" for a (future?) 
non-pacemaker scenario, but I still wanted to point out the reason why 
the conditionals are there in the first place

--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 17:36 +0200, Giulio Fidente wrote:
> On 05/07/2015 03:31 PM, Dan Prince wrote:
> > On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:
> 
> [...]
> 
> >> I think the change is good, I am assuming we don't want the shared parts
> >> to get duplicated into the two .pp though.
> >
> > So again. Duplicating the puppet class includes doesn't bother me too
> > much. Some of the logic (perhaps the DB creation) should move over to
> > puppet-tripleo however. But I would like to see us not go crazy with the
> > composition layer... using the stackforge/puppet modules directly is
> > best I think.
> 
> but it is not only includes really, I understand we would like it to be 
> so, but it isn't
> 
> eg, this would be duplicated, if not moved elsewhere:
> 
> https://github.com/openstack/tripleo-heat-templates/blob/master/puppet/manifests/overcloud_controller.pp#L166-L227
> 
> this as well:
> 
> https://github.com/openstack/tripleo-heat-templates/blob/master/puppet/manifests/overcloud_controller.pp#L296-L333
> 
> and there are quite a lot of similar examples, the change from marios as 
> well, ended up duplicating lots of code:

I would say the choice we make here is a bit of a gray area (A
preference).

Yes, there is some duplication but I suppose my preference (with our
present architecture) is to live with that as opposed to having the
conditional $enable_pacemaker mess that is occurring in
overcloud_controller.pp. I'm mostly looking at things like the rabbitmq
and galera implementations which have duplication w/ either approach
(using $enable_pacemaker or the fork approach I'm suggesting here). For
example:

http://git.openstack.org/cgit/openstack/tripleo-heat-templates/tree/puppet/manifests/overcloud_controller.pp#n229

Like Emilien points out a wrapper approach could significantly clean up
some things... but we don't have that yet w/ puppet-pacemaker. And also,
that moves us closer to use a composition layer anyways. All points
worth considering but I still think forking the two implementations
for now may be cleaner in the short term while we evolve our manifests.

Dan

> 
> https://review.openstack.org/#/c/180833/
> 
> > Any conversion code in Puppet (functions using split, downcase, etc) I
> > view as technical debt which should ideally we would eventually be able
> > to convert within the Heat templates themselves into formats usable by
> > Hiera directly. Any duplication around that sort of thing would
> > eventually get cleaned up as Heat gets an extra function or two.
> 
> FWIW, I do agree with the longish-term plan, most of the duplicated code 
> *should go away when some more string manipulation can be covered by 
> heat* but I still think that will be some of it, not all and yet this 
> isn't the case today (and I don't know when it will be honestly)
> 
> I think a split will still be worth some duplication when we will start 
> supporting *multiple controllers without pacemaker* as well, today not 
> so much
> 
> on the other hand, we can very well get rid of the ifs today by 
> deploying *with* pacemaker in single node scenario as well! we already 
> have EnablePacemaker always set to true for dev purposes, even on single 
> node

EnablePacemaker is set to 'false' by default. IMO it should be opt-in:

http://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=1f7426a014f0f83ace4d2b3531014e22f7778b4d

Dan




__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 10:42 -0400, Emilien Macchi wrote:
> 
> On 05/06/2015 10:32 PM, Dan Prince wrote:
> (...)
> > 
> > I think this split is a good compromise and would probably even speed up
> > the implementation of the remaining pacemaker features too. And removing
> > all the pacemaker conditionals we have from the non-pacemaker version
> > puts us back in a reasonably clean state I think.
> > 
> 
> I don't really like having a "fork" of the controller code, which could
> become pain to maintain. (ie: each new feature on controllers has to be
> coded in both manifests).

I wouldn't say the fork is ideal. But I do like it better than having
one big unreadable manifest. Keep in mind the Heat templates for both
implementations are mostly shared... so I think it is a reasonable
compromise to keep the code cleaner than it is with inline conditionals
for all pacemaker services.

This wasn't something that was planned... rather it just evolved. And
now actually seeing the pacemaker stuff I'd like to isolate it entirely
from the non-pacemaker implementation. If down the road we get a wrapper
and can cleanly de-duplicate some things then so be it.


> 
> I really prefer having parameters & Hiera in the party to enable or not
> Pacemaker.
> 
> FWIW, here is how we did in SpinalStack:
> 
> 1) Install Pacemaker / Corosync:
> https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/clustering.pp
> 
> 2) Create a wrapper to manage resources:
> https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/clustering/pacemaker_service.pp
> (that use collocation and order wrappers too)
> 
> 3) Example for Nova API: disable Pacemaker by default:
> https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L74
> 
> 4) Since puppet-openstack_extras manage the service in the catalog with
> Pacemaker, the code is the same for Pacemaker / non-pacemaker usage:
> https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L81-L90
> 
> 5) If pacemaker is enabled (yes, we have a condition...): we enable the
> resource:
> https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L81-L90
> 
> 
> The pacemaker wrapper is
> https://github.com/stackforge/puppet-openstack_extras/blob/master/manifests/pacemaker/service.pp
> so 100% upstream.
> We are using puppetlabs-corosync and not puppet-pacemaker so the code is
> much cleaner and Puppetish (providers, etc).
> The code is really lightweight in the composition layer and flexible to
> add more options easily.
> 
> We are using puppet-pacemaker now, so we can't (now at least) use
> openstack_extras wrapper, but still we can take this approach in
> consideration.

Right. I think this is the rub. puppet-pacemaker doesn't support the
wrapper. And for whatever reason we have choosen to use it instead of
the corosync based version. This is why we've ended up where we are at
today.

> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 10:26 -0400, Jay Dobies wrote:
> >>> Something like this:
> >>>
> >>> https://review.openstack.org/#/c/180833/
> 
> I'm not convinced this is a good user experience though. You have 
> configuration effectively in two places. If you want to enable Galera, 
> or enable ceph storage, it's a parameter. But not pacemaker. To enable 
> that, you have to look in the resource registry instead.

I'm not sure there is a better way to architect it that doesn't use the
resource registry. Heat doesn't support get_file: {get_param:
ManifestName} so we can't dynamically select an alternate template via a
parameter either.

We enable Puppet by changing the resource registry. Additionally, it is
very likely other things (network architecture related) are going to
make heavy use of the resource registry as well.

Yes, it is different from some of the other feature parameters... but I
think that is probably okay. In this regard maybe pacemaker is more of
an overlay rather than something you just enable via a parameter.

Dan

> 
> >
> >> I have two mild concerns about this approach:
> >>
> >> 1) We'd duplicate the logic (or at least the inclusion logic) for the
> >> common parts in two places, making it prone for the two .pp variants to
> >> get out of sync. The default switches from "if i want to make a
> >> difference between the two variants, i need to put in a conditional" to
> >> "if i want to *not* make a difference between the two variants, i need
> >> to put this / include this in two places".
> >
> > The goal for these manifests is that we would just be doing 'include's
> > for various stackforge puppet modules. If we have
> > 'include ::glance::api' in two places that doesn't really bother me.
> > Agree that it isn't ideal but I don't think it bothers me too much. And
> > the benefit is we can get rid of pacemaker conditionals for all the
> > things.
> >
> >>
> >> 2) If we see some other bit emerging in the future, which would be
> >> optional but at the same time "omnipresent" in a similar way as
> >> Pacemaker is, we'll see the same if/else pattern popping up. Using the
> >> same solution would mean we'd have 4 .pp files (a 2x2 matrix) doing the
> >> same thing to cover all scenarios. This is a somewhat hypothetical
> >> concern at this point, but it might become real in the future (?).
> >
> > Sure. It could happen. But again maintaining all of those in a single
> > file could be quite a mess too. And if we are striving to set all or our
> > Hiera data in Heat (avoiding use of some of the puppet functions we now
> > make use of like split, etc) this would further de-duplicate it I think.
> >
> > Again having duplication that includes just the raw puppet classes
> > doesn't bother me too much.
> >
> >>
> >>>
> >>> If we were to split out the controller into two separate templates I
> >>> think it might be appropriate to move a few things into puppet-tripleo
> >>> to de-duplicate a bit. Things like the database creation for example.
> >>> But probably not all of the services... because we are trying as much as
> >>> possible to use the stackforge puppet modules directly (and not our own
> >>> composition layer).
> >>
> >> I think our restraint from having a composition layer (extracting things
> >> into puppet-tripleo) is what's behind my concern no. 1 above. I know one
> >> of the arguments against having a composition layer is that it makes
> >> things less hackable, but if we could amend puppet modules without
> >> rebuilding or altering the image, it should mitigate the problem a bit
> >> [1]. (It's almost a matter that would deserve a separate thread though :) )
> >>
> >>>
> >>> I think this split is a good compromise and would probably even speed up
> >>> the implementation of the remaining pacemaker features too. And removing
> >>> all the pacemaker conditionals we have from the non-pacemaker version
> >>> puts us back in a reasonably clean state I think.
> >>>
> >>> Dan
> >>>
> >>
> >> An alternative approach could be something like:
> >>
> >> if hiera('step') >= 2 {
> >>   include ::tripleo::mongodb
> >> }
> >>
> >> and move all the mongodb related logic to that class and let it deal
> >> with both pacemaker and non-pacemaker use cases. This would reduce the
> >> stress on the top-level .pp significantly, and we'd keep things
> >> contained in logical units. The extracted bits will still have
> >> conditionals but it's going to be more manageable because the bits will
> >> be a lot smaller. So this would mean splitting up the manifest per
> >> service rather than based on pacemaker on/off status. This would require
> >> more extraction into puppet-tripleo though, so it kinda goes against the
> >> idea of not having a composition layer. It would also probably consume a
> >> bit more time to implement initially and be more disruptive to the
> >> current state of things.
> >>
> >> At this point i don't lean strongly towards one or the other solution, i
> >> just want us to have an o

Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Giulio Fidente

On 05/07/2015 05:36 PM, Giulio Fidente wrote:

On 05/07/2015 03:31 PM, Dan Prince wrote:

On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:


[...]


and there are quite a lot of similar examples, the change from marios as
well, ended up duplicating lots of code:

https://review.openstack.org/#/c/180833/


here I should have linked this change instead:

https://review.openstack.org/#/c/181015/1

from marios, built on your first
--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Giulio Fidente

On 05/07/2015 03:31 PM, Dan Prince wrote:

On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:


[...]


I think the change is good, I am assuming we don't want the shared parts
to get duplicated into the two .pp though.


So again. Duplicating the puppet class includes doesn't bother me too
much. Some of the logic (perhaps the DB creation) should move over to
puppet-tripleo however. But I would like to see us not go crazy with the
composition layer... using the stackforge/puppet modules directly is
best I think.


but it is not only includes really, I understand we would like it to be 
so, but it isn't


eg, this would be duplicated, if not moved elsewhere:

https://github.com/openstack/tripleo-heat-templates/blob/master/puppet/manifests/overcloud_controller.pp#L166-L227

this as well:

https://github.com/openstack/tripleo-heat-templates/blob/master/puppet/manifests/overcloud_controller.pp#L296-L333

and there are quite a lot of similar examples, the change from marios as 
well, ended up duplicating lots of code:


https://review.openstack.org/#/c/180833/


Any conversion code in Puppet (functions using split, downcase, etc) I
view as technical debt which should ideally we would eventually be able
to convert within the Heat templates themselves into formats usable by
Hiera directly. Any duplication around that sort of thing would
eventually get cleaned up as Heat gets an extra function or two.


FWIW, I do agree with the longish-term plan, most of the duplicated code 
*should go away when some more string manipulation can be covered by 
heat* but I still think that will be some of it, not all and yet this 
isn't the case today (and I don't know when it will be honestly)


I think a split will still be worth some duplication when we will start 
supporting *multiple controllers without pacemaker* as well, today not 
so much


on the other hand, we can very well get rid of the ifs today by 
deploying *with* pacemaker in single node scenario as well! we already 
have EnablePacemaker always set to true for dev purposes, even on single 
node

--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Emilien Macchi


On 05/06/2015 10:32 PM, Dan Prince wrote:
(...)
> 
> I think this split is a good compromise and would probably even speed up
> the implementation of the remaining pacemaker features too. And removing
> all the pacemaker conditionals we have from the non-pacemaker version
> puts us back in a reasonably clean state I think.
> 

I don't really like having a "fork" of the controller code, which could
become pain to maintain. (ie: each new feature on controllers has to be
coded in both manifests).

I really prefer having parameters & Hiera in the party to enable or not
Pacemaker.

FWIW, here is how we did in SpinalStack:

1) Install Pacemaker / Corosync:
https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/clustering.pp

2) Create a wrapper to manage resources:
https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/clustering/pacemaker_service.pp
(that use collocation and order wrappers too)

3) Example for Nova API: disable Pacemaker by default:
https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L74

4) Since puppet-openstack_extras manage the service in the catalog with
Pacemaker, the code is the same for Pacemaker / non-pacemaker usage:
https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L81-L90

5) If pacemaker is enabled (yes, we have a condition...): we enable the
resource:
https://github.com/stackforge/puppet-openstack-cloud/blob/master/manifests/compute/api.pp#L81-L90


The pacemaker wrapper is
https://github.com/stackforge/puppet-openstack_extras/blob/master/manifests/pacemaker/service.pp
so 100% upstream.
We are using puppetlabs-corosync and not puppet-pacemaker so the code is
much cleaner and Puppetish (providers, etc).
The code is really lightweight in the composition layer and flexible to
add more options easily.

We are using puppet-pacemaker now, so we can't (now at least) use
openstack_extras wrapper, but still we can take this approach in
consideration.
-- 
Emilien Macchi



signature.asc
Description: OpenPGP digital signature
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Jay Dobies

Something like this:

https://review.openstack.org/#/c/180833/


I'm not convinced this is a good user experience though. You have 
configuration effectively in two places. If you want to enable Galera, 
or enable ceph storage, it's a parameter. But not pacemaker. To enable 
that, you have to look in the resource registry instead.





I have two mild concerns about this approach:

1) We'd duplicate the logic (or at least the inclusion logic) for the
common parts in two places, making it prone for the two .pp variants to
get out of sync. The default switches from "if i want to make a
difference between the two variants, i need to put in a conditional" to
"if i want to *not* make a difference between the two variants, i need
to put this / include this in two places".


The goal for these manifests is that we would just be doing 'include's
for various stackforge puppet modules. If we have
'include ::glance::api' in two places that doesn't really bother me.
Agree that it isn't ideal but I don't think it bothers me too much. And
the benefit is we can get rid of pacemaker conditionals for all the
things.



2) If we see some other bit emerging in the future, which would be
optional but at the same time "omnipresent" in a similar way as
Pacemaker is, we'll see the same if/else pattern popping up. Using the
same solution would mean we'd have 4 .pp files (a 2x2 matrix) doing the
same thing to cover all scenarios. This is a somewhat hypothetical
concern at this point, but it might become real in the future (?).


Sure. It could happen. But again maintaining all of those in a single
file could be quite a mess too. And if we are striving to set all or our
Hiera data in Heat (avoiding use of some of the puppet functions we now
make use of like split, etc) this would further de-duplicate it I think.

Again having duplication that includes just the raw puppet classes
doesn't bother me too much.





If we were to split out the controller into two separate templates I
think it might be appropriate to move a few things into puppet-tripleo
to de-duplicate a bit. Things like the database creation for example.
But probably not all of the services... because we are trying as much as
possible to use the stackforge puppet modules directly (and not our own
composition layer).


I think our restraint from having a composition layer (extracting things
into puppet-tripleo) is what's behind my concern no. 1 above. I know one
of the arguments against having a composition layer is that it makes
things less hackable, but if we could amend puppet modules without
rebuilding or altering the image, it should mitigate the problem a bit
[1]. (It's almost a matter that would deserve a separate thread though :) )



I think this split is a good compromise and would probably even speed up
the implementation of the remaining pacemaker features too. And removing
all the pacemaker conditionals we have from the non-pacemaker version
puts us back in a reasonably clean state I think.

Dan



An alternative approach could be something like:

if hiera('step') >= 2 {
  include ::tripleo::mongodb
}

and move all the mongodb related logic to that class and let it deal
with both pacemaker and non-pacemaker use cases. This would reduce the
stress on the top-level .pp significantly, and we'd keep things
contained in logical units. The extracted bits will still have
conditionals but it's going to be more manageable because the bits will
be a lot smaller. So this would mean splitting up the manifest per
service rather than based on pacemaker on/off status. This would require
more extraction into puppet-tripleo though, so it kinda goes against the
idea of not having a composition layer. It would also probably consume a
bit more time to implement initially and be more disruptive to the
current state of things.

At this point i don't lean strongly towards one or the other solution, i
just want us to have an option to discuss and consider benefits and
drawbacks of both, so that we can take an informed decision. I think i
need to let this sink in a bit more myself.


Cheers

Jirka

[1] https://review.openstack.org/#/c/179177/

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread marios
On 07/05/15 16:34, Dan Prince wrote:
> On Thu, 2015-05-07 at 12:15 +0300, marios wrote:
>> On 07/05/15 05:32, Dan Prince wrote:
>>> Looking over some of the Puppet pacemaker stuff today. I appreciate all
>>> the hard work going into this effort but I'm not quite happy about all
>>> of the conditionals we are adding to our puppet overcloud_controller.pp
>>> manifest. Specifically it seems that every service will basically have
>>> its resources duplicated for pacemaker and non-pacemaker version of the
>>> controller by checking the $enable_pacemaker variable.
>>>
>>> After seeing it play out for a couple services I think I might prefer it
>>> better if we had an entirely separate template for the "pacemaker"
>>> version of the controller. One easy way to kick off this effort would be
>>> to use the Heat resource registry to enable pacemaker rather than a
>>> parameter.
>>>
>>> Something like this:
>>>
>>> https://review.openstack.org/#/c/180833/
>>
>> +1 I like this as an idea. Given we've already got quite a few reviews
>> in flight making changes to overcloud_controller.pp (we're still working
>> out how to, and enabling services) I'd be happier to let those land and
>> have the tidy up once it settles (early next week at the latest) -
>> especially since there's some working out+refactoring to do still,
> 
> My preference would be that we not go any further down the path of using
> $enable_pacemaker in the overcloud_controller.pp template.
> 
> I don't think it would be that hard to convert existing reviews to use
> the new file would it? And removing the conditionals would just make it
> read more cleanly too.

something like this should do it?:

 https://review.openstack.org/#/c/181015/1

I rebased onto yours and moved the enable_pacemaker stuff. If this is
what we want to do then I can rebase my other two dependent patches too
and do the same,

marios

> 
> Dan
> 
>>
>> thanks, marios
>>
>>>
>>> If we were to split out the controller into two separate templates I
>>> think it might be appropriate to move a few things into puppet-tripleo
>>> to de-duplicate a bit. Things like the database creation for example.
>>> But probably not all of the services... because we are trying as much as
>>> possible to use the stackforge puppet modules directly (and not our own
>>> composition layer).
>>>
>>> I think this split is a good compromise and would probably even speed up
>>> the implementation of the remaining pacemaker features too. And removing
>>> all the pacemaker conditionals we have from the non-pacemaker version
>>> puts us back in a reasonably clean state I think.
>>>
>>> Dan
>>>
>>>
>>> __
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>>
>> __
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 12:15 +0300, marios wrote:
> On 07/05/15 05:32, Dan Prince wrote:
> > Looking over some of the Puppet pacemaker stuff today. I appreciate all
> > the hard work going into this effort but I'm not quite happy about all
> > of the conditionals we are adding to our puppet overcloud_controller.pp
> > manifest. Specifically it seems that every service will basically have
> > its resources duplicated for pacemaker and non-pacemaker version of the
> > controller by checking the $enable_pacemaker variable.
> > 
> > After seeing it play out for a couple services I think I might prefer it
> > better if we had an entirely separate template for the "pacemaker"
> > version of the controller. One easy way to kick off this effort would be
> > to use the Heat resource registry to enable pacemaker rather than a
> > parameter.
> > 
> > Something like this:
> > 
> > https://review.openstack.org/#/c/180833/
> 
> +1 I like this as an idea. Given we've already got quite a few reviews
> in flight making changes to overcloud_controller.pp (we're still working
> out how to, and enabling services) I'd be happier to let those land and
> have the tidy up once it settles (early next week at the latest) -
> especially since there's some working out+refactoring to do still,

My preference would be that we not go any further down the path of using
$enable_pacemaker in the overcloud_controller.pp template.

I don't think it would be that hard to convert existing reviews to use
the new file would it? And removing the conditionals would just make it
read more cleanly too.

Dan

> 
> thanks, marios
> 
> > 
> > If we were to split out the controller into two separate templates I
> > think it might be appropriate to move a few things into puppet-tripleo
> > to de-duplicate a bit. Things like the database creation for example.
> > But probably not all of the services... because we are trying as much as
> > possible to use the stackforge puppet modules directly (and not our own
> > composition layer).
> > 
> > I think this split is a good compromise and would probably even speed up
> > the implementation of the remaining pacemaker features too. And removing
> > all the pacemaker conditionals we have from the non-pacemaker version
> > puts us back in a reasonably clean state I think.
> > 
> > Dan
> > 
> > 
> > __
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> > 
> 
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 11:22 +0200, Giulio Fidente wrote:
> hi Dan!
> 
> On 05/07/2015 04:32 AM, Dan Prince wrote:
> > Looking over some of the Puppet pacemaker stuff today. I appreciate all
> > the hard work going into this effort but I'm not quite happy about all
> > of the conditionals we are adding to our puppet overcloud_controller.pp
> > manifest. Specifically it seems that every service will basically have
> > its resources duplicated for pacemaker and non-pacemaker version of the
> > controller by checking the $enable_pacemaker variable.
> 
> not sure about the meaning of 'resources duplicated' but I think it is 
> safe to say that the pacemaker ifs are there coping mainly with the 
> following two:
> 
> 1. when pacemaker, we don't want puppet to enable/start the service, 
> pacemaker will manage so we need to tell the module not to
> 
> 2. when pacemaker, there are some pacemaker related steps to be 
> performed, like adding a resource into the cluster so that it is 
> effectively monitoring the service status
> 
> in the future, we might need to pass some specific config params to a 
> module only when pacemaker, but that looks like covered by 1) already
> 
> > After seeing it play out for a couple services I think I might prefer it
> > better if we had an entirely separate template for the "pacemaker"
> > version of the controller. One easy way to kick off this effort would be
> > to use the Heat resource registry to enable pacemaker rather than a
> > parameter.
> >
> > Something like this:
> >
> > https://review.openstack.org/#/c/180833/
> >
> > If we were to split out the controller into two separate templates I
> > think it might be appropriate to move a few things into puppet-tripleo
> > to de-duplicate a bit. Things like the database creation for example.
> > But probably not all of the services... because we are trying as much as
> > possible to use the stackforge puppet modules directly (and not our own
> > composition layer).
> 
> I think the change is good, I am assuming we don't want the shared parts 
> to get duplicated into the two .pp though.

So again. Duplicating the puppet class includes doesn't bother me too
much. Some of the logic (perhaps the DB creation) should move over to
puppet-tripleo however. But I would like to see us not go crazy with the
composition layer... using the stackforge/puppet modules directly is
best I think.


Any conversion code in Puppet (functions using split, downcase, etc) I
view as technical debt which should ideally we would eventually be able
to convert within the Heat templates themselves into formats usable by
Hiera directly. Any duplication around that sort of thing would
eventually get cleaned up as Heat gets an extra function or two.

Dan

> 
> What is your idea about those shared parts? To move them into 
> puppet-tripleo? To provision a shared .pp in addition to a 
> differentiated top-level template maybe? Something else?



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Dan Prince
On Thu, 2015-05-07 at 11:56 +0200, Jiří Stránský wrote:
> Hi Dan,
> 
> On 7.5.2015 04:32, Dan Prince wrote:
> > Looking over some of the Puppet pacemaker stuff today. I appreciate all
> > the hard work going into this effort but I'm not quite happy about all
> > of the conditionals we are adding to our puppet overcloud_controller.pp
> > manifest. Specifically it seems that every service will basically have
> > its resources duplicated for pacemaker and non-pacemaker version of the
> > controller by checking the $enable_pacemaker variable.
> 
> +1
> 
> >
> > After seeing it play out for a couple services I think I might prefer it
> > better if we had an entirely separate template for the "pacemaker"
> > version of the controller. One easy way to kick off this effort would be
> > to use the Heat resource registry to enable pacemaker rather than a
> > parameter.
> >
> > Something like this:
> >
> > https://review.openstack.org/#/c/180833/
> 
> I have two mild concerns about this approach:
> 
> 1) We'd duplicate the logic (or at least the inclusion logic) for the 
> common parts in two places, making it prone for the two .pp variants to 
> get out of sync. The default switches from "if i want to make a 
> difference between the two variants, i need to put in a conditional" to 
> "if i want to *not* make a difference between the two variants, i need 
> to put this / include this in two places".

The goal for these manifests is that we would just be doing 'include's
for various stackforge puppet modules. If we have
'include ::glance::api' in two places that doesn't really bother me.
Agree that it isn't ideal but I don't think it bothers me too much. And
the benefit is we can get rid of pacemaker conditionals for all the
things.

> 
> 2) If we see some other bit emerging in the future, which would be 
> optional but at the same time "omnipresent" in a similar way as 
> Pacemaker is, we'll see the same if/else pattern popping up. Using the 
> same solution would mean we'd have 4 .pp files (a 2x2 matrix) doing the 
> same thing to cover all scenarios. This is a somewhat hypothetical 
> concern at this point, but it might become real in the future (?).

Sure. It could happen. But again maintaining all of those in a single
file could be quite a mess too. And if we are striving to set all or our
Hiera data in Heat (avoiding use of some of the puppet functions we now
make use of like split, etc) this would further de-duplicate it I think.

Again having duplication that includes just the raw puppet classes
doesn't bother me too much.

> 
> >
> > If we were to split out the controller into two separate templates I
> > think it might be appropriate to move a few things into puppet-tripleo
> > to de-duplicate a bit. Things like the database creation for example.
> > But probably not all of the services... because we are trying as much as
> > possible to use the stackforge puppet modules directly (and not our own
> > composition layer).
> 
> I think our restraint from having a composition layer (extracting things 
> into puppet-tripleo) is what's behind my concern no. 1 above. I know one 
> of the arguments against having a composition layer is that it makes 
> things less hackable, but if we could amend puppet modules without 
> rebuilding or altering the image, it should mitigate the problem a bit 
> [1]. (It's almost a matter that would deserve a separate thread though :) )
> 
> >
> > I think this split is a good compromise and would probably even speed up
> > the implementation of the remaining pacemaker features too. And removing
> > all the pacemaker conditionals we have from the non-pacemaker version
> > puts us back in a reasonably clean state I think.
> >
> > Dan
> >
> 
> An alternative approach could be something like:
> 
> if hiera('step') >= 2 {
>  include ::tripleo::mongodb
> }
> 
> and move all the mongodb related logic to that class and let it deal 
> with both pacemaker and non-pacemaker use cases. This would reduce the 
> stress on the top-level .pp significantly, and we'd keep things 
> contained in logical units. The extracted bits will still have 
> conditionals but it's going to be more manageable because the bits will 
> be a lot smaller. So this would mean splitting up the manifest per 
> service rather than based on pacemaker on/off status. This would require 
> more extraction into puppet-tripleo though, so it kinda goes against the 
> idea of not having a composition layer. It would also probably consume a 
> bit more time to implement initially and be more disruptive to the 
> current state of things.
> 
> At this point i don't lean strongly towards one or the other solution, i 
> just want us to have an option to discuss and consider benefits and 
> drawbacks of both, so that we can take an informed decision. I think i 
> need to let this sink in a bit more myself.
> 
> 
> Cheers
> 
> Jirka
> 
> [1] https://review.openstack.org/#/c/179177/
> 
> 

Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Jay Dobies



On 05/07/2015 06:01 AM, Giulio Fidente wrote:

On 05/07/2015 11:15 AM, marios wrote:

On 07/05/15 05:32, Dan Prince wrote:


[..]


Something like this:

https://review.openstack.org/#/c/180833/


+1 I like this as an idea. Given we've already got quite a few reviews
in flight making changes to overcloud_controller.pp (we're still working
out how to, and enabling services) I'd be happier to let those land and
have the tidy up once it settles (early next week at the latest) -
especially since there's some working out+refactoring to do still,


+1 on not block ongoing work

as of today a split would cause the two .pp to have a lot of duplicated
data, making them not better than one with the ifs IMHO


I'm with Giulio here. I'm not as strong on my puppet as everyone else, 
but I don't see the current approach as duplication, it's just passing 
in different configurations.



we should probably move out of the existing .pp the duplicated parts
first (see my other email on the matter)


My bigger concern is Tuskar. It has the ability to set parameters. It's 
hasn't moved to a model where you're configuring the overcloud through 
selecting entries in the resource registry. I can see that making sense 
in the future, but that's going to require API changes.


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Giulio Fidente

On 05/07/2015 11:15 AM, marios wrote:

On 07/05/15 05:32, Dan Prince wrote:


[..]


Something like this:

https://review.openstack.org/#/c/180833/


+1 I like this as an idea. Given we've already got quite a few reviews
in flight making changes to overcloud_controller.pp (we're still working
out how to, and enabling services) I'd be happier to let those land and
have the tidy up once it settles (early next week at the latest) -
especially since there's some working out+refactoring to do still,


+1 on not block ongoing work

as of today a split would cause the two .pp to have a lot of duplicated 
data, making them not better than one with the ifs IMHO


we should probably move out of the existing .pp the duplicated parts 
first (see my other email on the matter)

--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Jiří Stránský

Hi Dan,

On 7.5.2015 04:32, Dan Prince wrote:

Looking over some of the Puppet pacemaker stuff today. I appreciate all
the hard work going into this effort but I'm not quite happy about all
of the conditionals we are adding to our puppet overcloud_controller.pp
manifest. Specifically it seems that every service will basically have
its resources duplicated for pacemaker and non-pacemaker version of the
controller by checking the $enable_pacemaker variable.


+1



After seeing it play out for a couple services I think I might prefer it
better if we had an entirely separate template for the "pacemaker"
version of the controller. One easy way to kick off this effort would be
to use the Heat resource registry to enable pacemaker rather than a
parameter.

Something like this:

https://review.openstack.org/#/c/180833/


I have two mild concerns about this approach:

1) We'd duplicate the logic (or at least the inclusion logic) for the 
common parts in two places, making it prone for the two .pp variants to 
get out of sync. The default switches from "if i want to make a 
difference between the two variants, i need to put in a conditional" to 
"if i want to *not* make a difference between the two variants, i need 
to put this / include this in two places".


2) If we see some other bit emerging in the future, which would be 
optional but at the same time "omnipresent" in a similar way as 
Pacemaker is, we'll see the same if/else pattern popping up. Using the 
same solution would mean we'd have 4 .pp files (a 2x2 matrix) doing the 
same thing to cover all scenarios. This is a somewhat hypothetical 
concern at this point, but it might become real in the future (?).




If we were to split out the controller into two separate templates I
think it might be appropriate to move a few things into puppet-tripleo
to de-duplicate a bit. Things like the database creation for example.
But probably not all of the services... because we are trying as much as
possible to use the stackforge puppet modules directly (and not our own
composition layer).


I think our restraint from having a composition layer (extracting things 
into puppet-tripleo) is what's behind my concern no. 1 above. I know one 
of the arguments against having a composition layer is that it makes 
things less hackable, but if we could amend puppet modules without 
rebuilding or altering the image, it should mitigate the problem a bit 
[1]. (It's almost a matter that would deserve a separate thread though :) )




I think this split is a good compromise and would probably even speed up
the implementation of the remaining pacemaker features too. And removing
all the pacemaker conditionals we have from the non-pacemaker version
puts us back in a reasonably clean state I think.

Dan



An alternative approach could be something like:

if hiera('step') >= 2 {
include ::tripleo::mongodb
}

and move all the mongodb related logic to that class and let it deal 
with both pacemaker and non-pacemaker use cases. This would reduce the 
stress on the top-level .pp significantly, and we'd keep things 
contained in logical units. The extracted bits will still have 
conditionals but it's going to be more manageable because the bits will 
be a lot smaller. So this would mean splitting up the manifest per 
service rather than based on pacemaker on/off status. This would require 
more extraction into puppet-tripleo though, so it kinda goes against the 
idea of not having a composition layer. It would also probably consume a 
bit more time to implement initially and be more disruptive to the 
current state of things.


At this point i don't lean strongly towards one or the other solution, i 
just want us to have an option to discuss and consider benefits and 
drawbacks of both, so that we can take an informed decision. I think i 
need to let this sink in a bit more myself.



Cheers

Jirka

[1] https://review.openstack.org/#/c/179177/

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread Giulio Fidente

hi Dan!

On 05/07/2015 04:32 AM, Dan Prince wrote:

Looking over some of the Puppet pacemaker stuff today. I appreciate all
the hard work going into this effort but I'm not quite happy about all
of the conditionals we are adding to our puppet overcloud_controller.pp
manifest. Specifically it seems that every service will basically have
its resources duplicated for pacemaker and non-pacemaker version of the
controller by checking the $enable_pacemaker variable.


not sure about the meaning of 'resources duplicated' but I think it is 
safe to say that the pacemaker ifs are there coping mainly with the 
following two:


1. when pacemaker, we don't want puppet to enable/start the service, 
pacemaker will manage so we need to tell the module not to


2. when pacemaker, there are some pacemaker related steps to be 
performed, like adding a resource into the cluster so that it is 
effectively monitoring the service status


in the future, we might need to pass some specific config params to a 
module only when pacemaker, but that looks like covered by 1) already



After seeing it play out for a couple services I think I might prefer it
better if we had an entirely separate template for the "pacemaker"
version of the controller. One easy way to kick off this effort would be
to use the Heat resource registry to enable pacemaker rather than a
parameter.

Something like this:

https://review.openstack.org/#/c/180833/

If we were to split out the controller into two separate templates I
think it might be appropriate to move a few things into puppet-tripleo
to de-duplicate a bit. Things like the database creation for example.
But probably not all of the services... because we are trying as much as
possible to use the stackforge puppet modules directly (and not our own
composition layer).


I think the change is good, I am assuming we don't want the shared parts 
to get duplicated into the two .pp though.


What is your idea about those shared parts? To move them into 
puppet-tripleo? To provision a shared .pp in addition to a 
differentiated top-level template maybe? Something else?

--
Giulio Fidente
GPG KEY: 08D733BA

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-07 Thread marios
On 07/05/15 05:32, Dan Prince wrote:
> Looking over some of the Puppet pacemaker stuff today. I appreciate all
> the hard work going into this effort but I'm not quite happy about all
> of the conditionals we are adding to our puppet overcloud_controller.pp
> manifest. Specifically it seems that every service will basically have
> its resources duplicated for pacemaker and non-pacemaker version of the
> controller by checking the $enable_pacemaker variable.
> 
> After seeing it play out for a couple services I think I might prefer it
> better if we had an entirely separate template for the "pacemaker"
> version of the controller. One easy way to kick off this effort would be
> to use the Heat resource registry to enable pacemaker rather than a
> parameter.
> 
> Something like this:
> 
> https://review.openstack.org/#/c/180833/

+1 I like this as an idea. Given we've already got quite a few reviews
in flight making changes to overcloud_controller.pp (we're still working
out how to, and enabling services) I'd be happier to let those land and
have the tidy up once it settles (early next week at the latest) -
especially since there's some working out+refactoring to do still,

thanks, marios

> 
> If we were to split out the controller into two separate templates I
> think it might be appropriate to move a few things into puppet-tripleo
> to de-duplicate a bit. Things like the database creation for example.
> But probably not all of the services... because we are trying as much as
> possible to use the stackforge puppet modules directly (and not our own
> composition layer).
> 
> I think this split is a good compromise and would probably even speed up
> the implementation of the remaining pacemaker features too. And removing
> all the pacemaker conditionals we have from the non-pacemaker version
> puts us back in a reasonably clean state I think.
> 
> Dan
> 
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


[openstack-dev] [TripleO] puppet pacemaker thoughts... and an idea

2015-05-06 Thread Dan Prince
Looking over some of the Puppet pacemaker stuff today. I appreciate all
the hard work going into this effort but I'm not quite happy about all
of the conditionals we are adding to our puppet overcloud_controller.pp
manifest. Specifically it seems that every service will basically have
its resources duplicated for pacemaker and non-pacemaker version of the
controller by checking the $enable_pacemaker variable.

After seeing it play out for a couple services I think I might prefer it
better if we had an entirely separate template for the "pacemaker"
version of the controller. One easy way to kick off this effort would be
to use the Heat resource registry to enable pacemaker rather than a
parameter.

Something like this:

https://review.openstack.org/#/c/180833/

If we were to split out the controller into two separate templates I
think it might be appropriate to move a few things into puppet-tripleo
to de-duplicate a bit. Things like the database creation for example.
But probably not all of the services... because we are trying as much as
possible to use the stackforge puppet modules directly (and not our own
composition layer).

I think this split is a good compromise and would probably even speed up
the implementation of the remaining pacemaker features too. And removing
all the pacemaker conditionals we have from the non-pacemaker version
puts us back in a reasonably clean state I think.

Dan


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev