| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-30 | |||
| 19:50:52 | mriedem | if you rebase like you spell, we might have problems | |
| 19:50:53 | artom | When you come back to your bae | |
| 19:51:03 | tssurya | mriedem, dansmith: thanks for the reviews, I just got back from a (2 weeks long) vacation and also don't have anything for the meeting. Will catch up on the summit stuff from tomorrow. | |
| 19:51:16 | dansmith | tssurya: cool | |
| 19:51:41 | mriedem | tssurya: rest assured we talked you up big time to belmiro and tim in vancouver | |
| 19:51:56 | tssurya | mriedem: haha thanks a ton!! | |
| 19:52:42 | mriedem | artom: just: git review -d 458820 && git rebase -i HEAD~2 | |
| 19:52:52 | mriedem | fix the one, then git rebase --continue && git review -R -y | |
| 19:53:29 | artom | Ah, yeah, rebasing on the one below it instead of master isn't a bad idea | |
| 19:53:35 | artom | Tiny bit patronizing tho ;) | |
| 19:54:08 | mriedem | right don't rebase on master | |
| 19:54:15 | mriedem | that makes the diffs between patches hard to follow | |
| 19:54:38 | mriedem | i only rebase on master if i have a merge conflict | |
| 19:54:59 | artom | At some point, someone somewhere will make a diff of diffs tool | |
| 19:55:08 | artom | I remember Diana talking about it before she left Red hat | |
| 19:55:17 | dansmith | git review can do that for ou | |
| 19:55:21 | mriedem | tonyb probably already has one | |
| 19:55:39 | artom | dansmith, yeah? Will need to look into that | |
| 19:55:44 | dansmith | git review -m XXX,4,5 | |
| 19:56:03 | mriedem | i'm currently explaining the difference, over text message, between bratwurst, kielbasa and summer sausage | |
| 19:56:10 | mriedem | and can't tell if it's some cruel joke | |
| 19:56:29 | dansmith | git review can't do that | |
| 19:56:48 | dansmith | git review will do anything for love, but it won't ... doo... thaaat. | |
| 19:56:51 | mriedem | git review --sausage-index | |
| 19:56:54 | dansmith | hah | |
| 19:57:24 | dansmith | I think you just did | |
| 19:57:38 | artom | So meta | |
| 20:01:03 | artom | mriedem, so is 'vf_trusted' always going to show in device metadata from now on? And set to True/False? | |
| 20:01:22 | artom | Or it'll only appear if the VF is trusted? | |
| 20:01:31 | artom | (Yeah, I could read, but... :/ ) | |
| 20:01:44 | mriedem | he's always putting it in, | |
| 20:01:50 | mriedem | default to False for old devices, which is correct | |
| 20:02:04 | mriedem | the new field shows up if the requested version on the metadata api is new enough | |
| 20:02:07 | artom | I'm cool with whatever, just wanted to be sure | |
| 20:02:32 | mriedem | i tend to like adding new fields like we do in the compute rest api, which is we add the new field always, we don't expose it conditionally based on it's value | |
| 20:02:50 | dansmith | definitely | |
| 20:03:10 | dansmith | not all languages have a __contains__ type operator, which makes checking for the presence before the value annoying | |
| 20:09:40 | mriedem | efried: jaypipes: +W on the mirror patch | |
| 20:09:53 | efried | süsse | |
| 20:18:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Skip ServerShowV254Test.test_rebuild_server in cells v1 job https://review.openstack.org/569454 | |
| 20:21:56 | artom | mriedem, https://review.openstack.org/#/c/566344/7/nova/tests/unit/test_metadata.py@244 | |
| 20:22:52 | artom | we need to include it though, no? We're asserting that the metadata will be equal to those dicts we define, so we need to add vf_trusted=False because we expect the code to generate that | |
| 20:23:36 | mriedem | oh is that the thing we're asserting against in the response? | |
| 20:23:39 | mriedem | if so, then sure | |
| 20:23:42 | mriedem | it wasn't obvious | |
| 20:24:04 | artom | Yeah, it's giving me a headache too | |
| 20:24:26 | mriedem | without reading it yet, is it fair to say the ML thread on review nit picking has itself succumbed to nit picking? | |
| 20:25:11 | artom | Duh | |
| 20:25:28 | artom | Really accessible topic, easy visibility points | |
| 20:25:32 | artom | Even *I* contributed | |
| 20:45:05 | artom | mriedem, need to relocate home (via daycare) will try to finish the metadata thing tonight | |
| 20:45:14 | artom | You east or west coast? | |
| 20:45:21 | artom | (Locality-wise, not rap) | |
| 20:45:44 | artom | Though I guess we already have your +2, it's jaypipes or whoever else we need to care about | |
| 20:46:28 | jaypipes | artom: hmm? | |
| 20:47:19 | artom | jaypipes, oh, apologies, for some reason I was sure mriedem roped you into reviewing https://review.openstack.org/#/q/topic:bp/sriov-trusted-vfs+(status:open+OR+status:merged) | |
| 20:47:28 | artom | He's +2 except for 1 patch with nits | |
| 20:47:31 | artom | Which I'm trying to fix | |
| 20:47:45 | mriedem | artom: MN | |
| 20:47:47 | mriedem | no coast | |
| 20:47:58 | jaypipes | artom: ok. can try tomorrow morning (there's a bunch of stuff ahead of that right now) | |
| 20:48:10 | artom | jaypipes, heh, it wasn't a request, but thank you! | |
| 20:48:16 | mriedem | we can throw it in the runways queue | |
| 20:48:23 | mriedem | sahid should have already done that anyway | |
| 20:48:28 | mriedem | i'll add it | |
| 20:48:45 | mriedem | artom: east coast rap-wise | |
| 20:49:00 | jaypipes | mriedem: definitely west coast. | |
| 20:49:04 | artom | mriedem, ok, you have an hour or so on me in terms of time left in day, you might still be around to +2 it if I succeed in making those damn tests pass | |
| 21:12:54 | mriedem | dansmith: before Kevin_Zheng goes off and adds some backoff looping call logic here, you might want to make sure we're not missing something on the problem for which he's trying to account https://review.openstack.org/#/c/568542/1/nova/compute/manager.py@6222 | |
| 21:13:02 | mriedem | basically a race window during abort of a live migration | |
| 21:26:30 | mriedem | hmm, also, on surya's patch, she already has a release note https://review.openstack.org/#/c/567878/5/releasenotes/notes/migration-tool-to-populate-inst.avz-29fed2fe57a9764d.yaml | |
| 21:26:37 | mriedem | so i'm not sure what to add to that in the backport | |
| 21:27:54 | dansmith | mriedem: right which is why I suggested a new one | |
| 21:28:01 | dansmith | mriedem: commented on that live migration abort thing | |
| 21:28:29 | dansmith | I think proper ordering will make that mostly unracy for our purposes without a polling loop | |
| 21:35:09 | mriedem | dansmith: alright this is what i had added, but it overlaps pretty heavily http://paste.openstack.org/show/722381/ | |
| 21:37:11 | dansmith | mriedem: I just wanted something that indicates that they have to run them again, if you think what is there now is enough, then that's fine | |
| 21:37:19 | dansmith | I agree your paste adds nothing to what is there, | |
| 21:37:28 | dansmith | I was just thinking about calling it out specifically | |
| 21:37:30 | dansmith | but it's cool | |
| 21:38:09 | dansmith | a couple days ago I was working an internal bug where someone updated the code, reported the fix didn't work | |
| 21:38:22 | dansmith | and turns out they didn't even restart nova-compute to get the new code | |
| 21:38:29 | dansmith | so maybe I'm overly sensitive at the moment | |
| 21:39:45 | mriedem | alright, i'll remove my extra reno in the backport. i figured that most deployment tools will run nova-manage db sync and the online data migrations during an update as well as a major upgrade, since it shouldn't hurt if there is nothing new, but can hurt if you don't run it and there is | |
| 21:40:22 | mriedem | also, thanks for the comments on kevin's patch, makes sense when i read it | |
| 21:40:54 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Metadata-API fails to retrieve avz for instances created before Pike https://review.openstack.org/571317 | |
| 21:41:47 | dansmith | I don't think they do actuall | |
| 21:41:50 | dansmith | fwiw | |
| 21:42:48 | dansmith | at least our process for rolling out updates (not upgrades) is basically just yum | |
| 21:43:56 | cfriesen | yeah, we normally wouldn't re-run online data migrations unless it's specifically highlighted. | |
| 21:44:17 | cfriesen | previous fixes have generally fixed things up on accessing the data, and the online data migrations fixed up ones that hadn't been accessed "live" | |
| 21:50:36 | mriedem | alright, well, meh | |
| 21:50:45 | mriedem | i guess people doing updates can run them if needed, | |
| 21:50:53 | mriedem | otherwise people rolling up to those during FFU can get it | |
| 21:52:09 | openstackgerrit | Merged openstack/nova master: Metadata-API fails to retrieve avz for instances created before Pike https://review.openstack.org/567878 | |
| 21:52:15 | openstackgerrit | Merged openstack/nova master: Make nova service-list use scatter-gather routine https://review.openstack.org/568271 | |
| 21:52:25 | openstackgerrit | Merged openstack/nova master: doc: cleanup API guide about instance faults https://review.openstack.org/560178 | |
| 21:54:07 | melwitt | interesting, looking at a recent fail of a VolumesBackupTest in the ceph job, it looks like if you issue a delete request twice for the same volume, the second one can fail with a 400 for having not allowed volume status http://logs.openstack.org/53/570653/1/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/c5d03e7/logs/screen-c-api.txt.gz#_2018-05-30_04_41_07_095 | |
| 21:54:29 | melwitt | anyone seen that before? is that usual? | |
| 21:56:40 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Metadata-API fails to retrieve avz for instances created before Pike https://review.openstack.org/571320 | |