16:00:43 <Sukhdev> #startmeeting ironic_neutron
16:00:45 <openstack> Meeting started Mon Feb 22 16:00:43 2016 UTC and is due to finish in 60 minutes.  The chair is Sukhdev. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:46 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:50 <openstack> The meeting name has been set to 'ironic_neutron'
16:01:04 <jroll> ohai!
16:01:26 <Sukhdev> #topic: Agenda
16:01:32 <Sukhdev> #link: https://wiki.openstack.org/wiki/Meetings/Ironic-neutron#Meeting_February_22.2C_2016
16:01:57 <Sukhdev> Today's agenda is light
16:02:10 <Sukhdev> #topic: Announcements
16:02:37 <Sukhdev> Anybody has any announcements?
16:02:57 <jroll> we made good progress on the first api patch during the midcycle last week \o/
16:03:15 <jroll> unfortunately, we also agreed the second one needs some heavy rework :(
16:03:44 <Sukhdev> jroll : can you elaborate, please
16:03:47 <devananda> o/
16:04:04 <jroll> so devananda probably is better with the words on this at this point
16:04:16 <sambetts> o/ Hi all
16:04:21 <jroll> but basically, the way the pluggability works and drivers are loaded, etc, needs to be refactored
16:04:27 <jroll> which is the majority of the patch
16:04:41 <vsaienko> \o
16:04:48 <jroll> for reference, this is https://review.openstack.org/139687
16:04:48 <devananda> as it stands, that patch is adding a per-node setting (exposed in the API) to change what network provider that node uses
16:05:10 <jroll> I was meant to work on this friday, but due to travel terribleness, did not get to it. I can hack on it tomorrow.
16:05:18 <devananda> that is basically how we plan to handle driver composition
16:05:44 <devananda> however, the patch doesn't have a well-defined internal API, eg. to support out of tree network provider implementations
16:05:55 <devananda> so we agreed to refactor it so that it does
16:06:27 <devananda> and create a NetworkProviderInterface API (internally) -- much like we have a DeployInterface and BootInterface, etc
16:06:39 <devananda> jroll: that about sum it up?
16:06:51 <jroll> devananda: yes, I think so
16:07:27 <vsaienko> devananda: who is going to handle refactoring?
16:07:30 <Sukhdev> devananda, jroll: thanks for clarification
16:07:44 <jroll> vsaienko: I plan to get to it tomorrow
16:07:52 <jroll> unless deva really wants to tackle it
16:08:47 <jroll> probably start today but likely not finish
16:10:17 <Sukhdev> jroll : just for clarification, with this refactoring, will there be still need to specify network_provider in the config file?
16:10:58 <jroll> Sukhdev: that was a default, if not specified on the node... I would think we would require it to be set on the node itself
16:11:50 <sambetts> I think we'd still need a default to fall back too if the nodes network interface is None?
16:12:28 <Sukhdev> Got it
16:12:45 <jroll> sambetts: if we're treating it as a driver, it should be a required field IMO
16:13:04 <jroll> just like node.driver is required, probably need to do it in validate
16:13:10 * jroll needs to check if driver comp spec covers this
16:13:33 <sambetts> yup, that sounds good
16:14:05 * sambetts was wondering if we had an optional config parameter that set the network interface on a newly enrolled node to the default
16:14:30 <jroll> we do in the current patch, I'm not so sure if it's a good idea now though
16:14:34 <jroll> devananda: ^ opinions?
16:15:49 <devananda> jroll: I need to review the driver comp spec as well
16:16:01 <Sukhdev> sambetts: the default in the config will apply to all nodes including newly enrolled nodes as well, no?
16:16:36 <devananda> honestly, I do not want to hold up the networking work for that spec, so if we don't get the internal API right, we can iterate -- but we have to get the REST API right
16:16:59 <devananda> jroll: I'll dive into the driver comp stuff today
16:17:14 <jroll> devananda: sorry, my question was about your opinion on whether to require setting node.network_provider or allow a default specified in config
16:17:27 <devananda> oh
16:17:29 <devananda> sorry
16:17:55 <sambetts> I think that node.network_provider should be a required field, but there should be an option to set the default one when you enroll the node
16:18:17 <sambetts> just an idea
16:18:20 <devananda> sambetts: that will be a cumbersome and breaking REST API change
16:18:34 <jroll> ?
16:18:42 <devananda> sambetts: everyone enrolling nodes would have to pas a new option
16:18:49 <jroll> how does that break the REST API, this doesn't exist in the API yet
16:18:51 <jroll> oh. ohhh.
16:18:58 <sambetts> my idea prevents them from having to pass an option everytime
16:19:49 <sambetts> e.g. if network_provider not passed, set the one from the config file
16:20:14 <jroll> to be clear, even without the default, I'm not proposing an error on POST /nodes without network_provider set, but rather failing node-validate if not set
16:20:25 <devananda> jroll: that is better
16:20:37 <jroll> still kind of a workflow break though
16:20:59 <devananda> jroll: not if the default is the no-op (flat) network provider
16:21:07 <jroll> I don't mind a default in config, or even hard coding flat default
16:21:09 <jroll> yep
16:21:15 <devananda> since that's the default today (because there isn't an alternative)
16:21:30 <jroll> nod
16:21:39 <jroll> yeah, I can get behind that
16:22:14 * sambetts nods
16:22:36 <jroll> cool
16:22:41 <jroll> let's plan on that then and move on?
16:22:51 <devananda> +1 for now
16:22:59 <Sukhdev> this makes sense to me
16:23:04 <devananda> I'm going to review dtantsur's spec in the meantime
16:23:34 <Sukhdev> In a way this is the current patch's behavior -
16:24:12 <jroll> sure, this isn't the problem with the current patch, to be clear :)
16:25:05 <Sukhdev> jroll : right - for a minute I thought it was being changed - but, now it is clear :-)
16:27:03 <Sukhdev> I plan on updating my system with the latest patches - seems like I should wait :-)
16:27:12 <Sukhdev> until this refactoring is done
16:29:07 <Sukhdev> Anything else on the patches we want to cover?
16:29:39 <Sukhdev> #topic: Open Discussion
16:29:41 <sambetts> I'm concerned about the patch above the network provider patch adding logic to the deploy drviers
16:29:49 <Sukhdev> #undo
16:29:49 <openstack> Removing item from minutes: <ircmeeting.items.Topic object at 0x9462590>
16:30:19 <sambetts> I'm a bit concerned that its making changes in functions that can't be overridden
16:30:44 <jroll> sambetts: got a link?
16:30:52 <Sukhdev> which one?
16:30:55 <sambetts> https://review.openstack.org/#/c/213262/72
16:31:01 <jroll> at first glance, I think the refactor will fix that given it will call driver.network.blah
16:31:34 <sambetts> https://review.openstack.org/#/c/213262/72/ironic/drivers/modules/iscsi_deploy.py
16:31:43 <sambetts> the finish deploy function ^
16:32:19 <devananda> jroll: just finished reading the composable driver spec. it's still incomplete
16:32:32 <devananda> jroll: doesn't really define any of the APIs yet
16:32:37 <jroll> sambetts: what pieces do you think would need to be overrideable there? or rather what would you like to see?
16:32:41 <jroll> devananda: oh fun :)
16:35:09 <sambetts> jroll: for our oob network driver we have to unconfigure the networking with the machine still turned on for the programming on the BMC to work, right now this shuts it down before that call happens, so I'd love to override that but can't because its a separate function outside the iscsi class, so I have to override the whole passthru function below that
16:35:39 <jroll> oh, yikes
16:36:26 <jroll> so, that isn't ideal, but it is also fixable later
16:36:38 <jroll> or we could quickly move it into the driver class so it's overrideable
16:36:45 <devananda> fix seems straight forward: make those module-level methods into class methods
16:36:47 <devananda> right
16:37:10 <jroll> yeah, seems sane, not much to discuss here :)
16:37:24 <jroll> sambetts: leave a comment on the patch and it should get done
16:37:33 <sambetts> :) cool, will do
16:38:40 <Sukhdev> Anything else?
16:38:56 <sambetts> something else I noticed is that depending on the ramdisk your using prepare instance happens either before or after local boot is configured, not sure if that will be affected by the networking work
16:44:38 <Sukhdev> sambetts : so, you are concerned about the timing of the flip?
16:46:28 * Sukhdev wonders if I am still connected to IRC
16:46:38 <sambetts> yes, I'm not sure if its problem or not in this case
16:47:40 <Sukhdev> sambetts : it seems you are testing these patches - if you notice any issue, it will be good to know
16:48:32 <sambetts> Sukhdev: I'm working on some custom out of tree network providers so I'm probably hitting weird things that come from that
16:49:29 <Sukhdev> :-)
16:49:54 <devananda> sambetts: you're still going through neutron, but just changing the order of calls, making some extra hardware calls, etc -- right?
16:50:18 <sambetts> yeah
16:51:26 <Sukhdev> sambetts : changing the order of calls to neutron - or making extra calls - I hope it the latter....
16:52:16 * Sukhdev time check 8 min remaining
16:52:27 <Sukhdev> #topic: Open Discussion
16:53:09 <Sukhdev> Lets continue to discuss sambetts issue, but, also open up for Open Discussion as well - since only few min left
16:53:13 <sambetts> making extra calls, a good example of the sort of thing we ran into is this: because we have to have the node on to configure the networking/bmc, the agent deploy driver complains that it can't soft power off the node because it can't access it
16:53:37 <sambetts> once we configure the tenant networks
16:53:43 <sambetts> and go to reboot it
16:54:20 <sambetts> tbh most of these issues are down to our crazy stuff rather than the existing code
16:55:08 <sambetts> anyway, RE our conversation 2 weeks ago, I've now posted this
16:55:08 <sambetts> https://review.openstack.org/#/c/277853/
16:55:40 <sambetts> and we had a good discussion about it during the Ironic mid-cycle
16:55:50 <Sukhdev> #link: https://review.openstack.org/#/c/277853/
16:55:53 <jroll> ++
16:56:19 <Sukhdev> sambetts : can you add the link to etherpad, please?
16:56:20 <sambetts> and I've now got this WIP patch up to show the Ironic side changes https://review.openstack.org/#/c/282431/
16:56:33 <sambetts> Sukhdev: the midcycle one?
16:56:58 <Sukhdev> sambetts : yes - that is where I track all these patches - makes it lot easier -
16:57:16 <Sukhdev> #link: https://review.openstack.org/#/c/282431/
16:57:55 <Sukhdev> sambetts : this is good stuff - thanks
16:58:51 <Sukhdev> sambetts : I will review it - but, is it in line with the nova/neutron work?
16:59:26 <Sukhdev> i.e. main port as access port and sub-ports as tagged ports?
16:59:59 <Sukhdev> this will be very useful for service BMs
17:00:15 * Sukhdev time check
17:00:26 <Sukhdev> Folks, we are out of time
17:00:42 <Sukhdev> This was very useful discussion - thanks for participation
17:00:45 <Sukhdev> bye
17:00:50 <sambetts> o/ thanks all
17:00:56 <Sukhdev> #endmeeting