Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-08
15:33:28 cdent sean-k-mooney: yes
15:33:30 efried oh, this isn't in libvirt, okay. Yeah, in libvirt max_unit == total and doesn't seem to change.
15:33:54 cdent but if you have N datastores in one cluster, no single request can be more than the free space on one of those datastores
15:34:23 efried okay, but as you say, fixing that is unrelated to the 409 bounce
15:34:24 cdent (this is why I've been eager for shared providers, as it could fix this (except that storage "policies" break all that)
15:34:27 sean-k-mooney cdent: well if you have over allcoation set to somthing other then 1 it could
15:34:30 mriedem sure, but why would max_unit change between initially reporting the inventory and then during the claim?
15:34:34 sean-k-mooney but ya i get you point
15:34:59 efried mriedem: the claim itself reduces the amount of free space on one of those disks
15:34:59 mriedem when the service starts up, it runs upt which should report the max_unit from the Nth datastore with the most amount of total disk right?
15:35:04 sean-k-mooney we physically could not allocate more then the free space on the data store
15:35:40 cdent let's assume for the moment that the max_unit hack is immutable as that's not really germane to the point, it's just the proximate cause
15:35:58 mriedem efried: couldn't we make that same argument for the libvirt driver then?
15:36:04 kashyap melwitt: efried: On that 'initenv' change (https://review.opendev.org/#/c/667976/2), doesn't it seem like a broken detection by 'cloud-init'?
15:36:09 gibi mriedem: done https://review.opendev.org/669188
15:36:11 cdent the point is that a failed _update_to_placement during instance claim (which is about inventory, traits and aggregates, not allocations) can fail an already suceeded instance
15:36:26 sean-k-mooney mriedem: i dont think we should be adjusting max_unit in either case
15:36:32 efried mriedem: In libvirt it's not an issue because we're only looking at one disk, so the (total - usage) alread ylimits.
15:36:35 kashyap [Ignore me, I'll add a comment in the change.]
15:36:37 mriedem cdent: what do you mean by "already succeeded?
15:36:48 efried kashyap: Note the subsequent patch and the bug it points to.
15:36:54 mriedem we do the claim before the driver.spawn right?
15:36:57 sean-k-mooney it should remian at total size and really on palcment to do the free calulation + allcoation raitio
15:37:08 cdent mriedem: let me look at my notes
15:37:19 sean-k-mooney or total size of largest data store i gues for vcenter
15:37:28 mriedem yeah driver.spawn is way late in the build process, it's like the last thing
15:37:40 mriedem first we claim, then we build volumes and networking, then we call driver.spawn
15:37:55 efried anyway, I think the point here is whether we can maybe do without that _update_to_placement at the end of instance_claim? What else is it doing?
15:38:28 mriedem instance_claim calls _update which was historically for updating info on the compute node record
15:38:30 cdent mriedem: yeah, sorry, not that it did work, but _would_ work
15:38:33 mriedem for things like free_vcpus and whatnot
15:38:54 mriedem yeah i was going to say... https://github.com/openstack/nova/blob/86524773b8cd3a52c98409c7ca183b4e1873e2b8/nova/compute/manager.py#L2223 :)
15:38:58 openstackgerrit Sundar Nadathur proposed openstack/nova master: ksa auth conf and client for cyborg access https://review.opendev.org/631242
15:38:59 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Create and bind Cyborg ARQs. https://review.opendev.org/631244
15:38:59 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Add Cyborg device profile groups to request spec. https://review.opendev.org/631243
15:39:00 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Get resolved Cyborg ARQs and add PCI BDFs to VM's domain XML. https://review.opendev.org/631245
15:39:50 mriedem so i think what's being asked is a kwarg on _update to say whether or not to call _update_to_placement
15:39:57 efried though the fact that it's doing something in this case is an argument in favor of keeping it. Kinda wondering what breaks if we don't have that. Presumably it would be possible for the disk allocation to be unsatisfiable late in the spawn process.
15:40:00 mriedem which in the startup and update_available_resource periodic would be yes,
15:40:06 mriedem but in instance_claim you're saying no
15:40:12 cdent mriedem: pretty much, yes
15:40:15 efried configurable ^ perhaps
15:40:30 cdent you'd have to know that if you chose to do that you have to keep the periodic regular
15:40:38 cdent s/regular/frequent/
15:41:32 mriedem reshapes are in that flow but it looks like reshape should only be allowed on startup
15:41:32 cdent what I really want to do is not have inventory changing all the time...
15:41:36 kashyap efried: Yeah, just read the bug. Miguel is right. And this change makes sense. I don't see any security implications here (checked also w/ a libvirt dev)
15:41:59 sean-k-mooney cdent: as mriedem asked earlier is there any reason on start that we could not set teh max_unit to the larges datasotre and then not update it
15:42:07 sean-k-mooney or not update it outside the periodic
15:42:32 mriedem sean-k-mooney: from the upt perspective i'm not sure if the driver knows if it's called from the periodic, on startup, or during a claim
15:42:34 sean-k-mooney e.g. does it really need to be set to the free space
15:42:53 mriedem note that _move_claim will have the same issue here i'd think
15:43:07 cdent a) what mriedem said, b) the idea is to make sure placement has a way to prevent someone asking for more physical disk than is available
15:43:45 cdent mriedem: it's less of a risk there as they a) don't happen as much, b) happen even less in the vmware environment, c) this problem really only shows up when throwing > 2000 vms as the same nova-compute
15:43:57 cdent s/as the/at the/
15:44:11 sean-k-mooney cdent: ya i get that. i wonder if this is somthing we coudl do when we make the inital allocation
15:44:29 mriedem the initial allocation doesn't involve the driver
15:44:30 sean-k-mooney e.g. is this somethign we shoudl consider for all driver in general
15:44:37 sean-k-mooney true
15:44:41 efried kashyap: Thank you. Your votes/comments on those patches will be beneficial. Sounded like melwitt will also appreciate it.
15:44:53 cdent no other driver (that I'm aware of) would need this as they report real and true inventory
15:44:56 sean-k-mooney but shoudl we be doing it for libvirt with ceph/nfs
15:45:21 cdent it's only when you have non-contiguous disk(s)
15:45:52 sean-k-mooney cdent: ok would this be solved in the futrue by representing
15:45:54 mriedem in the libvirt+ceph case, we report total and max_unit the same, and placement will reject requests that are too big because of the capacity calculation in placement, right?
15:46:00 sean-k-mooney the datastores as different shareing providers
15:46:27 cdent sean-k-mooney: yes, except for the thing I said above about something called "storage policy" which would disrupt placement's understanding of where things are
15:46:36 sean-k-mooney mriedem: yes which is fine for ceph as we have a singel pool for vms
15:47:02 mriedem i'd prefer not to have a config option for this behavior,
15:47:07 mriedem you could add a variable on the driver itself,
15:47:22 mriedem which could be controlled by the driver depending on if the vcenter driver is doing this shared storage pool modeling thing
15:47:24 cdent yeah, I'm not even clear if it should ever be upstreamed
15:47:30 mriedem the RT would check that, and by default we do as we do today
15:47:49 mriedem i've done similar hacks in the RT for ironic
15:47:54 mriedem b/c they are a special unicorn as well
15:48:02 mriedem *we've done
15:48:21 cdent unicorn's are a PITA
15:48:28 cdent hmm unicorns too
15:48:48 mriedem always pooping out frosting on my yard and such
15:48:59 cdent Thanks mriedem, sean-k-mooney, efried this has been very useful.
15:49:07 cdent and such
15:49:15 kashyap efried: Yes, will review
15:49:43 cdent If it turns out I can extract a little bit of 'if this virtdriver magic' i'll make it so
15:50:41 openstackgerrit Sundar Nadathur proposed openstack/nova master: ksa auth conf and client for cyborg access https://review.opendev.org/631242
15:50:42 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Create and bind Cyborg ARQs. https://review.opendev.org/631244
15:50:42 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Add Cyborg device profile groups to request spec. https://review.opendev.org/631243
15:50:43 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Get resolved Cyborg ARQs and add PCI BDFs to VM's domain XML. https://review.opendev.org/631245
15:53:44 mriedem gibi: looks like https://review.opendev.org/#/c/637955/31 and https://review.opendev.org/#/c/669188/2 conflict, maybe you want to rebase the fix underneath the bigger change?
15:57:09 sean-k-mooney cdent: by the way i assume there are buisness reasons that vmware would prefer not to map each compute node to a specifc data store and use cinder with volume type to express storage policies for everything else
15:57:39 cdent sean-k-mooney: yeah, the basic root of all these things is "let the DRS continue to work"
15:57:47 sean-k-mooney i know vmware have lots of fancy feature built into the hypervior and storage solution so im guessign the current approch is trying to expose them
15:58:02 sean-k-mooney right ok makes sense
15:58:31 cdent it creates a huge impedance mismatch between the way nova thinks and the way vmware thinks, but that's the way it goes
15:59:48 sean-k-mooney ya hyperv also has some limited clustering of vms and storage pools but its not as advanced as what vcenter does so the impedence miss match is less of an issue for them i guess
16:00:43 cdent there's been discussion of tying to listen to events from the DRS via some agent that would then "correct" placement when DRS makes a change
16:01:14 cdent but that would require more effort than currently available resources
16:01:45 gibi mriedem: will do
16:01:55 sean-k-mooney that might require you to reshap allocation if it moved things right if you had a shareing provider per datastore
16:02:30 sean-k-mooney although if you had just one RP and hid the datastore i guess it woul just have to modify the max unit
16:02:35 sean-k-mooney as it does today

Earlier   Later