Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-21
15:02:41 jroll and I really don't have time to deep dive this part of nova today :(
15:02:52 mriedem since i never think if ironic node re-balancing stuff, and we don't test it (at least in nova) i guess i didn't think about this
15:03:17 mriedem i would also think that johnthetubaguy (who added this workaround i think) and stackhpc would have noticed a problem by now
15:03:27 mriedem but i don't know what version of openstack they are giving to customers
15:03:38 mriedem https://review.opendev.org/#/c/602481/ was stein
15:04:08 sean-k-mooney when we are rebalcne we are moving the compute nodes between compute service right. and in an ironic case we have mupliple compute nodes (1 per server) each with there one placment RP associates to a singel compute service correct
15:04:18 sean-k-mooney just making sure i have that right in my head
15:04:44 mriedem ironic node : compute node : instance = 1:1:1
15:04:56 jroll correct, until the end - there is a placement RP for each ironic node / compute node
15:04:56 mriedem + : resource provider
15:05:27 sean-k-mooney jroll: yep that is want i ment
15:05:40 sean-k-mooney one placment RP per compute/ironic node
15:05:53 jroll cool, you are correct then
15:06:36 sean-k-mooney durign a reblance we are not actully delet ing he nova compute service we are just moveing the comnpute nodes so we really should not need to update placmeent
15:06:49 sean-k-mooney or rather delete the compute node/placmente RP
15:07:30 mriedem the compute manager doesn't know you're doing a re-balance
15:07:31 sean-k-mooney all we should need to do on the nova side is modify the service to compute node mapping but i dont know how we do that today
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,

Earlier   Later