| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 13:32:22 | smcginnis | sean-k-mooney: Cinder has this: https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L15-L18 | |
| 13:32:40 | smcginnis | sean-k-mooney: That's what you were saying the nova tox.ini needed to be changed to, right? | |
| 13:34:11 | cdent | smcginnis , sean-k-mooney I have a patch pending to fix lower constraints in nova for months and months https://review.openstack.org/#/c/622972/ | |
| 13:34:42 | cdent | it's so old now it is probably out of date | |
| 13:35:06 | smcginnis | Well wouldya look at that. | |
| 13:35:47 | smcginnis | I do think we should probably have the deps set separately than install_command though. | |
| 13:35:58 | smcginnis | Since that's the variable between these targets. | |
| 13:36:09 | smcginnis | But two ways to do the same thing I suppose. | |
| 13:39:24 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Update contributor guide for Train https://review.openstack.org/645581 | |
| 13:47:06 | openstackgerrit | Merged openstack/nova master: Trivial: remove unused var from policies.base.py https://review.openstack.org/645380 | |
| 13:58:22 | fried_rice | o/ | |
| 14:04:18 | kaisers | Looking for a second +2 for another driver bugfix at https://review.openstack.org/#/c/522245/ , anyone interested? :) | |
| 14:06:20 | mdbooth | Ooh, that's fun. | |
| 14:06:40 | mdbooth | In instance.save() we do: | |
| 14:06:42 | mdbooth | if not updates: | |
| 14:06:43 | mdbooth | ... | |
| 14:06:45 | mdbooth | return | |
| 14:07:10 | mdbooth | Which, I think, means that if 2 different threads do: | |
| 14:07:24 | mdbooth | instance.task_state='foo' | |
| 14:07:34 | mdbooth | instance.save(expected_task_state=[None]) | |
| 14:07:53 | mdbooth | They will *both* succeed, because the current state will never be evaluated for the second one | |
| 14:08:05 | mdbooth | And that would break a ton of stuff in nova api | |
| 14:08:31 | cdent | I feel like this is a known issue, but I can't remember why I think that | |
| 14:09:53 | mdbooth | I'm on my second interesting race condition of the day. I'm in race condition heaven. | |
| 14:13:33 | mdbooth | mriedem: Speaking of which you brought up gate failure https://bugs.launchpad.net/nova/+bug/1820337 yesterday, which I think is resolved by https://review.openstack.org/#/c/645546/ | |
| 14:13:34 | openstack | Launchpad bug 1820337 in OpenStack Compute (nova) "test_bfv_quota_race_local_delete intermittently fails with testtools.matchers._impl.MismatchError: ['bfv'] != []" [High,In progress] - Assigned to Matthew Booth (mbooth-9) | |
| 14:18:26 | mriedem | mdbooth: comment inline | |
| 14:18:33 | mriedem | i also noted that that bug doesn't show up on master for some reason | |
| 14:18:43 | mriedem | but yeah this seems reasonable | |
| 14:20:26 | mgoddard | hi nova | |
| 14:20:46 | mgoddard | has anyone seen this after sending SIGHUP to nova-compute: | |
| 14:20:47 | fried_rice | o/ mgoddard | |
| 14:20:48 | mgoddard | In shutdown, no new events can be scheduled | |
| 14:20:49 | mdbooth | mriedem: Yeah, the rocky/master thing is weird. My bet is it'll be some random quirk of eventlet scheduling I'll bet, caused by an additional context-switching operation added somewhere random and unimportant. | |
| 14:21:05 | mgoddard | http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_48_58_621 | |
| 14:21:07 | fried_rice | mgoddard: Does it go away after a little while? | |
| 14:21:33 | mgoddard | fried_rice: I'm not sure, but I don't think so | |
| 14:21:39 | mgoddard | fried_rice: how little a while? | |
| 14:22:08 | fried_rice | It wouldn't surprise me that SIGHUP would bork anything that's in flight. Was that vif creation in flight at the time? | |
| 14:23:38 | mgoddard | fried_rice: no, it's a post upgrade instance boot test | |
| 14:24:25 | fried_rice | looks like the SIGHUP happens here: http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_43_57_192 | |
| 14:24:42 | fried_rice | not sure if the errors after that are "normal" or not. | |
| 14:26:16 | fried_rice | your spawn is five minutes later, which should be plenty of time, yah. | |
| 14:26:28 | fried_rice | This sounds like a job for... | |
| 14:26:35 | fried_rice | gibi: ^ any ideas? | |
| 14:27:00 | gibi | looking | |
| 14:27:46 | mriedem | mdbooth: maybe, i guess that eventlet thing you're trying to fix was master-only | |
| 14:28:23 | mdbooth | mriedem: Yes, that's master only. Nova-api didn't need eventlet in Rocky, and wasn't monkey patched. | |
| 14:29:45 | mdbooth | The gate thing is in functional, which is monkey patched separately, same on both branches. The chance causing the difference in scheduling could be literally anywhere, though. Doesn't even have to be even vaguely related. | |
| 14:30:11 | openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support adding the reason behind a server lock https://review.openstack.org/638629 | |
| 14:30:47 | mgoddard | fried_rice: gibi: do the grenade jobs use a SIGHUP? | |
| 14:31:16 | cdent | mgoddard: unlikely | |
| 14:31:26 | gibi | I'm not sure what should nova is expected to do at SIGHUP | |
| 14:31:39 | gibi | I'm not sure what nova is expected to do at SIGHUP | |
| 14:31:59 | mgoddard | rolling upgrade docs say to SIGHUP to refresh upgrade levels: https://docs.openstack.org/nova/stein/user/upgrade.html#rolling-upgrade-process | |
| 14:32:46 | gibi | mgoddard: I understand the first log http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_48_58_621 as nova got an error from neutron so it would worth to check what is in the neutron log | |
| 14:34:22 | dansmith | gibi: there's a bug about this.. oslo.service is killing us during a sighup instead of letting us do graceful things | |
| 14:34:35 | gibi | mgoddard: ^^ | |
| 14:35:04 | dansmith | gibi: mgoddard: https://review.openstack.org/#/c/641907/ | |
| 14:35:16 | dansmith | I was looking at that with him early-on but I've lost all the context on it | |
| 14:35:36 | mgoddard | gibi: I think it's a red herring. I can't see any errors in neutron. I think that the VIF plug event pushes nova through a path that exposes this half-shutdown state | |
| 14:35:36 | gibi | dansmith: thanks | |
| 14:35:47 | mgoddard | dansmith: thanks | |
| 14:36:02 | dansmith | mgoddard: it's because nova has been restarted and has dropped a bunch of state on the ground | |
| 14:36:25 | dansmith | I have to run off for a sec, biab | |
| 14:36:37 | artom | http://logs.openstack.org/81/644881/2/check/tempest-full-py3/3b506d2/controller/logs/screen-n-cpu.txt.gz?level=WARNING#_Mar_20_20_39_18_723559 | |
| 14:36:58 | artom | Well crap, looks like there's still something missing from that whole revert resize external events mess | |
| 14:40:19 | mgoddard | It seems like the workaround here is going to be to restart nova-compute rather than HUP, I'll go with that | |
| 14:41:25 | mgoddard | thanks fried_rice gibi dansmith | |
| 14:49:35 | sean-k-mooney | cdent: regarding your lower constratins patch that is similar to mine but it also is technically wrong | |
| 14:49:55 | sean-k-mooney | cdent: placement and os-vif only use lower-constratis in our lower-constratis job | |
| 14:50:31 | cdent | sean-k-mooney: and? | |
| 14:50:34 | sean-k-mooney | but if we have a transitive depency that is capped in upper-constratits but not in our lower-constratis then it will be uncapped | |
| 14:51:02 | sean-k-mooney | given os-vif and placement have relitivly few depencies we are proably fine | |
| 14:51:34 | sean-k-mooney | but for nova or larger project we actully should use both but lower constraints needs to be passed first | |
| 14:51:47 | mgoddard | dansmith: qq for when you return - would you recommend a full restart of all nova services rather than HUP, including those where we haven't seen issues with HUP (i.e. all except compute)? | |
| 14:52:08 | cdent | my changes to nova's lower-constraints job make it so the upper-constraints file is not included in the test (which it had been before), and that caused conflicts | |
| 14:52:52 | sean-k-mooney | yes and that is more correct but the actually fix should be to include lower-constratis first followed by upper | |
| 14:53:02 | dansmith | mgoddard: yes to restart of compute instead of hup, although it'll still cause in-flight boots to fail.. don't think it's really a concern for most of the other services, so probably okay to hup them | |
| 14:53:16 | sean-k-mooney | the pip code uses the first constratit it finds for a dep | |
| 14:53:58 | sean-k-mooney | cdent: the main issue i have found is that mocking does not work correctly with the lower constrating deps as i think things moved | |
| 14:54:14 | sean-k-mooney | cdent: ill pull down your version and rebase and see if i see the same issue | |
| 14:54:15 | mgoddard | dansmith: at this point it's just been upgraded so will have restarted anyway - another restart won't hurt too much. I'll go with that, thanks | |
| 14:54:27 | dansmith | ack | |
| 14:54:43 | sean-k-mooney | i can combine both patches and see if i can get something that works fully | |
| 14:57:33 | sean-k-mooney | smcginnis: and yes the cinder code https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L15-L18 is what we do in os-vif and it almost right they should also have upper-constriatns listed here https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L198 after lower. | |
| 14:59:02 | smcginnis | sean-k-mooney: I don't think we want upper constraints listed in the lower constraints job on line 198. The point of that is to make sure it only uses the lower constraints. | |
| 14:59:12 | sean-k-mooney | smcginnis: the way os-vif placement and cinder execute lower-constraitns is actully probably fine 99% of the time as the are using lower-constraints for all there direct dependcies | |
| 14:59:57 | sean-k-mooney | smcginnis: if lower constratits does not list every dep we install then not having it in line 198 will leave deps uncapped | |
| 15:00:11 | sean-k-mooney | assuming those deps are capped in upper | |
| 15:00:20 | sean-k-mooney | that is the only case where it would be needed | |
| 15:00:24 | smcginnis | But lower should list every dependency. | |
| 15:00:39 | sean-k-mooney | it should but i dont think we enforce that | |
| 15:00:41 | smcginnis | Anything in requirements.txt should be in lower-constraints.txt | |
| 15:01:04 | smcginnis | The point of the job is to capture and validate all the requirement's minimum allowable versions. | |
| 15:01:38 | sean-k-mooney | smcginnis: yes i know but we dont validate that every file listed in pip feeze is in lower-constratis | |
| 15:02:03 | sean-k-mooney | the original file was generated that way but there is notheing to enforce it so it could be out of sync | |
| 15:02:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix return param docstring in check_can_live_migrate* methods https://review.openstack.org/645610 | |
| 15:02:44 | sean-k-mooney | lower-constratins also needs to have our depencies dependcies listed as we do not use our deps lower-constraitns recursivly | |
| 15:03:22 | sean-k-mooney | anyway ill email the list once i have a working fix for nova that work with master | |