| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 20:20:44 | mriedem | the setUp is mostly copy/paste from other similar functional tests | |
| 20:20:53 | mriedem | but thanks, i'll take all that credit | |
| 20:21:17 | tssurya | mriedem: I know, that's what I did took most of it from test_servers like Dan said | |
| 20:21:31 | tssurya | however I got tangled up | |
| 20:21:41 | arvindn05 | mriedem: incase of rebuild, can we make a allocation request to placement without resources and only traits(will require API behaviour change, new microversion?) and make sure the host list returned contains the host we are currently doing a rebuild of? | |
| 20:22:05 | mriedem | orangutangled? https://www.amazon.com/Orangutangled-Sudipta-Bardhan-Quallen/dp/1477847170 | |
| 20:22:29 | arvindn05 | mriedem: if that sounds like a good startergy...i can pursure the code changes required while that other patch is being merged.... | |
| 20:22:43 | tssurya | mriedem: :P | |
| 20:23:06 | mriedem | arvindn05: i think that is over-complicating things | |
| 20:23:26 | mriedem | to modify the allocation_candidates API to ignore resources | |
| 20:24:08 | arvindn05 | hmm....ok...any suggestions? | |
| 20:24:10 | mriedem | if anything, we'd want to change GET /allocation_candidates to pass in a specific root provider uuid | |
| 20:24:41 | mriedem | however....placement might say, "sorry no more room" | |
| 20:24:45 | arvindn05 | i though it could be useful for other cases where the user might want to take an inventory of RP's with specific traits etc | |
| 20:24:57 | mriedem | the user can do that via GET /resource_providers | |
| 20:25:24 | mriedem | hmm, i'm not sure how allocation_candidates is going to work here, since i guess it would have to make resources optional, | |
| 20:25:30 | mriedem | otherwise you can hit the bug that hongbin is fixing | |
| 20:25:34 | mriedem | if the compute is full | |
| 20:25:57 | arvindn05 | exactly... | |
| 20:26:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional test for deleting a compute service https://review.openstack.org/560626 | |
| 20:26:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/554920 | |
| 20:26:33 | mriedem | arvindn05: well, this is why your spec needs to be amended like i asked for | |
| 20:27:45 | arvindn05 | oh...didnt realize you were asking for ammending the spec...my assumption was to use the code review phase for addressing since it was a bug fix | |
| 20:28:29 | mriedem | what is a bug fix? | |
| 20:28:38 | mriedem | hongbin's patch is a bug fix | |
| 20:28:41 | mriedem | you're adding a new feature | |
| 20:30:40 | arvindn05 | got it. Thanks for letting me know...i thought specs need to be ammended for larger changes....i can propose an amendment to the spec | |
| 20:31:26 | mriedem | arvindn05: if we have to make an API change to GET /allocation_candidates, then it's a larger change, and all API changes require a spec anyway | |
| 20:32:13 | mriedem | dansmith: melwitt: i would like to phone a friend on https://review.openstack.org/#/c/554920/6/nova/api/openstack/compute/services.py@226 | |
| 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 | |