| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-17 | |||
| 15:30:55 | bauzas | mriedem: but we have a compute.utils helper module AFAIK | |
| 15:31:26 | bauzas | my main concern is that a modification would require two places | |
| 15:32:25 | mriedem | the only duplication is L698-L706 https://review.openstack.org/#/c/510938/5/nova/compute/manager.py | |
| 15:32:35 | bauzas | I agree | |
| 15:32:51 | mriedem | :) | |
| 15:32:51 | mriedem | whch is also in _delete_allocation_after_move now | |
| 15:33:03 | mriedem | and _rollback_live_migration | |
| 15:33:17 | bauzas | mriedem: but what if someone wants to add another call ? | |
| 15:33:33 | mriedem | so how about we do the consolidation into a util in a follow up? | |
| 15:33:47 | bauzas | I'm fine with that | |
| 15:33:50 | mriedem | hell, we could just do a private method inthe compute manager itself | |
| 15:33:52 | bauzas | I haven't reviewed the change | |
| 15:33:53 | mriedem | if you don't want to touch the RT | |
| 15:34:03 | bauzas | I just wanted to open a discussion | |
| 15:34:26 | mriedem | i'm cool with refactoring out the duplication, but not in this change since it has to be backported | |
| 15:34:27 | bauzas | but yeah, if we were doing the consolidation, would be worth doing all of it in a single patch | |
| 15:34:35 | bauzas | that's a good point | |
| 15:34:47 | bauzas | ok, I'll leave a comment and +W | |
| 15:45:02 | mriedem | bauzas: if you've got the time, we also need these stable/pike backports https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/pike+topic:bug/1715182 | |
| 15:45:15 | mriedem | lyarwood: ^ | |
| 15:45:27 | bauzas | mriedem: lyarwood is off sick | |
| 15:45:38 | mriedem | too much volume encryption will do that | |
| 15:47:15 | bauzas | I have an internal meeting until 4pm UTC, so I guess I have 13 mins of free time now :) | |
| 16:00:04 | melwitt | mriedem: I'm okay with this libvirt driver change now, to detach and reattach volumes during reboot. apparently there have been summit and PTG discussions about doing that in hard reboot for some time, so if that's the plan anyway, it seems fine to me https://review.openstack.org/#/c/400384 | |
| 16:00:36 | mriedem | i don't remember those discussions | |
| 16:02:36 | dansmith | I do, it was around the need to re-fresh the connection info for monitor ip changes, IIRC | |
| 16:02:54 | mriedem | that's not this though | |
| 16:04:16 | dansmith | I haven't looked at the patch, but okay | |
| 16:04:31 | dansmith | we did talk about re-building the storage connections on reboot for that reason though | |
| 16:05:21 | mriedem | yeah i remember that, and have the todo for it still | |
| 16:08:09 | mriedem | ok so we have an encrypted volume test in tempest, i could just modify that to hard reboot the instance after an ecrypted volume is attached and it should blow up the instance right? | |
| 16:08:10 | dansmith | efried: did alex_xu get everything resolved that he was concerned about on that spec? I saw a bunch of discussion here | |
| 16:08:40 | melwitt | mriedem: I think so, yes | |
| 16:09:02 | mriedem | melwitt: ok in that case i'm going to tweak tempest to recreate the failure and then make it depend on the nova fix | |
| 16:09:30 | mriedem | melwitt: jbernard: https://review.openstack.org/#/c/512622/ is passing | |
| 16:09:38 | mriedem | we need to get that in asap to unblock nova stable | |
| 16:09:43 | mriedem | jdurgin: ^ | |
| 16:09:59 | melwitt | mriedem: sounds like a good idea | |
| 16:10:25 | melwitt | mriedem: you did a Depends-On to see it pass on stable? | |
| 16:10:59 | mriedem | melwitt: i can quick | |
| 16:11:25 | melwitt | okay, just wanted to make sure that's going to work for you | |
| 16:11:47 | efried | dansmith Not really. We need another writeup. OTP, give me a few mins. | |
| 16:11:53 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: libvirt: add check for VIR_DOMAIN_BLOCK_REBASE_COPY_DEV https://review.openstack.org/512406 | |
| 16:11:55 | mriedem | melwitt: ^ | |
| 16:12:10 | dansmith | efried: more in this spec? i thought he was suggesting another spec.. but sure, np | |
| 16:13:56 | efried | dansmith Which spec are we talking about? | |
| 16:14:19 | dansmith | efried: your granular request word-a-palooza | |
| 16:16:24 | efried | dansmith Oh, no, that should be solid as is. Whatever else needs to come out of that will have to go... somewhere else. | |
| 16:16:45 | dansmith | efried: okay that's what I thought, but wanted to clarify | |
| 16:17:00 | dansmith | so I was going to drop my +2 on there, but didn't want to if alex_xu had more things implied from his +0 earlier | |
| 16:33:20 | jbernard | mriedem, melwitt: +A'd, thanks ya'll | |
| 16:33:50 | melwitt | jbernard: awesome, thanks | |
| 16:33:55 | mriedem | sean-k-mooney: comments inline https://review.openstack.org/#/c/502306/ | |
| 16:34:15 | mriedem | sean-k-mooney: honestly i think that's going to be a very tough mountain to climb at this point given all of the dependencies | |
| 16:34:32 | mriedem | sean-k-mooney: there seems to be a fundamental mistaken assertion in there that nova creates ports before a host is chosen | |
| 16:34:34 | mriedem | which is not the case | |
| 16:34:45 | mriedem | jbernard: thanks | |
| 16:35:12 | sean-k-mooney | mriedem: i taught nova called the create_or_get port api before calling placement? | |
| 16:35:40 | mriedem | sean-k-mooney: no, ports are created during allocate_for_instance in the compute service | |
| 16:35:40 | sean-k-mooney | mriedem: but thank for reviewing. yes this does have a lot of dependncies | |
| 16:35:44 | mriedem | after a host is chosen by the scheduler | |
| 16:36:13 | mriedem | johnthetubaguy has an older spec to move port creation to conductor, which might be another dependency for this | |
| 16:36:33 | mriedem | sean-k-mooney: isn't the qos policy applied to the port? | |
| 16:36:34 | sean-k-mooney | mriedem: but how will it know if it should be creating a sriov port or normal port if it only did it at the compute node | |
| 16:36:44 | mriedem | sean-k-mooney: nova doesn't create sriov ports | |
| 16:36:50 | mriedem | nova creates regular old boring ports | |
| 16:37:00 | mriedem | if you need a fancy nfv port, you create it in neutron and provide it when creating the server | |
| 16:37:28 | mriedem | if the qos policy is applied to the port, and nova creates the port, then i think we assert that nova-created ports don't have minimum bandwidth guarantees | |
| 16:37:30 | mriedem | easy peasy | |
| 16:37:32 | sean-k-mooney | mriedem: yes the qos policy is on the port but it can also be on the network which adds it to the port automatically | |
| 16:37:40 | mriedem | if you want the fancy, you precreate the port | |
| 16:37:53 | mriedem | ok then that gets complicated, | |
| 16:38:03 | sean-k-mooney | mriedem: yes you are right sriov or qos implies you precreate the port | |
| 16:38:05 | mriedem | because you can create a server with a nic uuid and nova will create the port in that network | |
| 16:38:21 | johnthetubaguy | that sounds a lot like the routed network case that triggered the move to conductor thingy | |
| 16:38:38 | mriedem | and if the qos policy on the network doesn't match the compute host i guess you have problems | |
| 16:38:41 | mriedem | johnthetubaguy: yes | |
| 16:40:15 | johnthetubaguy | sean-k-mooney: if you want to take on the refactor it is now I think just a case of moving the code, we create missing ports (the conductor bit), then update all the ports (the compute bit) | |
| 16:40:16 | mriedem | the spec also doesn't really mention any details about move operations, which we know are the hairiest thing to deal with | |
| 16:41:06 | mriedem | because during a move the allocations have to be made on the target host and eventually removed from the source host - is the neutron agent going to screwed up by that? | |
| 16:41:12 | mriedem | *to get | |
| 16:41:59 | sean-k-mooney | mriedem: it should not as neutron will not be doing allocations from placement iteself | |
| 16:42:30 | sean-k-mooney | mriedem: but that said we have not gone into details of the move operations in the spec | |
| 16:42:52 | melwitt | stephenfin: I refactored the unit tests to address your comment https://review.openstack.org/#/c/498983 | |
| 16:43:36 | mriedem | sean-k-mooney: that was another thing, the spec loosely talked about modifying the nova resource tracker, i think to create allocations for these bandwidth things, but we want the scheduler to create allocations, not the compute (RT) | |
| 16:44:09 | mriedem | because when you move the instance, the source host RT might overwrite the allocations for the instance which are meant for the dest host | |
| 16:44:27 | mriedem | because of periodic task race window wackiness | |
| 16:45:25 | sean-k-mooney | mriedem: yes in the future we defiently want to move the allocation out of the compute node the sepc was assuming the workflow we have today where the compute node does the allocation but i dont think it would break if the allocation was done earlier | |
| 16:46:19 | mriedem | sean-k-mooney: if today is >= pike, we don't create allocatoins in the computes anymore | |
| 16:46:31 | mriedem | allocations in the computes is soooooo 2 months ago :) | |
| 16:46:38 | sean-k-mooney | mriedem: oh did that land in pike cool | |
| 16:46:47 | mriedem | yeah i linked to the change in the spec | |
| 16:47:05 | mriedem | as a result of that change we've been doing a lot of cleanup of places where we dont cleanup allocations | |
| 16:47:10 | mriedem | still working on getting those fixes into stable/pike | |
| 16:47:10 | sean-k-mooney | mriedem: so is that now done in the conductor/scheduler | |
| 16:47:18 | mriedem | the scheduler claims yes | |
| 16:47:27 | mriedem | for VCPUS/MEMORY_MB/DISK_GB | |
| 16:47:43 | mriedem | if you have NUMATopologyLimits, those get claimed in the compute still, | |
| 16:47:53 | mriedem | since we don't model NUMA via nested RPs in placement yet | |
| 16:48:18 | sean-k-mooney | pci device im guessing are still on compute node too for same reason | |