| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-20 | |||
| 17:05:07 | dansmith | mordred: ack | |
| 17:05:12 | mordred | clients are going to need to handle servers in an unknown state regardless - there is no magical thing we can do to make this transparent for existing api consumers | |
| 17:05:20 | dansmith | tssurya: yeah, maybe, but that seems like mechanics we can work around if we need | |
| 17:05:29 | tssurya | mordred: setting it to NULL/empty is kind of like lying and not truth also | |
| 17:05:33 | dansmith | tssurya: mordred is a "real api consumer" which is why I want his opinion on this | |
| 17:05:39 | mordred | so I think saying "if you get a server in state UNKNOWN - here's what that means for the rest of the thing" | |
| 17:05:45 | tssurya | dansmith: ack :) | |
| 17:06:15 | mordred | tssurya: yah - totally - honestly I think we could also remove the whole entry (this is a microversion behavior, right?) | |
| 17:06:25 | mordred | sorry - when I said whole entry - I mean key and value | |
| 17:06:26 | dansmith | tssurya: well, saying we don't know what the value is is definitely more honest than saying it _is_ "unknown" or some fake value | |
| 17:06:47 | dansmith | mordred: yeah, so before a microversion you'd get a 500 if a cell is down I guess | |
| 17:06:47 | tssurya | dansmith: true, | |
| 17:06:57 | mordred | I know there is a concern about hitting key errors - but if you opt in to this microversion, you're saying you can deal with unknown state servers | |
| 17:07:03 | mordred | dansmith: ++ | |
| 17:07:04 | dansmith | which is an odd microversion, but.. | |
| 17:07:26 | mordred | well - it's a "list servers with this microversion because I can handle some of the servers in the list having less keys thn I was expecting" | |
| 17:07:32 | dansmith | it's a pretty well-baited one, because if you don't opt into that one for list, you're basically saying "meh, I can't handle the inconsistency" so we're helping by not showing you anything | |
| 17:07:34 | mordred | whereas existing code is just going to error | |
| 17:07:39 | dansmith | yes | |
| 17:07:45 | mordred | dansmith: exactly | |
| 17:08:13 | mordred | it's the old data consistency thing - it's better for the entire data store to be unavailable than to behave ina half-available state and maybe lose data when you thought it was ok | |
| 17:08:38 | dansmith | yeah, like gibi said, | |
| 17:08:41 | mordred | current behavior is 500 - nobody gets accidentally hosed - they can always retry the query until it stops 500ing | |
| 17:08:41 | tssurya | mordred: right | |
| 17:08:43 | mordred | ++ | |
| 17:08:52 | dansmith | if we report hostId=UNKNOWN, then a client thinks all the instances are suddenly on the same host :) | |
| 17:09:56 | dansmith | tssurya: okay so anyway, I think mordred and I agree on this, so I'll comment on the spec about this convo and maybe we can sync up with mriedem when he's back next week and see what he thinks | |
| 17:10:11 | dansmith | tssurya: are you opposed to making this unified or are you okay with either? | |
| 17:10:12 | tssurya | dansmith: ack | |
| 17:10:15 | dansmith | and which is your preference? | |
| 17:10:25 | tssurya | I am okay with either | |
| 17:10:27 | dansmith | all things equal, I mean, which do you think is better? | |
| 17:10:33 | mordred | dansmith: would we have the addresses dict since those are coming from neutron? or do those get stored in the cells db? | |
| 17:10:40 | tssurya | as long as we give at least the uuid back | |
| 17:10:57 | dansmith | mordred: we would have to hit neutron for it since our _cache_ of those values are in the cell db | |
| 17:11:07 | tssurya | like keeping the current 500 behavior if 1 out of 70 goes down is wrong for sure | |
| 17:11:19 | dansmith | tssurya: yeah okay | |
| 17:11:21 | mordred | nod. I mean - I already have code to hit neutron api if the addresses dict is empty - so I'm fine if it's not | |
| 17:11:43 | tssurya | 1) we fix this with skipping down cells as first step 2) return just the uuids with rest as empty | |
| 17:11:48 | mordred | tssurya: uuid and name (filtering on name client-side is frequent/important) | |
| 17:11:50 | tssurya | makes sense ? | |
| 17:12:17 | mordred | if it's possible | |
| 17:12:18 | dansmith | mordred: we don't have easy access to the name, but we can get it from the reqspec if you think it's important | |
| 17:12:21 | tssurya | mordred: we were thinking of skipping a cell if filters are used | |
| 17:12:40 | dansmith | yeah, if you're filtering, and we don't know the value, we assume no match | |
| 17:12:51 | tssurya | dansmith: we don't have the name in req_spec right ? | |
| 17:12:57 | dansmith | tssurya: the display name? | |
| 17:13:05 | tssurya | yes | |
| 17:13:32 | tssurya | meaning we don't have the display_name | |
| 17:13:36 | tssurya | in req_spec | |
| 17:13:52 | dansmith | oh, I guess I'm thinking of buildreq, which stores it while we're building | |
| 17:13:56 | dansmith | so yeah maybe not | |
| 17:14:31 | mordred | ah - we don't have the name? hrm. | |
| 17:15:15 | tssurya | mordred: no :( the only info we can get is uuid, flavor, image, avz, project_id | |
| 17:15:49 | dansmith | the point here is to not store the same thing in multiple places where it gets out of sync (like in cellsv1) | |
| 17:16:19 | stephenfin | zcorneli|lunch: One nit left in that that should be fixed but not blocking on it. If you could address that (and anything else you fancy fixing) in a follow up, that would be appreciated. | |
| 17:16:20 | tssurya | dansmith: yea and I agree with not adding info into top API, but we should make use of the existing info | |
| 17:16:37 | dansmith | tssurya: yeah | |
| 17:16:42 | tssurya | infact just the uuid would also be good enough with the state as UNKNOWN like you said | |
| 17:16:58 | tssurya | and we would anyways need a "odd microversion" bump | |
| 17:17:23 | mordred | hrm. well - it would make things a bit awkward for ansible dynamic inventory without name ... but if that's life, I guess that's life | |
| 17:17:56 | tssurya | mordred: :) | |
| 17:18:19 | mordred | basically it'll make operations that are working with servers that are up not able to find the server if part of the api control is down if/when those servers are being managed by name and not uuid | |
| 17:18:22 | mordred | BUT | |
| 17:18:53 | mordred | maybe that's just life - the existing system would mean you can't deal with ANY of your servers - so it's still an improvement | |
| 17:19:08 | dansmith | yeah | |
| 17:19:15 | dansmith | if we do the 1a option then it gets better | |
| 17:19:18 | mordred | yup | |
| 17:19:28 | tssurya | yea | |
| 17:19:40 | mordred | so yeah - I think 1 followed by 1a would be great | |
| 17:20:23 | dansmith | mordred: thanks for the discussion! | |
| 17:20:26 | tssurya | mordred: totally appreciate your view and opinion on this :) thanks! | |
| 17:20:49 | tssurya | dansmith: so you will write a comment on yours and mordred's behalf in the spec ? | |
| 17:20:58 | dansmith | tssurya: am now yeah | |
| 17:21:04 | tssurya | and then we can discuss it with mriedem and gibi next week | |
| 17:21:18 | tssurya | dansmith: thanks! | |
| 17:21:29 | dansmith | yup | |
| 17:29:10 | openstackgerrit | Merged openstack/nova master: Optimize member_of check for nested providers https://review.openstack.org/573203 | |
| 17:45:55 | zcorneli | stephenfin: Ack. I'll fix that up. | |
| 18:00:39 | dansmith | zcorneli: bonus points if you fix up the stuff in the devstack patch so we can merge that | |
| 18:00:45 | dansmith | if not, I'll try to get around to it at some point | |
| 18:01:13 | zcorneli | Sure, IIRC it looked pretty simple/quick to fix up the stuff mriedem was looking for. | |
| 18:01:27 | dansmith | yup | |
| 18:01:28 | zcorneli | dansmith: I've also got the discard patch almost ready to review as well. | |
| 18:01:34 | dansmith | zcorneli: excellent | |
| 18:02:05 | zcorneli | dansmith: Going to finish the fix up patch, rebase onto it, then post, I think. | |
| 18:02:14 | zcorneli | Then I can take a crack at the devstack fixup. | |
| 18:02:19 | dansmith | cool | |
| 18:11:09 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: [placement] Add /reshaper handler for POST https://review.openstack.org/576927 | |
| 18:23:02 | openstackgerrit | Merged openstack/nova master: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857 | |
| 18:39:45 | mnaser | so mriedem was working on a patch adding `nova-status upgrade check as part of openstack ansible deployment which is really sweet, but one of the issues we're having is detecting what action we're missing | |
| 18:39:56 | mnaser | is it possible to perhaps have a different exit code depending on which 'issue' | |
| 18:40:13 | mnaser | in this case, upgrading from queens to master (rocky), we need to restart placement | |
| 18:41:15 | mnaser | like maybe some sort of .. cli friendly output so we can run `nova-status upgrade check` once | |
| 18:41:35 | mnaser | then have conditionals depending if it returns something like out_of_date_placement => restart placement, etc | |
| 18:42:21 | dansmith | returning different statuses per check would make it unstable across releases, which is probably undesirable | |
| 18:42:38 | dansmith | how about just some sort of robot-friendly output? | |
| 18:42:57 | mnaser | yeah i was just thinking how complicated that would become at some point | |
| 18:43:10 | mnaser | but robot friendly output, something like one line per 'check' .. only if there is something | |
| 18:43:25 | mnaser | that way, we can run it once, ignore exit code, and then have tasks after it conditional if output matches that string | |