| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 15:13:26 | mriedem | efried: one new question from me inline https://review.openstack.org/#/c/438598/12 | |
| 15:13:26 | zioproto | also the instances have been deleted I guess | |
| 15:13:37 | efried | mriedem Ack, looking... | |
| 15:13:39 | openstackgerrit | Steve Noyes proposed openstack/nova master: Update detach to use V3 Cinder API https://review.openstack.org/438750 | |
| 15:13:41 | zioproto | dansmith: I think we are talking about the instances that are marked as deleted | |
| 15:14:10 | zioproto | dansmith: are you going to submit this patch for review in master ? | |
| 15:14:17 | dansmith | zioproto: no, let me get you an example | |
| 15:14:29 | mriedem | efried: same question about exception handling in power_off | |
| 15:14:37 | mriedem | we try to avoid handling Exception if possible | |
| 15:14:39 | dansmith | zioproto: 2017-04-20 15:11:10.584 787 WARNING nova.objects.instance [req-d1cbe1e7-1f03-46b6-9009-bba793eb73ba - - - - -] Instance b750d102-4fbb-4401-80b2-cc36888c9749 keypair c_rally_5aabcf4c_Xg4N7es6 not found | |
| 15:14:49 | efried | mriedem Roger that. Responding. | |
| 15:14:50 | dansmith | zioproto: yeah, we'll fix in master and backport | |
| 15:15:01 | sdague | mriedem: so, given that powervm driver is pretty self contained, is there particular primary concerns before merging? | |
| 15:15:06 | bauzas | oh man, /me needs to get drugs in order to think about https://review.openstack.org/#/c/437424/ | |
| 15:15:16 | bauzas | let's prepare a new battle | |
| 15:15:27 | zioproto | dansmith: ok, please give me the review number, I will cherry-pick this patch in my deployment or I cant upgrade next week | |
| 15:15:36 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for resource providers https://review.openstack.org/457532 | |
| 15:15:37 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for inventories https://review.openstack.org/457533 | |
| 15:15:37 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for allocations https://review.openstack.org/457534 | |
| 15:15:38 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: CLI for usages https://review.openstack.org/457535 | |
| 15:15:38 | openstackgerrit | Roman Podoliaka proposed openstack/osc-placement master: tests: add a hook for functional testing in the gate https://review.openstack.org/452122 | |
| 15:15:46 | dansmith | zioproto: yeah, but we need to get the problem fully fixed first :) | |
| 15:15:51 | mriedem | sdague: i had questions in the 2nd patch | |
| 15:16:23 | zioproto | I am available to test more patches :) | |
| 15:16:37 | zioproto | dansmith: should I open a bug on LP ? | |
| 15:16:49 | dansmith | zioproto: yep, working on a new patch | |
| 15:17:09 | dansmith | zioproto: yeah, please do file a bug, we'll need it for the review | |
| 15:17:56 | efried | sdague Not sure if you've been aware, but FYI we have a CI running (a subset of) tempest on the in-tree patches, and publishing to gerrit. | |
| 15:18:12 | sdague | efried: while you are changing things, I think you misunderstand LOG.exception(e) :) | |
| 15:18:22 | efried | sdague Entirely possible ;-) | |
| 15:18:28 | efried | Please enlighten me. | |
| 15:19:00 | imacdonn | mdbooth, you asked earlier if a rescan would make the device(s) go away .. if I'm interpreting your question properly, the answer is "no" ... rescan picks up new LUNs, but doesn't do anything about ones that have become unresponsive | |
| 15:19:12 | efried | Thought it was LOG.error + stack trace | |
| 15:19:18 | sdague | efried: right | |
| 15:19:22 | mdbooth | imacdonn: Ok, thanks | |
| 15:19:49 | sdague | efried: comment left | |
| 15:19:54 | efried | sdague Thanks. | |
| 15:20:52 | lyarwood | mdbooth / imacdonn ; https://bugzilla.redhat.com/show_bug.cgi?id=1367805 is a dup of this that I could never reproduce correctly FWIW | |
| 15:20:52 | openstack | bugzilla.redhat.com bug 1367805 in openstack-nova "Previously removed iSCSI multipath volumes can be rediscovered by future disconnect_volume calls before being unmapped on the backend by os-terminate_connection." [High,Closed: insufficient_data] - Assigned to lyarwood | |
| 15:21:06 | mdbooth | imacdonn lyarwood: So what I think we need to achieve here is that after calling disconnect nothing is subsequently allowed to connect until the associated terminate connection has defintely resulted in the device having been killed by cinder | |
| 15:21:22 | imacdonn | lyarwood, \o/ I'm not alone ;) | |
| 15:22:00 | openstackgerrit | STEW TY proposed openstack/nova master: Transform instance.unrescue notifications https://review.openstack.org/388275 | |
| 15:22:21 | lyarwood | mdbooth: yes and the locking in your change would do that now | |
| 15:22:33 | imacdonn | mdbooth, right .. and I've sortof hacked that by putting a lock on things that cover that window .. like _shutdown_instance() in compute.manager | |
| 15:23:04 | imacdonn | mdbooth, and the same lock on connect_volume() in virt.libvirt.volume.iscsi | |
| 15:23:10 | efried | sdague So I don't need to pass the exception itself to LOG.exception - it picks it up automatically? | |
| 15:23:25 | mriedem | efried: yeah | |
| 15:23:31 | efried | Beautiful, thanks. | |
| 15:23:33 | mriedem | LOG.exception() just takes a message if you want to say something | |
| 15:23:38 | mriedem | and it dumps the current stacktrace | |
| 15:23:56 | mdbooth | lyarwood: Assuming we can verify the sync behaviour of terminate connection all the way down | |
| 15:24:07 | sdague | yeh, it's probably worth logging the instance there, I put a suggested string in a comment right now | |
| 15:24:41 | imacdonn | mdbooth, lyarwood AFAIK, it's sync as far as nova-compute is concerned - it's making an API call to cinder ... it's an RPC call within cinder, but nova-compute doesn't know about that | |
| 15:25:24 | zioproto | dansmith: https://bugs.launchpad.net/nova/+bug/1684861 | |
| 15:25:24 | openstack | Launchpad bug 1684861 in OpenStack Compute (nova) "Database migrations from mitaka to newton fail due to missing keypairs" [Undecided,New] | |
| 15:25:48 | zioproto | wait the title is wrong, I just realized | |
| 15:25:50 | dansmith | zioproto: thanks, still working on the fix if you can hang on for a bit | |
| 15:26:15 | zioproto | https://bugs.launchpad.net/nova/+bug/1684861 | |
| 15:26:21 | zioproto | I updated the title | |
| 15:26:32 | imacdonn | mdbooth, lyarwood If those calls were not sync, we'd have other problems .. like we'd ask cinder to connect a volume, then do the rescan "too early" | |
| 15:26:36 | zioproto | if you link the fix to the bug I will track it | |
| 15:27:03 | mdbooth | imacdonn: I've learned not to assume these things :) | |
| 15:27:08 | imacdonn | hah ;) | |
| 15:27:11 | dansmith | zioproto: it looks to me like you have something that shouldn't be allowed with the foreign key constraint | |
| 15:27:24 | dansmith | zioproto: have you disabled those constraints on your database by chance? | |
| 15:27:50 | zioproto | how can I check ? | |
| 15:27:59 | zioproto | it is possible something happened in this database since Icehouse | |
| 15:28:41 | zioproto | is possible we messed up something in the upgrade Juno to Kilo if I remember correctly | |
| 15:28:45 | dansmith | zioproto: well, can you check that you have something in instance_extra with instance_uuid=e7eb4196-994e-4968-bcf0-aa6f783bbb18, but nothing in instances.uuid for that? | |
| 15:29:10 | dansmith | zioproto: are you using a hand-rolled db archiving script or something? | |
| 15:29:21 | dansmith | zioproto: something like this maybe? https://tecadmin.net/temporarily-disable-foreign-key-checks-in-mysql/ | |
| 15:31:09 | zioproto | dansmith: yes I have data in instance_extra but not in instances | |
| 15:31:15 | zioproto | I know what happened | |
| 15:31:19 | dansmith | zioproto: that shouldn't be possible | |
| 15:31:35 | zioproto | we were upgrading to Kilo to Liberty | |
| 15:31:47 | zioproto | and when doing the prod update we had a problem with the database migration | |
| 15:31:56 | zioproto | and we had to do some fast operations on the database | |
| 15:32:04 | zioproto | it is possible that we deleted some very old instances | |
| 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 | |