| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-05 | |||
| 17:08:01 | mriedem | i got confused by all of the options | |
| 17:08:13 | mriedem | plus, someone jangled some keys in front of me | |
| 17:09:10 | mriedem | sdague: well with https://review.openstack.org/#/c/447728/ merged i think we're ready for novaclient 8.0.0 which drops all of the proxy crap | |
| 17:09:11 | melwitt | heh, yeah. I've had a lot of meetings this morning so I didn't get to it yet | |
| 17:32:21 | dane-fichter | mriedem: sorry I had to step out for a meeting | |
| 17:32:24 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Update reno for removed network CLIs https://review.openstack.org/453800 | |
| 17:32:32 | mriedem | melwitt: dansmith: want to get ^ quick and then i can releaes? | |
| 17:32:33 | mriedem | *release | |
| 17:32:41 | mriedem | dane-fichter: np, +2 | |
| 17:32:51 | dane-fichter | mriedem: great, thanks. | |
| 17:36:24 | melwitt | mriedem: +W | |
| 17:50:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-manage: Rename 'api_db', 'cellsv2' commands https://review.openstack.org/453809 | |
| 17:50:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: nova-manage: Deprecate '--version' parameters https://review.openstack.org/453808 | |
| 17:50:50 | sfinucan | ^ artom Per your comments in the 'move-nova-cmds-to-cliff' spec yesterday | |
| 17:51:09 | sfinucan | the first one is particularly disgusting :) | |
| 17:57:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: make it clear that os-cells is for cells v1 https://review.openstack.org/453433 | |
| 17:58:00 | mriedem | this is a pretty simple fix for a long-standing bug https://review.openstack.org/#/c/452577/ | |
| 17:58:06 | mriedem | which i'm going to backport once it's merged | |
| 17:59:11 | dansmith | ooh | |
| 18:00:36 | mriedem | ooh is right | |
| 18:16:30 | mriedem | ericyoung: what's your launchpad id? | |
| 18:21:22 | mriedem | dansmith: melwitt: we also have this regression from ocata to get backported https://review.openstack.org/#/c/452359/ - i've been waiting on that one before fixing https://review.openstack.org/#/c/449334/ since they are going to conflict | |
| 18:22:03 | dansmith | guh | |
| 18:22:10 | dansmith | I cannot wait for counting quotas | |
| 18:22:46 | artom | sfinucan, so you're going to add deprecation warnings in Pike, and remove the old commands in Queens entirely? | |
| 18:22:56 | artom | I think there needs to be a period of overlap when both coexist | |
| 18:23:25 | artom | For instance, does tripleo call them? | |
| 18:23:33 | artom | I think we've broken tripleo enough as is ;) | |
| 18:23:45 | artom | Let's give them a chance to adjust because we yank commands from under them | |
| 18:23:50 | artom | s/because/before | |
| 18:24:41 | mriedem | i hope tripleo isn't using deprecated nova clis | |
| 18:25:28 | artom | mriedem, they're not deprecated yet :) I'm trying to think of ways to make the deprecation as painless as possible | |
| 18:25:35 | mriedem | artom: i mean novaclient | |
| 18:25:36 | mriedem | not nova-manage | |
| 18:25:38 | mriedem | https://review.openstack.org/#/c/453819/ | |
| 18:26:09 | mriedem | https://governance.openstack.org/tc/reference/tags/assert_follows-standard-deprecation.html | |
| 18:27:34 | dansmith | our delete path(s) are just hilarious | |
| 18:27:40 | mriedem | dansmith: i know, | |
| 18:27:45 | dansmith | maybe we should get rid of delete? | |
| 18:27:51 | mriedem | dansmith: that's why i'm holding https://review.openstack.org/#/c/449334/ to the end, | |
| 18:28:04 | mriedem | because i think the fix for that bug is to re-use the _local_delete method in the compute api | |
| 18:28:09 | mriedem | but i think there will be dragons | |
| 18:28:17 | artom | soft-delete in the client is the way to go ;) | |
| 18:28:29 | mriedem | _local_delete is what cleans up ports and volumes | |
| 18:28:38 | mriedem | and was missing from the duplicate 'local delete' that was added for the cells v2 case | |
| 18:29:02 | mriedem | once these ocata regression bugs are fixed, i think we spend some time just removing the transition code in the delete path for cells v2 | |
| 18:29:16 | mriedem | but need the fixes and tests in place before we unwind that onion | |
| 18:30:00 | mriedem | artom: per the deprecation policy, we can deprecate in pike and remove in queens as long as it's been deprecated for at least 3 months | |
| 18:30:16 | mriedem | so you can't deprecate something at the very end of pike and drop it immediately in queens if it's under 3 months | |
| 18:30:24 | mriedem | the 3 months thing is for CD | |
| 18:30:35 | artom | mriedem, I can't argue with the policy | |
| 18:31:03 | artom | I just think it's safer to have a period when both coexist - so that whatever depends on them can change to the new way of doing things while the old way still works | |
| 18:31:23 | mriedem | that's the whole point of the 3 months | |
| 18:31:27 | mriedem | and full stable branch | |
| 18:32:19 | artom | Right, assuming the new way is ready and available during those 3 months ;) | |
| 18:32:24 | artom | Good point for stable though | |
| 18:33:31 | sdague | mriedem: you have 7.0.0 instead of 8.0.0 in https://review.openstack.org/#/c/453819/1 | |
| 18:33:37 | sdague | which is why it's failing tets | |
| 18:33:39 | sdague | tests | |
| 18:33:53 | mriedem | derp | |
| 18:33:55 | mriedem | copy/paste | |
| 18:34:22 | mriedem | it's dependent on a novaclient reno change for now anyway | |
| 18:36:41 | mriedem | dansmith: don't forget the regression test patch below it | |
| 18:37:10 | dansmith | yeah dude, it takes a while :) | |
| 18:37:43 | mriedem | sorry | |
| 18:40:01 | ericyoung | mriedem: eric-aceshome | |
| 18:40:09 | mriedem | ok | |
| 18:41:04 | melwitt | mriedem: is this part not a separate fix on its own? https://review.openstack.org/#/c/452351/4/nova/compute/api.py | |
| 18:41:40 | dansmith | it is and he says that in the commit message, | |
| 18:41:49 | mriedem | i could split that out if needed | |
| 18:41:58 | dansmith | but there's not much to indicate what the thinking is there | |
| 18:42:11 | melwitt | okay | |
| 18:42:11 | melwitt | like, continuing on to the other delete code seems like a bug in itself | |
| 18:42:12 | mriedem | if you undo that and run the functional test, you'll see | |
| 18:42:14 | dansmith | I think I have it worked out in my head.. we would just go on to try to delete bdms and maybe fail or something? | |
| 18:42:20 | mriedem | yes, the problem is it continues to the rest of the other delete code | |
| 18:42:24 | mriedem | yes | |
| 18:42:53 | mriedem | i can split that out if it helps | |
| 18:43:02 | dansmith | so the user would see it as a 500 or something on the racing second delete attempt? | |
| 18:43:23 | dansmith | mriedem: since these are for backports, I'm fine with either, but might be good to just add more words about what that change is | |
| 18:43:33 | mriedem | i think it results in a 404 | |
| 18:43:34 | melwitt | I was just thinking out loud | |
| 18:43:37 | mriedem | looking up the bdms | |
| 18:43:43 | mriedem | i can test here locally quick to be sure | |
| 18:43:52 | dansmith | mriedem: "500 or something" includes "404" :P | |
| 18:44:02 | mriedem | "sh is f'ed" | |
| 18:44:05 | mriedem | includes both | |
| 18:44:10 | dansmith | heh | |
| 18:45:13 | melwitt | I understand what the change is for, I was just asking if that needs to be documented as a separate bug I guess | |
| 18:45:44 | mriedem | and return from _delete if we hit InstanceNotFound." | |
| 18:45:44 | mriedem | "This also fixes another bug where we would not short-circuit | |
| 18:45:44 | mriedem | i just lazily lumped it in in the commit message | |
| 18:46:13 | dansmith | right | |
| 18:46:22 | dansmith | that's the thing I think is a little, uh, light on the detail | |
| 18:46:42 | mriedem | heh, well, spent a half day writing the recreate functional regression test, | |
| 18:46:48 | mriedem | i was too tired to care about that detail :P | |
| 18:46:55 | mriedem | psh | |
| 18:47:01 | melwitt | yeah, I see it now. was just a surprise to see a major "let's stop continuing on to the other delete code we shouldn't be running" lumped in with "add regression test" :P | |
| 18:47:02 | dansmith | I haven't even gotten to that part yet :P | |
| 18:47:06 | mriedem | let me see what it does when i remove the return and rerun the func test | |