| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 13:15:22 | tssurya | mriedem: I have a question regarding the notification change: https://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@168 not particularly what/why it should change | |
| 13:17:31 | gibi | tssurya: I think it is not mandatory to add the new fields to the notification payload but it would be nice to add | |
| 13:17:44 | gibi | tssurya: if we are not adding now, then we can add it later | |
| 13:18:49 | tssurya | gibi: ah ok, yea I can add them in this spec itself, np, I wasn't sure of the workflow since I haven't dealt with notification changes before | |
| 13:19:04 | tssurya | so that means I need to add the fields into the InstancePayload | |
| 13:19:14 | tssurya | and then send that via the notifications event right ? | |
| 13:20:07 | gibi | tssurya: yes, you extend the IntancePayload and with the new fields and make sure that the value of the new fields are populated from the instance | |
| 13:20:17 | tssurya | https://github.com/openstack/nova/blob/b33fa1c054ba4b7d4e789aa51250ad5c8325da2d/nova/compute/utils.py#L447 somewhere there ? | |
| 13:20:52 | tssurya | oh ok | |
| 13:21:24 | gibi | tssurya: there is a schema def in the InstancePayload ovo that defined how to copy data from the Instance ovo so if you fill the schema then the copy is automatic | |
| 13:21:32 | gibi | https://github.com/openstack/nova/blob/master/nova/notifications/objects/instance.py#L59 | |
| 13:22:22 | tssurya | gibi: ah okay that's great | |
| 13:22:36 | tssurya | sure then I will add this change too to the spec | |
| 13:22:40 | tssurya | thanks gibi! :) | |
| 13:22:50 | gibi | tssurya: here is a patch from the past that extended the InstancePayload https://github.com/openstack/nova/commit/2bca6431e69bf2c6e657736b7fe11f5a2fbb9433 | |
| 13:26:40 | tssurya | gibi: nice, I can follow that workflow :) | |
| 13:27:04 | gibi | tssurya: and you can invite me to review the patch when it is up. :) | |
| 13:27:10 | openstackgerrit | Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352 | |
| 13:27:22 | tssurya | gibi: absolutely! :D thank you | |
| 13:27:26 | gibi | np | |
| 13:27:26 | mnaser | i added a note to this bug. it's been around for a few cycles ^ | |
| 13:32:17 | smcginnis | sean-k-mooney: I see now what you were saying with the lower-constraints issue yesterday. | |
| 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 | gibi | dansmith: thanks | |
| 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: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 | |