15:00:05 <EmilienM> #startmeeting puppet-openstack
15:00:08 <openstack> Meeting started Tue Nov 17 15:00:05 2015 UTC and is due to finish in 60 minutes.  The chair is EmilienM. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:00:09 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:00:11 <openstack> The meeting name has been set to 'puppet_openstack'
15:00:12 <mwhahaha> hi
15:00:16 <iurygregory> hello
15:00:20 <mkarpin> hi
15:00:21 <degorenko> hey o/
15:00:21 <mentat> hi all
15:00:23 <EmilienM> #link agenda https://etherpad.openstack.org/p/puppet-openstack-weekly-meeting-20151117
15:00:28 <crinkle> o/
15:00:32 <clayton> o/
15:00:44 <EmilienM> I missed you guys
15:00:55 <EmilienM> #topic review past items
15:01:07 <mdorman> o/
15:01:11 <EmilienM> sbadia to investigate Trove bring broken in RDO (openstack-trove-taskmanager appears to be missing)
15:01:32 <sbadia> hello
15:01:33 <EmilienM> sbadia: do you have any status on this so far?
15:01:57 <richm> hello
15:01:59 <angdraug> o/
15:02:12 <sbadia> on the root issue yes (it's linked to the voting change of integration jobs)
15:02:25 <sbadia> but for taskmanager the package is in testing on centos repos
15:02:37 <EmilienM> ok, and not in GA repo, right?
15:02:41 <sbadia> and wasn't in stable yet
15:02:41 <aderyugin> 0/
15:02:43 <sbadia> yep
15:02:58 <EmilienM> ok, thanks for working on that, we need to make sure it lands soon
15:03:02 <sbadia> heikel confirmed that
15:03:11 <sbadia> yep shure!
15:03:30 <EmilienM> #topic keystone federation status
15:03:32 <EmilienM> iurygregory: o/
15:03:37 <EmilienM> what's up?
15:03:42 <iurygregory> Let's go
15:04:06 <iurygregory> Well, to have K2K federation working we need https://review.openstack.org/#/c/216821/
15:04:46 <iurygregory> Service Provider use Shibboleth, and to have this working on RedHat osfamily it is necessary to enable other repo
15:04:57 <iurygregory> https://github.com/puppetlabs/puppetlabs-apache/blob/master/manifests/params.pp#L88
15:05:19 <iurygregory> In Debian everything is working
15:05:36 <EmilienM> we won't enable any exteral third party repo
15:05:43 <EmilienM> it will be up to people deploying that feature
15:05:49 <iurygregory> yeah ;) i know
15:05:59 <iurygregory> so i think we have two options
15:06:04 <iurygregory> Fail when try to use keystone::federation::shibboleth on RedHat
15:06:12 <EmilienM> though for testing purpose, I think it's ok to add the repo in acceptance
15:06:12 <iurygregory> Show a warning about not configuring Keystone VirtualHost (Should we configure keystone configuration file? /etc/keystone/keystone.conf
15:06:33 <chem> EmilienM: I think this should be included in the module ...
15:06:42 <EmilienM> chem: the third party repo?
15:07:02 <EmilienM> chem: http://lists.openstack.org/pipermail/openstack-dev/2015-September/075491.html
15:07:05 <chem> EmilienM: because the apache module can use it
15:07:25 <EmilienM> richm: any thoughts?
15:07:38 <chem> iurygregory is it epel ?
15:07:44 <richm> I think it is fine if this is not supported on Red Hat family platforms for now
15:08:06 <richm> Or, you have to somehow manually tell it that you are using the 3rd party repo to make it work
15:08:15 <EmilienM> I don't think puppet-keystone should take care of Shibboleth apache mod
15:08:35 <clayton> agreed/
15:08:42 <EmilienM> I would rather patch puppetlabs-apache
15:08:43 <richm> why not?  It takes care of mod_wsgi?
15:09:10 <clayton> I think at most we document that it's missing on some distros and point people at where to get it
15:09:13 <EmilienM> richm: mod_wsgi is packages everywhere afik
15:09:22 <EmilienM> clayton: +1
15:09:26 <iurygregory> chem, the repo is from opensuse http://wiki.aaf.edu.au/tech-info/sp-install-guide
15:10:08 <xarses> hi
15:10:30 <iurygregory> i think show a warning about not installing and configure shibboleth on redhat is fine
15:10:37 <EmilienM> so I guess a first step would be to deliver it for debian only now, and add documentation explaining this repo thing
15:10:46 <iurygregory> yeah
15:10:51 <EmilienM> iurygregory: a puppet warning?
15:10:57 <iurygregory> yep =)
15:11:12 <EmilienM> that sounds good to me
15:11:43 <iurygregory> it's fine with me =)
15:11:55 <EmilienM> any concern?
15:12:01 <clayton> it would show a warning in what cases?
15:12:15 <EmilienM> clayton: iiuc, when deploying shibboleth on redhat systems
15:12:20 <degorenko> in case of redhat
15:12:24 <degorenko> +1 goodto me
15:12:29 <clayton> and you'd have a flag to disable the warning?
15:12:52 <iurygregory> when using redhat say that we will not configure shibboleth in the keystone virtualhost =)
15:13:04 <EmilienM> +1 for the flag, yes. But by default we need to send it
15:13:05 <iurygregory> yeah clayton
15:13:06 <richm> I think Red Hat is more focused on mellon than shibboleth
15:13:16 <chem> EmilienM: why i though it would be a good idea is because of this https://github.com/puppetlabs/puppetlabs-apache/blob/master/manifests/params.pp#L122
15:13:16 <iurygregory> we can do this ^^
15:13:19 <clayton> sure, but someone that's already solved that problem isn't going to want to see the warning all the time.  I think this is probably going too far to protect people from themselves personally.
15:13:27 <chem> EmilienM: (took me a while to retrieve it)
15:13:34 <clayton> I'd just document that it's missing on redhat and point people at where to look to fix it
15:13:59 <EmilienM> chem: mhh, interesting o_O
15:14:01 <chem> EmilienM: iurygregory richm it's supported in the puppet apache module, provided we give the repo
15:14:09 <richm> ok
15:14:36 <chem> EmilienM: iurygregory richm that's why I think that it's a good thing to leverage the official upstream code
15:14:38 <holser> o/
15:15:07 <EmilienM> iurygregory: have you tried what chem suggests?
15:15:50 <iurygregory> i didn't try to set the repo and see how is working on centos
15:16:50 <EmilienM> iurygregory: maybe you should first try it
15:16:55 <chem> EmilienM: iurygregory richm it would be obviously a exeption to a good rule (no external repo), but in the end I'm not that strong minded about this, it's just that the code is there ...
15:17:37 <iurygregory> ok EmilienM ^^
15:17:37 <Guest20594> sorry Im late
15:17:46 <clayton> np Guest20594
15:17:49 <EmilienM> Guest20594: hey matt
15:17:55 <Guest20594> dang it
15:18:40 <EmilienM> iurygregory: ok, let's continue the discussion on Gerrit. Please test if the module is in centos : https://github.com/puppetlabs/puppetlabs-apache/blob/master/manifests/params.pp#L122 and if not, we send a warning
15:18:56 <iurygregory> ok =)
15:19:06 <chem> EmilienM: it is not : https://github.com/puppetlabs/puppetlabs-apache/blob/master/manifests/params.pp#L88
15:19:28 <iurygregory> if we enable the repo it will be i think XD
15:19:42 <chem> iurygregory correct :)
15:19:59 <chem> ok, I'm done
15:20:08 <EmilienM> for now, we won't add an extra repo in puppet-keystone
15:20:28 <iurygregory> so we go with the warning right?
15:20:29 <EmilienM> we need to investigate if fedora is interested by packaging this module, (not sure regarding what richm said)
15:20:38 <EmilienM> otherwise, yes a warning *for now*
15:20:48 <chem> ack
15:20:53 <iurygregory> ok ;) could some cores review https://review.openstack.org/#/c/216821/?
15:20:58 <iurygregory> Tks ^^
15:21:17 <EmilienM> #action review federation patche https://review.openstack.org/#/c/216821
15:21:30 <EmilienM> iurygregory: anything else?
15:21:34 <iurygregory> np
15:21:40 <EmilienM> thanks for this work
15:21:40 <iurygregory> tks people
15:21:45 <EmilienM> #topic os_service_defaults
15:21:54 <EmilienM> clayton, spredzy|afk: o/
15:22:38 <clayton> We've got a lot of pending reviews that are using this.  I think the concensus is that we can start using it, but just to be careful about not accidently changing existing values
15:22:45 <clayton> but I'd like to make sure that's the case
15:23:21 <EmilienM> that's sounds a great plan: 1/ switch to the new feature 2/ keep backward compatible
15:23:34 <iurygregory> +1
15:23:35 <mfisch> what else is holding it back from being uses everywhere?
15:23:38 <clayton> ok, I'd also like to know if people have any thoughts on requiring it for new changes?
15:23:55 <EmilienM> clayton: like any new patch should require it?
15:23:55 <clayton> Converting everything to use it is going to be a ton of work, but we can try not to make it worse
15:24:00 <clayton> EmilienM exactly
15:24:10 <EmilienM> clayton: I guess that's something we need to do *after* it lands
15:24:19 <clayton> after what lands?
15:24:23 <EmilienM> and perform a iterative migration
15:24:31 <EmilienM> first, a big patch per module
15:24:47 <EmilienM> but for sure we will miss some params, because WIP patches in the same repo
15:24:59 <clayton> I don't think we should block new uses of it on existing params being converted.  Not sure if that's what you're proposing
15:25:09 <EmilienM> so I guess we can drop a comment in all patches, but not -1 maybe, if the patch is ready to be merged
15:25:26 <EmilienM> clayton: I agree
15:25:39 <clayton> I'm fine with grandfathering existing patches, but anything new past x date should use it
15:25:48 <EmilienM> clayton: +1
15:25:58 <clayton> what is x?  This week?  Next?
15:26:03 <EmilienM> today
15:26:10 <clayton> Sounds good.
15:26:14 <mfisch> +1
15:26:20 <iurygregory> +1
15:26:33 <EmilienM> #info os_service_defaults feature can be implemented in all modules from today
15:27:02 <EmilienM> the only concern I'm aware of was the logging stuff
15:27:16 <EmilienM> clayton: I haven't followed yet but is it fixed^ ?
15:27:44 <clayton> no, I think we were kind of waiting for a decision on service defaults, since I think it was generally agreed that was probably the best solution
15:28:04 <EmilienM> the best solution is to keep backward compatible, even if upstream is different
15:28:11 <EmilienM> for now, I think that's the best solution
15:28:22 <clayton> this was introduced in liberty, so only people on master should have this issue
15:28:36 <EmilienM> if we really want to align with upstream, we will need to send some warnings to our users and change the default later one day
15:28:52 <clayton> and it's potentially a huge problem for people upgrading from kilo, since it will double the number of db connections for people on stable/kilo
15:28:53 <EmilienM> clayton: the logging stuff?
15:29:05 <EmilienM> are we talking about the same thing?
15:29:12 <clayton> oh, sorry, I misread, I thought we were on the db topic.
15:29:19 <EmilienM> nope
15:29:22 <clayton> I jumped ahead :)
15:29:39 <clayton> EmilienM: yes, I agree on the logging, I think it should stay where it is
15:29:45 <EmilienM> ok great
15:29:47 <clayton> I think the puppet module defaults are more sane than the upstream ones
15:29:54 <EmilienM> and also for any other potential divergence
15:29:59 <clayton> I think that is a rare situation, but one that will occasionally occur
15:30:09 <EmilienM> use os_service_defaults when we can, but if different from upstream -> use the default we had
15:30:21 <EmilienM> we need to document that
15:30:42 <degorenko> +1 to this approach
15:30:43 <EmilienM> any volunteer to write something in https://wiki.openstack.org/wiki/Puppet/Coding_style ?
15:31:12 <EmilienM> if not clayton :-P
15:31:52 <clayton> you can assign it to me
15:31:56 <iurygregory> like add a section about when use os default?
15:32:03 <EmilienM> #action clayton to update https://wiki.openstack.org/wiki/Puppet/Coding_style about os_service_defaults
15:32:06 <EmilienM> clayton: thanks a lot
15:32:14 <clayton> iurygregory : yeah, I think that's the idea
15:32:29 <EmilienM> #topic incorrect defaults for *::db classes
15:32:34 <EmilienM> clayton, degorenko: hey
15:32:36 <iurygregory> if you need help you can ping ^^
15:32:39 <clayton> this is the topic I thought we were on :)
15:32:43 <EmilienM> #link https://bugs.launchpad.net/puppet-tuskar/+bug/1515273)
15:32:43 <openstack> Launchpad bug 1515273 in puppet-tuskar "Defaults for *::db classes are incorrect" [Undecided,New]
15:32:44 <degorenko> yep :)
15:32:56 <EmilienM> not sure spredzy|afk is aroung
15:33:00 <EmilienM> around*
15:33:07 <degorenko> regarding https://bugs.launchpad.net/puppet-tuskar/+bug/1515273  i guess we should use defaults we had and raise warnings
15:34:26 <EmilienM> clayton: what do you prefer?
15:34:38 <clayton> I think we should use $::os_service_default for all of these values
15:34:39 <EmilienM> I vote for keeping old default, which is 10 iirc
15:34:52 <EmilienM> clayton: why? because openstack changed it?
15:34:54 <clayton> the default in sqlalchemy is currently 5
15:34:57 <EmilienM> (in liberty)
15:35:01 <clayton> and it's always been 5 afaict
15:35:09 <clayton> it's documented incorrectly in the neutron config file
15:35:26 <EmilienM> what is the impact on the migration?
15:35:37 <clayton> so the current way of doing it will double the number of connections most deployers have to their databases
15:35:38 <EmilienM> is it fine for people who upgrade?
15:35:41 <clayton> when they upgrade
15:36:00 <clayton> which would be fine if we had a good reason for it and we documented it, but spredzy indicated it was basically an accident
15:36:26 * xarses checks in fuel
15:36:41 <clayton> I did the math for us, and it would take us from around 3k db connections in production to 6k.  Given load balancer and process fd limits, it would likely break some people, for probably no good reason
15:37:02 <iurygregory> wow
15:37:11 <mwhahaha> so we never released those values
15:37:24 <EmilienM> mwhahaha: right
15:37:26 <mwhahaha> so if we switch them before the liberty release, then they won't have been released
15:37:35 <EmilienM> mwhahaha: double right
15:37:42 <mwhahaha> so we should fix them now :D
15:37:44 <mfisch> +1
15:37:47 <EmilienM> +1
15:37:51 <iurygregory> +1
15:38:00 <xarses> how do we end up with 3/6k from this value?
15:38:13 <clayton> xarses let me explain
15:38:36 <clayton> for us, we have nova, neutron, heat, glance and cinder running on our control nodes, so 5 services, each has one process that connects to the db
15:38:48 <clayton> each of them spawns one worker per cpu core by default, so for us, 32 or 40
15:39:04 <clayton> and previously we had 5 db connections for *each* of those processes, and this changes it to 10
15:39:13 <clayton> oh, and we have 3 control nodes
15:39:38 <clayton> so 5 * 40 * 5 * 3 vs 5 * 40 * 10 * 3
15:39:51 <clayton> and that's the *minimum* number of db connections
15:40:01 <clayton> overflow allows it to go over that number, although we rarely see that happening
15:40:04 <clayton> given the number of workers
15:41:00 <EmilienM> clayton: can you send a patch in puppet-cinder with the fix so we can start from here and help to fix it before the release?
15:41:13 <clayton> it's actually already fixed in puppet-cinder
15:41:18 <clayton> that's why it's not listed in the bug report
15:41:22 <EmilienM> oh nice
15:41:33 <clayton> because it's been converted over already
15:42:04 <xarses> ok, thanks
15:42:45 <EmilienM> clayton: so we need to use $::os_service_default for these params?
15:42:59 <clayton> I think that's the best way to go
15:43:07 <EmilienM> ok
15:43:30 <EmilienM> any concern about it?
15:43:59 <clayton> nope, we're already doing it for puppet-cinder.  We have the code in production for that already
15:44:07 <EmilienM> just a stupid question: is someone working on $::os_service_default conversion for our modules?
15:44:17 <EmilienM> mwhahaha, degorenko, clayton, spredzy|afk ^ ?
15:44:20 <iurygregory> degorenko, i think
15:44:24 <degorenko> yep
15:44:27 <xarses> we are frequently setting max_pool_size = min($::processorcount * 5 + 0, 30 + 0)
15:44:28 <iurygregory> i've see some patches
15:44:46 <mwhahaha> was going to try and get some of the initial test stuff started for everything if someone else hadn't yet
15:44:49 <EmilienM> degorenko: great. Do you have a LP to track the work?
15:44:49 <degorenko> EmilienM, https://review.openstack.org/#/q/status:open+branch:master+topic:os_service_default,n,z
15:44:50 <xarses> as a result of scale testing we have preformed
15:44:56 <EmilienM> or a topic, nice!
15:45:01 <mwhahaha> i know we have some outstanding items for the os_package_type too
15:45:16 <degorenko> EmilienM, our trello board
15:45:18 <degorenko> https://trello.com/c/XLJJJBF0/71-move-modules-to-the-os-service-default-pattern
15:45:25 <EmilienM> degorenko: great
15:46:16 <EmilienM> anything else about this topic?
15:46:18 <xarses> clayton: ^
15:46:27 <clayton> nope
15:46:33 <EmilienM> #topic Open Discussion, Bug and Review triage
15:46:34 <degorenko> nope, i'm good with solution from clayton
15:46:48 <EmilienM> if you have questions, patches, bugs, or just want to say something, go ahead now
15:47:04 <mwhahaha> https://review.openstack.org/#/c/244900/
15:47:13 <mwhahaha> i ran into that when trying out the upstream modules in fuel
15:47:39 <myatsenko1> Guys plz review this patch https://review.openstack.org/#/c/233011/ , its about floating ip range support.
15:48:02 <mwhahaha> as well as https://review.openstack.org/#/c/243912/
15:48:08 <aderyugin> https://review.openstack.org/#/c/220238/
15:48:40 <EmilienM> mwhahaha: sounds good to me
15:48:42 <degorenko> one more bug faced in fuel :) https://review.openstack.org/245876
15:49:21 <chem> mwhahaha: that's strange the default should be taken into account
15:49:26 <chem> mwhahaha: that's strange the default case should be taken into account
15:49:45 <chem> mwhahaha: that is, "if default don't add the domain suffix"
15:50:10 <chem> mwhahaha: I'll look into the whole thing, and review later.  Thank for bringing this up
15:50:12 <xarses> clayton: We set them high in fuel as a result of scale testing up to 30 / 60. https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/openstack-controller/openstack-controller.pp#L67-L71
15:50:31 <mwhahaha> chem it's from the fact that when it goes to look up the resources the names get changed to include the domain
15:50:43 <mwhahaha> chem: that review is a workaround but we need to touch that code (it's really old from 2013)
15:51:30 <EmilienM> aderyugin: where is functional testing on https://review.openstack.org/#/c/220238/ ?
15:51:31 <chem> mwhahaha: but to keep backward comptatibility if the domain is default, the module shouldn't add "::Default" suffix to the name
15:51:57 <chem> mwhahaha: that's how I though it was working anyway :)  but maybe I missed something
15:52:00 <mwhahaha> chem: right but we broke backwards compatibility with the review that put it in i thought :D
15:52:20 <chem> mwhahaha: not for this
15:52:26 <mwhahaha> we don't specify the resource in our catalog so it only affects lookups on tenants
15:52:50 <chem> mwhahaha: anyway I'll have a look today.
15:52:54 <mwhahaha> sure
15:52:54 <degorenko> EmilienM, do you mean acceptance? for 220238
15:53:30 <EmilienM> degorenko: yes
15:54:01 <EmilienM> mwhahaha: one question - are all modules on the same page about https://github.com/openstack/puppet-cinder/commit/70daad9ea4a25471f4d40b78ad70ae0aa2f99a1e ?
15:54:06 <degorenko> EmilienM, we can try to add it, but probably in upstream murano packages missed one file :)
15:54:17 <EmilienM> degorenko: ok, just asking
15:54:21 <degorenko> aderyugin, can provide more information i guess
15:54:24 <mfisch> I will review the murano one today
15:55:22 <EmilienM> do we have anything else for today?
15:55:23 <aderyugin> EmilienM: right now application provider will not pass acceptance, since default application is missing from upstream package
15:55:38 <mwhahaha> yea i think they are
15:55:40 <EmilienM> aderyugin: ok - please make sure it's wip by packaging people
15:55:47 <EmilienM> mwhahaha: ok, just checking
15:56:01 <aderyugin> EmilienM: ok
15:56:15 <EmilienM> aderyugin: it won't block anything
15:56:52 <degorenko> one more request for review :) https://review.openstack.org/#/c/216654/
15:57:04 <EmilienM> #action emilien to check https://github.com/openstack/puppet-cinder/commit/70daad9ea4a25471f4d40b78ad70ae0aa2f99a1e is consistent across all modules
15:57:21 <myatsenko1> and this one https://review.openstack.org/#/c/233011/
15:57:26 <mfisch> we should use the meeting commands for reviews so they're in the logs
15:57:45 <delatte> if possible would like this reviewed as well https://review.openstack.org/#/c/197572/  I know clayton reviewed but was looking for others
15:58:00 <mfisch> you have a -1 from ody
15:58:09 <EmilienM> mfisch: #action is here for that
15:58:34 <mfisch> EmilienM: yes lets use it
15:58:41 <mfisch> #action everyone to use #action when posting reviews they want reviewed
15:58:54 <degorenko> #action review https://review.openstack.org/245876
15:59:13 <degorenko> #action review https://review.openstack.org/#/c/216654/
15:59:16 <EmilienM> ok, anything else but reviews before closing the meeting?
15:59:17 <myatsenko1> #action review https://review.openstack.org/#/c/233011/
15:59:22 <xarses> I have not so much as a problem, but I need some help splitting up some of the ci in puppet-ceph and was wondering if any one was up to helping me through the process
15:59:30 <delatte> mfisch:  if you read his comments, it a "way we do things" -1.  was hoping for cores to weigh in
15:59:40 <EmilienM> xarses: you come up 30s before the end :(
15:59:41 <mkarpin> #action review https://review.openstack.org/#/c/241614/
15:59:42 <aderyugin> #action review https://review.openstack.org/#/c/220238/
15:59:52 <iurygregory> #action review  https://review.openstack.org/#/c/216821/
15:59:54 <EmilienM> xarses: mailing list
15:59:57 <xarses> EmilienM: its not so important to address here
16:00:04 <EmilienM> thanks everyone, have a great day
16:00:05 <mfisch> we're out of time
16:00:06 <EmilienM> #endmeeting