| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-16 | |||
| 18:33:32 | mriedem | uh oh | |
| 18:33:36 | mriedem | someone woke the mrodden troll | |
| 18:33:38 | owalsh | oop ^^ | |
| 18:36:48 | owalsh | mriedem (too many m... nicks here), dansmith do we need to worry about it for pip or setup.py? | |
| 18:37:19 | dansmith | owalsh: pip will do the right thing I think | |
| 18:37:28 | dansmith | owalsh: setup.py has this kind of issue in other places, so I think we're probably okay there | |
| 18:43:40 | mordred | mriedem, jaypipes: just reading a little scrollback - may or may not be relevant - but we're seeing not-infrequent flaky test issues in shades functional tests these days inside the flow "delete server ; delete volumes that were attached to server" | |
| 18:44:06 | mordred | mriedem, jaypipes: specifically because the volume throws an error that you can't delete an attached volume | |
| 18:44:22 | mordred | I know that's a cinder not a nova thing - but since you're tlaking about the attach flow I figured I'd mention it | |
| 18:45:17 | mriedem | mordred: you're racing | |
| 18:45:32 | smcginnis | Yeah, that sounds like the attach hasn't finished before trying to delete. | |
| 18:45:34 | melwitt | does anyone know if there's a way to see all of the code module docs on https://docs.openstack.org/developer/nova ? | |
| 18:45:37 | smcginnis | *dettach | |
| 18:45:40 | mriedem | smcginnis: the detach | |
| 18:45:43 | mriedem | yeah | |
| 18:45:53 | smcginnis | Keyboard... | |
| 18:46:04 | mriedem | mordred: tempest flow is usually create volume, create server, attach volume to server, detach volume, delete volume, delete server, | |
| 18:46:06 | mriedem | sometihng like that | |
| 18:46:21 | sdague | melwitt: we pulled that part out of the docs a while ago because so little was actually docs | |
| 18:46:24 | sdague | docced | |
| 18:46:27 | mriedem | nova will detach the volume, but you have to wait for the volume to be 'available' before you can delete it | |
| 18:46:33 | melwitt | k, thanks sdague | |
| 18:50:46 | mordred | mriedem: yah - that's the basic flow we're doing now (I just recently added the wait-for-volume-available) - tempest is apparently also seeing some flakiness in the same general flow | |
| 18:50:56 | mordred | mriedem: jordanP is looking in to it some | |
| 18:51:15 | mriedem | guess it depends on the specific low, | |
| 18:51:16 | mriedem | *flow | |
| 18:51:29 | mriedem | if you do boot from volume with delete_on_termination=True, then nova deletes the volume | |
| 18:51:37 | mriedem | when you delete the server | |
| 18:55:49 | mordred | yah. we have different tests for that - it was "boot server ; create volume ; attach volume ; delete server ; delete volume" that's the issue - but I'll keep working to figure out more places I can poll :) | |
| 19:12:23 | dansmith | mriedem: melwitt: do you have any opinion on this? https://review.openstack.org/#/c/433362/3/nova/api/openstack/compute/servers.py@1159 | |
| 19:12:47 | dansmith | like, whether we should target from the REST API side, or modify every compute/api method to take the mapping, | |
| 19:12:59 | dansmith | or stash the mapping legally or illegally inside the instance somewhere? | |
| 19:14:54 | frickler | mriedem: not sure if you have seen this, there seems to be another nonvc issue, unrelated to py3, but in the multinode case. I'm thinking https://review.openstack.org/434996 might help but that surely needs some more eyes on it | |
| 19:15:54 | melwitt | dansmith: I think I remember we discussed this a bit before with alaski where an idea came up about stashing the targeted context in instance._context to avoid having to target everywhere | |
| 19:16:11 | dansmith | hmm | |
| 19:16:52 | dansmith | melwitt: the thing that kinda concerns me about that is that it becomes super obscure such that someone will come along and use that context to try to do something elsewhere and not know it's targeted | |
| 19:16:53 | melwitt | there's pros and cons to each way, so I think there wasn't like an obvious choice. I don't think I like the idea of modifying every compute/api method to take a mapping, at least | |
| 19:17:02 | dansmith | I mean, I guess that's the problem with targeting in the REST side anyway | |
| 19:17:33 | melwitt | yep. that's why I was thinking I'm wary of making it too obscure | |
| 19:17:36 | dansmith | melwitt: yeah, well, glad we agree on that, I'd like to rule that out right here and now :) | |
| 19:17:43 | dansmith | the api modification route I mean | |
| 19:17:48 | melwitt | yeah | |
| 19:18:22 | dansmith | melwitt: right now target_cell modifies the context you pass it I think, | |
| 19:18:37 | dansmith | which means we'd need to change that or make another thing to say "give me a targeted context" | |
| 19:19:33 | alaski | I can't recall the various pros/cons I worked through in my head, but I do like the idea of stashing the target somewhere. that could be the targeted context on instance, or update the context to be targeted when used to pull from a cell | |
| 19:19:42 | melwitt | yeah. if you do with...as then I think you could set it on the instance._context and let it go on its way from then on. or am I wrong on that | |
| 19:20:13 | dansmith | melwitt: you can't I don't think | |
| 19:20:23 | melwitt | because it would set it back. okay | |
| 19:20:24 | dansmith | melwitt: because returning from the with will untarget the context you set on instance | |
| 19:20:28 | dansmith | yeah | |
| 19:20:40 | dansmith | alaski: yeah, I like that because it is less work, I just worry about it being super confusing later | |
| 19:21:02 | melwitt | yeah. originally I had two methods for targeted context, one that returns one and one that is the context manager. got rid of one because at the time because there wasn't a strong case for needing it yet | |
| 19:21:46 | dansmith | the event patch needs to be more prescriptive because it has to collate multiple events, | |
| 19:22:00 | dansmith | but I can make the top patch do the magic thing and see how much has to change all at once I guess | |
| 19:22:53 | alaski | dansmith: yeah. I'm slowly remembering that I liked storing the targeted context on object._context and just treating an object as attached to the cell it came from, since nothing else should pull object._context to try to use it for other purposes | |
| 19:23:39 | dansmith | alaski: meaning it's a copy and targeted, whereas the context we pass to methods remains clean? | |
| 19:23:52 | dansmith | that is probably less obscure than how I was thinking about it | |
| 19:24:02 | alaski | yeah | |
| 19:24:23 | alaski | there's still some risk, but it seemed the least magic | |
| 19:24:34 | dansmith | melwitt: if you agree, I'll go do that thing and we can blame alaski for any fallout sometime when he's not looking | |
| 19:25:18 | melwitt | I guess the thing is, will that do the job. because the target_cell modifies the external context, not the internal one. so will things look at instance._context to switch | |
| 19:26:09 | melwitt | dansmith: yeah, I'm not too opinionated about it. alaski's thinking makes sense to me, plus we can blame him | |
| 19:26:37 | dansmith | melwitt: well, we would still target the external context if and when we need to, | |
| 19:26:43 | dansmith | melwitt: but instance.save() would just work (tm) | |
| 19:26:54 | melwitt | I'm just not sure how it's going to work out because of context vs instance._context, but yeah. yeah, true | |
| 19:27:01 | openstackgerrit | Sean Dague proposed openstack/nova master: WIP: verify project_id when quotas are checked https://review.openstack.org/435010 | |
| 19:27:19 | dansmith | melwitt: well, given the fun I just had with the test fixture, | |
| 19:27:33 | dansmith | it'll be interesting to see what doesn't fall into that realm with just the instance context set | |
| 19:27:42 | dansmith | like, looking up BDMs for an instance in the middle of an operation or something | |
| 19:28:12 | dansmith | melwitt: like, we'll make the rpc call with an untargeted context, which means my serializer hack won't work | |
| 19:28:19 | dansmith | which will suck | |
| 19:28:20 | dansmith | a lot. | |
| 19:28:41 | dansmith | unless the rpc layer auto-targets the context before it makes the call | |
| 19:28:50 | melwitt | hm, yeah that's what I was just thinking | |
| 19:28:56 | dansmith | that makes me sad | |
| 19:28:57 | melwitt | we have the lookup things in there | |
| 19:29:19 | dansmith | right, which I thought we wanted to maybe replace by going back to targeting... | |
| 19:29:26 | dansmith | we added transport back into the context, right? | |
| 19:29:33 | dansmith | ugh, I dunno | |
| 19:29:41 | melwitt | really? I didn't know we wanted to :P | |
| 19:29:56 | melwitt | not yet. I have a patch up that does that but you crapped on it | |
| 19:29:58 | dansmith | didn't we discuss that? I thought we added it back in | |
| 19:30:06 | dansmith | hah, well | |
| 19:30:15 | dansmith | link? | |
| 19:30:28 | melwitt | we don't technically need it, it would just save lookups if we did | |
| 19:30:39 | melwitt | https://review.openstack.org/411543 | |
| 19:30:51 | dansmith | right, I thought that was the plan.. to not have to do that lookup again in the rpc layer and thus go with the targeting keeping things pointed in the right place | |
| 19:31:08 | dansmith | melwitt: oh please.. that's like a small dingleberry | |
| 19:31:19 | dansmith | I didn't even -1 | |
| 19:31:25 | melwitt | I did ^ weirdly because we have tried to not pass cell mapping around so user/pass don't get dumped out in some exception somewhere, potentially | |
| 19:31:40 | melwitt | haha, I know | |
| 19:32:26 | melwitt | okay, well, the original plan was to target everything explicitly but then we thought it would be easier if we just looked it up in there | |
| 19:33:20 | dansmith | so I know I raised it as an issue, but I wonder what kind of things might come up from just having a targeted external context all the time | |
| 19:33:22 | melwitt | another thing is belliott had said the construction of the rpc transport objects is expensive, so there was value in caching them that way instead of creating them on-the-fly each target | |
| 19:33:34 | dansmith | the biggest thing would be going back to the api database again, but that shouldn't be affected I think right? | |
| 19:33:56 | dansmith | melwitt: we can still cache them even if we target them early | |
| 19:34:31 | melwitt | yeah. we'd have to find a place for that, maybe nova.context is not the best place is another thing belliott didn't like about it | |
| 19:34:55 | dansmith | yeah, I don't really get those concerns, but.. | |
| 19:34:56 | melwitt | dansmith: yeah going back to api database isn't affected. those can use the global api db context managers | |
| 19:35:18 | dansmith | yeah, so, maybe I'm just being overly concerned about doing the target hard and early | |