| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-22 | |||
| 03:50:10 | openstackgerrit | OpenStack Release Bot proposed openstack/nova stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/645462 | |
| 03:50:16 | openstackgerrit | OpenStack Release Bot proposed openstack/nova master: Update master for stable/stein https://review.openstack.org/645463 | |
| 06:22:40 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove deprecated 'default_flavor' config option https://review.openstack.org/645476 | |
| 06:26:32 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | |
| 06:33:16 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | |
| 06:33:21 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | |
| 09:27:39 | openstackgerrit | Matthew Booth proposed openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | |
| 09:28:28 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Use migration_status during volume migrating and retyping https://review.openstack.org/637224 | |
| 09:45:07 | openstackgerrit | Boxiang Zhu proposed openstack/nova master: [WIP] Scheduler filters evaluated even forced host https://review.openstack.org/645520 | |
| 09:50:20 | kashyap | mdbooth_: Nice work on that eventlet patch (although I'm not fully qualified to comment on it). | |
| 09:50:23 | kashyap | "I started with what seemed like the simplest and most obvious change, and moved on as I discovered more interactions which broke." | |
| 09:50:26 | kashyap | :D | |
| 09:51:01 | kashyap | (Also kudos for the "commit message mini essay") | |
| 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 | mnaser | i added a note to this bug. it's been around for a few cycles ^ | |
| 13:27:26 | gibi | np | |
| 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 | |