Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-11
20:32:23 dansmith hah
20:32:38 melwitt heh, awesome reference
20:33:17 mriedem re: is it ok to cascade delete the resource provider while the service has instances on it
20:33:38 arvindn05 mriedem: API change is just my first proposal...i would really like to avoid it if i can with a much more localized fix...but you are right its better to be discussed on the spec in case we need to make that API change
20:33:47 dansmith mriedem: we currently allow them to do this anyway right?
20:33:57 dansmith mriedem: deleting the resource provider doesn't affect the quota
20:34:08 dansmith mriedem: it just avoids us leaking data in placement
20:34:32 melwitt well, thinking ahead where we want to ask placement for cores/ram allocation usage
20:34:38 mriedem dansmith: we currently allow admins to delete compute services that have instances running on them, yes
20:34:49 melwitt it seems like we shouldn't nuke allocations if instances are still alive, right?
20:34:51 dansmith so that creates the quota issue
20:34:53 mriedem but we don't currently delete the resource provider (and allocations)
20:35:08 mriedem and since the compute doesn't heal allocations,
20:35:29 mriedem even if you bring the compute back up, those instances are screwed and the resource provider for that node isn't going to report proper inventory
20:35:36 mriedem s/inventory/consumption/
20:35:37 melwitt like, instances are still consuming resources from placement
20:35:49 dansmith instances don't consume from placement,
20:36:06 dansmith but yes, they'll be consuming from a provider that is no longer present
20:36:13 mriedem it seems to me that if the node has instances with allocations in placement, we shouldn't delete the resource provider in that case
20:36:19 dansmith mriedem: that's still true today right? if you delete it, you'll end up with a compute node that looks to be empty in placement
20:36:38 mriedem dansmith: i guess the node uuid would change wouldn't it
20:36:43 melwitt well, this cascade is talking about deleting the allocations too right? I thought we need to keep the allocations as long as the instance is alive, no?
20:36:44 mriedem so you'd have a new provider
20:36:47 dansmith so you just leak data today
20:37:01 dansmith you still get out of sync the same way, but with leaked data you can never clean up
20:37:27 dansmith so maybe we should block the service delete, but that's separate from if we're deleting the service, we should delete the RP I think
20:39:01 mriedem i can't parse that sentence
20:39:08 dansmith meaning,
20:39:23 dansmith if we're going to delete the service (and thus compute node), we should delete the RP in placement
20:39:24 dansmith but
20:39:38 dansmith if we want to block deleting the service because of instances, then that's fine and it would also block the RP
20:39:59 melwitt if we want to be able to count quota usage for cores and ram from placement allocations (which we do, so we can handle cell down, and that's the right place to count cores and ram anyway), then deleting allocations when a service is deleted if instances are still running associated with that service is going to mess that up
20:40:13 tssurya so we should allow a service delete only if doesn't have any living instances on that compute node ?
20:40:33 dansmith melwitt: that's the same concern as deleting instances before the service for today's counting method
20:41:02 dansmith melwitt: and if we don't delete the RP when we delete the service, you have no way of going back to delete the allocations you leaked later,
20:41:22 mriedem well, you have ways, but not fun ways
20:41:23 dansmith well, maybe you can by instance uuid, but you can't delete them by compute node uuid in bulk
20:41:24 dansmith yeah
20:41:51 openstackgerrit Jackie Truong proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949
20:41:52 openstackgerrit Jackie Truong proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204
20:41:53 openstackgerrit Jackie Truong proposed openstack/nova master: Add certificate validation docs https://review.openstack.org/560158
20:41:55 mriedem you'd have to take all instances that were running on that host, find their allocations, and then remove any against RPs that no longer exist
20:42:06 melwitt okay, I'm not getting the "same concern with today's counting method" part yet
20:42:19 dansmith so I guess the question is.. why do we allow you to delete the service today with instances on it?
20:42:30 mriedem yeah i'm trying to get there
20:42:31 dansmith because that would cause accounting trouble in the RT the way it was when that was added
20:42:34 melwitt if you delete instances before the service today, the allocations will be deleted when you delete the instances, which is fine
20:43:44 dansmith if you just delete the service, then we'll still count those instances for quota, yes, is that what you mean?
20:43:53 mriedem excluding placement, if you delete the service and later restart it, we'll create a new service, but the RT will still look up instances on that service via host and nodename yeah?
20:44:00 efried johnthetubaguy_: I think your -1 on https://review.openstack.org/#/c/553605/ is moot now, yes? The flag changes are being done in the previous patch, and what you were seeing in PS6 was a bad rebase.
20:44:06 dansmith but you've broken things now, like you can't find the cell that the instance's host is in anymore
20:44:07 dansmith so you might not be able to delete instances
20:44:08 melwitt I think you meant the opposite, deleting the service before the instances. with counting today, we count from the instances table so it works. but it would break if we were counting from placement
20:44:33 efried johnthetubaguy_: That delta is no longer there (that file is unchanged by this patch)
20:44:40 dansmith sure, what I mean is, other things will break if you delete the service before the instances I think
20:44:52 melwitt I see
20:44:55 mriedem quota counting uses instance mappings right?
20:44:59 mriedem to find the cell they are in
20:45:09 melwitt yes
20:45:14 dansmith I guess delete will as well, not host mapping
20:45:18 mriedem i'm not sure what other things will break for the instance if the host mapping is gone
20:45:28 dansmith alright maybe not
20:45:41 dansmith that said,
20:45:55 dansmith I don't think that we should not delete in placement if we're deleting our record of what those allocations are,
20:46:01 dansmith because then they're leaked
20:46:09 dansmith and my point being,
20:46:22 dansmith we should tie the deleting of those to deleting of our service (and thus our record of how to find them)
20:46:41 dansmith it seems weird to me to let people delete the service with instances on it,
20:46:52 dansmith but I guess it might not fail anything today because we use the hostname as they key
20:47:23 mriedem i'm totally fine with deleting the resource provider if we're going to delete the service and compute node
20:47:36 dansmith we kindof have to
20:47:45 mriedem i'm trying to parse if you're confirming what tssurya asked, "so we should allow a service delete only if doesn't have any living instances on that compute node ?"
20:47:47 melwitt yeah, I agree we can't be leaking the allocations. but so far I'm leaning to we should block service delete if there are instances. I'm concerned about losing allocation records for instances in that case
20:48:12 mriedem the easy thing to do here is not delete anything if there are instances on the node
20:48:17 dansmith melwitt: yeah, I'm saying I think that makes the most sense, I'd just like to figure out why that wasn't done way back when
20:48:33 dansmith maybe because it would re-heal and so meh,
20:48:38 dansmith but today that's not the case
20:48:38 melwitt yeah, okay. I'd also like to know
20:48:38 mriedem dansmith: right, because "if you delete the service and later restart it, we'll create a new service, but the RT will still look up instances on that service via host and nodename yeah?"
20:49:02 dansmith yes, I mean,
20:49:14 tssurya melwitt: umm another doubt on quota counting - we don't use inst_mappings right ? we just query through all the cells DBs instances tables ? -> https://github.com/openstack/nova/blob/master/nova/quota.py#L1325
20:49:15 mriedem https://github.com/openstack/nova/blob/2c5da2212c3fa3e589c4af171486a2097fd8c54e/nova/compute/resource_tracker.py#L714
20:49:38 dansmith deleting the service today which deletes the compute node will break all manner of stuff, if not deleting instances and quotas, because we'll get a new compute node uuid
20:50:09 dansmith but likely just placement-forward things where the uuid started to be a thing
20:50:25 mriedem yeah. if we block the service delete, you have options to not f*ck yourself,
20:50:29 mriedem like migrate the instance
20:50:30 melwitt tssurya: oh, yep, you're right. I forgot ... cause we have to be able to get all instances for a project/user combo and we can't know that from instance_mappings (because it lacks user_id). but we could have done a pruning by project_id based on instance_mappings
20:50:59 tssurya melwitt: which is what we plan to do once we have your spec in :D
20:51:05 mriedem this is sort of like the "should we allow AZ renames for aggregates with hosts that have instances on them"
20:51:23 mriedem knowing that if you do ^ you'll likely break some stuff
20:51:27 dansmith yeah
20:51:43 melwitt tssurya: heh, yeah :)
20:51:53 mriedem ok so is this considered a separate bug?
20:52:01 mriedem or do we roll it into the same fix?
20:52:02 dansmith it's a separate patch IMHO
20:52:09 dansmith which is why I was saying they're two different concerns
20:52:12 mriedem that's kind of what i was thinking, it would be easier that way anyway
20:52:28 melwitt +1 to separate patches
20:52:30 mriedem we can build on the same functional test base i started
20:53:00 tssurya sure

Earlier   Later