| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-08 | |||
| 13:54:51 | mriedem | https://review.opendev.org/#/c/674770/ | |
| 13:55:18 | mriedem | dansmith: https://review.opendev.org/#/c/672747/ | |
| 13:55:31 | mriedem | well or lee can upgrade to +W that one | |
| 13:56:02 | mriedem | https://review.opendev.org/#/q/topic:bug/1764556+branch:stable/rocky | |
| 14:00:23 | dansmith | mriedem: I'm not sure what https://review.opendev.org/#/c/674770/1 is actually fixing | |
| 14:00:51 | mriedem | https://review.opendev.org/#/c/669393/ | |
| 14:01:11 | mriedem | dansmith: redundant db calls | |
| 14:01:41 | mriedem | efried: meeting? | |
| 14:02:02 | efried | thanks mriedem, got distracted. | |
| 14:02:03 | dansmith | mriedem: is it actually doing less work? | |
| 14:02:55 | mriedem | the bug report has the details | |
| 14:03:23 | mriedem | oh you mean since it still calls group.get_hosts() right? | |
| 14:04:35 | dansmith | I guess I need to dig further into what is going on under the covers, but it seems like this is just a tiny optimization, not really a fix no? | |
| 14:05:07 | dansmith | the bug seems to be "clean up dead code" or so | |
| 14:05:48 | mriedem | yes i think it is, | |
| 14:05:51 | mriedem | otherwise we'd change to: | |
| 14:05:56 | mriedem | group_hosts = group.hosts yeah? | |
| 14:07:06 | mriedem | guess i never saw the original https://review.opendev.org/#/c/665954/ | |
| 14:08:06 | dansmith | I'm just not sure what the point is | |
| 14:08:17 | dansmith | but it seems like kinda pointless for a backport | |
| 14:09:10 | mriedem | dropped my +2 | |
| 14:09:30 | mriedem | the point was to remove the redundant db query | |
| 14:09:32 | mriedem | which it's not | |
| 14:10:19 | dansmith | which redundant query? the hit to list cells first or something? | |
| 14:10:35 | mriedem | group.get_hosts (and the scatter/gather thing) | |
| 14:17:47 | openstackgerrit | François Palin proposed openstack/nova master: Add retry when attachment_delete or terminate_connection fails https://review.opendev.org/669674 | |
| 14:20:35 | mriedem | stephenfin: https://review.opendev.org/#/c/583247/2 leads to a change in merge conflict, i'd like to abandon the entire series unless there is a valid reason we should move forward with adding code that's not used | |
| 14:20:56 | stephenfin | mriedem: Kill it | |
| 14:21:30 | mriedem | gladly | |
| 14:22:02 | stephenfin | (Nice idea but I don't have time to finish it and guess he won't now) | |
| 14:37:03 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Add functional recreate test for regression bug 1825537 https://review.opendev.org/675355 | |
| 14:37:03 | openstack | bug 1825537 in OpenStack Compute (nova) rocky "finish_resize failures incorrectly revert allocations" [Medium,In progress] https://launchpad.net/bugs/1825537 - Assigned to Matt Riedemann (mriedem) | |
| 14:39:14 | dansmith | mriedem: okay I think I got them all, yeah? | |
| 14:40:29 | mriedem | dansmith: yup i think so, thanks! | |
| 14:44:31 | dansmith | cool | |
| 14:45:03 | gibi | mriedem: I'm reading your grenade + resize comment in https://review.opendev.org/#/c/655721 Would it be enough to tag one of the resize tempest test with "smoke" to get the coverage you miss or we need to change some grenade logic as well? | |
| 14:49:59 | mriedem | tagging a resize tempest test with smoke would make it run during grenade yes | |
| 14:50:17 | artom | Lol, we ain't very smart | |
| 14:50:33 | artom | When we detach an interface, we catch a bunch of exceptions: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L209 | |
| 14:51:07 | artom | We then do some RPC | |
| 14:51:09 | artom | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4305 | |
| 14:51:32 | artom | ... which is a cast: https://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L572 | |
| 14:52:01 | mriedem | artom: the only thing that wouldn't be in that list is PortNotFound | |
| 14:52:08 | mriedem | the others are all failures from the api | |
| 14:52:40 | mriedem | for all i know some years ago it might have been a call, or the api might have looked up the port first | |
| 14:52:52 | artom | mriedem, ah, good point | |
| 14:53:28 | mriedem | who's not smart now | |
| 14:53:28 | artom | Yeah, I'm sure there's a valid historical reason for it | |
| 14:53:39 | artom | mriedem, when did I ever claim to be smart? | |
| 14:53:54 | artom | It was a just a cute "end state" | |
| 14:54:05 | mriedem | https://review.opendev.org/#/c/130028/2/nova/api/openstack/compute/plugins/v3/attach_interfaces.py@158 | |
| 14:54:09 | mriedem | eli pointed this out in 2014 | |
| 14:54:09 | artom | (Got reported as a bug downstream, that we can pass anything as a port ID and it still accepts it) | |
| 14:54:45 | artom | How in the hell did you pull that review from 5 years ago so fast | |
| 14:55:06 | openstackgerrit | Shilpa Devharakar proposed openstack/nova master: Enable request filter isolate_aggregates https://review.opendev.org/667952 | |
| 14:56:42 | mriedem | pycharm annotation | |
| 14:57:15 | efried | mriedem: been seeing this weirdness of late https://logs.opendev.org/41/673341/10/check/nova-tox-functional-py36/b2b2fa5/testr_results.html.gz | |
| 14:57:15 | efried | is this the race you were talking about yesterday? | |
| 14:57:48 | mriedem | no, but i don't remember talking about weird races yesterday either | |
| 15:01:59 | gibi | efried, mriedem: this was seen in https://review.opendev.org/#/c/666857/ which was reviewed by both of you | |
| 15:02:03 | mriedem | artom: it was all dansmith's fault https://review.opendev.org/#/c/21819/ | |
| 15:02:16 | mriedem | in...grizzly | |
| 15:02:24 | artom | I do love it when that happens | |
| 15:02:39 | mriedem | well, note the size of the change | |
| 15:03:02 | mriedem | Dan Smith <danms@us.ibm.com> | |
| 15:04:10 | mriedem | artom: push a patch to remove the PortNotFound handling if you want | |
| 15:04:10 | dansmith | what's up now? I tried to catch exceptions around a rpc cast? | |
| 15:04:23 | mriedem | dansmith: yes, and 6 years later much hilarity ensued | |
| 15:04:53 | artom | mriedem, I'd rather look up the port ID at the REST API level, and throw the 404 | |
| 15:04:58 | artom | Would that work? | |
| 15:05:01 | mriedem | you mean your customer would | |
| 15:05:12 | mriedem | let's front-load validate all incoming IDs because of one red hat customer! | |
| 15:05:13 | mriedem | yay! | |
| 15:06:02 | artom | I actually don't think there's a customer | |
| 15:06:24 | artom | It's more a matter of aligning with our api-ref docs: https://docs.openstack.org/api-ref/compute/?expanded=detach-interface-detail#detach-interface | |
| 15:06:29 | artom | Which state a 404 could happen | |
| 15:06:35 | artom | ... or we should change the doc | |
| 15:06:43 | mriedem | technically the 404 is for the instance not being found | |
| 15:06:47 | mriedem | the port not being found is a 400 | |
| 15:06:55 | mriedem | according to what i've learned over the years from cdent and edleafe | |
| 15:06:57 | mriedem | and the api sig | |
| 15:07:09 | mriedem | missing things that aren't the main resource are 400s | |
| 15:07:29 | edleafe | mriedem is a good student :) | |
| 15:07:39 | mriedem | i only had to ask about 10 times | |
| 15:08:34 | artom | Ah, so since the api-ref doens't state a 400 is a possible response, we're technically good? | |
| 15:08:40 | artom | Still feels weird | |
| 15:09:12 | artom | Although I suppose there's precedent | |
| 15:09:17 | artom | With live-migration, for example | |
| 15:12:19 | stephenfin | efried: You should look at this too, I'd say https://review.opendev.org/#/c/666032/ | |
| 15:13:15 | efried | dansmith: mriedem: FYI I'm +2 all the way up on aggregate isolation https://review.opendev.org/#/c/671075/ | |
| 15:13:29 | efried | stephenfin: I've been avoiding that one | |
| 15:13:34 | efried | I never understood the original spec | |
| 15:14:01 | efried | but I'll have a look at some point if you get desperate. | |
| 15:14:48 | stephenfin | efried: TBH, it's what I'm doing anyway so it really just needs a sanity check | |
| 15:16:03 | efried | point is, if I do manage to review the code, I'll be doing it against whatever the spec says. So if I rubber-stamp the spec based on the fact that that's what you're coding... https://previews.123rf.com/images/sudowoodo/sudowoodo1708/sudowoodo170800024/84484656-ouroboros-symbol-snake-eating-its-own-tail-modern-alchemy-logo-vector-illustration-.jpg | |
| 15:30:53 | openstackgerrit | Shilpa Devharakar proposed openstack/nova-specs master: Update spec: filtering of alloc candidates by forbidden aggregates https://review.opendev.org/675384 | |
| 15:37:06 | aspiers | how can I debug placement returning no allocation candidates on devstack, when debug is already set to True and showing nothing useful? | |
| 15:37:14 | aspiers | GET /placement/allocation_candidates?limit=1000&required=%21COMPUTE_STATUS_DISABLED&resources=DISK_GB%3A20%2CMEMORY_MB%3A2048%2CVCPU%3A1 | |
| 15:38:27 | aspiers | I've already gone through the three resource classes via openstack resource provider inventory show $rp_uuid | |
| 15:38:51 | aspiers | each one is showing a total and max_unit greater than what is being asked for | |