| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 19:20:59 | dansmith | mdbooth: the context in the instance doesn't have anything to do with where the rpc call will go | |
| 19:21:13 | dansmith | and on the other side of the call, the context is reset to what was used for the rpc call | |
| 19:22:24 | mdbooth | dansmith: In this case the cell_mapping comes from a weird mappings dict passed in | |
| 19:23:02 | mdbooth | The bug is that if the first instance in the host list is actually from the wrong cell, it'll use the wrong cell context | |
| 19:23:05 | cdent | jaypipes: are we decided that step_size of 1 is okay on the ironic inventory because the modulo math comes out right in that (special) case? | |
| 19:23:24 | jaypipes | cdent: I think so | |
| 19:23:34 | cdent | cool, just checking | |
| 19:24:39 | openstackgerrit | melanie witt proposed openstack/nova master: Count resources to check quota for cells https://review.openstack.org/416521 | |
| 19:25:06 | mdbooth | dansmith: NM. I have to shoot, but I'm going to leave this open and look at it again first thing in the morning. | |
| 19:25:12 | dansmith | mdbooth: okay I see what you mean, but I don't think that's right.. the instancemapping will point to the hostmapping where its record currently resides | |
| 19:25:23 | dansmith | mdbooth: then we add in the other destination, | |
| 19:25:40 | dansmith | but the instance gets added to a host list based on instance.host, which matches hostmapping, and thus the cell | |
| 19:25:54 | mdbooth | dansmith: That's the bug, btw | |
| 19:26:06 | mdbooth | instance is added to host list based on _get_relevant_hosts | |
| 19:26:24 | mdbooth | So a migrating instance will be in *2* host lists, one of which it's not actually on | |
| 19:26:50 | mdbooth | If the first entry in the list just happened to be a migrating instance which wasn't actually on this host | |
| 19:26:59 | mdbooth | We'd use its cell_mapping anyway | |
| 19:27:07 | mdbooth | That's ok as long as we're not migrating between cells | |
| 19:27:07 | dansmith | ah, this is actually what you mean: | |
| 19:27:13 | dansmith | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4082 | |
| 19:27:17 | mdbooth | But it's seriously obtuse | |
| 19:27:24 | dansmith | yeah, I see now | |
| 19:27:33 | dansmith | I was looking at the api side | |
| 19:27:46 | mdbooth | dansmith: Yeah, that. | |
| 19:28:23 | dansmith | we should just pass the mapping in from the api side instead of inferring it late there | |
| 19:28:39 | dansmith | my NOTE was probably really "be lazy and don't add a new parameter" | |
| 19:29:24 | mdbooth | dansmith: As I say, I'll leave it open and poke at it in the morning | |
| 19:29:30 | dansmith | alright | |
| 19:30:02 | openstackgerrit | Dan Smith proposed openstack/nova master: Make discover_hosts only query for unmapped ComputeNode records https://review.openstack.org/427903 | |
| 19:33:59 | mriedem | gah, mocking out a context manager and asserting calls on the mock is not fun | |
| 19:36:42 | openstack | Launchpad bug 1671615 in openstack-manuals "Networking with nova-network in Administrator Guide" [Undecided,New] | |
| 19:36:42 | mriedem | docs bug if someone wants it https://bugs.launchpad.net/openstack-manuals/+bug/1671615 | |
| 19:37:35 | dansmith | mriedem: so this is my patch series to make the discovery faster: https://review.openstack.org/#/c/427903/ | |
| 19:37:41 | dansmith | maybe should have a specless blueprint? | |
| 19:37:53 | mriedem | yeah probably | |
| 19:38:20 | mriedem | mdbooth: dansmith: got any tricks for asserting calls on a context manager method? | |
| 19:38:27 | mriedem | like, i want to use mock.assert_has_calls, | |
| 19:38:33 | mriedem | on just the args to the method, | |
| 19:38:39 | mriedem | but the context manager enter/exit stuff messes that up | |
| 19:38:54 | dansmith | mriedem: mock it with a side effect that is a context manager | |
| 19:39:04 | dansmith | there's some other way to mock just __enter__ but it's always obscure so I don't do it | |
| 19:39:42 | dansmith | mriedem: I have been meaning to switch the way that fixture works a bit so we can more easily mock the non-context-manager set_target_cell in the tests | |
| 19:39:46 | efried | __enter__ can be mock.patched like anything else, neh? | |
| 19:40:15 | mriedem | yes, the problem is i don't need to mock __enter__ and __exit__ | |
| 19:40:17 | mriedem | the problem is, | |
| 19:40:24 | mriedem | assert_has_calls things i have 3 more calls than i actually care about | |
| 19:40:29 | dansmith | yeah | |
| 19:40:32 | dansmith | it's ugly | |
| 19:40:37 | dansmith | I made it work one time, | |
| 19:40:40 | dansmith | and then undid it | |
| 19:41:02 | mriedem | maybe i can just use mock.method_calls | |
| 19:41:08 | mriedem | which doesn't include magic methods | |
| 19:42:07 | asettle | Sorry mriedem was off for the evening (I'm UK based), but basically, yes :) big fat warning. | |
| 19:42:15 | asettle | "This is deprecated as of Ocata" type thing | |
| 19:42:17 | asettle | Thanks for noticing that! | |
| 19:43:14 | efried | mriedem https://github.com/powervm/pypowervm/blob/develop/pypowervm/tests/tasks/test_storage.py#L93-L99 | |
| 19:43:18 | asettle | cdent and jaypipes - how did you both get on? | |
| 19:43:41 | efried | mriedem Or perhaps https://github.com/powervm/pypowervm/blob/develop/pypowervm/tests/tasks/test_storage.py#L355-L363 is closer to what you want. | |
| 19:44:02 | jaypipes | asettle: was there an updated revision pushed? I didn't see one? | |
| 19:44:26 | asettle | jaypipes: nope. sorry, that's not what I meant. Just checking in to see if you or cdent had other thoughts/concerns since we last spoke | |
| 19:44:30 | mriedem | efried: that's a bit ugly imo | |
| 19:44:33 | asettle | I see your comments on patch 8 | |
| 19:44:36 | asettle | Thank you :) | |
| 19:44:40 | mriedem | the .__enter__.return_value | |
| 19:44:56 | cdent | asettle: i put more comments on the review, but basically I was able to confirmation that the placement parts are okay | |
| 19:45:09 | efried | mriedem Would it be less ugly if __enter__ wasn't a double-underscore method? | |
| 19:45:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Make discover_hosts only query for unmapped ComputeNode records https://review.openstack.org/427903 | |
| 19:45:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Add ComputeNode.mapped field https://review.openstack.org/427901 | |
| 19:45:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Add ability to query for ComputeNodes by their mapped value https://review.openstack.org/427902 | |
| 19:45:36 | asettle | Terrific! Ugh thank you so much cdent :D | |
| 19:45:56 | jaypipes | asettle: nope, besides those suggestions I put on the initial review, I'm good! :) | |
| 19:47:40 | asettle | Ugh you guys are excellent :D thank you both and to mriedem for looking at this all week :) Crossing my fingers that some of these edits should set us on the right path for both RDO and Ubuntu | |
| 19:47:53 | asettle | I'll check back in with Brian in the morning. | |
| 19:50:08 | openstackgerrit | christopher.uhler proposed openstack/nova master: Libvirt volume driver for Veritas HyperScale https://review.openstack.org/443951 | |
| 20:00:28 | mriedem | guh | |
| 20:00:41 | mriedem | annoying when you have the fix for a thing all day, but spend 2 hours trying to get tests to work | |
| 20:00:45 | mriedem | *unit* tests | |
| 20:01:05 | cdent | mriedem: after all this time you haven't been able to accept that as the new normal? | |
| 20:01:57 | mriedem | sometimes i don't mind, | |
| 20:02:02 | mriedem | i only mind when i have other stuff to be doing | |
| 20:04:50 | efried | mriedem Then you have to rebase the thing... | |
| 20:06:18 | mriedem | i do actually have to rebase this | |
| 20:06:20 | mriedem | but want it working first | |
| 20:06:35 | mriedem | i want the *mocks* working first | |
| 20:08:47 | mriedem | gee thanks everyone | |
| 20:09:09 | dansmith | mriedem: basically, we all acknowledge your mundane plight, now get back to work | |
| 20:09:09 | mriedem | what i really need is coffee | |
| 20:09:11 | mriedem | and metal | |
| 20:09:15 | mriedem | ha | |
| 20:09:18 | dansmith | :) | |
| 20:10:41 | mriedem | i need comstud here to kick me in my ass | |
| 20:11:54 | efried | I could put you in an armbar, but that would be counterproductive to your typing abilities. | |
| 20:12:09 | melwitt | what's special about comstud kicking you? what about us?? | |
| 20:12:34 | dansmith | er, "scaredy" | |
| 20:22:13 | mriedem | melwitt: because you'd probably apologize afterward | |
| 20:22:21 | mriedem | comstud on the other hand... | |
| 20:22:44 | melwitt | heh | |
| 20:26:47 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in admin_password.py https://review.openstack.org/443957 | |
| 20:28:08 | mriedem | f yes | |
| 20:36:36 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in admin_password.py https://review.openstack.org/443957 | |