| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-25 | |||
| 15:11:28 | mriedem | kashyap: that's why i said, "meh, it'll eventually be fixed upstream so who cares" isn't a good response to a user | |
| 15:11:29 | kashyap | B is the "active layer", where A is the backing file of B. | |
| 15:11:51 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Bumping functional test job timeouts https://review.openstack.org/537933 | |
| 15:12:02 | gibi | mriedem: ^^ | |
| 15:12:17 | mriedem | mdbooth: we don't currently block swap volume with a multiattach volume | |
| 15:12:21 | mriedem | on the nova side | |
| 15:13:12 | mriedem | i believe on the cinder side, you can't retype a multiattach volume that is attached to an instance | |
| 15:13:12 | mdbooth | mriedem: Cool, so we just prevent it? I didn't look specifically last time I reviewed that patch. | |
| 15:13:14 | mriedem | jgriffith: right ^ ? | |
| 15:13:27 | mriedem | mdbooth: i said we don't bloc kit | |
| 15:13:29 | mriedem | meaning we allow it | |
| 15:14:06 | mdbooth | mriedem: Ah... | |
| 15:14:08 | mriedem | mdbooth: if the volume's secondary attachments are only read-only, it shouldn't be an issue right? | |
| 15:14:36 | mdbooth | mriedem: Well that's going to get a bit weird, but it won't cause data corruption. | |
| 15:15:13 | mdbooth | Although presumably they're also going to have to swap volume, because the old one goes away, right? | |
| 15:15:58 | mriedem | not parsing that | |
| 15:16:05 | mriedem | see L50 here for what i'm talking about https://etherpad.openstack.org/p/nova-ptg-rocky | |
| 15:18:07 | mdbooth | The way swap volume works is we create a local COW for the volume, which means we're no longer making changes to it. We (Nova) then copies all the data from the source to the dest. During this copy, all writes are written to the COW, which is local to a compute. When the copy is complete, we commit all the changes in the COW to the new copy, then flip to the new copy. | |
| 15:18:14 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs object https://review.openstack.org/489408 | |
| 15:18:16 | mriedem | https://specs.openstack.org/openstack/cinder-specs/specs/queens/enable-multiattach.html | |
| 15:18:21 | mriedem | "Again NOTE we will NOT allow retype of multiattachment setting for an in-use volume." | |
| 15:18:48 | mdbooth | mriedem: Right. If I'm understanding you correctly we missed that? | |
| 15:19:01 | mriedem | it's supposed to be handled on the cinder side | |
| 15:19:11 | mriedem | i don't know if that includes volume migration | |
| 15:19:16 | mriedem | where it's not a retype | |
| 15:19:21 | mriedem | jgriffith: smcginnis: ildikov: ^ | |
| 15:19:23 | jgriffith | mriedem: it's set in the policy | |
| 15:19:45 | mriedem | jgriffith: i thought we said you simply couldn't retype a multiattach in-use volume, regardless of policy | |
| 15:19:48 | jgriffith | mriedem: You can if policy permits, default policy being admin/owner | |
| 15:20:06 | jgriffith | mriedem: Oh | |
| 15:20:11 | jgriffith | yes you're correct | |
| 15:20:16 | mriedem | ok cool | |
| 15:20:19 | jgriffith | sorry, I was confusing the bootable policy | |
| 15:20:27 | mriedem | jgriffith: and does that apply to both retype and migration? | |
| 15:20:29 | jgriffith | You're correct, in-use should just return and error immediately | |
| 15:20:41 | giodeco | When trying to create a new instance on my Newton Openstack env, I have an error in /var/log/nova-conductor.api of type "Corrupt image download. Checksum was d41d8cd98f00b204e9800998ecf8427e expected f8ab98ff5e73ebab884d80c9dc9c7290". I believe it is something related to network or rabbitmq. Any suggestion? | |
| 15:21:02 | mriedem | jackie-truong: ^ | |
| 15:21:24 | jgriffith | mriedem: that's a great question :) | |
| 15:21:57 | mriedem | jgriffith: heh :) basically anything that triggers a call to nova's swap volume api | |
| 15:22:08 | jackie-truong | mriedem: which question are you pointing me to? | |
| 15:22:14 | mriedem | jackie-truong: the image checksum one | |
| 15:22:29 | mriedem | sounds like a john hopkins image signature validation question | |
| 15:23:12 | jackie-truong | it does | |
| 15:23:17 | mdbooth | mriedem: Bizarrely, we've also had users call it directly. However, I don't think that would cause a data corruption issue specifically. | |
| 15:23:19 | jackie-truong | lemme look | |
| 15:24:08 | mriedem | mdbooth: it's an admin-only api in nova by default | |
| 15:24:09 | jgriffith | mriedem: so currently anything that calls retype on a multiattach that's in use is going to return an exception | |
| 15:24:13 | mriedem | so normal end users shouldn't be able to do that | |
| 15:24:21 | jgriffith | mriedem: regardless of *how* it tries to do it | |
| 15:24:35 | mriedem | jgriffith: does volume migration go through the retype flow even if it's not a retype? | |
| 15:24:42 | jgriffith | mriedem: nope | |
| 15:24:43 | mriedem | sort of like how we call cold migration a resize in nova | |
| 15:24:59 | jgriffith | I don't know that swap ever goes through a cidner retype? | |
| 15:25:24 | jgriffith | remember it's cinder's retype with migration that calls swap and get us in that weird case | |
| 15:26:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: DNM: test if tempest-full is triggered or not https://review.openstack.org/537936 | |
| 15:26:32 | mriedem | jgriffith: looks like nova's swap volume is called from VolumeManager._migrate_volume_generic | |
| 15:26:37 | mdbooth | mriedem: Anyway, I only brought that up to point out that it's not without precedent. Ideally it would work, but given that it has never worked I think it's a simple enough message to users that: 'This doesn't work'. In the meantime we take the reduced complexity in Nova and wait for the upstream fix. | |
| 15:26:46 | mriedem | called from migrate_volume | |
| 15:26:59 | jgriffith | mriedem: right, the live-migration of volumes is the trigger | |
| 15:27:13 | jgriffith | either from the Cinder side or the Nova side; but not the swap call itself | |
| 15:28:13 | jgriffith | mriedem: what I was getting at is that the swap process itself takes two volumes so that's not an issue; migrate is another story | |
| 15:29:28 | mriedem | yeah ok i found in the volume manager code where retype calls migrate_volume | |
| 15:29:36 | mriedem | and then there is just the migrate_volume called directly from the api | |
| 15:29:56 | jgriffith | mriedem: my point is that I don't think there is ever a case where Nova calls cinders retype is there? | |
| 15:30:02 | mriedem | no | |
| 15:30:06 | mdbooth | No | |
| 15:30:07 | mriedem | i'm only worried about the case that cinder calls nova | |
| 15:30:11 | mriedem | to swap with a multiattach volume | |
| 15:30:35 | jgriffith | mriedem: yeah, but in that case Cinder should've already completed (or failed) to do the work of creating the destination volume etc | |
| 15:30:39 | mriedem | and this is the retype check in the API https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L1647 | |
| 15:30:41 | jgriffith | so we should be *ok* | |
| 15:31:23 | mriedem | jgriffith: isn't ^ saying, that if i'm retyping from a multiattach=True to a multiattach=True type, we don't care if it's attached or not? | |
| 15:31:26 | jgriffith | mriedem: right, no options there; it just won't let you do it with in-use | |
| 15:31:41 | mriedem | well, the in-use check is only if the src/dst multiattach values are changing | |
| 15:32:08 | mriedem | s/changing/different/ | |
| 15:32:14 | jgriffith | mriedem: yes | |
| 15:32:30 | jgriffith | mriedem: yes to the if the multiattach setting is the same we don't care | |
| 15:32:39 | jgriffith | then we behave like any other retype command | |
| 15:33:07 | mriedem | ok so we can call swap_volume in nova for a retype/migrate on a multiattach volume | |
| 15:33:09 | mriedem | mdbooth: ^ | |
| 15:33:33 | mriedem | we == cinder | |
| 15:33:55 | SpazZz | hehe when yo uassociate too closely to your code | |
| 15:34:04 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/537897 | |
| 15:34:11 | jgriffith | hehe | |
| 15:34:19 | mdbooth | mriedem: Yeah, that would definitely cause a problem. | |
| 15:34:23 | mdbooth | SpazZz: hehe | |
| 15:35:03 | mdbooth | I think you're right though, that cinder is probably best placed to handle that. | |
| 15:36:26 | mriedem | well i'll try to write up a cinder api-ref docs patch | |
| 15:36:29 | mriedem | for all the things | |
| 15:37:54 | mdbooth | jgriffith: Incidentally, this might be handled by context, but at first glance it looks like you've got a race there even if you fix the multiattach check. | |
| 15:38:59 | mdbooth | We're checking that volume.status is available, and if it is then we're proceeding. However, I don't see any assertion that volume.status continues to be available. | |
| 15:39:22 | mdbooth | In Nova we typically do this by atomically changing task state. | |
| 15:40:09 | mriedem | mdbooth: they do that here https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L1699 | |
| 15:40:37 | jgriffith | mdbooth: We have a conditional on the volume object IIRC | |
| 15:40:37 | mdbooth | So we'd do: if instance.status == 'all good': atomic_update(instance.task_state = 'doing my thang', expected task_state = 'whatever') | |
| 15:41:11 | jgriffith | mdbooth: after that yeah we don't lock or anything if that's what you mean | |
| 15:41:26 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204 | |
| 15:41:26 | openstackgerrit | Jackie Truong proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949 | |
| 15:41:53 | SpazZz | IBM CI looks like it's cleaning up.. can hopefully get the my +1 back and look sexier for the Core Reviewers | |
| 15:42:03 | SpazZz | Walk by and be like, "girlll look at all these +1's over here.." | |