Open Stack

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