16:01:57 <Sukhdev> #startmeeting ironic_neutron
16:01:57 <openstack> Meeting started Mon Nov  7 16:01:57 2016 UTC and is due to finish in 60 minutes.  The chair is Sukhdev. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:01:58 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:02:00 <openstack> The meeting name has been set to 'ironic_neutron'
16:02:07 <jroll> \o
16:02:11 <Sukhdev> #topic: Agenda
16:02:19 <Sukhdev> #link: https://wiki.openstack.org/wiki/Meetings/Ironic-neutron#Meeting_November_7.2C_2016
16:02:42 <Sukhdev> Welcome to first meeting after the summit
16:02:57 <Sukhdev> Hope everybody is fully recovered from travels by now
16:03:12 <Sukhdev> #topic: Announcements
16:03:28 <Sukhdev> Anybody has any announcements?
16:03:39 <sambetts> Port groups API patch merged
16:04:03 <Sukhdev> yup - that is good news
16:04:15 <Sukhdev> I updated it on the agenda
16:04:33 <Sukhdev> lets dive into the agenda
16:04:36 <hshiina> nova schedule was announced  http://lists.openstack.org/pipermail/openstack-dev/2016-October/106423.html
16:04:48 <jroll> ^^
16:04:56 <hshiina> spec freeze is o-1, nov 17
16:04:57 <jroll> need to get that nova portgroups spec merged
16:05:02 <hshiina> it's important
16:05:39 <sambetts> Nova team accepted attach/detach BP but we need to review/merge the Ironic RFE/spec first
16:06:33 <sambetts> wow thats 1 week away.. .
16:07:02 <Sukhdev> yup - its almost upon us
16:08:40 <Sukhdev> lets quickly go through the agenda items
16:08:50 <Sukhdev> #topic Security Groups
16:09:08 <Sukhdev> I posted the documentation patch for this
16:09:29 <Sukhdev> #link: https://review.openstack.org/#/c/393962/
16:10:17 <Sukhdev> when ever you have a moment, please have a look at it
16:10:28 <Sukhdev> for the patch - https://review.openstack.org/#/c/361451
16:11:06 <Sukhdev> sambetts and I had a brief discussion on Friday regarding the flag security_group_enabled
16:11:52 <Sukhdev> i.e. if the security_group_enabled flag False, and SGs are configured, how to deal with this
16:12:26 <Sukhdev> I checked on the neutron and nova side as to how this is handled.
16:13:10 <Sukhdev> basically, if this flag is set to false and somebody tries to set security groups at the time of nova boot - the instance fails
16:14:09 <Sukhdev> in our case it is slightly different - as everything is being set/done during the init
16:14:55 <Sukhdev> for tenant networks, it will have the same behavior - however, how do we want to deal with it when this happens for provisioning or cleaning networks
16:15:24 <Sukhdev> we can take two approaches -
16:15:51 <Sukhdev> 1 ) Since all of these operations are managed by admin, we should simply document it
16:16:07 <Sukhdev> 2) we fail the operation
16:16:25 <Sukhdev> or may be both actullay
16:16:40 <Sukhdev> any thoughts?
16:17:07 <jroll> both seems right to me
16:17:47 <jroll> if someone has configured some security group, they expect that port to be locked down, we shouldn't allow a port to be up that isn't locked down the way they expect
16:17:50 <jroll> if that makes sense
16:19:43 <Sukhdev> the way the SG patch is written, it will behave like that - i.e. port_create() will fail and log the exception
16:21:22 <Sukhdev> l just noticed sambetts comment about it on the patch -
16:21:25 <sambetts> Sorry for being slow to respond, I was in another call, I put a more detailed https://review.openstack.org/#/c/361451/14/ironic/common/neutron.py
16:21:48 <Sukhdev> I think option 2 in your comment - i.e. misconfiguration seems more appropriate
16:22:01 <jroll> +1
16:23:35 <sambetts> the only reason I prefer option 1 is that port security can be enabled and disabled at will in the neutron API, however the secuirty groups in the ironic configuration can only be disabled by editing the ironic config file and restarting ironic, so it the admin wants to debug something and turns off port secuirty it'll prevent all ironic deployments
16:24:21 <jroll> sambetts: as an operator, I never want to be surprised with something less secure than I expect
16:24:25 <jroll> is why I'm on the option 2 side
16:24:59 <Sukhdev> sambetts : that scenario is not really applicable in this case - it is the same operator who is adding both options
16:25:58 <sambetts> I'm talking about post having Ironic running, I don't want to have to restart ironic once its running to do something which in other cases can simply be enabled/disable via an API
16:26:36 <jroll> yeah, I get it, but I think it can put someone in a "dangerous" position
16:27:53 <Sukhdev> sambetts : i chatted with both nova and neutron PTLs about this on Friday after our discussion - it is used to fail the instance - in case somebody tries to specify SG, when they should not be
16:28:24 <sambetts> yeah they changed it in mitaka to just not apply the SG right?
16:28:40 <Sukhdev> in our case, these operations are managed by the admin
16:29:49 <Sukhdev> I mean to say is that the net effect of this mis-configuration is that the instance is faied
16:30:09 * sambetts wonders if this is the sort of check we should have on the neutron network driver validate() function
16:32:25 <Sukhdev> I am of the openion that we update the documentation - where we talk about the provisioning/cleaning network creation
16:32:56 <Sukhdev> actually, the patch that I pushed for documentation - I should add text to describe this
16:34:48 <Sukhdev> thoughts?
16:35:26 <jroll> yes, agree
16:35:49 <sambetts> I think we should definatly document it, but I wonder if we should fail earlier in the process if the network has port security disabled
16:36:47 <jroll> yeah, I wouldn't be opposed to checking in validate
16:37:00 <Sukhdev> sambetts : failing earlier does not really help - as you pointed out earlier - if somebody changes the network properties after everything is running
16:37:07 <sambetts> e.g. make the nodes configured the neutron network interface fail validation or have an explict check so that we don't fire off N port creates even though we know they are going to fail
16:39:07 <Sukhdev> so, this is really a weird scenario - I can see the usefulness if there are multiple operators
16:39:46 <Sukhdev> failing earlier helps if someone creates networks and SGs and also sets this flag to false - which is kind of weird :-)
16:40:51 <sambetts> Sukhdev: nova runs node validation has part of its resource discovering process and will prevent nova scheduling instances on the node if they are misconfigured
16:41:23 <jroll> sambetts: no, validation happens post-schedule
16:41:44 <jroll> but agree, it will save some time/resources
16:42:05 <Sukhdev> see here - https://github.com/openstack/nova/blob/14.0.1/nova/network/neutronv2/api.py#L783
16:42:10 <sambetts> jroll: ? really?! I'm sure nodes that fail validation don't show up as available to nova?
16:42:23 <Sukhdev> this is where this is checked
16:42:57 <jroll> sambetts: yeah, it happens here https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L763
16:43:17 <jroll> sambetts: it also happens between enroll and manageable, so you're partially right
16:44:45 <Sukhdev> nova only checks this flag at the time of port create
16:46:40 <Sukhdev> sambetts : even if we go with the idea of checking this flag - what would you see the end-result? - not fail the operation?
16:47:01 <sambetts> No fail it, but we'd fail here: https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L763
16:47:12 <sambetts> before we've even starting deployment
16:47:52 <sambetts> but this is something we can always expand on later
16:48:25 <Sukhdev> hmm...
16:50:00 <Sukhdev> I really believe this is an issue of misconfiguration - and, we can come back and this check here if it becomes an issue -
16:50:16 <Sukhdev> I mean add this check here
16:50:44 <jroll> I mean... we agreed to add a check. why not add it sooner in the deploy process?
16:51:19 <sambetts> jroll: we technically don't need the check right now, it'll just catch the failure like any other port create error right now
16:51:42 <jroll> s/add a check/have it fail/
16:51:57 <jroll> :)
16:52:25 <Sukhdev> +1
16:52:48 <jroll> so, agree to fail early in validate()?
16:52:49 <sambetts> I guess the only potential arguement might be to do with validate being syncronous and making an external call */me ducks*
16:53:13 <jroll> sambetts: it already hits the bmc, if neutron ever gets slower than a bmc we have bigger problems :)
16:53:19 <jroll> net-show, anyway
16:53:30 <sambetts> true true
16:53:45 <sambetts> does validate hit the BMC?
16:53:53 <jroll> yes
16:54:04 <sambetts> for power or something?
16:54:11 <persia> Some folk talk about running neutron on fairly low-spec switch hardware.  Some BMCs have fairly high-clock cores.  Take care with assumptions.
16:54:33 <jroll> oh wait, apparently we do not: https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L774
16:54:43 <jroll> :/
16:54:52 <jroll> persia: fair point
16:55:28 * Sukhdev time check - 5 min
16:55:39 <Sukhdev> lets draw some conclusions
16:56:10 <sambetts> I'm happy with the patch as it is today if we're happy for it to be a misconfiguration
16:56:16 <sambetts> I've removed my -1
16:56:23 <jroll> I'm still happiest with doing this in validate(), but okay with current status quo too
16:57:06 <Sukhdev> I think we can come back with a new patch to address it
16:57:41 <Sukhdev> I will push an update to the documentation patch to describe this misconfiguration patch
16:58:34 * Sukhdev 2 min left
16:58:43 <Sukhdev> anything critical to discuss?
16:59:21 <sambetts> nothing from me as far as I can remember
16:59:21 <Sukhdev> Thanks folks, this was a very good discussion -
16:59:55 <Sukhdev> see ya'll next week when US will have new president :-)
17:00:03 <Sukhdev> bye
17:00:07 <sambetts> :/
17:00:10 <sambetts> cya o/
17:00:18 <Sukhdev> #endmeeting