Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-11
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
20:53:02 dansmith yeah
20:53:31 mriedem tssurya: you want me to open a separate bug?
20:53:58 tssurya mriedem: yea
20:54:04 mriedem also, it's 11pm there right?
20:54:15 tssurya I will put up a patch tomorrow
20:54:17 mriedem do you get to bill belmiro for overtime?
20:54:30 tssurya mriedem: yes its 11pm :)
20:54:33 tssurya haha no
20:55:04 tssurya this is me learning stuff from the veterans :P
21:22:40 openstackgerrit Merged openstack/nova master: Remove mox in tests/unit/test_utils.py https://review.openstack.org/557883
21:52:10 mriedem cdent: edleafe: oh seers of http status code wisdom, hear my call, shall thou return a 400 or 409 for https://bugs.launchpad.net/nova/+bug/1763183 ?
21:52:10 openstack Launchpad bug 1763183 in OpenStack Compute (nova) "DELETE /os-services/{service_id} does not block for hosted instances" [High,Triaged] - Assigned to Matt Riedemann (mriedem)
21:52:41 mriedem i think 409
21:53:03 cdent i feel like I should have some kind of a 409phone
21:53:26 mriedem oh and if 409, guess what, @wsgi.expected_errors((400, 404))
21:53:30 mriedem we get to have a microversion debate
21:53:40 mriedem alex_xu: ^
21:54:16 mriedem although, it's an admin api...
21:54:40 edleafe mriedem: 400 usually means that you can make the call succeed by fixing the parameters, etc
21:54:44 cdent mriedem: yes 409
21:54:53 cdent because of what edleafe just said
21:55:06 cdent the server has to change to make things work, not the request
21:55:17 cdent but if the server does change, then things might work when you try later
21:55:47 mriedem yeah 409 is correct, but need to figure out if i can get by the existing expected errors decorator
21:55:57 mriedem but i think with the legal caveat in https://docs.openstack.org/nova/latest/contributor/microversions.html#when-a-microversion-is-not-needed 3rd bullet
21:55:58 mriedem we maybe can

Earlier   Later