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
