| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-26 | |||
| 07:46:38 | openstackgerrit | mashuting proposed openstack/nova master: VMware: Support volumes backed by VStorageObject https://review.openstack.org/521690 | |
| 08:52:04 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Integration tests using gabbi-tempest https://review.openstack.org/613386 | |
| 09:19:50 | openstackgerrit | Yikun Jiang proposed openstack/nova master: Add ratio online data migration when load compute node https://review.openstack.org/613499 | |
| 10:49:14 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Integration tests using gabbi-tempest https://review.openstack.org/613386 | |
| 10:52:43 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: Integration tests using gabbi-tempest https://review.openstack.org/613386 | |
| 10:53:59 | openstackgerrit | Yikun Jiang proposed openstack/nova master: Add ratio online data migration when load compute node https://review.openstack.org/613499 | |
| 10:53:59 | openstackgerrit | Yikun Jiang proposed openstack/nova master: Add compute_node ratio online data migration script https://review.openstack.org/609995 | |
| 12:39:10 | jaypipes | aspiers: +2 from me. | |
| 13:02:48 | jaypipes | efried: done | |
| 13:18:51 | mpasserini | Hi, what is the service which has the task of creating the ephemeral volumes on the compute nodes? Nova-compute? | |
| 13:34:56 | mriedem | dansmith: i'd ping tssurya, but she's not around and it seems you're updating the down cell series. i've gone up through https://review.openstack.org/#/c/584829/ | |
| 13:35:14 | dansmith | mriedem: just going through my gerrit mail right now | |
| 13:35:37 | mriedem | i've got comments/questions in the changes with +1s too, so you could look at those as well. like wondering about applying limits when listing from down cells. | |
| 13:35:43 | mriedem | seems we could do that, but we don't. | |
| 13:36:11 | dansmith | roger | |
| 13:48:39 | mriedem | cdent: requests.GET question for you, if i have a call like GET /servers?all_tenants=1&project_id=1&project_id=2, does that result in {'project_id': [1,2]} ? | |
| 13:49:00 | mriedem | with the all_tenants key in the dict as well, but i care about the double project_id | |
| 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 | cdent | (re that link: my eyes!!!!) | |
| 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: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 | 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:26 | fried_rice | in this case, the next spin through the periodic will hopefully work. | |
| 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 | |