Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-07-27
16:36:53 opendevmeet Meeting ended Tue Jul 27 16:36:53 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
16:36:53 bauzas #endmeeting
16:36:54 bauzas even
16:37:22 sean-k-mooney melwitt: bauzas do you want to chat about the allocation delete issue
16:37:38 bauzas sean-k-mooney: I guess I need to look at the original change
16:38:12 sean-k-mooney it didnt really contain much more motivation then we surmised already
16:38:33 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/591597
16:39:31 sean-k-mooney it looks like the current intent was to put the instance in error if there was a conflict
16:39:47 sean-k-mooney which you would preumable fix by deleting it again
16:40:21 sean-k-mooney so based on that https://bugs.launchpad.net/nova/+bug/1836754 is invlid
16:40:29 sean-k-mooney since that is the expect behavior
16:40:55 sean-k-mooney and presumable tempest does not handel the fact delete can fail and it shoudl retry
16:41:39 melwitt no it does not. I debated whether that should be the fix, to retry on 409 during resource cleanup
16:41:52 melwitt but then I found the WIP change and thought it didn't seem like good UX to ever reject a delete request from a user
16:42:08 sean-k-mooney right
16:42:22 melwitt that was in fact one of the things customers within yahoo when I worked there were vehement about, delete should never fail
16:42:26 sean-k-mooney but if we decied its not a good ux to delete it form the user instead of https://review.opendev.org/c/openstack/nova/+/688802
16:42:40 sean-k-mooney we should revert the previous patch and start callign delete again on plamcnet
16:44:06 melwitt I'm cool with that too. I wasn't 100% sure whether there's internal cases where we would want the chance to know about a conflict
16:44:18 sean-k-mooney melwitt: basiclaly i think we have 2 options. mark the bug as invilad and adapt tempset for the 409, or always call placemtn with DELETE
16:44:29 sean-k-mooney i dont think we need a new arguement that default to ture
16:45:04 bauzas I see two different things here
16:45:07 sean-k-mooney melwitt: do we know of any internal cases this would protect form
16:45:20 bauzas 1/ a delete should always work and never return an exception, for sure
16:45:39 bauzas 2/ a racing delete could tho leave orphaned allocations and that's fine
16:45:46 melwitt sean-k-mooney: they're in the WIP patch, one of them was during a soft-delete reclaim periodic. I need to look again to see the other cases where we would pass force=False
16:46:19 sean-k-mooney melwitt: ack
16:46:29 melwitt but yeah for sure when it's an end user requesting delete we should ensure that cannot fail
16:48:22 melwitt bauzas: only adjacent to how this discussion came about, I would love for you to be the second reviewer on the placement consumer types series, if you might have interest
16:48:43 bauzas melwitt: I can help
16:48:59 melwitt \o/
16:49:26 bauzas I mostly work on the generic mdev stuff but I'm not sure I'll be able to put all the changes before I leave for 3 weeks, so I'll maybe turn into reviews next week
16:50:29 melwitt gibi has gone through it thoroughly already (thank you gibi!) so it's in a nice shape now
16:51:24 sean-k-mooney i can try and take a look too but while i dicuss the api of placment alot i never really look at the code so millage will vary
16:52:14 bauzas melwitt: ping me your series so I'll open a tab and make it a priority
16:53:39 melwitt sean-k-mooney: ++
16:54:08 melwitt bauzas: it's 3 patches starting here https://review.opendev.org/c/openstack/placement/+/669170 I will remove the -W, I put up a DNM nova change to run with the new stuff and it all worked well _except_ for hitting the generation conflict seemingly guaranteed
16:55:42 opendevreview Stephen Finucane proposed openstack/nova master: mypy: Add nova.cmd, nova.conf, nova.console https://review.opendev.org/c/openstack/nova/+/705657
16:55:43 opendevreview Stephen Finucane proposed openstack/nova master: mypy: Add type annotations to top-level modules https://review.opendev.org/c/openstack/nova/+/705658
16:55:44 opendevreview Stephen Finucane proposed openstack/nova master: WIP: Expand type hints for nova.block_device https://review.opendev.org/c/openstack/nova/+/742170
16:55:44 opendevreview Stephen Finucane proposed openstack/nova master: trivial: Clean manager.Manager, service.Service signatures https://review.opendev.org/c/openstack/nova/+/764806
16:57:15 bauzas melwitt: cool, will read tomorrow
16:57:42 melwitt thanks bauzas++
16:59:26 sean-k-mooney melwitt: bauzas this is a summary of my view on the WIP patch https://review.opendev.org/c/openstack/nova/+/688802/2#message-70a28bde99719c723c954c947d0af17227a344e9
17:00:00 melwitt thanks sean-k-mooney
17:00:13 sean-k-mooney melwitt: there are other reasonce lients can recive a 409 namely locked instances so we can go either way with it
17:00:28 melwitt I feel strongly that delete (by an end user) should never fail
17:00:46 melwitt internals can handle 409s if there is some reason to
17:00:47 sean-k-mooney right but the api contract is it must fail if the instance is locked
17:01:38 melwitt well, yeah, agree that is expected
17:01:42 melwitt that's the point (or one of the points) of locking
17:02:08 sean-k-mooney https://docs.openstack.org/api-ref/compute/?expanded=force-delete-server-forcedelete-action-detail#force-delete-server-forcedelete-action im kind of suprised force delete can also return a 409
17:02:43 melwitt yeah :\
17:02:48 sean-k-mooney force-delet i do expect to never fail
17:03:04 sean-k-mooney well 401/403/404 sire
17:03:07 sean-k-mooney *sure
17:03:10 sean-k-mooney but not 409
17:03:38 melwitt yeah
17:08:24 melwitt fyi there's a new spec proposed around migrating instances between projects that I had -2ed the implementation proposal as this majorly needs a spec https://review.opendev.org/c/openstack/nova-specs/+/802034
17:20:48 sean-k-mooney melwitt: oh i forgot to bring this up in the meeting
17:21:05 sean-k-mooney melwitt: bauzas how do you feel about approving/reviewing yoga specs now?
17:21:37 sean-k-mooney we normally wait untill around m3 but ^ and any other specs can be targeted again the yoga release now
17:21:43 sean-k-mooney we have the folder created
17:23:13 sean-k-mooney melwitt: and ya that needs a spec
17:23:36 melwitt I personally think review is welcome at any time. approval I think is natural post PTG at least but obviously it doesn't have to be
17:23:39 sean-k-mooney we sould also need support in every other serivce to move the ownership of the resouces
17:24:19 sean-k-mooney we have approved spec pre ptg before but mostly onse that missed the previous cycle
17:24:32 melwitt yeah. we've talked about it many times as we know, and some of the proposal were to only support the project move within nova and then let other tools call all the different projects' APIs
17:24:59 sean-k-mooney os-chown was the latest iteration of that right
17:25:34 sean-k-mooney https://github.com/kk7ds/oschown
17:26:03 sean-k-mooney i think there was a sepereate repo created form dansmith poc
17:26:19 melwitt yeah there is that and I didn't know about a separate repo
17:26:55 sean-k-mooney the only one i can find is os-migrate
17:27:02 sean-k-mooney but that is different
17:27:12 sean-k-mooney that is inter cloud migration
17:28:16 sean-k-mooney melwitt: so i think my issue with nova just doing the nova bits is we end up in a situration where for a time the ownser ship of each resouce is split between multipel porojects/users/domains
17:28:38 opendevreview Stephen Finucane proposed openstack/placement master: tests: Silence noisy tests https://review.opendev.org/c/openstack/placement/+/801101
17:28:39 opendevreview Stephen Finucane proposed openstack/placement master: setup: Replace dashes with underscores https://review.opendev.org/c/openstack/placement/+/801102
17:28:39 opendevreview Stephen Finucane proposed openstack/placement master: tox: Remove psycopg2 warning filter https://review.opendev.org/c/openstack/placement/+/800911
17:28:40 opendevreview Stephen Finucane proposed openstack/placement master: db: Replace 'as_scalar()' with 'scalar_subquery()' https://review.opendev.org/c/openstack/placement/+/801100
17:28:40 opendevreview Stephen Finucane proposed openstack/placement master: db: Replace implicit conversion of SELECT into FROM https://review.opendev.org/c/openstack/placement/+/800910
17:28:41 opendevreview Stephen Finucane proposed openstack/placement master: db: Remove use of non-integer/slice indices https://review.opendev.org/c/openstack/placement/+/801104
17:28:41 opendevreview Stephen Finucane proposed openstack/placement master: db: Update 'select()' calls https://review.opendev.org/c/openstack/placement/+/801103
17:28:42 opendevreview Stephen Finucane proposed openstack/placement master: db: Use explicit transactions https://review.opendev.org/c/openstack/placement/+/801106
17:28:42 opendevreview Stephen Finucane proposed openstack/placement master: db: Replace deprecated 'FromClause.select().whereclause' parameter https://review.opendev.org/c/openstack/placement/+/801105
17:28:44 opendevreview Stephen Finucane proposed openstack/placement master: tox: Enable SQLAlchemy 2.0 warnings https://review.opendev.org/c/openstack/placement/+/801108
17:28:44 opendevreview Stephen Finucane proposed openstack/placement master: db: Remove unnecessary use of '_mapping' https://review.opendev.org/c/openstack/placement/+/801107
17:29:16 sean-k-mooney for example what happens if you move the ownwer ship of the neutron port of cinder volumes first before the nova instance
17:29:30 sean-k-mooney if we created the port and volume and delete on terminate is set
17:29:36 sean-k-mooney then you delete the instance
17:29:47 melwitt yeah... I think that's why we've nacked it in the past. if we add something like that, it needs a big fat warning on it. and maybe a dragon icon
17:29:48 sean-k-mooney what do we do then
17:30:05 sean-k-mooney but dragons are cool
17:30:21 sean-k-mooney we should put a honey badger on it instead
17:30:24 melwitt "if you do this and it messes stuff up, you are on your own"
17:30:33 melwitt hehe
17:31:07 dansmith how about we only allow the transition if the instance is locked?
17:31:24 dansmith I don't remember how much lock really restricts, and we've probably not done a good job of honoring lock everywhere,
17:31:39 melwitt yeah true dragons might attract people to call the API
17:31:43 dansmith but it would at least be something procedural

Earlier   Later