| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-20 | |||
| 14:44:26 | mdbooth | imacdonn: Fun | |
| 14:44:28 | mriedem | because add_link defaults to False | |
| 14:44:35 | alex_xu | mriedem: that works for me. I guess initialy we think there is only one migration, and we didn't want to trouble with user to find out which migration is in-progress. | |
| 14:44:37 | imacdonn | mdbooth :) indeed | |
| 14:45:17 | alex_xu | with filter, we can reach the same goal | |
| 14:45:21 | mriedem | at microversion >= 2.23 with os-migrations, we provide a link and migration_type in the response for an in-progress live migration, otherwise just the migration_type | |
| 14:45:35 | mriedem | alex_xu: note that os-migrations also takes a filter parameter | |
| 14:45:40 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/migrations.py#L77 | |
| 14:45:47 | alex_xu | so i'm ok with that. and the user have one more way to query all the migraitons for a specific servers, so nothing bad at here | |
| 14:45:50 | mriedem | it's not validated, but it's there | |
| 14:46:21 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Canary test against multi-cell devstack https://review.openstack.org/458537 | |
| 14:46:42 | mdbooth | imacdonn: Let me think about that for a bit. lyarwood may also be interested in https://bugs.launchpad.net/nova/+bug/1683972 | |
| 14:46:42 | openstack | Launchpad bug 1662483 in OpenStack Compute (nova) "duplicate for #1683972 detach_volume races with delete" [Undecided,In progress] - Assigned to Matthew Booth (mbooth-9) | |
| 14:47:22 | alex_xu | takashin: what would you think? | |
| 14:47:43 | mriedem | alex_xu: this is how os-migrations gets the migrations from the db + filters https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4789 | |
| 14:47:47 | imacdonn | mdbooth, OK, will check that one out too. I think my problem is a bit more extensive than 1683972, but maybe we can find a solution that covers both | |
| 14:48:00 | mdbooth | imacdonn: It's the same bug, just repeating for lyarwood | |
| 14:48:16 | mdbooth | I just unlinked the bug it was marked as a dup of, btw | |
| 14:48:17 | mriedem | alex_xu: so technically you could get all migration types via os-migrations v2.1 and filter on instance_uuid | |
| 14:48:28 | imacdonn | mdbooth, oh, duh .. still working on my first coffee :) | |
| 14:48:32 | traskat | efried, are there other options then changing the flavor. i tried now a reboot but that didn't work either. the flavor i use for the rebooted instance has the extra specs before. | |
| 14:48:34 | takashin | alex_xu: So I would like to make new APIs to show in-progress migrations only. | |
| 14:48:42 | mdbooth | I don't think they're the same, although they're cousins | |
| 14:48:48 | imacdonn | mdbooth, agreed | |
| 14:49:51 | zioproto | dansmith: I upgraded to 14.0.5 but there are still some strange different messages https://pastebin.com/vLNWdiRZ | |
| 14:49:56 | mriedem | takashin: why would you rather filter all migrations and return only in-progress migrations without any option for the user to choose the filtering? | |
| 14:50:05 | zioproto | looks like a problem with the keypairs | |
| 14:50:08 | efried | traskat Sorry, I'm not familiar enough with this to be of help. The resize was just an off-the-cuff idea. But you probably want to take this discussion to the #openstack channel (see channel topic). | |
| 14:50:38 | traskat | ok thanks | |
| 14:50:51 | mriedem | takashin: that seems inconsistent behavior with the rest of our apis, which if you list resources, you get undeleted resources in their current state, | |
| 14:51:02 | mriedem | you can take actions on those resources, and those actions may be invalid for the state that the resource is in, | |
| 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 | mriedem | alex_xu: we actually have CLIs for these in novaclient already | |
| 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: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 | mriedem | from a CLI perspective, it's very confusing | |
| 14:58:31 | zioproto | dansmith: https://pastebin.com/n0THHGSG | |
| 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 ! | |