| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-26 | |||
| 13:49:37 | cdent | mriedem: I think it depends on how you are making the call. let me refresh my memory | |
| 13:50:23 | cdent | mriedem: looks like you need getall to get what you want: https://docs.pylonsproject.org/projects/webob/en/stable/reference.html#query-post-variables | |
| 13:51:03 | cdent | GET() will get you both, in tuples | |
| 13:51:44 | cdent | GET[key] gets the second, getall(key) gets both | |
| 13:51:50 | mriedem | ok i forgot about getall(), i think i found the list of tuples while digging through requests code yesterday | |
| 13:53:20 | mriedem | ok and looking at the list servers code, the only param we do getall on is 'status' so you can filter the list of servers on multiple status values, anything else would just be whatever the last entry is in the query string | |
| 13:53:30 | mriedem | we don't document that of course, but that's what i'm seeing | |
| 13:53:52 | mriedem | i just wasn't sure how search_opts.update(req.GET) would work | |
| 13:55:54 | cdent | there are lots of things about the nova-api that are "unique" | |
| 14:09:37 | openstackgerrit | Jay Pipes proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.openstack.org/555081 | |
| 14:10:21 | jaypipes | fried_rice, stephenfin, artom, alex_xu: thx for the reviews. I think I got all of your comments. please check it again. thanks in advance! mriedem, I added a bunch more to the upgrade impact section. ^^ | |
| 14:10:41 | fried_rice | ack | |
| 14:25:57 | finucannot | mriedem: This is another one of those changes that I thought would require a microversion but apparently does not. The output is wrong but it doesn't raise a HTTP 5xx https://review.openstack.org/#/c/608685/ | |
| 14:26:48 | finucannot | What distinguishes a fix from this from, say, modifying the output of os-hypervisor to respect (cpu|ram)_allocation_ratio. Both are currently giving "wrong" output | |
| 14:29:03 | mriedem | i don't have a good answer for that except the os-hypervisors stuff has been around forever and i don't know if including allocation ratios in that calculation would be considered by all to be "right" | |
| 14:30:33 | finucannot | Yeah, probably not the best example | |
| 14:30:51 | finucannot | mriedem: Would it be fair to ask for 'bug'-type reno for these kind of changes, in that case? | |
| 14:30:59 | finucannot | I'm thinking of the case where different clouds would give different results for what's supposed to be the same version | |
| 14:31:33 | mriedem | i wouldn't say it's unfair, although this simple tenant usage thing lpetrut is fixing is admin-only by default | |
| 14:31:39 | mriedem | non-admins can't list usage across all tenants | |
| 14:31:51 | mriedem | so that's not really an interop concern | |
| 14:32:11 | finucannot | Ohh, I didn't know that. In that case, not an issue here | |
| 14:32:55 | mriedem | well, | |
| 14:33:05 | mriedem | his change isn't specific to list (admin), it's also show (non-admin) | |
| 14:33:17 | mriedem | so idk, if you want a "fixes" release note then ask for one | |
| 14:33:26 | mriedem | "paging was broken, now it's fixed" | |
| 14:34:00 | finucannot | Yeah, I think I will. Probably makes sense assuming we want to backport this too (and to alert folks who'd want this downstream) | |
| 14:35:10 | mriedem | in general, we don't have a great formula for when a microversion is required. obviously major behavior changes that people need to opt into. and request/response schema changes. but there are so many other behavior-ish and bug fix type things that could be argued require a microversion we'd never do anything | |
| 14:35:17 | mriedem | or we'd have a thousand microversions by now | |
| 14:35:32 | mriedem | e.g. the trusted VFs thing from sahid technically could have required a microversion | |
| 14:36:19 | mriedem | and your requested networks physnet stuff | |
| 14:37:16 | finucannot | All good points | |
| 14:37:38 | finucannot | Clearly my "must raise HTTP 5xx" rule is too rigid | |
| 14:37:47 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Integration tests using gabbi-tempest https://review.openstack.org/613386 | |
| 14:45:37 | openstackgerrit | Lucian Petrut proposed openstack/nova master: Fix os-simple-tenant-usage result order https://review.openstack.org/608685 | |
| 14:47:55 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix os-simple-tenant-usage result order https://review.openstack.org/608685 | |
| 14:50:39 | mriedem | i can't figure out for the life of me why i can't send a RequestSpec object to prep_resize over rpc https://gist.github.com/mriedem/9fce3c68336f8abb5d4753c156d8f96f but we pass RequestSpec objects over rpc in several other places, like build_and_run_instance and select_destinations | |
| 14:51:14 | mriedem | there is an old oslo.messaging bug for the fake impl rpc driver https://bugs.launchpad.net/oslo.messaging/+bug/1529084 but how does that not break on those other calls? | |
| 14:51:15 | openstack | Launchpad bug 1529084 in oslo.messaging "RPC fake driver should accept datetime items for data" [Undecided,Invalid] | |
| 15:22:44 | openstackgerrit | Eric Fried proposed openstack/nova master: Kill @safe_connect in _get_provider_traits https://review.openstack.org/613613 | |
| 15:22:59 | fried_rice | edmondsw: ^ (and leakypipes and other haters of @safe_connect) | |
| 15:24:29 | edmondsw | fried_rice thanks | |
| 15:24:47 | cdent | fried_rice: where does the exception go, now? | |
| 15:25:11 | fried_rice | cdent: up | |
| 15:25:13 | fried_rice | as it should | |
| 15:25:16 | cdent | indeed | |
| 15:25:19 | cdent | but then what? | |
| 15:25:32 | fried_rice | well, it will go the same place as the AttributeError did. | |
| 15:25:37 | fried_rice | http://184.172.12.213/04/602804/8/check/nova-in-tree-pvm/9a0e925/logs/n-cpu.txt.gz#_Oct_26_02_24_17_490196 | |
| 15:26:02 | cdent | okay, but surely that's not enough | |
| 15:26:34 | cdent | either the attributeerror of the keystone error needs to be handled in some approrpriate fashion | |
| 15:26:48 | cdent | s/of/or/ | |
| 15:27:17 | fried_rice | It's really just so we can know what actually happened rather than having to rely on the tribal knowledge that when a placement helper method returns None, it's probably because comm failed. | |
| 15:27:49 | fried_rice | Getting rid of @safe_connect is mostly about ^ that, not about doing something more appropriate with the now-not-as-mysterious exception. That would be a separate thing. | |
| 15:27:49 | cdent | (re that link: my eyes!!!!) | |
| 15:28:54 | fried_rice | cdent: hold on, we've got some of my log coloring experimental code on that server, I think there's a secret option to change themes... | |
| 15:29:42 | cdent | sure, but the reason safe_connect existed in the first place was to do some kind of "right thing" when comm's failed. what's wrong here is that safe_connect is broken, not that it exists | |
| 15:30:02 | cdent | safe_connect was supposed provided away for interactions with the placement api to be correctly retried (or logged) when things went awry | |
| 15:30:18 | cdent | somewhere along the way it lost that and became simply a bad thing that returned None when it shouldn't | |
| 15:30:51 | cdent | There's no doubt (at all) that it is a bad thing, now. But the lost of the intention is also a bad thing. | |
| 15:31:26 | fried_rice | in this case, the next spin through the periodic will hopefully work. | |
| 15:31:26 | cdent | If we're cool with the idea of removing it (because its masks real things that need to be fixed) will to stuff actually being fixed, cool | |
| 15:31:48 | cdent | that seems an odd approach to take, to me | |
| 15:31:57 | fried_rice | That's "retry" | |
| 15:32:04 | fried_rice | or if this call happened as the result of an actual operation, we've got retries built in, IIRC. Sec... | |
| 15:32:06 | cdent | because the next periodic could be many minutes away if the deployer has chosen to do so | |
| 15:32:22 | fried_rice | Yeah, and if an operation isn't happening, who cares | |
| 15:32:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Pass request_spec from compute to cell conductor on reschedule https://review.openstack.org/582417 | |
| 15:33:06 | cdent | if the failed request was e.g., updating inventory, scheduling will go wrong during that window | |
| 15:33:55 | cdent | What I'm trying to get at here is that I suspect (actually, am confident that) the system is more complex than "just let the expceptions rise" can bear | |
| 15:33:57 | fried_rice | So this is all happening under the umbrella of _update, which has a retry around it for conflicts | |
| 15:34:19 | cdent | but what about for a network partition? | |
| 15:35:33 | cdent | (or things that look like a network partition (such as the load balance being reconfigured)) | |
| 15:36:16 | fried_rice | Look, I'm not disputing that we could do a better job of handling placement comm errors in this whole flow (i.e. the resource tracker). But first we have to know we're having a placement comm error. | |
| 15:36:36 | fried_rice | If we want to put in retries at some level based on ClientException, I'm all behind that. | |
| 15:36:42 | fried_rice | But that's not what this patch is for. | |
| 15:38:41 | cdent | Okay, but "handling placement comm errors" is why safe_connect was created, so simply killing it is probably not the right approach. Fixing it probably is. | |
| 15:39:12 | cdent | I'm harping on it because it is clear that the reasson for its existence has now been lost to the sands of time (because somewhere along the way it borked) | |
| 15:39:41 | cdent | But we seem to want to say "it sucks, let's just kill it" (at least jokingly) and that's not really sufficient in a distributed system. | |
| 15:41:00 | cdent | These issues will come back around again, after safe_connect has gone, and we'll realize "you know, we need to create this thing that allows us to safely connect" | |
| 15:41:08 | fried_rice | We have made several assays toward fixing it. It can't be done. Because it's at too low a level in the call stack. | |
| 15:41:12 | SteelyDan | cdent: actually safe_connect was really to handle the "have they set up placement or not" situation | |
| 15:41:33 | SteelyDan | which is why we've been talking about removing it entirely since placement became required | |
| 15:41:50 | cdent | SteelyDan: that was _one_ of the reasons, and yes, the very first reason, but very quickly it took on "deal with errors from keystoneclient" | |
| 15:42:31 | SteelyDan | yes, it has evolved | |
| 15:42:42 | openstackgerrit | Eric Fried proposed openstack/nova master: Kill @safe_connect in _get_provider_traits https://review.openstack.org/613613 | |
| 15:42:46 | cdent | we had: is it in the service catalog, then can I reach it | |
| 15:42:52 | fried_rice | cdent: added words clarifying intent --^ | |
| 15:43:32 | cdent | thanks | |
| 15:44:31 | openstackgerrit | sean mooney proposed openstack/nova master: harden placement init under wsgi https://review.openstack.org/610034 | |
| 15:44:34 | cdent | I'm trying really hard to not to think to much about the resource tracker lately but sometimes I get sucked in. | |
| 15:45:34 | sean-k-mooney | cdent: ^ uses the run_once decorator | |
| 15:45:38 | fried_rice | For example, if we decided to make it retry, we would have to restrict the decorator to *only* single primitive API calls. We would have to remove it from all the methods that do more complex things e.g. involving multiple API calls. | |
| 15:45:38 | fried_rice | cdent: Anything we would do to @safe_connect, beyond making it essentially a no-op shell (which isn't better than removing it IMO), would also entail a thorough audit of everywhere it is used, which is a nearly impossible task due to how many different ways and in how many layers things get called. | |
| 15:46:02 | cdent | sean-k-mooney: yeah, thanks, reading | |
| 15:46:25 | sean-k-mooney | cdent: ill propose the placement version shortly if people are fine with it | |
| 15:47:07 | cdent | fried_rice: yes, I know, thus why I'm trying really hard not to think about the resource tracker lately. It was already really complicated before placement, and we somehow managed to make it worse, not better. | |
| 15:47:28 | fried_rice | I have made a couple of attempts, and I think SteelyDan has as well, to do something to @safe_connect itself to improve it. At some point in a review leakypipes said we should just target one usage at a time and get rid of it. Which so far has been the only approach that has gotten us to actually merge code around this. | |
| 15:47:45 | SteelyDan | yup | |
| 15:47:54 | fried_rice | cdent: I still hope that it will be better in the long run, once we've purged all the legacy stuff from this painful transitional period. | |