Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-08-09
14:40:48 dtantsur thanks dansmith, I think I know what to do
14:42:59 dtantsur mmmm, coffee :)
14:43:25 edleafe dtantsur: yeah, if the API already returns a particular code, and what you're changing isn't a thing that people rely on, it's a bugfix, not a version bump
14:43:36 edleafe dtantsur: so yeah, what mordred said
14:44:11 jaypipes mriedem: k, sorry, done now.
14:44:18 dtantsur edleafe: okie, I'll bake a patch today
14:44:22 edleafe dtantsur: if a node has no resource_class set and an instance on it, and then the resource_class gets set, the next time through the instance flavor will get updated
14:44:29 jaypipes mriedem: I'm not entirely sure how one would add tests for a continue block...
14:44:49 edleafe dtantsur: dansmith: that's why the 'seen' cache is keyed on (instance_uuid, rc)
14:45:48 mriedem jaypipes: i guess you'd assert that something *didn't* happen
14:45:50 mriedem since you continued
14:46:04 mriedem but anyway, it can be a follow up given the circumstances with time
14:46:47 jaypipes mriedem: k. answered your comment about local delete..
14:47:24 edleafe dtantsur: I'll revise that migration patch to remove the support for a node with an instance changing its resource_class
14:47:42 dansmith edleafe: dtantsur thanks a bunch
14:48:46 dtantsur np
14:55:08 openstackgerrit Eric Fried proposed openstack/nova master: Get auth from context for glance endpoint https://review.openstack.org/490057
14:55:08 openstackgerrit Eric Fried proposed openstack/nova master: nova.utils.get_ksa_adapter() https://review.openstack.org/488137
14:55:08 mriedem dansmith: jaypipes: is this required for rc1 or just sugar? https://review.openstack.org/#/c/491012/
14:55:19 dansmith mriedem: required
14:55:47 jaypipes dansmith: to be perfectly frank, I don't know. :(
14:55:55 jaypipes sorry, that was for mriedem ^
14:56:12 jaypipes mriedem: see the comment I just responded to edleafe and dansmith with.
14:56:45 jaypipes mriedem: basically boils down to "I totally think we probably might need to maybe do something here, but I'm not sure what..."
14:56:48 dansmith jaypipes: huh? before that patch pike computes are behaving like ocata ones
14:56:56 dansmith that's where it was when I left it anyway
14:58:00 mriedem alright i'll review https://review.openstack.org/#/c/488510/ in the meantime then
14:58:34 jaypipes dansmith: well, I guess after going through the scenarios in my head, I'm wondering what we really need to do there. Probably just need another hangout session with you on it.
14:58:36 dansmith jaypipes: L1038 is the critical bit: https://review.openstack.org/#/c/491012/7/nova/compute/resource_tracker.py
14:58:40 openstackgerrit Eric Fried proposed openstack/nova master: Get auth from context for glance endpoint https://review.openstack.org/490057
14:58:48 openstackgerrit Sean Dague proposed openstack/nova master: Clean up *most* ec2 / euca2ools references https://review.openstack.org/492166
14:58:48 openstackgerrit Sean Dague proposed openstack/nova master: Add documentation for documentation contributions https://review.openstack.org/492124
14:59:04 dansmith jaypipes: without that line, we don't move the ball forward with comptues just stomping all over everything on both sides of a boot or move operation
14:59:24 jaypipes dansmith: oh, I totally get that. the part I'm wondering about is whether that "heal allocations" method needs to change now.
14:59:27 dansmith er, without gating that line on the version
14:59:44 dansmith jaypipes: that's not what he was asking, right? he was asking if the whole change is required for rc1 right?
15:00:02 jaypipes dansmith: ah, sorry. mriedem, yes, it is.
15:00:19 jaypipes mriedem: I was -Workflow on it to discuss that bit about the heal allocations.
15:00:26 jaypipes sorry for confusion.
15:00:31 mriedem np
15:00:56 mnaser so: there should never be a scenario where an instance fails to spawn with local variable referenced before assignment.. right?
15:00:56 dansmith jaypipes: your non-functional continue in a conditional is part of your extra debug logging I assume, and that was all you so..whatever you want to do there
15:01:16 mriedem mnaser: correct
15:01:21 mnaser because im trying to go through this code and i cant find how/why it's happening
15:01:32 mnaser okay. i'll go do some more in depth checking then
15:01:39 mriedem UnboundLocalError right?
15:01:48 jaypipes dansmith: heh, yeah... I was tempted to put into the log debug message something like "we're really not sure whether we even get here any more and if we do, what we should do anyway" ;)
15:01:52 mriedem seems that would be obvious
15:02:09 mnaser mriedem the manager is catching the original exception so its making it a tad harder
15:02:42 mnaser based on my search, it should either be in nova/objects/numa.py or nova/virt/hardware.py as those are the two references to it
15:02:43 mriedem oh, LOG.exception?
15:02:55 dansmith jaypipes: yeah, and it might be useful. Just don't say "We were completely burned out at the end of pike and this seems like a bad thing we probably should have handled. Sorry about that."
15:03:12 jaypipes dansmith: don't give me ideas... :P
15:03:27 mnaser :313
15:03:27 mnaser mriedem i'll have to do, it's def not catching by default, compute logs is just showing: 2017-08-09 14:33:00.758 4242 DEBUG nova.compute.utils [req-91225f58-71c5-44d3-ae5c-734986b4a3f7 16e021e0ed5f47b68c095d6885f18f4b d7594b0298b54bcc9e4e0f252e1da2e4 - - -] [instance: 18b91008-d42f-4eac-b57d-07195e7774ba] local variable 'sibling_set' referenced before assignment notify_about_instance_usage /usr/lib/python2.7/site-packages/nova/compute/utils.py
15:03:36 dansmith mriedem: comment in here about the wording https://review.openstack.org/#/c/491582/6
15:03:45 jaypipes dansmith: how about this? LOG.debug("We used to think we were indecisive. Now we're not so sure.")
15:04:07 dansmith jaypipes: that seems fine to me. highly truth-based
15:04:15 dansmith jaypipes: but I'd only +1 it and wait for others to +2
15:04:15 jaypipes very truthy indeed.
15:07:04 mriedem dansmith: replied https://review.openstack.org/#/c/491424/7/releasenotes/notes/pike_prelude-fedf9f27775d135f.yaml - so you want me to add those or leave it?
15:08:22 dansmith mriedem: I didn't realize that wasn't +A.. I would have.. I was just suggesting that maybe circling back and putting it in there might be worthwhile, but it's really not a big deal
15:08:38 dansmith it's on its way now
15:08:46 openstackgerrit Balazs Gibizer proposed openstack/nova master: replace chance with filter scheduler in func tests https://review.openstack.org/491529
15:08:48 dansmith I was just commenting on your comment
15:10:06 mriedem want to send this in also https://review.openstack.org/#/c/491581/ ?
15:10:11 mriedem it was previously approved
15:10:27 mriedem pad your stats before i wreck your stats on these RT patches :)
15:10:42 mnaser would someone be kind enough to just eye this for a second with me before i dive deeper? is it possible that i get an error of sibling_set being referenced before assignment if: the siblings_set is empty, therefore the loop next occurs and it is referenced in line 805 (outside the loop) -- https://github.com/openstack/nova/blob/stable/newton/nova/virt/hardware.py#L777-L807
15:11:11 mnaser sorry poorly worded that, the loop is pretty much skipped over so sibling_set is never set to anything and it's referenced in the if statement below it
15:11:32 dansmith mriedem: :(
15:12:05 dansmith mnaser: looking
15:12:23 mriedem mnaser: https://github.com/openstack/nova/blob/stable/newton/nova/virt/hardware.py#L805 would be the problem right?
15:12:32 mriedem if sibling_sets.items() was empty
15:12:41 mriedem then there is no sibling_set variable set in the for loop above
15:12:57 mnaser thats what i was guessing -- kinda wanted a second pair of eyes before i dive in deeper in the wrong place
15:13:04 mnaser i will check and see what the value of sibling_sets is
15:13:07 dansmith yeah it's referencing the loop variable
15:13:39 mnaser ok cool, i'll do some more checking and see what the sibling set value is when it works and when it doesnt
15:13:56 mnaser (oddly enough, it fails only on the *last* instance to go in the server -- ex: if it fits 15 VMs, 14 will go in, the 15th will fail with that)
15:14:17 dansmith mnaser: you could just put 798 and below inside an "if sibling_sets" conditional
15:14:28 dansmith mnaser: then it won't run if the loop didn't do a thing, which would avoid the problem
15:14:55 mriedem if only stephenfin were around to harass
15:15:05 dansmith it'd be much better to just set something before the loop to None, and then set it inside the loop and only run the bottom code if we found a thing
15:15:18 dansmith because it's just using the last value it iterated over
15:15:40 mnaser yeah that seems cleaner, but i also wonder if the issue is siblings_set being empty
15:15:44 dansmith sibling_set has to be a tuple for the use on L805
15:15:50 mnaser becuase maybe it shouldn't be and thats the issue
15:15:50 dansmith so you can use None as the sentinel
15:16:19 dansmith mnaser: well, it's fragile code so it deserves fixing regardless, IMHO
15:16:21 mnaser so i just want to make sure the original issue isnt siblings_set being empty?
15:16:43 mnaser true
15:16:44 mnaser i was a bit taken aback seeing a variable referenced before assignment in nova's code :p
15:16:45 dansmith if it being empty is possible (which it clearly is) and that's fatal, then this needs to check for it and log a warning
15:17:38 dansmith we could ask sahid
15:17:50 openstackgerrit Matt Riedemann proposed openstack/nova master: Add release note for shared storage known issue https://review.openstack.org/491582
15:18:31 gibi mriedem, jaypipes, dansmith: As Jay's resize confirm fix is almost done could you take a look on the other resize bugfix https://review.openstack.org/#/c/491491
15:19:54 mnaser yum, theory validated - SIBLING_SETS: defaultdict(<type 'list'>, {}) _pack_instance_onto_cores /usr/lib/python2.7/site-packages/nova/virt/hardware.py:781
15:20:04 mnaser now time to understand the siblings_set and why its empty
15:20:35 danpb dansmith: ?

Earlier   Later