| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-27 | |||
| 14:38:44 | artom | dansmith, it'll exercise the code, but assuming the workers have identical NUMA topologies, we'll have no way of knowing | |
| 14:38:53 | artom | Because in those cases, current live migration "works" | |
| 14:39:08 | stephenfin | sean-k-mooney: fyi, six doesn't have a wrapper for the collections -> collections.abc thing | |
| 14:39:14 | stephenfin | there's an open bug but no patch for it | |
| 14:39:15 | artom | Both hosts being identical means we don't need to update the XML or claim any "new" resources | |
| 14:39:35 | sean-k-mooney | stephenfin: right but it does for is this python 3 or not | |
| 14:40:00 | sean-k-mooney | stephenfin: but ya i need to stil fix that actully so ill respin shortly | |
| 14:40:03 | stephenfin | Oh, right, gotcha | |
| 14:40:16 | stephenfin | Could you spin out 'flatten_iterable' into a separate patch too, in that case? | |
| 14:40:22 | stephenfin | If you have uses for it elsewhere | |
| 14:40:45 | dansmith | artom: no way of knowing what? if we see the debugs that show that you're doing the stuff, we'll know it's at least running your code, and not breaking something simple | |
| 14:40:52 | sean-k-mooney | i had thought you were using sum in a few places to flatten lists which was the other usecase i wanted to adress with it | |
| 14:41:06 | dansmith | artom: even if it generates the same xml on the other side, it's still doing all that work, even if for just a boring topo | |
| 14:41:12 | stephenfin | Yeah, I still don't think it's necessary when the sum thing is a well known pattern, but if you're going that way then it's definitely a patch in its own right | |
| 14:41:15 | sean-k-mooney | but i cant find them anymore. did you get rid of them recently | |
| 14:41:19 | dansmith | artom: seems like a worthwhile thing to have when a reviewer feels uneasy | |
| 14:41:19 | sean-k-mooney | and yes ill break it out | |
| 14:41:30 | bauzas | stephenfin: are you about to respin ? | |
| 14:41:33 | stephenfin | oh, no other uses? | |
| 14:41:34 | stephenfin | Hmm | |
| 14:41:35 | bauzas | I was on https://review.opendev.org/#/c/674894/11//COMMIT_MSG | |
| 14:41:36 | dansmith | artom: and I would think it would be pretty much just tweaking the flavor we create for a regular live migration job | |
| 14:41:44 | stephenfin | do we _really_ need it then? | |
| 14:41:48 | sean-k-mooney | yes | |
| 14:42:00 | sean-k-mooney | the sum pattern is not a common idiom | |
| 14:42:14 | sean-k-mooney | you are the only person i know of that has suggested it | |
| 14:42:35 | sean-k-mooney | so i would like to have a fucntion that names that algoritium | |
| 14:42:56 | sean-k-mooney | so we can reuse it and not have to rely on trible knowladge | |
| 14:43:02 | stephenfin | could we use itertools.chain? | |
| 14:43:15 | stephenfin | that does this same thing, right? | |
| 14:43:39 | sean-k-mooney | its close but it does not do the right things for maps | |
| 14:43:46 | stephenfin | no? | |
| 14:43:49 | sean-k-mooney | it allos does not flatten a list | |
| 14:44:00 | sean-k-mooney | *list of list | |
| 14:44:06 | sean-k-mooney | i could use it internally | |
| 14:44:39 | stephenfin | you can use '*' for that | |
| 14:44:42 | stephenfin | I mean, you've one caller | |
| 14:44:49 | stephenfin | This is so overengineered :D | |
| 14:44:58 | bauzas | sean-k-mooney: I honestly feel flattening an iterable of iterables is just a common pattern that doesn't really need to have a general method | |
| 14:44:59 | sean-k-mooney | but based on your feedback to not use the functional part of the standard libary like reduce i wrote it as genertor | |
| 14:45:26 | stephenfin | Yeah, I prefer reduce to this /o\ | |
| 14:45:30 | bauzas | sean-k-mooney: it's more like, you have so many ways to do what you want, pick one wisely | |
| 14:45:41 | stephenfin | I mean, I'm not in love with it, but 2 lines >> ~50 lines | |
| 14:45:47 | sean-k-mooney | ok i am going to take a break form irc now before i say some i should not | |
| 14:46:01 | bauzas | sean-k-mooney: like, returning a generator if you pass a couple of lists isn't really helpful, righT? | |
| 14:56:07 | mriedem | i think at this point i might just take this over to move it along https://review.opendev.org/#/c/663851/42 | |
| 14:56:30 | mriedem | alex_xu: gmann: efried: ^ any problem with me doing that? | |
| 14:57:14 | efried | I haven't been following it | |
| 14:57:14 | mriedem | it's close, in a runway for a couple more days, and there are at least 3 other changes open for review that are conflicting for the same microversion | |
| 14:57:42 | efried | you planning to +2 it after you "move it along"? | |
| 14:57:43 | mriedem | efried: i know, just saying from a PM standpoint | |
| 14:57:58 | mriedem | that's the question - my changes would be docs/test really | |
| 14:58:06 | mriedem | but alex is the only other core that's really gone through it much | |
| 14:58:10 | mriedem | so yeah i'd still want to be able to +2 | |
| 14:58:22 | efried | If your changes are docs/test, then I'm good with it. | |
| 15:13:08 | artom | dansmith, true. I don't mind doing something like that as a follow-up, as I think func tests that test more scenarios are more important to get done first | |
| 15:13:33 | dansmith | artom: they're important for sure, they just don't give me all that much confidence | |
| 15:13:43 | artom | dansmith, honest question, why? | |
| 15:14:03 | artom | We have no way to be sure real libvirt would accept the XML we generate? | |
| 15:15:07 | dansmith | artom: because every other thing we've landed like this has been fine in the contrived tests, but not actually work in the real world | |
| 15:15:14 | artom | dansmith, heh, fair | |
| 15:15:22 | dansmith | like because we generate bad libvirt xml, or were digesting not-real-world host xml, etc | |
| 15:15:41 | dansmith | numa itself was DoA, IIRC, and pci wasn't actually usable for a long time, etc | |
| 15:16:07 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Make _rever_allocation nested allocation aware https://review.opendev.org/676138 | |
| 15:16:07 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140 | |
| 15:17:33 | artom | dansmith, would the current combination of manual tests + func tests be enough initially? | |
| 15:18:04 | artom | ... I guess it doens't address your point, because all previous things you mention were exactly in that situation | |
| 15:18:08 | dansmith | artom: you mean, similar to the manual + func tests we had for things we landed borken in the past? | |
| 15:18:27 | dansmith | artom: it just seems like this should be relatively simple to try, by altering the flavor on a LM job, while you're waiting for review or something | |
| 15:18:56 | dansmith | mriedem: how hard is it to float a new job or changed job which does one of our multinode LM tests, but with a different flavor that specifies a simple numa topo? | |
| 15:20:02 | mriedem | dansmith: the hard part there is likely just hacking the flavor, | |
| 15:20:22 | mriedem | but we could probably do that within the script that runs the live migration tests themselves after configuring tempest.conf to use the new flavor | |
| 15:20:50 | dansmith | yeah, I was thinking that would be doable | |
| 15:20:52 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate re-schedule with bandwidth https://review.opendev.org/676972 | |
| 15:20:58 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/live_migration/hooks/run_tests.sh | |
| 15:21:01 | mriedem | dansmith: in there ^ | |
| 15:21:02 | dansmith | not necessarily landable, but just hacked into place so we can see it run | |
| 15:21:07 | dansmith | artom: ^ | |
| 15:21:31 | mriedem | we should be able to create a new flavor and then re-configure tempest (on the controller host) to use that for the flavor before running the tests | |
| 15:21:40 | artom | dansmith, yeah, I was looking at that as well | |
| 15:22:11 | mriedem | you can see we already configure tempest from the script using ansible | |
| 15:22:11 | mriedem | $ANSIBLE primary --become -f 5 -i "$WORKSPACE/inventory" -m ini_file -a "dest=$BASE/new/tempest/etc/tempest.conf | |
| 15:22:43 | mriedem | so yeah, just create a new flavor and re-configure tempest with the id | |
| 15:23:03 | mriedem | this one https://github.com/openstack/tempest/blob/master/tempest/config.py#L285 | |
| 15:23:18 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support migrating SRIOV port with bandwidth https://review.opendev.org/676980 | |
| 15:25:41 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow migrating server with port resource request https://review.opendev.org/671497 | |
| 15:25:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827 | |
| 15:26:04 | artom | mriedem, ack, appreciated :) | |
| 15:27:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add online migration for legacy NUMA objects https://review.opendev.org/537414 | |
| 15:27:10 | dansmith | artom: sean-k-mooney wrote these functional tests didn't he? | |
| 15:27:30 | artom | dansmith, no... You're implying there's loads of typos? | |
| 15:27:58 | dansmith | hehe, yes | |
| 15:28:03 | dansmith | you must be hanging around him too much then | |
| 15:28:43 | sean-k-mooney | :) | |
| 15:29:06 | artom | I suck up to the best :D | |
| 15:29:07 | sean-k-mooney | eventrully my influnce will spread to all in nova | |
| 15:29:37 | dansmith | over my dead body | |
| 15:29:48 | artom | That can be arranged | |
| 15:31:15 | dansmith | believe me, I know | |
| 15:31:20 | artom | o_O | |