| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 14:51:09 | mriedem | in which case, the api will tell you that with a 400 response | |
| 14:51:10 | dansmith | zioproto: hmm | |
| 14:51:27 | takashin | mriedem: Because we have os-migrations API and there should be a difference between new APIs and old one. | |
| 14:51:43 | lyarwood | mdbooth: thanks, I missed that you had a change up to add locking to detach_volume in the compute layer somehow | |
| 14:51:48 | zioproto | dansmith: Some instances are still missing keypair information. Unable to run keypair migration at this time. | |
| 14:52:19 | dansmith | zioproto: yeah, but the problem is the "error attempting to run ..." one | |
| 14:52:21 | dansmith | zioproto: just a sec | |
| 14:52:24 | mdbooth | lyarwood: Not related to this issue. I should probably abandon that patch as it's more complex than I was going for. | |
| 14:52:30 | mriedem | takashin: so if i understand your point, if i want to list all migrations, regardless of type or status, i should use os-migrations | |
| 14:52:38 | alex_xu | takashin: /servers/{id}/migrations is more easy for user to query migration for specific server | |
| 14:52:53 | mriedem | takashin: but if i want to list migrations for a specific server in order to cancel the migration, i should use /servers/server_id/migrations | |
| 14:52:55 | zioproto | dansmith: I wait no problem :) | |
| 14:53:32 | takashin | mriedem: alex_xu: Yes. In specific state. | |
| 14:53:50 | mriedem | takashin: the thing that worries me about saying just use os-migrations, is the docs have a warning on it https://developer.openstack.org/api-ref/compute/#migrations-os-migrations-frozen | |
| 14:53:54 | alex_xu | takashin: From the ML, operators have use-case to query specific server's migrations. so | |
| 14:53:56 | mriedem | basically saying, don't use this, use the other API | |
| 14:54:02 | lyarwood | mdbooth: is it not? wouldn't locking against instance.uuid for both attach and detach help here? | |
| 14:54:12 | dansmith | zioproto: well, I was looking to see if we had any updates to that migration but I don't think we have | |
| 14:54:19 | mriedem | alex_xu: argubally they could, i.e. GET /os-migrations?instance_uuid=1234 | |
| 14:54:23 | dansmith | zioproto: you can make a code tweak to dump more info since this is a test right? | |
| 14:54:35 | mdbooth | lyarwood: Don't think so. This is racing with cinder internals. | |
| 14:54:43 | imacdonn | lyarwood, mdbooth, my problem is that the iSCSI connection termination is separate from detach | |
| 14:55:28 | mdbooth | imacdonn: If it rescans again before the lun has been reallocated, does the device go away? | |
| 14:55:40 | dansmith | zioproto: can you do this and see what it shows us? https://pastebin.com/3RJdsJE1 | |
| 14:55:44 | lyarwood | imacdonn: it shouldn't be anymore, we've moved everything into the bdm.detach() call | |
| 14:55:57 | alex_xu | mriedem: but more straight forward with /servers/{id}/migrations? but actually we should think about more how the API user write the client code | |
| 14:56:01 | lyarwood | imacdonn: so locking that against instance.uuid should help avoid this now unless I'm missing something | |
| 14:56:03 | mdbooth | lyarwood: terminate connection isn't atomic, right? | |
| 14:56:27 | mdbooth | lyarwood: So on completion of the call to terminate connection, the connection hasn't necessarily been terminated? | |
| 14:56:46 | mdbooth | atomic is the wrong word. Synchronous. | |
| 14:56:48 | lyarwood | mdbooth: yeah good point | |
| 14:56:51 | gibi | mriedem: hi! I'm sorry for missed the nova meeting, today is a pretty crazy day. Thanks for giving the subteam update. | |
| 14:56:51 | mriedem | alex_xu: we actually have CLIs for these in novaclient already | |
| 14:57:20 | mriedem | gibi: np | |
| 14:57:22 | imacdonn | lyarwood, mdbooth, terminate connection is NOT part of bdm.detach, though | |
| 14:57:23 | mdbooth | lyarwood: That's a question, btw. Do we know for sure that is/isn't the case? | |
| 14:57:34 | mriedem | alex_xu: nova migration-list [--host] [--status] | |
| 14:57:49 | mdbooth | imacdonn: If it was synchronous it would at least be a lever we could use to fix the problem. | |
| 14:57:50 | mriedem | alex_xu: we don't allow listing migrations via os-migrations and filtering on instance uuid from the CLI | |
| 14:57:57 | mriedem | we could certainly add that | |
| 14:57:58 | mdbooth | If it's async this could be messy. | |
| 14:58:18 | mriedem | alex_xu: then we also have nova server-migration-list <server> | |
| 14:58:24 | mriedem | which is using /servers/server_id/migrations | |
| 14:58:31 | zioproto | dansmith: https://pastebin.com/n0THHGSG | |
| 14:58:31 | mriedem | from a CLI perspective, it's very confusing | |
| 14:58:34 | imacdonn | mdbooth, you mean like a "callback when it's done" thing ? | |
| 14:58:44 | dansmith | zioproto: interesting | |
| 14:58:55 | mdbooth | I mean: don't return control until it's done | |
| 14:59:06 | zioproto | dansmith: my production database is always interesting :( | |
| 14:59:18 | dansmith | zioproto: no, this is super helpful | |
| 14:59:23 | mdbooth | If nova knew that immediately after terminate connection returned the connection was definitely already be gone, we could use that | |
| 14:59:29 | imacdonn | mdbooth, I believe that's how it is already ... cinderclient waits for the response to an HTTP request | |
| 14:59:31 | lyarwood | imacdonn: sorry, I'm talking about the driver bdm - https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L294-L352 | |
| 14:59:55 | mriedem | alex_xu: takashin: so maybe we just leave /servers/server_id/migrations as doing the server-side in-progress filtering as it does today, and expand the "nova migration-list" CLI to allow filtering on instance uuid too | |
| 14:59:58 | zioproto | dansmith: I have upgrade date on Saturday 29th of April :) Do you think I can find a fix by then ? :) | |
| 15:00:10 | mriedem | and clarify the help text in the api ref docs and CLI help text | |
| 15:00:13 | dansmith | zioproto: yep | |
| 15:00:40 | mriedem | alex_xu: takashin: part of the problem is we just don't document os-migrations very well in the api-ref | |
| 15:00:42 | zioproto | dansmith: I have the feeling everytime I chat with you a new Nova release comes out :D | |
| 15:00:57 | dansmith | zioproto: heh | |
| 15:01:01 | mdbooth | lyarwood kashyap: Any idea what the status of qemu iscsi multipath is? | |
| 15:01:10 | lyarwood | mdbooth: yeah looking at the cinderv2 code it looks synchronous | |
| 15:01:22 | mdbooth | lyarwood: Got a link? | |
| 15:01:26 | zioproto | I am building the ubuntu packages for 14.0.5 ... but I will wait a 14.0.6 if something still needs to be changed for the database migrations | |
| 15:01:38 | lyarwood | mdbooth: AFAIK it's in and we have some config support for it in the libvirt driver but it's not wired up at the moment | |
| 15:01:47 | dansmith | zioproto: so I'm going to give you another patch to try and see if you get farther, but it's not the fix I want, so I'll ask you to test the real fix as well, okay? | |
| 15:01:51 | lyarwood | mdbooth: https://github.com/openstack/cinder/blob/master/cinder/api/contrib/volume_actions.py#L192-L208 | |
| 15:01:56 | mriedem | lyarwood: mdbooth: terminate_connection in cinder is an RPC call to the cinder-volume backend | |
| 15:01:58 | zioproto | dansmith: sure ! | |
| 15:02:04 | mdbooth | lyarwood: Sweet, because the real solution here is to stop doing this and use qemu. | |
| 15:02:09 | mriedem | lyarwood: that's not what you want | |
| 15:02:24 | alex_xu | mriedem: at least that match the requirement from the operator | |
| 15:02:28 | mriedem | lyarwood: https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L749 | |
| 15:02:54 | mriedem | https://github.com/openstack/cinder/blob/master/cinder/volume/rpcapi.py#L217 | |
| 15:03:11 | mriedem | alex_xu: ok so i guess i'll just go with that then, and unblock takashin | |
| 15:03:20 | alex_xu | mriedem: thanks | |
| 15:03:23 | mriedem | i can fix the os-migrations api-ref docs (i've been meaning to do that for awhile now), | |
| 15:03:24 | mdbooth | mriedem: It's a call, which is nice. | |
| 15:03:27 | lyarwood | mriedem: yeah I was just getting there, thanks for the links :) | |
| 15:03:31 | takashin | mriedem: alex_xu: Thank you. | |
| 15:03:36 | mriedem | and add --server as a filter option to nova migration-list | |
| 15:03:52 | mriedem | mdbooth: well normally a block rpc call from the api is not nice | |
| 15:03:55 | mriedem | *blocking | |
| 15:04:09 | dansmith | zioproto: https://pastebin.com/yHueQi4s | |
| 15:05:06 | alex_xu | mriedem: I can help you on those | |
| 15:05:08 | mdbooth | mriedem: Nice for me :) | |
| 15:05:15 | mdbooth | But yeah | |
| 15:05:18 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Microversion 2.42 - Fix tag attribute disappearing https://review.openstack.org/429512 | |
| 15:05:35 | mriedem | sdague: another one if you have time https://review.openstack.org/#/c/429512/ - unblocks novaclient for new microversions in pike | |
| 15:06:45 | zioproto | dansmith: should I revert the previous patch with the 'raise' ? | |
| 15:06:56 | dansmith | zioproto: no | |
| 15:07:10 | dansmith | zioproto: the raise lets us see if I typo'd this one :P | |
| 15:07:33 | zioproto | dansmith: https://pastebin.com/eLsiFn9W | |
| 15:08:08 | kashyap | mdbooth: So, no, upstream QEMU's built-in iSCSI initiator doesn't support multipathing yet | |
| 15:08:29 | kashyap | mdbooth: And I haven't seen any related to it for over 2 years | |
| 15:09:12 | dansmith | zioproto: hmm, that ignoring line should mean we're skipping the thing that gives us the error | |
| 15:09:30 | dansmith | zioproto: I hate to ask, but did you apply with patch or paste it in by hand? | |
| 15:09:46 | mdbooth | kashyap: Sucks :( | |
| 15:09:52 | zioproto | I pasted by hand ! | |
| 15:10:04 | dansmith | zioproto: any chance you missed the 'continue' line under the log? | |