| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-26 | |||
| 13:55:53 | sfinucan | jaypipes: Aye | |
| 13:56:00 | sfinucan | another +2 too, mind | |
| 13:56:29 | sfinucan | :) | |
| 13:56:31 | jaypipes | sfinucan: hmm? | |
| 13:56:43 | alex_xu | mriedem: if we have wraps for the shell, 'nova --os-compute-api-version 2.44 help' will hide the shell which isn't supported anymore. Then the user can know which commands supported in the specific version | |
| 13:57:29 | sfinucan | /bug | |
| 13:57:58 | mriedem | alex_xu: does anyone do that with the CLI? | |
| 13:58:01 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova master: Read Neutron port 'binding_profile' during boot https://review.openstack.org/449257 | |
| 13:58:02 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova master: Add Neutron port capabilities to devspec in request https://review.openstack.org/451777 | |
| 13:58:10 | jaypipes | sfinucan: doh, apologies. for some reason I thought you or sean-k-mooney had +2'd moshele's hw-tc-offload patch :( | |
| 13:58:39 | alex_xu | mriedem: not sure :( | |
| 13:58:46 | sfinucan | jaypipes: Oh, I heard something about bug 1662650 before. I think that guy, Trevor, talked about it in an SR-IOV meeting ages ago | |
| 13:58:46 | openstack | bug 1662650 in neutron "[RFE] Advance configuration of SR-IOV ports- api extension" [Wishlist,Triaged] https://launchpad.net/bugs/1662650 - Assigned to Trevor McCasland (twm2016) | |
| 13:58:56 | edleafe | jaypipes: in meetings - added to my queue | |
| 13:58:59 | jaypipes | sean-k-mooney, sfinucan: I must have confused your +2's on the dependent patches for a +2 on that one. my apologies to you both. | |
| 13:59:11 | sfinucan | jaypipes: It's all goooood | |
| 13:59:25 | jaypipes | no, it's really not... pretty bad mistake of mine :( | |
| 13:59:36 | mriedem | jbernard: i don't know what happened in https://review.openstack.org/#/c/459832/ | |
| 14:00:54 | openstackgerrit | Stephen Finucane proposed openstack/os-vif master: Revert "hardware offload support for openvswitch" https://review.openstack.org/460120 | |
| 14:01:19 | sfinucan | jaypipes: Do we need to +2 reverts? :D | |
| 14:01:26 | jaypipes | sfinucan: no, go for it. | |
| 14:02:38 | openstackgerrit | Stephen Finucane proposed openstack/os-vif master: Standardize README https://review.openstack.org/460094 | |
| 14:03:55 | sfinucan | mriedem: Thanks for the info on https://review.openstack.org/#/c/373293/ - I'd no idea what the policy around stable/* requirements bumps was | |
| 14:04:11 | mriedem | sfinucan: it's a long and sordid tale of woe | |
| 14:04:28 | mriedem | and coincidentally as a result of the TC meeting yesterday, is going to come back up real soon in the ML | |
| 14:05:21 | mriedem | sfinucan: https://docs.google.com/presentation/d/1k0mCHwRZ3_Z8zJw_WilsuTYYqnUDlY2PkgVJLz_xVQc/edit?usp=sharing | |
| 14:05:26 | mriedem | oldy but goody | |
| 14:06:16 | bauzas | mriedem: dansmith: jaypipes: do you remember the exact problem that I tried to solve when I wrote "Amend RT to filter out allocations for instances that don't have a host set | |
| 14:06:16 | bauzas | yet when it does self-heal check." ? | |
| 14:06:39 | mriedem | bauzas: that was a race that edleafe pointed out i think | |
| 14:06:51 | bauzas | mriedem: dansmith: jaypipes: the fact that we post allocations for instances that we discover, right? | |
| 14:06:58 | dansmith | the pre-scheduling race or something? | |
| 14:07:06 | sfinucan | mriedem: niiiiice | |
| 14:07:06 | bauzas | I think so | |
| 14:07:16 | bauzas | anyway, just writing the patch, comments welcome | |
| 14:07:34 | mriedem | oh right, | |
| 14:07:35 | dansmith | like, you create an instance, don't assign the host, do create allocations, and that host deletes the allocations before building because of self-heal | |
| 14:07:42 | mriedem | because instance.host isn't set until we're in the RT | |
| 14:07:45 | dansmith | yeah | |
| 14:09:39 | bauzas | dansmith: wait | |
| 14:09:56 | bauzas | so we create allocations in self-heal | |
| 14:10:06 | dansmith | and delete them | |
| 14:10:09 | bauzas | but if we can't track it, then we delete the allocations | |
| 14:10:15 | bauzas | correct | |
| 14:10:34 | bauzas | so, we should filter out to delete the allocation, or to post it ? | |
| 14:11:11 | dansmith | not sure what you mean | |
| 14:11:21 | dansmith | we should avoid deleting it if we think we should but no host is set on the instance | |
| 14:11:36 | dansmith | like instance.host==None | |
| 14:11:56 | bauzas | we delete the allocations for instances that aren't tracked | |
| 14:12:16 | dansmith | ...right | |
| 14:12:17 | bauzas | okay, so the problem is about deleting allocations | |
| 14:12:21 | dansmith | yes | |
| 14:12:23 | bauzas | lemme see the code | |
| 14:13:13 | bauzas | I mean, can't see the race | |
| 14:13:29 | dansmith | 1. conductor creates the instance in the db, instance.host=None | |
| 14:13:40 | dansmith | 2. Conductor/scheduler/whatever creates the allocations in placement | |
| 14:13:52 | dansmith | 3. Conductor gets ready to send the instance to compute to build | |
| 14:14:07 | dansmith | 4. Compute runs self heal before that happens, sees allocations it doesn't know about, deletes them | |
| 14:14:14 | dansmith | 5. Instance arrives at compute ready to build | |
| 14:14:18 | dansmith | make sense? | |
| 14:14:23 | bauzas | ah ok | |
| 14:14:25 | bauzas | yup | |
| 14:14:37 | bauzas | no races yet, but possible race after my changes, gotcha | |
| 14:14:41 | dansmith | yes | |
| 14:16:12 | mriedem | althought, | |
| 14:16:16 | mriedem | *although, | |
| 14:16:30 | mriedem | now that you say this, i'm not sure where in the code #4 happens | |
| 14:16:45 | dansmith | hang on I'll show you | |
| 14:16:56 | bauzas | unless I'm blind, the only case where we get allocations, it's per instance | |
| 14:17:04 | bauzas | and we never delete them | |
| 14:17:19 | bauzas | only if when tracking, we removed the instance uuid | |
| 14:17:21 | dansmith | I had to find this for jaypipes a couple weeks ago | |
| 14:17:48 | bauzas | which is happening because we do get the list of instances per host | |
| 14:17:51 | dansmith | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L873-L885 | |
| 14:17:57 | bauzas | and we compare to what we have in memory | |
| 14:18:04 | dansmith | we get allocations based on compute node, | |
| 14:18:14 | dansmith | then any instances we don't have in the instances_dict provided get their allocations deleted | |
| 14:18:25 | bauzas | oh fuck, missed that one | |
| 14:18:30 | mriedem | dansmith: i saw that, | |
| 14:18:48 | mriedem | but how is an instance we're about to build that doesn't have instance.host set showing up in that list? | |
| 14:18:51 | bauzas | dansmith: but we should get the instance uuid to be scheduled part of the argument | |
| 14:19:04 | mriedem | via the allocations tied to the RP? | |
| 14:19:13 | mriedem | oh i see | |
| 14:19:14 | mriedem | yeah | |
| 14:19:16 | dansmith | mriedem: it's not in instances_uuids yet, but is in get_allocations() | |
| 14:19:20 | mriedem | right | |
| 14:19:23 | dansmith | bauzas: no | |
| 14:19:24 | mriedem | instance_uuids could be empty | |
| 14:19:27 | dansmith | right | |
| 14:20:12 | bauzas | if there are no tracked instances, correct | |
| 14:20:21 | openstackgerrit | John Garbutt proposed openstack/nova master: Remove all discoverable policy rules https://review.openstack.org/459608 | |
| 14:21:06 | mriedem | self.tracked_instances[uuid] = obj_base.obj_to_primitive(instance) | |
| 14:21:09 | bauzas | mriedem: dansmith: oooh man, I see the problem | |
| 14:21:19 | mriedem | so rather than pass instance_uuids to remove_deleted_instances, we'll have to pass the actual instances | |
| 14:21:23 | mriedem | to see if host is set or not | |
| 14:21:25 | bauzas | since instance.host is none, then the get_instances_per_host doesn't get it | |
| 14:21:26 | dansmith | mriedem: no | |
| 14:21:34 | dansmith | mriedem: because instances with no host won't be in tracked_instances | |
| 14:21:39 | bauzas | yup | |
| 14:21:43 | dansmith | mriedem: we have to look up instances we think we want to delete allocations for | |