| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-30 | |||
| 13:58:37 | takashin | mriedem: I will check it. | |
| 13:59:57 | openstackgerrit | Eric Fried proposed openstack/nova master: Make _Provider really private https://review.openstack.org/523932 | |
| 13:59:57 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.uuid_set() https://review.openstack.org/520243 | |
| 14:05:47 | gmann | alex_xu: gibi: stephenfin oomichi_afk can u check this, it is good to go now - https://review.openstack.org/#/c/500347/12 | |
| 14:18:45 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Assert that we restrict cold migrations to the same cell https://review.openstack.org/524035 | |
| 14:19:45 | mriedem | edleafe: before sydney we glanced at the conductor changes and had mentioned something about how we thought we should do something differently in there, which would require changes to the selection object, but i can't remember the details off hand atm, but that's what i'm referring to, and i haven't been back to the conductor changes in the series yet to see what you have changed | |
| 14:20:10 | gmann | jaypipes: for you - https://review.openstack.org/#/c/500347/12 | |
| 14:20:47 | jaypipes | gmann: ah, that's the one from Kevin Chen, yeah? | |
| 14:20:58 | gmann | yea | |
| 14:20:58 | jaypipes | gmann: you happy with that one now> | |
| 14:20:59 | jaypipes | ? | |
| 14:21:01 | jaypipes | k | |
| 14:21:09 | gmann | yea it looks good, m +1 | |
| 14:21:33 | edleafe | mriedem: the only thing I remember was the two RPC params instead of one | |
| 14:22:04 | jaypipes | gmann: +2 | |
| 14:22:25 | gmann | jaypipes: thanks | |
| 14:22:30 | jaypipes | gmann: np :) | |
| 14:22:34 | edleafe | mriedem: nothing about the Selection object | |
| 14:23:10 | edleafe | mriedem: and if something *does* come up, it can be addressed in a subsequent patch, no? | |
| 14:26:47 | mriedem | edleafe: referring to this https://review.openstack.org/#/c/511358/24/nova/conductor/manager.py@587 | |
| 14:28:05 | mriedem | i'm sorry this isn't all wrapped up already, sydney happened and dan and i have been dealing with cve fallout before sydney and after, draining time and energy on much else | |
| 14:28:51 | edleafe | mriedem: ah, that was changed the day you mentioned it. | |
| 14:29:16 | edleafe | it no longer relies on the emptiness of the allocation_request field | |
| 14:37:44 | efried | jaypipes I'd like to fix https://bugs.launchpad.net/nova/+bug/1735430 pretty quick here, would like to discuss possibilities. | |
| 14:37:45 | openstack | Launchpad bug 1735430 in OpenStack Compute (nova) "Report client doesn't handle RP create conflict (409) properly" [Undecided,New] | |
| 14:38:12 | jaypipes | efried: ok dokey | |
| 14:38:40 | jaypipes | efried: wouldn't it be nice if we had error *codes*? :( | |
| 14:38:55 | efried | jaypipes Like in the body of the response? | |
| 14:39:01 | jaypipes | efried: ya | |
| 14:39:23 | jaypipes | efried: so you could distinguish between bad name and bad UUID... for example | |
| 14:39:25 | efried | jaypipes Is there a reason we don't do that? | |
| 14:39:36 | jaypipes | efried: just never did it. I blame cdent. | |
| 14:39:50 | cdent | jaypipes: hah, I validated the api guideline that says it should happen | |
| 14:39:53 | cdent | I blame jaypipes | |
| 14:40:13 | efried | Okay, if there's no reason we're avoiding it, we could get started on that as a long-term solution. | |
| 14:40:13 | jaypipes | cdent: :) | |
| 14:40:26 | gibi | gmann: looking... | |
| 14:40:31 | jaypipes | efried: ack, though there are more pressing concerns at this point, no? | |
| 14:40:39 | efried | In the meantime: in PS2 I actually made us raise an exception: https://review.openstack.org/#/c/523932/2 | |
| 14:40:44 | cdent | efried: I think it was mostly just a kind of distraction in the early days and then forgotten | |
| 14:40:50 | cdent | however, it wouldn't make any difference in this bug | |
| 14:41:06 | efried | jaypipes That broke a functional test that runs _init_compute_host twice. cdent was helping me look at that last night. | |
| 14:41:28 | cdent | the 409 is the same in either case: uniqueness is violated and we require uniqueness. the bug isn't so much the response code as the return of None | |
| 14:41:35 | jaypipes | efried: sorry, I didn't mean the bug wasn't pressing. I meant reworking things so we have proper error codes. | |
| 14:41:45 | jaypipes | efried: as opposed to grepping through an error string | |
| 14:41:46 | efried | jaypipes Yeah, I'm with you. | |
| 14:42:00 | efried | jaypipes Hold on, are you saying we could grep the error string? | |
| 14:42:05 | efried | as an interim solution? | |
| 14:42:33 | jaypipes | efried: we've already done that in a couple places, yeah. | |
| 14:42:43 | cdent | the error string doesn't matter in this bug | |
| 14:43:12 | cdent | or rather the fact that we hide the error response in the expection message made it hard to debug | |
| 14:43:16 | jaypipes | cdent: well, the whole "the error is saying I used a bad UUID, when I actually used a bad name" is problematic. | |
| 14:43:26 | jaypipes | cdent: since they both have the same error message | |
| 14:43:26 | cdent | that's a different bug: | |
| 14:43:30 | jaypipes | cdent: ack | |
| 14:43:42 | cdent | https://bugs.launchpad.net/nova/+bug/1735405 | |
| 14:43:44 | openstack | Launchpad bug 1735405 in OpenStack Compute (nova) "Error message from placement when creating resource provider uses ambiguous identifier" [Undecided,New] | |
| 14:43:44 | efried | So let's say we do grep the error string - how would report client react that would be appropriate here? | |
| 14:43:53 | jaypipes | ok, agreed, diff bug | |
| 14:43:55 | cdent | the db duplicate response message does include which columns were duplicated | |
| 14:44:03 | cdent | I changed that some months ago | |
| 14:44:10 | cdent | but we hide that in the report client, and shouldn't | |
| 14:44:15 | jaypipes | efried: raise an exception back to the caller. | |
| 14:44:29 | jaypipes | cdent: gotcha | |
| 14:44:39 | cdent | why do we need to grep the error response? | |
| 14:44:48 | cdent | if uniqueness fails, uniqueness fails | |
| 14:44:58 | cdent | you can only get a 409 from post to /resource_providers for uniqueness fails | |
| 14:45:00 | jaypipes | cdent: in order to log an appropriate message, that's all | |
| 14:45:17 | jaypipes | cdent: it's just misleading to write an error saying bad UUID, when it was the name that was problematic | |
| 14:45:17 | cdent | pass on the response's error message, no need to parse it, just use it | |
| 14:45:40 | jaypipes | cdent: sure, can do that | |
| 14:46:09 | efried | Hold on | |
| 14:46:13 | efried | I don't agree | |
| 14:46:45 | efried | Right now we assume it's UUID conflict, so we GET the provider by UUID and return it (or None) | |
| 14:47:03 | cdent | the code is incomplete | |
| 14:47:16 | efried | Right | |
| 14:47:28 | cdent | if we want to assume that a 409 _could_ be "another thread created" | |
| 14:47:33 | cdent | then yes, parsing it required | |
| 14:47:52 | cdent | (or adding more detailed error codes, eventually) | |
| 14:48:36 | gibi | gmann_afk: +W | |
| 14:49:47 | cdent | but, then we have 409 meaning potentially 3 things: uuid uniqueness violation (good luck!), name uniqueness violation, concurrent violation | |
| 14:50:09 | cdent | and while telling some of those apart is gravy, the uuid ones, unclear | |
| 14:50:12 | jaypipes | cdent: right. thus the need for more specific error codification. | |
| 14:50:35 | cdent | right, but even with better error codification how do you distinguish conficts 1 and 3? | |
| 14:50:43 | bauzas | oh shit, DST shift impact :( | |
| 14:50:46 | bauzas | mriedem: ^ | |
| 14:50:53 | efried | cdent We have to assume #1 doesn't happen. | |
| 14:51:02 | efried | Or rather, assume the risk | |
| 14:51:51 | efried | Realistically, we don't get to that point if there's a UUID "conflict" - we would have just returned that provider from the first GET in _ensure_resource_provider. | |
| 14:51:57 | mriedem | bauzas: ? | |
| 14:52:38 | cdent | brb | |
| 14:52:49 | mriedem | sdague: i think you will enjoy this https://review.openstack.org/#/c/507693/ | |
| 14:53:20 | efried | cdent jaypipes IMO the first thing we should do is the simplest: raise exceptions instead of returning None in those various paths. | |
| 14:54:03 | efried | Today that 404 will be indicating name conflict 99% of the time. | |
| 14:55:07 | efried | The only excepton would be concurrent create-followed-by-delete. So I'd even go so far as to say 99.99999% | |
| 14:55:09 | cdent | efried: yeah, I agree None is not right | |
| 14:55:13 | jaypipes | efried: go for it. | |
| 14:55:27 | cdent | you'll have to have the fun of figuring out what's up with the api_samples test :( | |
| 14:55:47 | efried | Exactly. jaypipes it looks like this: http://logs.openstack.org/32/523932/2/check/openstack-tox-functional/05f8149/testr_results.html.gz | |
| 14:56:24 | efried | It happens because those test cases are starting the fake compute host twice. Each one seems to be getting its own database. But still just one placement service. | |