| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 09:51:22 | mdbooth | kashyap: Heh, thanks. The commit message has also grown over time :) | |
| 09:51:42 | kashyap | Yes, I can see. I'm sure you spent many hours fine-tuning it as you discovered things. | |
| 09:53:05 | mdbooth | I feel that enormous comments are an indicator of code smell, though, and nova.monkey_patch is almost all comment. | |
| 09:53:39 | mdbooth | Most code should not require that much explanation. | |
| 09:56:47 | openstackgerrit | Merged openstack/nova master: Cleanup comments around claim_resources method https://review.openstack.org/644576 | |
| 09:56:53 | openstackgerrit | Merged openstack/nova master: Address old TODO in claim_resources_on_destination https://review.openstack.org/644596 | |
| 10:30:45 | lyarwood | mdbooth: re https://review.openstack.org/#/c/637224/ - good news, I can't reproduce the cinder weirdness locally after rebasing and recloning everything this morning. Just pushed https://review.openstack.org/#/c/637527/ again, hopefully this also passes in the check gate now. | |
| 10:30:54 | lyarwood | s/gate/queue/g | |
| 10:33:59 | kashyap | mdbooth: Yeah, was about to remark on that, too. THe large commentary section. | |
| 11:53:14 | openstackgerrit | Matthew Booth proposed openstack/nova master: Fix incomplete instance data returned after build failure https://review.openstack.org/645546 | |
| 12:55:06 | kaisers | mdbooth: Hi! If possible, could you revisit the driver bugfix at https://review.openstack.org/#/c/554195/ ? Needs a second +2 and you already looked into it some time ago. :) | |
| 12:55:45 | mdbooth | kaisers: I don't have that mojo, but I can take a look. | |
| 12:56:20 | kaisers | mdbooth: still fine, thanks | |
| 13:04:55 | openstackgerrit | Merged openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | |
| 13:11:46 | openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support adding the reason behind a server lock https://review.openstack.org/638629 | |
| 13:13:18 | tssurya | cdent, edleaf, mriedem: thanks for the spec review ^ I have updated it with respect to your comments. | |
| 13:13:35 | cdent | tssurya: great, thank you. will look again soon. | |
| 13:13:44 | tssurya | edleafe* | |
| 13:13:58 | edleafe | tssurya: :) | |
| 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/ | |