| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 15:32:09 | sdague | efried: is there a reason why this goes fatal - https://review.openstack.org/#/c/391288/51/nova/virt/powervm/vm.py@405 | |
| 15:32:14 | zioproto | and we disabled the foreigh checks to do that | |
| 15:32:22 | sdague | because I don't see any higher level error recovery for that exception | |
| 15:32:34 | dansmith | zioproto: ah, okay that would explain it | |
| 15:33:06 | zioproto | dansmith: we probably dropped a lot of stuff from table instances to make a kilo to liberty database migration go through, while we were in maintanance window | |
| 15:33:14 | zioproto | that is why now we improved and we do these migrations before | |
| 15:33:47 | zioproto | dansmith: so this means we behaved bad and we dont get a patch that fixes the problem ?? :| | |
| 15:33:51 | efried | sdague If we got there, somebody passed an extra spec with a 'powervm:' prefix, but that we don't recognize. | |
| 15:34:03 | efried | We definitely want to fail the deploy, nah? | |
| 15:34:14 | dansmith | zioproto: okay, I'm trying to decide if we should try to handle that possibility in the mainstream code or not.. because technically you broke the data model :) | |
| 15:35:14 | dansmith | zioproto: I'm leaning towards catching the problem, logging it, but making you do your own cleanup to fix it | |
| 15:35:30 | dansmith | zioproto: which would just mean deleting the orphaned instance_extra records | |
| 15:35:47 | dansmith | mriedem: you following this at all? | |
| 15:35:49 | efried | sdague I *suppose* an argument could be made for ignoring, so if we add a new spec in a later release, you can still use a flavor containing it on an older compute node... | |
| 15:35:54 | efried | thorst thoughts? | |
| 15:36:11 | thorst | efried: I just logged a comment. I think its sufficient to just log a warning saying 'Unknown' or something | |
| 15:36:14 | sdague | efried: maybe? though by throwing an attribute error, it's not super clear to me where things go into sane recovery to the user to explain to them what went wrong | |
| 15:36:17 | thorst | but I agree...full stop, meh | |
| 15:36:20 | zioproto | dansmith: of course all other operators that asked my how I upgraded from kilo to liberty did the same. | |
| 15:36:42 | thorst | warning tells them that something came in you didn't understand. | |
| 15:36:45 | dansmith | zioproto: yeah, I'm sure you're not the only one | |
| 15:36:52 | sdague | efried: I guess the real question is who can solve the problem | |
| 15:36:56 | efried | thorst ...IF they look at the log. | |
| 15:36:58 | mriedem | dansmith: not a word | |
| 15:37:15 | sdague | is it someone that can change their API request? in which case tell them over the API channel how to fix it | |
| 15:37:32 | sdague | but if only admins can fix it, then log warning seems better | |
| 15:37:53 | dansmith | mriedem: so, zioproto disabled FK constraints in kilo, deleted a bunch of instances, and then re-enabled them, which orphaned some instance extra records, which now don't behave properly since they assume they have a FK to instances | |
| 15:37:53 | efried | sdague thorst I guess if they misspell a keyword, they'll not get the corresponding behavior. Which they may not notice for... a while. And once they do, they'll have to look at the log to figure out why that happened. | |
| 15:38:04 | dansmith | mriedem: which is why their migration is failing | |
| 15:38:16 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: add tags field to instance.update notification https://review.openstack.org/407228 | |
| 15:38:35 | dansmith | mriedem: handling it is a little tricky, so I'm leaning towards catching the error, logging it and the uuid so they can manually clean the affected instance extra records, but not actually try to magically handle this, | |
| 15:38:49 | dansmith | mriedem: in case it's actually for some other reason | |
| 15:38:58 | mriedem | why were the FK constraints disabled? | |
| 15:39:10 | dansmith | mriedem: especially since the way you get here is breaking the model | |
| 15:39:33 | dansmith | mriedem: dunno, they probably had stale/corrupt data and wanted to delete some broken instances, and didn't realize they should have deleted the things depending on the instance records instead of disabling FKs | |
| 15:39:46 | mdbooth | lyarwood imacdonn: So this call isn't synchronous unfortunately, or at least it's driver dependent and for zfssa iscsi it's not sychronous | |
| 15:40:05 | mriedem | dansmith: ummm | |
| 15:40:20 | mriedem | it seems weird to put something into code for a thing you shouldn't have done in the first place, | |
| 15:40:27 | openstackgerrit | Pavel Gluschak (scsnow) proposed openstack/nova master: VStorage: changed default log path https://review.openstack.org/458557 | |
| 15:40:34 | mriedem | and if you did, then you (as the person that did it) patch the fix on your end | |
| 15:40:35 | dansmith | mriedem: well, right, so that's why I just want to log/explain and be graceful | |
| 15:40:42 | imacdonn | mdbooth lyarwood: where do you see that ? | |
| 15:40:43 | mriedem | even that is odd | |
| 15:40:52 | mdbooth | The cinder api makes a synchronous rest call to c-vol, which synchronously makes a rest call to zfssa | |
| 15:40:54 | dansmith | mriedem: not fix the problem, but just warn about the affected uuid(s) | |
| 15:40:56 | mriedem | "someone reported they did this odd thing they shouldn't have, so we're going to handle and log it for them" | |
| 15:41:04 | mdbooth | However, the rest call explicitly isn't synchronous | |
| 15:41:05 | zioproto | dansmith: I have the proof https://bugs.launchpad.net/nova/+bug/1511466/comments/9 | |
| 15:41:05 | openstack | Launchpad bug 1511466 in OpenStack Compute (nova) "migrate flavor data impossible" [Medium,Confirmed] | |
| 15:41:08 | mriedem | dansmith: i know, i'm saying it's weird | |
| 15:41:10 | dansmith | mriedem: right now they just get "error, failed, kthx" | |
| 15:41:17 | zioproto | dansmith: somebody wrote on LP to do that, and I did it ! | |
| 15:41:28 | mdbooth | The driver waits for the return, but the return it's looking for is ACCEPTED | |
| 15:41:31 | dansmith | zioproto: do you do everything random people tell you to do? nevermind, don't answer that :P | |
| 15:41:47 | mdbooth | Which doesn't indicate the requested change has happened yet, only that it has been requested | |
| 15:41:51 | mriedem | dansmith: so are people doing this because of some latent bug that forced them to do it, but we have since fixed? | |
| 15:41:56 | dansmith | mriedem: well, yeah, but especially if it was advice on a LP bug that multiple people followed, :/ | |
| 15:42:00 | imacdonn | mdbooth lyarwood OK, yeah, but doesn't cinder-api wait for the response from cinder-volume before responding to the (cinder) API request? | |
| 15:42:23 | dansmith | mriedem: it doesn't matter, disabling FKs isn't the right workaround for a problem anyway, it's deleting extra, then deleting instance, etc | |
| 15:42:28 | mdbooth | It does, yes, but that response only indicates that zfssa has accepted the request | |
| 15:42:32 | mriedem | dansmith: i agree | |
| 15:42:42 | mdbooth | It doesn't indicate that the request has been processed | |
| 15:42:56 | mriedem | which is why it's odd to put something into code that is for "people worked around this incorrectly, but we're going to patch something in for them" | |
| 15:42:58 | dansmith | mriedem: but regardless, it's out there so I at least want to log/warn and not just fail quietly with no indication about why | |
| 15:43:11 | imacdonn | mdbooth lyarwood: The request to the ZFSSA is another REST API .. it also waits for the operation to complete before returning | |
| 15:43:24 | mriedem | dansmith: where does this handle/log thing go? a migration? an object? | |
| 15:43:28 | dansmith | mriedem: well, I wasn't going to be that nice, I was going to say "Instance Extra (uuid) has no Instance, unable to migrate" | |
| 15:43:32 | mdbooth | imacdonn: It waits for the rest call to return an http response | |
| 15:43:35 | imacdonn | mdbooth lyarwood I know this quite well, because it times out sometimes ;) | |
| 15:43:43 | dansmith | mriedem: in the online migration function that nova-manage is calling | |
| 15:43:50 | mdbooth | But the http resonse doesn't (according to the docs) indicate that it has completed | |
| 15:44:06 | mriedem | dansmith: as you like | |
| 15:44:10 | mriedem | i won't block it | |
| 15:44:17 | dansmith | mriedem: well, I'll put it up and you can decide then | |
| 15:44:33 | mriedem | geddy lee told me that if i choose not to decide i've still made a choice | |
| 15:44:35 | Tahvok | I'm installing nova on ubuntu, and according to the guide I need to create the cell1 cell. In the command they say I need to run it with some sort of id? 109e1d4b-536a-40d0-83c6-5f121b82b650 | |
| 15:44:37 | imacdonn | mdbooth lyarwood empirically, I can say that it does wait for the operation to complete .... how else would the status be returned ?? | |
| 15:44:45 | Tahvok | What is that id? https://docs.openstack.org/ocata/install-guide-ubuntu/nova-controller-install.html | |
| 15:44:48 | dansmith | mriedem: heh | |
| 15:44:51 | mdbooth | imacdonn: It returns 202 ACCEPTED | |
| 15:45:13 | mriedem | Tahvok: which command? nova-manage cell_v2 create_cell? | |
| 15:45:22 | Tahvok | mriedem: yes | |
| 15:45:28 | mriedem | su -s /bin/sh -c "nova-manage cell_v2 create_cell --name=cell1 --verbose" nova | |
| 15:45:36 | mriedem | that doesn't say anything about providing an id | |
| 15:45:39 | Tahvok | Oh, it's the output | |
| 15:45:41 | mriedem | it outputs an id | |
| 15:45:42 | mriedem | yes | |
| 15:45:50 | Tahvok | Yes, I thought it's part of the command | |
| 15:45:55 | Tahvok | mriedem: thanks! | |
| 15:46:16 | imacdonn | mdbooth: the ZFSSA REST API call ?? | |
| 15:46:18 | mriedem | Tahvok: also https://docs.openstack.org/developer/nova/man/nova-manage.html#nova-cells-v2 in case you need more info on the commands | |
| 15:46:21 | lyarwood | mdbooth: sorry back, isn't this a bug within this and any other driver that doesn't terminate_connection synchronously then? | |
| 15:46:23 | mdbooth | imacdonn: Yeah | |
| 15:46:38 | lyarwood | mdbooth: I skimmed over a few target drivers and they appeared to be sync | |
| 15:46:39 | mdbooth | lyarwood: Right, we're discussing zfssa, but other drivers will be different | |
| 15:46:54 | Tahvok | mriedem: is that create_cell command idempotent? | |
| 15:47:10 | imacdonn | mdbooth: that doesn't sound right ... looking for a debug log to verify | |
| 15:47:10 | Tahvok | Could I run it again, or it will try creating another cell? | |
| 15:47:12 | lyarwood | mdbooth: yeah, I think there's still value to adding the lock on our side if we also follow up with cinder to get all drivers in line there | |
| 15:48:02 | zioproto | dansmith: the bug is still open https://bugs.launchpad.net/nova/+bug/1511466 SOOO I guess any operator with a big deployment that managed to upgrade to liberty probably used this workaround. This means that soon this problem will explode :D | |