| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-27 | |||
| 15:19:17 | mriedem | dansmith: yup that's this https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4138 | |
| 15:19:33 | mriedem | well, we wouldn't hit that yet | |
| 15:19:44 | mriedem | the migration consumer will only have VCPU and MEMORY_MB allocations against the source node | |
| 15:19:52 | mriedem | so this https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4155 | |
| 15:20:25 | mriedem | so that's definitely busted - we could easily test that with a resize revert test and verify the DISK_GB allocation for the instance is gone | |
| 15:20:41 | dansmith | and forced live | |
| 15:20:52 | mriedem | i haven't stepped through forced live yet (or evac for that matter) | |
| 15:21:06 | sean-k-mooney | dansmith: we should only be able to pick a different provider in a block migrate case correct? if we do not set that flag we should not allow the shareing provider to change | |
| 15:21:07 | dansmith | also, | |
| 15:21:26 | dansmith | migrate to an older node that doesn't have this will drop the shared disk allocation | |
| 15:21:44 | dansmith | because placement will allocate from its own disk inventory, even though it's the same pool, | |
| 15:21:49 | mriedem | sean-k-mooney: re: "for example the nano flavor with with the cirros image in devstack with no volume for the guest." i don't know what you're asking me | |
| 15:21:52 | dansmith | and then when we upgrade that node, we won't convert the allocations | |
| 15:21:54 | dansmith | in fact, | |
| 15:22:09 | dansmith | any upgrade where we boot up on rocky code and change our inventory will break all our allocations right? | |
| 15:22:32 | dansmith | fried_rice: cdent what happens if I have allocations against my disk_gb inventory and then I update my inventory with no disk_gb ? | |
| 15:22:49 | fried_rice | The inv update will bounce 409 InventoryInUse. | |
| 15:22:49 | mriedem | i don't think you can do that | |
| 15:22:51 | mriedem | yeah | |
| 15:23:03 | fried_rice | on every periodic | |
| 15:23:11 | dansmith | okay, so anyone with MISC_SHARES now will failboat on upgrade to rocky | |
| 15:23:14 | fried_rice | update_from_provider_tree will never succeed. | |
| 15:23:23 | dansmith | and anyone that sets that on non-empty computes will stop reporting | |
| 15:23:55 | mriedem | oh right b/c upt removes the DISK_GB from the node provider if it sees it's in a sharing provider relationship | |
| 15:24:00 | dansmith | yeah | |
| 15:24:02 | fried_rice | Note that we didn't document that you could do this. | |
| 15:24:09 | mriedem | and if that DISK_GB is being used it will blow up on the remove | |
| 15:24:15 | mriedem | fried_rice: heh i know | |
| 15:24:17 | dansmith | fried_rice: and yet, it's in documentation and people have tried it, hence the bug yeah? | |
| 15:24:24 | sean-k-mooney | mriedem: in that instance. the flavor has root_gb=0 the imange is like 20MB in glance and we boot it on the dest without claim space in placement. the vm can use as much space as disk topology in the image specifies | |
| 15:24:50 | fried_rice | dansmith: The bug was opened because bhagyashri was working on it and I said it should have a bug report. | |
| 15:25:07 | mriedem | shared storage providers is definitely a feature/spec | |
| 15:25:17 | fried_rice | ...which we don't claim works yet. | |
| 15:25:19 | mriedem | given the upgrade/CI/etc | |
| 15:25:24 | mriedem | i know, but | |
| 15:25:31 | fried_rice | we should document that we *don't* support it. | |
| 15:25:42 | dansmith | well, there's mention of that trait in our own docs, and given what it breaks it's not trivial, IMHO | |
| 15:25:43 | fried_rice | and then take some time to resolve these issues correctly. | |
| 15:25:44 | mriedem | that's what dansmith and melwitt were talking about last night | |
| 15:25:57 | sean-k-mooney | mriedem: anyway thats unrelated to dans question excpet for the fact we dont track the disk usage correctly in placemnet | |
| 15:26:21 | melwitt | yes, described here L8 https://etherpad.openstack.org/p/nova-rocky-release-candidate-todo | |
| 15:26:35 | mriedem | sean-k-mooney: yes i think that's correct and likely a bug; i'm not entirely sure how the resource tracker reports disk usage for a flavor like that which *is* using local disk | |
| 15:26:38 | mriedem | where root_gb=0 | |
| 15:27:05 | mriedem | sean-k-mooney: we've also said you shouldn't use root_gb=0 except for volume-backed flavors | |
| 15:27:15 | mriedem | and added a policy rule in rocky to disable that | |
| 15:27:27 | sean-k-mooney | mriedem: oh cool | |
| 15:27:37 | sean-k-mooney | is is set by default | |
| 15:28:02 | mriedem | sean-k-mooney: https://github.com/openstack/nova/commit/763fd62464e9a0753e061171cc1fd826055bbc01 | |
| 15:28:14 | mriedem | the plan was to disable that by default starting in stein | |
| 15:28:27 | mriedem | so you can't boot a server with a root_gb=0 flavor unless you're doing boot from volume | |
| 15:28:35 | cdent | dansmith: microversions :( | |
| 15:28:52 | mriedem | how are microversions related to this? | |
| 15:29:08 | dansmith | I assume because I messed up a version in my test | |
| 15:29:10 | cdent | (sorry, his test) | |
| 15:29:13 | mriedem | ah | |
| 15:29:16 | mriedem | whew | |
| 15:29:48 | sean-k-mooney | mriedem: right ok cool. if you have existing instces booted that way we sould have to update teh embeeded flavor or resouce dict to indicate or live migration will explode | |
| 15:29:51 | mriedem | sean-k-mooney: so figuring out how we track disk usage for those types of flavors in the resource tracker would be good to know | |
| 15:30:05 | mriedem | because if it was never tracked as usage before, then it's not really a huge regression to not be tracking it in placement | |
| 15:30:08 | sean-k-mooney | mriedem: im pretry sure we track it as 0 | |
| 15:30:20 | sean-k-mooney | e.g. we dont track it at all | |
| 15:30:42 | mriedem | sean-k-mooney: i think that too b/c https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1461 | |
| 15:30:49 | mriedem | object_or_dict.flavor.root_gb | |
| 15:31:00 | sean-k-mooney | it was a way to bypass qouta in the past | |
| 15:31:01 | mriedem | the is_bfv in there was just recently added in the same series of fixes for the bfv thing | |
| 15:31:32 | mriedem | right, so to summarize, don't set flavor root_gb=0 *unless* those flavors are only used with bfv instances, | |
| 15:31:44 | mriedem | and we have the is_bfv root_gb reporting in the RT and placement fixed in rocky | |
| 15:32:31 | dansmith | melwitt: aight, well, anyway, my recommendation is that we just remove that inventory quirk for rocky since it can't work and it's one line. alternatively, at least a known-issue reno just to cover our butts in case someone hits it | |
| 15:32:35 | sean-k-mooney | mriedem: ya i think though we will have to fix up the allcoation for existing instance that are not bfv going forward | |
| 15:32:51 | mriedem | sean-k-mooney: we do | |
| 15:32:52 | dansmith | melwitt: it's like having a half-merged feature.. doesn't really serve any purpose and is externally tickle-able to failure | |
| 15:33:02 | dansmith | obviously your call on what to do | |
| 15:33:12 | mriedem | sean-k-mooney: https://review.openstack.org/#/c/583715/ | |
| 15:33:24 | mriedem | sean-k-mooney: we'll heal on move | |
| 15:33:26 | cdent | any swag on how hard to make it go, now-ish? | |
| 15:33:38 | mriedem | "make it go" == make it work? | |
| 15:33:53 | mriedem | we don't even have multi-node shared storage provider CI | |
| 15:33:56 | mriedem | so very high risk IMO | |
| 15:33:57 | sean-k-mooney | mriedem: in the non BFV case we need to read the size form the image if the flavor root_gb=0 | |
| 15:34:00 | mriedem | way too late | |
| 15:34:11 | mriedem | sean-k-mooney: yup | |
| 15:34:13 | dansmith | yeah, way too late to try to make any of the broken non-broken | |
| 15:34:19 | mriedem | but that's not reported to the RT as far as i know | |
| 15:34:27 | fried_rice | dansmith: I can propose that if you like. | |
| 15:34:39 | dansmith | fried_rice: which? | |
| 15:34:41 | mriedem | dansmith: so we should likely start with a bug saying this stuff will nuke your DISK_GB allocations on failure or revert at least | |
| 15:34:51 | mriedem | fried_rice: melwitt: ^ | |
| 15:34:53 | fried_rice | dansmith: Taking that line out of the libvirt driver. | |
| 15:35:09 | dansmith | mriedem: for sure | |
| 15:35:24 | dansmith | fried_rice: sure, I'm happy to do it as well, either way | |
| 15:35:28 | mriedem | and we can track the various bugs in a spec in stein if we're going to go full on and support this | |
| 15:36:01 | fried_rice | dansmith: You want to write up the bug, I'll do the patch? | |
| 15:36:02 | mriedem | b/c we need a spec for the upgrade impacts obviously, and how to deploy the thing, plus CI requirements (which i'm already half-way done with) | |
| 15:36:03 | melwitt | sounds like a plan | |
| 15:36:21 | dansmith | fried_rice: if mriedem isn't going to | |
| 15:36:34 | mriedem | go ahead | |
| 15:36:43 | dansmith | fried_rice: I think mriedem really wants to do it | |
| 15:36:46 | dansmith | I heard him say earlier | |
| 15:36:53 | dansmith | so I don't want to step on his toes | |
| 15:36:57 | mriedem | i'm cleaning up stephenfin's last 2 changes in his vswitc hseries | |