Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-05
14:25:56 mriedem because in swap volume we know if we're doing old or new flow based on the bdm.attachment_id,
14:25:59 mriedem in attach_volume we don't
14:26:10 mriedem so we need to leave the attach_volume changes for the last patch that adds the new flow
14:27:24 ildikov ah, ok
14:27:46 ildikov so you wanted to split out like 10 lines of code change?
14:28:07 mriedem it's more than that
14:28:17 mriedem it's the new method, plus the usage in swap_volume, plus tests
14:28:18 ildikov ok, 20
14:28:51 mriedem ok - just leave it all in a 2K LOC change and we won't merge any of it if that's what you want
14:29:09 ildikov Jesus, Mary, St Joseph and the camel
14:29:19 ildikov sigh, no, I'll go and start over
14:29:52 mriedem i'm trying to help you split the things out that can be split out to make the main end patch more manageable for reviewers,
14:29:58 mriedem if we don't want to do that, then i'll give up
14:31:16 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix ValueError if invalid max_rows passed to db purge https://review.openstack.org/525628
14:31:16 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix TypeError in nova-manage db archive_deleted_rows https://review.openstack.org/525629
14:31:46 ildikov no, don't, I'm sorry, flying freaks me out, so I'm a few levels up regarding stress right now
14:32:56 ildikov and I've just uploaded the 167th revision which gives my stomach an extra bump... :/ :)
14:33:07 ildikov will ping you when I have a next version
14:33:25 mriedem ok
14:38:56 sambetts jaypipes: Is there a bug in Nova tracking the placement race condition we identified at the PTG between nova releasing the allocation and the ironic virt driver setting the number of resources available to zero?
14:40:06 jaypipes sambetts: not sure I follow you...
14:42:43 sambetts jaypipes: the race condition where on "nova delete" of an instance the allocation in placement is released so the node becomes free again, but its not actually free because Ironic is cleaning the node, so we set the resources to zero but for a brief period of time the node in placement can be reallocated because the resources are updated in a timed loop
14:44:34 jaypipes sambetts: but the node is not "available" according to the Ironic virt driver when it's being cleaned and therefore will not appear to the scheduler as passing the compute filter.
14:46:10 sambetts jaypipes: its only not avaiable because we set the avaiable resources for that node to zero, but the avaiable resources isn't refreshed instantly after an instance is deleted, but the allocation in placement is freed
14:46:45 sambetts so the node in placement can be reallocated until the resource tracker updates the avaiable resources
14:47:25 jaypipes sambetts: the allocation is deleted properly (the instance is no longer consuming resources on that node). it is the node itself that is marked as not available and therefore won't be scheduled to.
14:49:35 sambetts jaypipes: I'm not sure what you mean the node is marked as not avaiable, as far as I'm aware that happens by setting the resources to zero and then that causes the code to remove it from placement so it can't be scheduled too, is that what you are refering too
14:49:45 sambetts ?
14:49:47 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Add regression test for rebuilding a volume-backed server https://review.openstack.org/525632
14:49:48 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Only query BDMs once in API during rebuild https://review.openstack.org/525633
14:49:48 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Get original image_id from volume for volume-backed instance rebuild https://review.openstack.org/525634
14:51:19 jaypipes sambetts: I'm referring to the scheduler, not placement. the scheduler checks to see whether a compute node/service is up and available to take requests before it attempts to schedule an instance to it. that check will return False for an Ironic node that is cleaning state.
14:51:23 openstackgerrit Merged openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/525399
14:52:25 sambetts jaypipes: is that new? I've never heard of that before
14:52:54 jaypipes sambetts: so placement might return that Ironic node to the scheduler as having space (now that the allocation was deleted), but the scheduler won't pick it until the node is available.
14:53:06 jaypipes sambetts: no, that's been like that since the beginning.
14:54:06 jaypipes sambetts: https://github.com/openstack/nova/blob/master/nova/scheduler/driver.py#L55-L61
14:54:39 jaypipes though I'm looking at that now and seeing it's referring to the service (i.e. the nova-compute), not the baremetal node
14:54:42 jaypipes ffs
14:54:53 jaypipes this is why we can't ever have nice code...
14:55:09 sambetts :/ never seen anything like that in our driver, its all done based on resources
14:55:13 sambetts yeah :/
14:57:07 jaypipes sambetts: well, there's this patch which should at least help with the Ironic situation: https://review.openstack.org/#/c/513526/
14:57:41 jaypipes sambetts: that randomizes the returned results from placement so that (as is the case with Ironic) you won't always get back the same top node.
14:59:18 sambetts that'll certainly help, the other thing I think we suggested at the PTG was that we somehow instead of the resources getting marked as zero and then removed from placement all the time, we use the reserved field (although I'm still not sure that solves the race)
15:01:58 sambetts the race occurs because the resource tracker is async from the allocation getting deleted, so I'm sure what the right thing to do is there, unless we can force an refresh of that as soon as the resources are freed but then there is still a small period of time while the resource tracker runs
15:03:01 sambetts someone mentioned at the PTG that there are some hypervisors that have the same behaviour as this where even though an allocation has been deleted the resources aren't actually available for use yet
15:03:31 sambetts I'm trying to dig up the notes from the PTG session on it
15:10:10 sambetts hmmm I can't find any notes on the topic in the etherpads :( johnthetubaguy do you remember this conversation at the PTG ^
15:19:00 openstackgerrit Ildiko Vancsa proposed openstack/nova master: Add a new check to volume attach https://review.openstack.org/525622
15:19:00 openstackgerrit Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
15:19:01 openstackgerrit Ildiko Vancsa proposed openstack/nova master: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
15:20:47 ildikov mriedem: ^^
15:26:32 mriedem ack
15:44:12 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Add regression test for rebuilding a volume-backed server https://review.openstack.org/525664
15:44:13 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Only query BDMs once in API during rebuild https://review.openstack.org/525665
15:44:14 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Get original image_id from volume for volume-backed instance rebuild https://review.openstack.org/525666
15:48:38 openstackgerrit Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285
15:48:38 openstackgerrit Ildiko Vancsa proposed openstack/nova master: libvirt: Allow multiple volume attachments https://review.openstack.org/267587
16:01:40 edleafe mriedem: time for a question re: alternates for migration?
16:02:54 edleafe if I set return_alternates to True as you suggest, what should then happen to the alternates?
16:03:23 mriedem cold migrate / resize right?
16:03:47 edleafe mriedem: yeah: https://review.openstack.org/#/c/516707/17/nova/conductor/tasks/migrate.py@244
16:04:02 mriedem well, presumably the same thing as happens when build_and_run_instances gets alternates and sends them to compute, which can send them back to build_instances on a reschedule
16:04:42 mriedem so we have 2 reschedule loops:
16:05:11 edleafe well, I'm unfamiliar with those code pathways, so IYO will that be a major change to those methods?
16:05:54 mriedem 1. superconductor:schedule_and_build_instances -> compute:build_and_run_instance -> cellconductor:build_instances
16:05:54 mriedem 2. superconductor:resize_instance -> compute:prep_resize -> cellconductor:resize_instance
16:06:07 mriedem well, looking at https://review.openstack.org/#/c/511358/29/nova/compute/manager.py
16:06:18 mriedem it looks like it would be a matter of passing host_list to prep_resize in the compute
16:07:01 mriedem and like what you have here in conductor manager (this is cell conductor at this point during a reschedule):
16:07:04 mriedem https://review.openstack.org/#/c/511358/29/nova/conductor/manager.py@517
16:07:46 mriedem you would have to do the same for the resize reschedule here https://review.openstack.org/#/c/511358/29/nova/conductor/api.py@87
16:07:53 edleafe ok, next question: should I add that all to the existing patches, or split it into two?
16:08:01 mriedem now, we could arguably do the build and resize + alternates in separate patches
16:08:05 mriedem heh
16:08:14 mriedem split is obviously easier for review
16:08:20 mriedem it would mean 2 compute rpc version bumps
16:08:29 mriedem but service version bumps are free, so i don't think that's a big deal
16:08:33 mriedem dansmith: ^ agree?
16:09:55 edleafe ok, so I'll leave return_alternates=False in the current patch, and then flip it when in a new patch that bumps the compute RPC again.
16:10:07 mriedem makes sense
16:10:13 edleafe ok
16:11:26 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix doubling allocations on rebuild https://review.openstack.org/521662
16:11:28 mriedem jaypipes: ^ is that cve fix (now disclosed) with the bug link and a release note; melwitt can you also look at ^
16:12:40 melwitt mriedem: sure
16:13:09 dansmith mriedem: I like to avoid bumps of both rpc and service version when possible, and it usually is.. but yes, they're free
16:24:28 bauzas mriedem: just +2d the cve fix
16:24:42 mriedem bauzas: thanks
16:24:52 bauzas now the bug is disclosed, we can move on
16:25:13 bauzas sorry for having paid a lot of attention for that bug tho, but the overall direction looked good to me a while ago
16:25:20 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Add regression test for rebuild with new image doubling allocations https://review.openstack.org/523213
16:25:20 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix doubling allocations on rebuild https://review.openstack.org/523214
16:25:31 bauzas ie. using the hints for passing whether it's a rebuild or not
16:30:04 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix doubling allocations on rebuild https://review.openstack.org/523214
16:32:51 mriedem sdague: can you hit https://review.openstack.org/#/c/523194/ again? alex had pointed out something so i lost your +2
16:46:32 openstackgerrit Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239
16:46:32 openstackgerrit Ed Leafe proposed openstack/nova master: Refactor the code to check for sufficient hosts https://review.openstack.org/520242
16:46:33 openstackgerrit Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854
16:46:33 openstackgerrit Ed Leafe proposed openstack/nova master: Move the to_dict() method to the Selection object https://review.openstack.org/523492

Earlier   Later