Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-12
16:57:12 aspiers here's sean-k-mooney's take: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-12.log.html#t2019-06-12T13:13:16
16:57:18 efried In spawn specifically, we are getting the allocation as a param, so that's fine. But there are (aspiers tells me) other places where we need the same information but don't (currently) have access to the allocations.
16:57:21 dansmith spawn having access to the allocations makes sense, but storing them on/with the instance doesn't really
16:57:52 aspiers here's an example: https://review.opendev.org/#/c/662558/5/nova/virt/libvirt/driver.py
16:58:08 dansmith I imagine bauzas would want access to allocations for gpu stuff as well, and I'm sure we discussed making that a param to spawn in the past
16:58:15 aspiers currently this is broken because it's insufficient to pass only the flavor to _sev_required()
16:58:23 efried allocations are already passed to spawn
16:58:33 dansmith ack, okay I was sure we had discussed
16:58:36 aspiers right, the problem is that they are not passed much further on
16:58:58 aspiers sean-k-mooney's other suggestion was to pass from spawn() to _get_guest_xml() and so on
16:59:21 dansmith yeah, pass them further if you want, but storing them .. no so much
16:59:33 aspiers but that doesn't solve things like attaching volumes to an SEV instance
16:59:39 efried you could cache a map of instance:allocations on the virt driver
16:59:47 dansmith efried: dude
16:59:53 efried but if you restart the compute driver you'd have to rebuild that by querying placement
16:59:57 dansmith aspiers: so look them up and pass them in if you need
17:00:09 efried we've said we don't like virt drivers talking to placement
17:00:19 dansmith yeah
17:00:31 efried though I guess `read` may be acceptable
17:00:32 dansmith and caching placement data is also smelly, IMHO
17:00:46 dansmith efried: no, make compute manager look them up and pass them in
17:01:04 efried dansmith: You mean augment every ComputeDriver method to accept allocations?
17:01:30 dansmith surely we don't need them on every call
17:01:40 efried creeping death of a thousand cuts
17:02:15 efried we already store other (pre-placement) allocation-like things on the Instance, don't we? PCI devices and such?
17:02:31 dansmith but we own those things
17:02:40 dansmith we cache the neutron port information and it's a disaster
17:03:02 efried why?
17:03:12 efried because that information can get changed via neutron?
17:03:19 efried and then we have to figure out how to sync it?
17:03:24 efried In this case nova does "own" the information.
17:03:30 efried the allocation should not change unless nova changes it
17:03:36 efried whereupon we're editing the Instance obj anyway.
17:03:58 aspiers so how would something like DriverVolumeBlockDevice.attach() get hold of allocations?
17:04:04 dansmith okay, whatever, you asked my my opinion.. I hate it. My opinion doesn't matter anyway, only mriedem's, so just do whatever he says
17:04:10 aspiers if not via the Instance?
17:04:43 aspiers (Yes, that is an honest stupid question)
17:04:54 efried aspiers: If I'm understanding dansmith correctly, he's suggesting adding an allocations param to attach_volume and then passing it down through the call stack to DVBD.attach()
17:05:17 efried doing that ^ as needed every time we encounter a ComputeDriver operation who(se guts) require access to the allocation
17:05:21 aspiers efried: so through attach_volume() in the manager?
17:05:40 efried yup. The compute manager would call placement to get the allocations for the instance before invoking the method.
17:05:54 aspiers it seems pretty much every instance lifecycle method is already passing context and instance through
17:06:00 efried yes
17:07:43 aspiers looks like adding allocations would bloat the already long parameter lists of many methods
17:08:26 aspiers long parameter lists seem like a code smell to me in general, but I don't really enough about nova to judge here
17:08:51 aspiers I'll do whatever you gurus think is best :)
17:08:57 efried let's see what mriedem thinks
17:09:00 aspiers OK
17:09:37 dansmith we have virtapi for compute drivers to ask compute manger for help. compute manager already has an in-memory cache of a bunch of this information, so letting the driver ask manager for allocation info for an instance when it needs it, which can be a readthrough cache operation would also be reasonable
17:10:05 efried ah, that could work. What's that virtapi called?
17:10:41 dansmith storing this on the instance is pointless to me because I think we always need to get it from placement to be sure it's right I think, and the instance record is already a massive thing we shoot across the stressed RPC bus with a ton of extra crap we don't need 90% of the time
17:11:02 efried ComputeDriver.virtapi...
17:11:06 dansmith but, as I said, it matters only what mriedem thinks, so ... no point in even discussing until he opines
17:11:28 aspiers RPC bloat sounds like a very valid concern
17:16:23 sean-k-mooney RPC bloat being the reason not to put it in the instance?
17:16:40 aspiers that's how I interpreted what dansmith said
17:16:47 efried This virtapi thing is pretty small right now, basically just has wait_for_instance_event
17:16:51 aspiers yeah
17:16:59 aspiers also, I don't see allocations cached in the manager
17:17:06 sean-k-mooney dansmith: the allcoation should not change over the lifetime of the instance without a server action to modify it
17:17:10 dansmith efried: it used to have a bunch of services the driver used, but has whittled down
17:17:29 sean-k-mooney e.g. it will only cahnge on move operattion/resize/rebulds/or attaches/detaches
17:17:57 dansmith sean-k-mooney: in some future where I can allocate non-compute resources for the instance, the instance's allocation can change not from nova right?
17:18:02 efried but if dansmith likes the idea of extending it with allocations_for_instance, which we can cache in the manager, I'm down with that idea.
17:18:15 dansmith aspiers: no, we cache inventory AFAIK, but not allocations at the moment
17:18:26 sean-k-mooney dansmith: im notsure about that
17:18:43 sean-k-mooney are you thinking baout things like changing the bandwith allcoation via neutron
17:18:51 sean-k-mooney e.g. by changing the qos policy
17:19:17 efried aspiers: The inventory (basically all the provider tree information for all the hosts managed by this compute) is cached in the SchedulerReportClient today. But yeah, not the allocations.
17:19:40 sean-k-mooney i guess that could happen in the future
17:19:55 dansmith sean-k-mooney: I'm thinking of allocating things like a new volume, or even ephemeral things like networked secure enclave keys or something
17:20:17 sean-k-mooney dansmith: well a new volume would be a volume attach right
17:20:19 efried as long as those ^ things happen in nova, we're good, but yeah, if they happen outside of nova, weirdness.
17:20:22 dansmith sean-k-mooney: it also means our rpc messages and database footprint scales with whatever else we add into the allocation in the future
17:20:48 efried I think we have other problems to solve ("heal allocations"??) if we start allowing outside entities to modify instance allocations
17:20:56 dansmith sean-k-mooney: not always in the case of manilla or similar higher level services
17:21:01 sean-k-mooney that is true altheough we have a 64k userdata blob in there already
17:21:11 efried I'm happy with the virtapi solution personally.
17:21:18 efried I don't even think we need to bother caching it, at least initially.
17:21:28 sean-k-mooney for mania the filesystem is not own by the instnace
17:21:30 efried Since it's on demand and should be a pretty rare ask.
17:21:53 sean-k-mooney the instace can have acess too it but its not tied to the lifetime of the instnace
17:22:30 sean-k-mooney efried: i do kindof dislike the idea that if placement goes offline you cant reboot an instnace
17:22:32 dansmith sean-k-mooney: okay, you understand I'm talking about high-level services that sit on top of infra that would have per-instance allocations right?
17:22:45 sean-k-mooney dansmith: yes
17:23:01 sean-k-mooney and i get that it could be extendted that way in the future
17:24:51 sean-k-mooney without storing the allcoation somewhere in nova then placment becmoes a singel point of failture for starting or stroping vms if they use sev or vgpus
17:25:33 dansmith I definitely don't like that we'd depend on placement to reboot an instance or do other things, but I'd rather us not depend on the placement *data* in that case,
17:25:38 dansmith and not just solve it by replicating everything everywhere
17:26:01 efried yeah, when I found out we rebuild the domxml every time we reboot I was like whaaa?
17:26:16 sean-k-mooney well that makes sense in some ways
17:26:26 efried it must
17:26:30 efried in some ways
17:26:31 sean-k-mooney that is not really the root cause of the dependcy in the vgpu case
17:26:32 dansmith is this all coming up because we've already translated a request into a "will have SEV" and the only way we can think to know that later is persist the whole allocation?
17:26:46 efried dansmith: more or less
17:26:46 sean-k-mooney no
17:26:49 efried heh
17:26:53 efried anti-jinx
17:26:58 sean-k-mooney we can look at the request spec

Earlier   Later