Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-21
15:07:51 jroll except that a rebalance happens when the number of compute services changes. so if you shut down a compute service (which deletes compute nodes and RPs), ironic nodes get moved to another, and those compute nodes and RPs are re-created
15:07:54 mriedem it knows, "oh i was managing node x before, but now the driver says i'm not, so delete"
15:08:29 mriedem jroll: yeah and before rocky, those new compute nodes and RPs would all have a unique uuid
15:08:34 mriedem since rocky, they re-use the node uuid
15:08:48 jroll yeah
15:09:04 mriedem so i think with my recent fix that's all fine,
15:09:16 mriedem the question is what happens with allocations against the re-balanced node resource provider
15:09:20 mriedem b/c i'm guessing since stein those are gone
15:09:23 sean-k-mooney mriedem: right but im wonder that im not "managing because the drive said so, therefor delete is required"
15:09:28 jroll yeah, that's what worries me
15:09:54 mriedem i guess someone needs to do some testing
15:10:00 sean-k-mooney in a libvirt case we would normaly delete the compute service and have it delete the compute nodes under neat it
15:10:26 mriedem sean-k-mooney: the compute service doesn't delete itself,
15:10:28 mriedem that happens in the API
15:10:34 mriedem DELETE /os-services/{service_id}
15:10:35 sean-k-mooney well yes
15:10:41 mriedem which is related to a number of other fun issues
15:10:43 sean-k-mooney i know that but that is what i ment
15:10:44 mriedem which are in the ML
15:11:07 sean-k-mooney we externally trigger the deletion of the compute service when removing a libvir host
15:11:21 sean-k-mooney does vmware do something similar to ironic
15:11:44 sean-k-mooney they have multiple compute nodes in the same service too right
15:11:51 mriedem http://lists.openstack.org/pipermail/openstack-discuss/2019-June/007135.html
15:12:06 mriedem sean-k-mooney: no, vmware driver does not do that since kilo or liberty
15:13:44 mriedem TheJulia: tl;dr, i think you need https://review.opendev.org/#/q/topic:bug/1839560+branch:stable/stein for your grenade failures,
15:14:01 mriedem and there might be a regression due to https://review.opendev.org/#/c/602481/ where allocations in placement are lost during a re-balance
15:14:06 mriedem but someone needs to test that
15:14:19 TheJulia mriedem: sean-k-mooney: I added a depends-on flag to one of my test patches for the patches above, if I see the issue re-appear on that job, I'll let you all know
15:18:33 mriedem so if the lost allocations on re-balance is a real thing, i might have a solution, which i wrote down in a comment in https://review.opendev.org/#/c/602481/
15:19:01 mriedem b/c the code that deletes the resource provider and it's allocations pre-dates the removal of the RT allocation healing code, and unsurprisingly we lost track of all those random one off moving parts
15:19:29 mriedem especially since we don't have functional testing of that stuff in tree
15:37:40 openstackgerrit Adam Spiers proposed openstack/nova master: libvirt: Parse the 'os' element from domainCapabilities https://review.opendev.org/673790
15:39:26 gibi mriedem: hi! I have no problem moving the small parts of the cross-cell resize series to the front. I can re-apply my +2 if you rebase the series
15:39:57 mriedem gibi: ok thanks
15:46:42 openstackgerrit Matt Riedemann proposed openstack/nova master: FUP for I5576fa2a67d2771614266022428b4a95487ab6d5 https://review.opendev.org/677769
15:55:45 aspiers anyone object to me splitting libvirt/test_driver.py into smaller pieces? at 23k LoC it's almost *double* the size of the next largest file in nova (compute/test_compute.py in case you're curious)
15:55:58 aspiers my editor really hates such a huge file, and so do I
15:56:09 aspiers efried_afk, sean-k-mooney: ^^^
15:56:40 aspiers of course this will probably cause a few merge conflicts so maybe we need to time it right
15:56:55 sean-k-mooney aspiers: you mean it does not like 10s of 1000s or lines
15:57:41 aspiers sean-k-mooney: yes, syntax highlighting something that big is really expensive
15:57:56 aspiers unless your highlighting algorithm cuts corners
15:58:43 sean-k-mooney pycharm does not mind too much
15:58:52 sean-k-mooney once it has built the index
15:59:53 aspiers Yikes this is a huge list https://review.opendev.org/#/q/project:openstack/nova+path:nova/tests/unit/virt/libvirt/test_driver.py+status:open+branch:master
16:00:16 aspiers Still, we could split stuff out in smaller steps to be less disruptive
16:00:34 aspiers I was thinking to move all the capabilities / domcaps tests out for a start
16:01:08 aspiers libvirt/driver.py is 10k LoC, that should also be split up
16:02:24 aspiers anyway I'm done for today
16:02:32 aspiers probably won't do much the rest of this week
16:02:40 aspiers since I've been doing way too much overtime recently
16:12:30 sean-k-mooney aspiers: yes it shoudl but with the rate of change that is hard to do without a dedicated effort
16:12:52 sean-k-mooney aspiers: and ya dont burn out.
16:21:05 mriedem efried_afk: did cern reach out to you about the deepcopy of the ProviderTree? i think it came up last week, but was reminded after reading https://techblog.web.cern.ch/techblog/post/nova-ironic-at-scale/
16:21:56 openstackgerrit Matthew Booth proposed openstack/nova master: libvirt: Fix service-wide pauses caused by un-proxied libvirt calls https://review.opendev.org/677736
16:22:37 mriedem aspiers: you mean moving driver.py tests out of test_driver.py? but leave the code in driver.py? that would be confusing to me, and becomes a mess for git history tracking
16:23:25 mriedem there is probably a ton of copy/paste patterns in libvirt driver tests that could be a simple cleanup to start with, rather than moving things *out* of the test_driver.py module
17:02:49 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Get instances outside of update_available_resource lock https://review.opendev.org/677790
17:29:59 openstackgerrit Merged openstack/nova master: Move calls to ovs-vsctl to privsep. https://review.opendev.org/639282
17:32:01 openstackgerrit Merged openstack/nova master: Move setting of device trust to privsep. https://review.opendev.org/639283
17:44:58 efried_afk aspiers: I love the idea of splitting huge files. sean-k-mooney, syntax highlighting isn't the problem so much as the inspection. Every change causes it to reinspect the file, which takes forever on monsters like test_libvirt. If I'm spending any amount of time there, I have to turn inspection off.
17:45:50 dansmith efried_afk: without any other context or target articles,
17:46:01 dansmith that first sentence sounds like it belongs on your online dating profile
17:46:10 dansmith "I like long walks on the beach and splitting huge files"
17:47:03 sean-k-mooney efried_afk: i have not had that problem but it does annoy me that thost files are so large
17:47:06 efried dansmith: It was a straight copy/paste
17:47:14 dansmith efried: I figured
17:47:46 dansmith Male seeking cybernetic female analog capable of introspection of long python files and/or automated splitting thereof
17:47:52 efried mriedem: I don't recall anything specific about a ProviderTree deepcopy...
17:48:25 efried oo, that's good dansmith. Clearly you have a lot of experience here.
17:48:38 dansmith my kink knows no bounds
17:51:23 efried dansmith: are you happy with https://review.opendev.org/#/c/671072/ series at this point?
17:52:11 efried gibi: also ^
17:52:15 dansmith efried: I haven't circled back
17:56:00 openstackgerrit Eric Fried proposed openstack/nova master: Remove deprecated [neutron]/url option https://review.opendev.org/677755
17:56:16 aspiers mriedem: I would happily move the corresponding code out from driver.py at the same time if that was feasible
17:57:03 aspiers I'm not sure why it would become a mess for git history tracking? even git blame has for a long time been able to figure out when stuff gets moved between files
17:57:17 aspiers efried: exactly
17:57:47 mriedem i'm not a fan of the tests for something in one module living in another module
17:57:48 aspiers mriedem: that said, pointers on stuff to deduplicate are welcome
17:58:00 mriedem and like i said, i think you can just refactor the duplication in test_driver.py for lots of tests to probably make it smaller
17:58:34 aspiers I think our messages crossed :) ^^^
17:58:43 mriedem this isn't an exercise i'm the one that's wanting to take on, so that's up to you,
17:58:55 mriedem but i know there is a ton of just copy test foo and tweak 1-2 lines for test bar
17:59:12 mriedem i also have no problem with the thing in pycharm
18:00:00 efried aspiers: git history is an issue, unfortunately - afaik blame will show the new commit on the new file. You would have to back up to see the previous iteration, which is awkward. (I've run into this a number of times already in placement, and it's pretty debilitating.)
18:00:25 mriedem correct, it sucks
18:00:36 mriedem the mox removal series is a prime example
18:00:45 mriedem it also causes issues with merge conflicts for backports
18:00:53 sean-k-mooney well perhaps this is jsut somethin we need to be aware of and think should we be moving things out of the driver into modules used by the driver in the future
18:01:04 aspiers efried: no, blame really can show where it came from
18:01:25 sean-k-mooney aspiers: yes but git blame on github crashes on those files
18:01:29 sean-k-mooney so you have to do it locally
18:01:43 aspiers ?
18:01:49 efried what's the blame trick, without iterating backward across commits?
18:01:58 aspiers mriedem's point about backports is a much more convincing argument against
18:02:19 aspiers however, if our policy is "you can't move stuff around, ever" then we're in for a miserable future
18:02:50 mriedem i'm not against refactoring giant methods in the libvirt driver module, deduplicating test code in test_driver, etc. i just don't want to see random tests moving out of test_driver into some other module when the code being tested is still in driver.py
18:03:20 aspiers mriedem: sure, that's why I said <aspiers> mriedem: I would happily move the corresponding code out from driver.py at the same time if that was feasible
18:03:21 mriedem aspiers: no one said that was the policy

Earlier   Later