| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-27 | |||
| 14:47:05 | dansmith | so, that tells me that a single non-fancy request to placement should return a split allocation | |
| 14:47:07 | mriedem | dansmith: we should know shortly from this ceph patch i have | |
| 14:47:32 | cdent | If we need a specific functional test for something, I'm semi idle right now, so could make something if someone tells me what it needs to be | |
| 14:47:33 | dansmith | and the scheduler is doing a non-fancy request, so it should be getting back a split allocation I guess | |
| 14:47:40 | melwitt | mriedem: in https://review.openstack.org/586568 is that taking care of the live migration rollback scenario? or is that still an open question | |
| 14:47:53 | dansmith | cdent: well, I've tried writing a very hacky one and placement is returning no allocation requests | |
| 14:47:53 | mriedem | melwitt: i looked at rollback and didn't see anything that needed this type of thing | |
| 14:47:59 | melwitt | mriedem: ack | |
| 14:48:13 | cdent | dansmith: do you want to push it up and I'll tune it and you can go review something? | |
| 14:48:29 | mriedem | melwitt: i'd say if we ever go the generic route in _post_live_migration, we'd want to do the same in _rollback_live_migration | |
| 14:48:48 | mriedem | rollback is likely less of an issue b/c if we failed live migration, we won't activate the dest host port bindings and get into this mess | |
| 14:49:21 | [fcandido] | http://eavesdrop.openstack.org/meetings | |
| 14:49:36 | melwitt | ack | |
| 14:49:45 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: funtional test with sharing providers https://review.openstack.org/586589 | |
| 14:49:51 | dansmith | cdent: ^ | |
| 14:49:52 | cdent | on it | |
| 14:50:00 | dansmith | cdent: warning, it's very, uh, forced | |
| 14:50:10 | cdent | ha, noted | |
| 14:50:42 | fried_rice | dansmith/superdan: I haven't caught up on the whole conversation, but you're asking about a compute node that's marked as a sharing provider? | |
| 14:50:45 | dansmith | cdent: attempts to create a provider with disk, associate with the compute host providers, nuke the disk inventory from one and then try to boot and see if we got the shared bit | |
| 14:51:04 | cdent | ✔ | |
| 14:51:07 | dansmith | fried_rice: no, not a compute node marked as sharing, just a compute with no disk because it's associated to a shared disk provider | |
| 14:52:16 | mriedem | dansmith: why not write a simple fake virt driver that doesn't report DISK_GB inventory? | |
| 14:52:27 | dansmith | mriedem: because this was quick | |
| 14:52:40 | dansmith | mriedem: obviously not mergeable | |
| 14:54:16 | mriedem | your max_unit is wrong | |
| 14:54:21 | melwitt | argh, gate bug fix just failed merge for POST_FAILURE | |
| 14:54:22 | mriedem | your sharing provider has 1gb | |
| 14:55:33 | mriedem | unless flavor1 doesn't have any root_gb | |
| 14:56:20 | dansmith | it has 1024 GB | |
| 14:56:46 | dansmith | oh max_unit | |
| 14:56:46 | mriedem | but max you can request in a chunk is 1 right? | |
| 14:57:02 | cdent | i'll mess with it | |
| 14:57:07 | mriedem | max_unit should equal total | |
| 14:57:23 | dansmith | still no dice | |
| 14:57:51 | dansmith | er, hmm it didn't update | |
| 14:58:31 | dansmith | ah, I'm setting inventory twice for some reason | |
| 14:58:42 | mriedem | yeah | |
| 14:58:42 | sean-k-mooney | melwitt: the live migrate one? | |
| 14:58:56 | mriedem | you might be using a 1 root_gb flavor anyway | |
| 14:59:02 | mriedem | so the max_unit being 1 might not make a difference | |
| 14:59:03 | dansmith | I was, and still no difference | |
| 14:59:05 | dansmith | yeah | |
| 14:59:12 | melwitt | sean-k-mooney: yeah | |
| 14:59:14 | fried_rice | dansmith/superdan: Okay, you're trying to make a setup that has its disk allocated from a sharing provider, not the compute node. And then what, migrate it? | |
| 14:59:28 | mriedem | boot and then migrate | |
| 14:59:37 | mriedem | but boot fails? | |
| 14:59:40 | dansmith | fried_rice: well, boot first would be nice | |
| 15:00:03 | fried_rice | bhagyashri got that working live and in a func test with the libvirt driver. | |
| 15:00:12 | fried_rice | Have you located that func test yet? | |
| 15:00:12 | dansmith | fried_rice: I believe migrate will mangle the allocations, but trying to prove it | |
| 15:00:21 | dansmith | nope | |
| 15:00:22 | fried_rice | dansmith: I suspect you may be right. | |
| 15:00:25 | fried_rice | okay, stand by... | |
| 15:00:59 | mriedem | fried_rice: that libvirt func test doesn't go through the scheduler though right? | |
| 15:01:03 | dansmith | fried_rice: yeah, so in that case, I want to remove the bit of the libvirt inventory thing that will not expose disk_gb, because people may turn that on, and then be mangling their allocations with migrations for a couple days before realizing it | |
| 15:01:09 | fried_rice | mriedem: I sure thought it did. | |
| 15:01:16 | fried_rice | https://review.openstack.org/#/c/560459/ | |
| 15:01:56 | mriedem | hmm yeah https://review.openstack.org/#/c/560459/17/nova/tests/functional/libvirt/test_shared_resource_provider.py | |
| 15:02:01 | fried_rice | yup | |
| 15:02:57 | dansmith | yeah, so I dunno why it's not working for me | |
| 15:03:00 | dansmith | but that's fine | |
| 15:03:27 | sean-k-mooney | dansmith: only the allocation for the compute resouces need to be migrated correct. the shard storage allocation should remain the same. | |
| 15:03:38 | mriedem | sean-k-mooney: well, that's the point of the test, | |
| 15:03:39 | dansmith | sean-k-mooney: right, but we don't do that properly | |
| 15:03:43 | fried_rice | dansmith: Building on that one and trying a migration would be informative. I would be surprised if it works properly, because we have no logic to do ^ | |
| 15:03:45 | mriedem | because we have FIXME notes all over the migration code | |
| 15:03:52 | sean-k-mooney | i guess unless we are migrating with a block migraion to a different storage provider | |
| 15:03:58 | dansmith | fried_rice: I have fixmes about it being broken and known | |
| 15:04:02 | fried_rice | yup | |
| 15:04:22 | dansmith | fried_rice: so, yeah, I'm not sure why we landed the patch to do that for inventory in that case, but.. alas | |
| 15:04:58 | fried_rice | dansmith: So that we wouldn't be double-reporting inventory allocations. | |
| 15:05:20 | fried_rice | dansmith: Can't you only migrate an instance that's on volume storage anyway? | |
| 15:05:22 | dansmith | fried_rice: right, but that has been broken since forever, and this change means we *lose* data | |
| 15:05:25 | dansmith | no | |
| 15:05:34 | fried_rice | what happens to the disk? | |
| 15:05:37 | mriedem | ssh to the dest | |
| 15:05:43 | fried_rice | eek, really? | |
| 15:05:43 | dansmith | hah | |
| 15:05:44 | dansmith | it gets migrated | |
| 15:05:48 | dansmith | either block migration or shared (non-volume) storage in teh backend | |
| 15:05:57 | fried_rice | Okay, so what are we expecting to happen here? | |
| 15:06:06 | dansmith | for live, and yeah, scp to dest for the cold migration case | |
| 15:06:10 | fried_rice | I would have thought we would ssh the data to whatever disk got allocated on the dest. | |
| 15:06:29 | dansmith | I think we need to remove that bit of the inventory logic that doesn't expose DISK_GB | |
| 15:06:39 | dansmith | so that we don't get split allocations that we trash during a migration | |
| 15:06:48 | dansmith | because we'll end up with instances with no DISK_GB allocation at all | |
| 15:06:50 | fried_rice | which may or may not be the same provider as we started on, but to a different spot on that disk - which would be something to fix later | |
| 15:06:52 | dansmith | and then start overcommitting | |
| 15:07:20 | fried_rice | I don't understand that thinking. And IMO it is premature to land a patch to yank that out until we've demonstrated that anything bad happens. | |
| 15:07:34 | dansmith | that's why I'm trying to write a test | |
| 15:07:40 | fried_rice | sounds good. | |
| 15:07:43 | fried_rice | need help? | |
| 15:08:14 | dansmith | I asked for help and now am working on using that functional test to do my bidding | |
| 15:09:22 | mriedem | i believe this is the problem https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L48 | |
| 15:09:32 | mriedem | b/c we're assuming only allocations on the source compute node provider | |
| 15:09:34 | fried_rice | I think the worst that happens is we fail to remove the original allocation for the DISK_GB on the sharing provider. What happens after that depends on whether we migrated to a compute node with or without sharing disk. But the doubled allocation leaves us in no worse shape than we were before this fix, I would have thought. | |
| 15:09:35 | mriedem | and copy those to the migration consumer | |
| 15:09:41 | mriedem | which won't include the DISK_GB allocation on the shared provider | |
| 15:09:54 | sean-k-mooney | fried_rice: dansmith do we handel flavors with root_gb=0 in placement by the way. preplacement we jsut did not track there disk usage properly. im assuming that is stil broken | |