Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-07-31
15:30:43 cdent gibi_: see what happens if you get rid of the first == in the and_
15:30:52 gibi_ checking
15:31:01 bauzas yeah that will work
15:31:09 bauzas obviously
15:31:17 cdent dansmith: we had in our brains that we were removing all allocations for the consumer
15:31:24 cdent but the code is only removing some of them
15:32:33 dansmith meaning, the api is such that all should be removed, but some bug in the db side of placement prevents that from happening?
15:32:50 cdent yeah
15:32:59 dansmith that seems ungood
15:33:15 cdent it looks like the original goal was to _only_ remove exact matches (not sure why)
15:33:22 cdent (the comment says as much)
15:33:27 dansmith hmm
15:33:41 dansmith jaypipes seemed to think that should be fully atomic, so that seems weird
15:33:59 gibi_ I can confirm that removing the rp == from the db code makes the placement API behave as expected in this particular case http://paste.openstack.org/show/617034/
15:34:16 cdent and we had three authors on that particular change, so is probably going to be hard to remember the whys and wherefores
15:34:18 cdent gibi_: nice
15:34:56 cdent dansmith: since we've been assuming all this time that the behavior is one thing and not the other, I think we should just change it
15:35:12 cdent also I don't think there is much risk, because we haven't been doing any dual provider allocations up til now
15:35:39 cdent edleafe: you listening ^ ?
15:35:42 cdent brb
15:36:19 gibi_ I'll let you guys to report a bug and propose a fix as my workday is over soon.
15:36:46 cdent gibi_: right on, thanks very much for all your digging
15:37:16 jaypipes rock on, thanks gibi
15:37:25 dansmith cdent: right, and I think that's the thing the api espouses anyway
15:38:10 edleafe cdent: sorry, distracted by meeting. Trying to follow, though
15:38:29 jaypipes dansmith, cdent: so shall I remove that == rp_uuid line in a separate patch or in the same confirm_resize() patch?
15:38:33 cdent edleafe: no worries, just wanted your memory if anything
15:38:39 dansmith jaypipes: definitely separate
15:38:49 dansmith jaypipes: we probably need to backport that right?
15:38:51 jaypipes dansmith: ahead of confirm_resize() yeah?
15:38:55 dansmith yeah
15:38:55 jaypipes dansmith: ya
15:39:00 jaypipes ok, I'm on it.
15:39:35 bauzas jaypipes: if we remove the == rp, wouldn't that be a problem for racy calls ?
15:39:55 jaypipes bauzas: no
15:39:57 bauzas jaypipes: because previously we were just supposing the generation bit to be updated
15:40:06 bauzas given that generation is per RP
15:40:18 jaypipes bauzas: the generation is on the RP. this code is removing allocation records.
15:40:43 jaypipes bauzas: this is leftover code that was assuming a single RP for an allocation
15:41:14 jaypipes 1509gg
15:41:17 jaypipes ffs
15:43:02 cdent the bug: https://bugs.launchpad.net/nova/+bug/1707669
15:43:04 openstack Launchpad bug 1707669 in OpenStack Compute (nova) "[placement] put allocations does not do a full overwrite of existing allocations" [High,Triaged]
15:44:34 jaypipes cdent: danke
15:50:59 sdague 2 easy bugs to close - https://review.openstack.org/#/c/486642/
15:51:07 sdague https://review.openstack.org/#/c/488530/
15:51:39 sdague plus mriedem here is my nova-manage patch to do list_cells with urls by default - https://review.openstack.org/#/c/487860/
15:58:00 mriedem sdague: wasn't the limits stuff added here https://review.openstack.org/#/c/486642/ for a security bug?
15:58:11 edleafe cdent: ok, catching up. Looking through the long paste, I don't see where we clean up the allocations for the source host
15:58:11 mriedem or was the security issue just that it was completely unbounded to begin with?
15:58:22 sdague mriedem: yes, the security issue was completely unbounded
15:58:28 sdague it was originally set to 2 seconds
15:58:41 edleafe We PUT the allocations for the target, but don't seem to remove the source allocs
15:58:43 sdague then after that actually failed in the gate some times, it was bounced to 8
15:58:45 cdent edleafe: it was the third of three PUTs
15:58:54 sdague but there are real world reports that times out in legit cases
15:59:28 sdague it still gives us a backstop though so you can't make a malicious image that eats all the cpu forever
15:59:46 jaypipes sdague: done
16:00:20 sdague jaypipes: thank you
16:01:07 edleafe cdent: ok, I see that now. So is that PUT not doing a true PUT?
16:01:25 edleafe cdent: or just for the consumer/RP combo?
16:01:50 cdent it's not doing a true put
16:01:59 cdent https://bugs.launchpad.net/nova/+bug/1707669
16:01:59 openstack Launchpad bug 1707669 in OpenStack Compute (nova) "[placement] put allocations does not do a full overwrite of existing allocations" [High,Triaged]
16:02:09 cdent server-side is borked
16:02:43 edleafe cdent: what I'm asking is if it's just doing a true PUT for RP/consumer
16:02:54 edleafe cdent: and not for consumer
16:02:58 edleafe *just
16:03:29 cdent edleafe: sorry, I'm not parsing you
16:03:37 mriedem sdague: comments on more testing in https://review.openstack.org/#/c/487860/3/nova/tests/unit/test_nova_manage.py
16:03:46 mriedem the db url parsing has broken a few times
16:03:52 cdent the bodies of the PUTs, all three, are correct, according to our expectations of how the api behaves. the api doesn't not behave as we expect
16:03:58 mriedem and we have an outstanding bug for db urls with TLS info in them
16:05:44 sdague mriedem: so, using urlparse we're never going to hit that, but I'm fine putting some more in there
16:06:21 mriedem we were originally using urlparse
16:06:24 mriedem which was causing problems
16:07:00 edleafe cdent: yes I see that. Is the disconnect due to the fact that we expect when we PUT allocations for a consumer, it first removes any existing allocs for that consumer?
16:07:33 ralonsoh_ stephenfin: hi, can you take a look at my question in https://review.openstack.org/#/c/427145/? PS7
16:07:53 cdent edleafe: that's the bug. we expect that, but it does not do that. what it was doing is removes only those allocations where rp uuid and consumer uuid match
16:07:57 cdent which is not enough
16:09:25 edleafe cdent: ok, that's what I was asking above about "or just for the consumer/RP combo?"
16:09:33 edleafe I'm following now
16:09:39 edleafe Is this being worked on?
16:09:45 edleafe IOW, can I help?
16:09:47 cdent yes, jay's integrating it in his stack
16:09:59 jaypipes just running new tests now..
16:10:00 edleafe ok cool
16:10:10 openstackgerrit Matt Riedemann proposed openstack/nova master: Clean variable names and docs around neutron allocate_for_instance https://review.openstack.org/489267
16:10:11 cdent edleafe: I think the main thing to do is try to break stuff
16:10:11 mriedem i hate the allocate_for_instance code ^
16:11:27 edleafe cdent: roger that.
16:15:54 mdbooth gibi_: Just looking at https://review.openstack.org/#/c/487958/4/nova/tests/functional/test_servers.py
16:16:02 mdbooth gibi_: Any idea how close that might be to landing?
16:16:52 mdbooth gibi_: I need to add a test for https://review.openstack.org/#/c/462521/ and ServerMovingTests looks like an obvious place for it
16:17:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Clean variable names and docs around neutron allocate_for_instance https://review.openstack.org/489267
16:20:42 cdent mdbooth: I think gibi_'s gone. We could potentially land that code soon if dansmith and jaypipes think it belongs alongside jay's stack, but it is primarily for testing how allocations are handled, not verifying reverts etc. Not sure if that makes it better or worse.
16:21:42 jaypipes cdent: five minutes.
16:22:03 mdbooth cdent: Well the setUp there creates an environment with 2 computes sufficient for running resize(), which is exactly what I need
16:22:50 cdent mdbooth: yeah, it apparently also fixes some issues with doing that
16:23:11 mdbooth cdent: I'm not surprised in the slightest there are dragons.

Earlier   Later