Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-27
15:17:39 dansmith oh you're saying we drop the disk allocation but only because we don't copy it for the migration
15:17:42 mriedem we don't go through the scheduler there
15:17:48 mriedem dansmith: yeah
15:18:10 dansmith so,
15:18:22 mriedem on a revert or failed resize, we'll delete the allocs for the instance on the dest host (created by the scheduler) and move those back from the migration to the instance, but the migration allocs won't be on the sharing provider
15:18:24 dansmith what happens if placement picks a different sharing provider than we had before? our disk doesn't actually move
15:18:27 mriedem so we'd lost the DISK_GB alocs in that case
15:18:38 dansmith ah, yeah, anything where we use the migration's allocations would be wrong
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 mriedem i don't think you can do that
15:22:49 fried_rice The inv update will bounce 409 InventoryInUse.
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?

Earlier   Later