Earlier  
Posted Nick Remark
#openstack-nova - 2019-03-08
15:54:25 aspiers SUSE knows all about that ...
15:54:27 kashyap Once they start cutting long-term 'stable' release, everyone starts running around headless-chickens
15:54:41 kashyap ... "we need to get this into the "long-term" release!"
15:54:56 sean-k-mooney kashyap: honestly i think that is better the feature backports
15:55:00 gibi mriedem, melwitt: I'm wondering about how to close the https://blueprints.launchpad.net/nova/+spec/bandwidth-resource-provider the spec talks about the server move operations that was not implemented in Stein so if we use that as a scope then the bp cannot be marked implemented
15:55:18 kashyap sean-k-mooney: Not saying it's bad :-) Just the reality and the incentive structure I guess
15:55:31 mriedem gibi: i think we can complete the blueprint in stein and have a separate blueprint for move operations in train
15:55:39 aspiers OK I'm done for the week. I'll look at splitting that out into 3 reviews next week
15:55:56 aspiers Have a good w/e all! o/
15:56:16 kashyap aspiers: Just a heads-up, next week the first three days I'm travelling; so, if I'm slow on responding ... you know why :-)
15:56:20 kashyap Have a good one!
15:56:22 gibi mriedem: shall I remove the move operation from the old spec when I update that now?
15:56:22 aspiers OK np :)
15:56:45 mriedem gibi: are you going to amend the stein spec for the microversion and all the other stuff that came up?
15:57:07 mriedem i guess i'm not sure what you're asking me :)
15:57:51 gibi mriedem: yeah I'm plannig to amend the spec
15:57:58 gibi mriedem: so I can remove the move from the scope there
15:58:53 mriedem ok
15:59:01 gibi mriedem: ok
16:03:01 gibi mriedem: there are two trailing patche for the bandwidth series both can be considered bugfixes, https://review.openstack.org/#/c/638711/ and https://review.openstack.org/#/c/639608/ what do you think, shall I push them forward before RC1?
16:09:33 openstackgerrit guang-yee proposed openstack/nova master: pass endpoint interface to Ironic client https://review.openstack.org/640879
16:27:09 mriedem gibi: on https://review.openstack.org/#/c/639608/ i think that's already a potential bug because of not handling errors from claim_resources
16:27:27 mriedem so it's low priority for me, but i think it's a bug we can report and work on
16:27:57 mriedem as i said before that whole while loop needs to be refactored at some point (that whole damn build_instances method actually)
16:28:46 mriedem gibi: https://review.openstack.org/#/c/638711/ isn't really a bug per se, but it's a performance optimization, so again i don't know that we need to rush it, but it's still good to have
16:29:14 mriedem you could report a low priority bug for tracking that if you wanted - as a perf issue
16:29:28 mriedem like i said, if i'm multi-creating 100 instances in one request, that's 100 separate GET /allocations calls
16:29:49 mriedem although, can you multi-create with a pre-created port...?
16:29:55 mriedem i'm not sure that is supported
16:30:19 mriedem that would be like multi-create with the same pre-created volume
16:30:23 mriedem so that likely doesn't work anyway
16:31:35 openstackgerrit Eric Fried proposed openstack/nova master: update gate test for removal of force evacuate https://review.openstack.org/641986
16:32:09 fried_rice mriedem: sean-k-mooney stephenfin bauzas tssurya ^
16:32:53 fried_rice http://logs.openstack.org/86/641986/2/check/nova-live-migration/6f6b678/job-output.txt.gz#_2019-03-08_14_32_56_004357
16:33:47 tssurya fried_rice: oops
16:34:42 cfriesen mriedem: pretty sure you can't multi-create with pre-created port
16:34:43 openstackgerrit Kashyap Chamarthy proposed openstack/nova-specs master: Re-propose the spec to allow specifying a list of CPU models https://review.openstack.org/642030
16:36:27 mriedem cfriesen: yeah i didn't think so..
16:36:56 mriedem fried_rice: ah crap
16:37:09 fried_rice mriedem: Did I do it right? I have very little experience with the CLI
16:38:14 cfriesen Could someone take a look at this and clue me in why my new unit test (test_executable_exists_in_path(), in test_utils.py) is failing? Is this a zuul thing? It works fine locally.
16:38:26 cfriesen https://review.openstack.org/#/c/641932
16:38:43 mriedem cfriesen: i looked at that the last time you asked and i couldn't spot anything obvious, but i'm sure there is some global you're hitting
16:39:05 mriedem fried_rice: yeah i think so, the 'common' options come before the command, and then the per-command options
16:39:12 mriedem i'm not sure if osc is that picky but i think novaclient is
16:39:40 fried_rice cfriesen: Have you tried full-pathing ls?
16:40:21 cfriesen fried_rice: the "ls" is fine, it's the call to libvirt_utils.executable_exists_in_path() that is unexpectedly failing
16:40:26 fried_rice duh, sorry, yeah
16:40:40 fried_rice I hadn't really parsed yet, that was just something that jumped out at a glance.
16:40:48 cfriesen it's totally reliable locally, fails every time in zuul
16:42:07 fried_rice cfriesen: ah, `type`. Shell builtins can be funny. There's a way to invoke a builtin as an executable, looking...
16:44:06 fried_rice cfriesen: Tough that it can't be reproduced locally; but you can try changing "type ..." to "builtin type ..."
16:45:16 fried_rice cfriesen: also, based on the comment, if what you truly care about is that it's an executable (as opposed to a function, alias, or builtin), you could use /usr/bin/which instead of type.
16:45:21 mriedem cfriesen: there is also this https://wiki.openstack.org/wiki/Testr#Reproducing_Failures
16:45:24 mriedem if you get desperate
16:45:38 mriedem ^ is how you can try to spot racing globals
16:46:02 fried_rice mriedem: whoosh, that could use some updating for stestr, huh
16:46:50 mriedem yeah probably
16:47:01 mriedem good thing anyone with a lp account can change it :)
16:50:25 fried_rice cfriesen: oh, sorry again, the problem is likely to be with $PATH, which definitely shouldn't include /tmp in real life.
16:50:54 fried_rice cfriesen: I'm able to repro locally.
16:52:01 cfriesen fried_rice: ah, excellent. wonder why it works locally here
16:52:13 cfriesen I don't have /tmp in PATH
16:52:31 fried_rice yeah, that's really strange cfriesen, because even if you had /tmp in your $PATH, mktemp creates in a randomly-named subdirectory.
16:56:13 cfriesen I think my bash must be messed up, with "type -P" returning whether the binary is executable even if it's not in $PATH
16:58:10 cfriesen fried_rice: interesting, fedora 29 behaves the same as mine
16:58:36 cfriesen maybe I'll just switch to "which".
16:58:47 openstackgerrit Artom Lifshitz proposed openstack/nova master: Dont' wait for VIF plugging during revert resize https://review.openstack.org/639396
16:58:49 mriedem gibi: smallish thing in https://review.openstack.org/#/c/638711/
16:58:59 mriedem artom: are you going to ever fix that typo in your commit title?
16:59:06 mriedem don't make me -5 you
16:59:55 artom Christ :(
16:59:56 openstackgerrit Artom Lifshitz proposed openstack/nova master: Don't wait for VIF plugging during revert resize https://review.openstack.org/639396
17:00:03 artom It's, err, the possessive form?
17:01:28 fried_rice cfriesen: using type -P with a full path is weird anyway.
17:02:08 fried_rice cfriesen: Are you actually wanting to know if a fully-pathed file is executable, or are you wanting to know if a name (not a full path) is in your PATH *and* executable?
17:02:49 fried_rice cause the former should really be done natively (os.stat | O_EXEC kind of thing)
17:03:33 cfriesen fried_rice: the latter. using the full path was just for the testcase. I'm trying to make sure that "swtpm" exists on the system before advertising support for emulated TPM on that node.
17:04:43 fried_rice cfriesen: Okay. Then the test case shouldn't use the full path either.
17:04:57 cfriesen one could argue that's up to the operator to ensure, in which case I could skip the check entirely.
17:05:11 fried_rice meh, it seems reasonable, if you can get it to work.
17:07:30 mriedem artom: so i don't think you need to change that finish_revert_migration interface at all,
17:07:47 mriedem we just need to be registering an event callback prior to call migrate_instance_fininsh
17:07:52 fried_rice cfriesen: Is / in your $PATH?
17:08:12 cfriesen nope
17:08:46 cfriesen I think it must be a non-exclusive test, where it checks $PATH but also the full path to the executable if specified
17:09:10 artom mriedem, I thought that's what dansmith *didn't* want
17:09:12 fried_rice weird that it's different on different platforms.
17:09:43 mriedem artom: this is the kind of thing that the 3 of us would probably need to get on a hangout to discuss
17:09:55 mriedem so we stop talking past each other while half paying attention
17:10:03 dansmith aren't we just reverting mriedem's patch to expect an event here?
17:10:27 artom dansmith, you just want to +W a mriedem revert, don't you :)
17:10:50 artom I have to go do lunch with colleagues, but I like the hangouts idea
17:10:54 mriedem yes he's reverting my patch but in an uglier way
17:10:56 dansmith artom: no, I want to avoid the other thing, and since we've been in that "don't wait for the event" state a few times now, I think first step is just getting back to that, unless there's some reason not to
17:11:03 cfriesen fried_rice: if I modify os.environ['PATH'] in the testcase to include the directory containing the temp dir, do you think that'd work?
17:11:14 fried_rice cfriesen: I tried that locally and it didn't work, no.
17:11:17 fried_rice but I'm not sure why.
17:11:37 artom dansmith, so you'd rather I just straight up revert mriedem's patch? Functionally it's the same
17:11:39 cfriesen guess I'm switching to "which" then. :)

Earlier   Later