| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-08 | |||
| 16:12:06 | lyarwood | mdbooth: it's hardly band-aid | |
| 16:12:08 | mdbooth | Are we handling that somewhere else? | |
| 16:12:21 | efried | cdent I'm writing up a summary and adding it to the review (https://review.openstack.org/#/c/526539/) | |
| 16:12:35 | cdent | efried++ | |
| 16:15:12 | lyarwood | mdbooth: https://bugs.launchpad.net/nova/+bug/1633033 - sorry that took a little digging | |
| 16:15:15 | openstack | Launchpad bug 1633033 in OpenStack Compute (nova) "live migration with encrypted volume fails" [Undecided,In progress] - Assigned to Lee Yarwood (lyarwood) | |
| 16:16:02 | mdbooth | lyarwood: Ok, so looks like that's broken too, same root cause | |
| 16:16:07 | lyarwood | yup | |
| 16:16:21 | mriedem | lyarwood: mdbooth: why don't you do the short-term backportable fix and then do the refactor on master which isn't backported? | |
| 16:16:32 | mriedem | don't refactor a bunch of code that you're going to backport | |
| 16:16:33 | mdbooth | Looking back over my notes, actually I think the _get_guest_xml fix is really easy | |
| 16:17:14 | lyarwood | mriedem: I'm trying to suggest that, I really don't want to play around with _get_guest_xml in backports | |
| 16:17:42 | mdbooth | lyarwood: The reason is that everywhere that calls _get_guest_xml either subsequently calls _create_domain_and_network, or doesn't pass block_device_info, so doesn't call _connect_volume | |
| 16:18:08 | mdbooth | Which means that all you need to do is move _connect_volume from _get_guest_xml to _create_domain_and_network | |
| 16:18:15 | mdbooth | Which is also totally intuitive | |
| 16:18:28 | mdbooth | I've audited ^^^, but feel free to do the same | |
| 16:18:51 | mdbooth | _create_domain_and_network already has context, which also makes sense | |
| 16:19:03 | mdbooth | So that change isn't going to be invasive at all | |
| 16:19:04 | matrohon | mriedem: I'm not sure networks=none would help me. My VM needs a port, but a port without IP. | |
| 16:19:29 | mriedem | matrohon: yeah it won't | |
| 16:19:34 | mriedem | networks=none means no ports | |
| 16:19:48 | mriedem | i didn't know about ip_allocation='none' | |
| 16:20:01 | mdbooth | lyarwood: Whatever way you look at it, we need context in swap_volume anyway | |
| 16:20:09 | mdbooth | So that part needs to be done anyway | |
| 16:20:14 | lyarwood | mdbooth: and is in the bugfix | |
| 16:20:31 | mdbooth | lyarwood: I don't see it in that review stack | |
| 16:20:47 | lyarwood | mdbooth: https://review.openstack.org/#/c/531233/3/nova/virt/libvirt/driver.py | |
| 16:20:50 | mdbooth | NP, now I do | |
| 16:21:03 | matrohon | mriedem: ok, I can work on fixing 1741575, with the attached patch as a first step | |
| 16:21:11 | mdbooth | lyarwood: Ok, that bit's already done. | |
| 16:21:37 | mdbooth | With those bits in place, moving the encryptor call into _connect/_disconnect is trivial | |
| 16:21:46 | mdbooth | And no harder to backport | |
| 16:21:52 | mdbooth | And also fixes pre_live_migration | |
| 16:22:04 | mdbooth | And anything else we missed | |
| 16:22:33 | mdbooth | Does lxc have encryptors? | |
| 16:23:01 | matrohon | mriedem: do you think I need something else than the bug report to start submitting code review? | |
| 16:23:14 | lyarwood | mdbooth: well, the frontend encryptors are host level so yeah, I can't see why not. | |
| 16:23:23 | lyarwood | mdbooth: not that they are tested anywhere. | |
| 16:23:30 | mdbooth | lyarwood: Ok, that's probably a bug too, then | |
| 16:23:52 | mdbooth | So it's a 3 for 1 fix | |
| 16:24:02 | openstackgerrit | Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766 | |
| 16:24:22 | lyarwood | mdbooth: right, I'll respin without moving _connect_volume out of _get_guest_xml | |
| 16:24:35 | mdbooth | I don't think you can do that | |
| 16:24:54 | lyarwood | right, context | |
| 16:25:02 | mdbooth | Actually it's already there | |
| 16:25:19 | mdbooth | But you'd have to remove the encryptor stuff from _create_domain_and_network | |
| 16:25:28 | mdbooth | Which is just backwards | |
| 16:25:32 | mriedem | matrohon: no | |
| 16:25:35 | mdbooth | Might as well go the other way | |
| 16:25:37 | mriedem | bug fix is fine | |
| 16:25:50 | matrohon | mriedem: ack, thanks | |
| 16:25:58 | mriedem | matrohon: but i looked at the patch and it doesn't really make sense to me, unless a port with ip_allocation='none' results in port.get('ip_allocation') == None? | |
| 16:27:12 | efried | jaypipes cdent What is the behavior when I try to delete a provider that has "stuff" associated with it? | |
| 16:27:34 | efried | I think I've seen some cases where we automatically delete the "stuff" and some where we bounce. | |
| 16:27:44 | cdent | fails, if there are allocations we bounce | |
| 16:27:48 | matrohon | mriedem: port.get('ip_allocation') == None is the unexpected result, I'm currently trying to find the reason why. I probably need to fix that first on the neutron side | |
| 16:27:55 | cdent | I think that's the only one, but let confirm | |
| 16:28:28 | efried | cdent I'm talking about inventory, aggregates, traits - but most especially aggregates at the moment. | |
| 16:29:09 | jaypipes | efried: we just delete. | |
| 16:29:27 | jaypipes | efried: allocations are the only thing that prevents deletion (and child providers) | |
| 16:29:28 | mriedem | matrohon: if neutron needs changes to support this case, then things get sticky with regards to interoperability | |
| 16:29:39 | cdent | efried: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L816 | |
| 16:29:44 | efried | jaypipes Okay. So this is tricky. | |
| 16:29:55 | mriedem | because if you upgrade nova to handle this none scenario but neutron doesn't support it, things fail, and vice-versa | |
| 16:30:22 | efried | cdent Thanks, it's all right there! | |
| 16:31:36 | edleafe | mriedem: just getting caught up (sitting in traffic is *so* productive!) | |
| 16:31:43 | edleafe | mriedem: your changes look fine | |
| 16:32:43 | edleafe | mriedem: some of the issues you asked about on the review were from the original test, which was supposed to test alternate host usage; hence the 4 computes and such | |
| 16:33:02 | matrohon | mriedem: but ip_allocation | |
| 16:33:04 | edleafe | I'm going to add that test to the resize patch | |
| 16:33:09 | matrohon | mriedem: sorry | |
| 16:33:17 | efried | ggregate) so we'll... drop it from the local provider tree at that time, somehow. | |
| 16:33:17 | efried | cdent jaypipes So this might be okay. A virt that's "leaving" an aggregate should only delete the aggregate association from self, not delete it from the aggregate or delete the shared provider. The sharing provider will still show up in the ProviderTree, so report client won't delete it. Then the next time we pull the compute RP and its associations, that sharing provider will be absent (because no longer associated by a | |
| 16:33:22 | mriedem | edleafe: add the test to the resize patch? | |
| 16:33:31 | mriedem | edleafe: the test is already there since it comes before it in the series | |
| 16:33:51 | edleafe | mriedem: the regression test really doesn | |
| 16:34:06 | edleafe | doesn't check that the alternate hosts stuff is working as expected | |
| 16:34:16 | edleafe | just that it doesn't blow up :) | |
| 16:34:23 | cdent | efried: I think that sounds right, but as your patient, I'm going to the bath, will bbl | |
| 16:34:28 | mriedem | so you're going to modify the regresssion test to add to it in the resize patch | |
| 16:34:42 | efried | cdent Enjoy | |
| 16:35:29 | edleafe | mriedem: no, more or less copy parts to make a new functional test that's alternate host-specific | |
| 16:35:30 | ktibi | dansmith Hi do you know why consecutive_build_service_disable_threshold default value is 10 ? | |
| 16:35:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add pagination and Changes-since filter support for os-migrations. https://review.openstack.org/330406 | |
| 16:35:47 | ktibi | because I found a method for disable all compute service with a simple user :/ | |
| 16:35:57 | matrohon | mriedem: I'll keep on digging on the neutron side first, since not having the ip_allocation field set in port's attributes retrieved by a REST request, when it is explicitly set in the database is not really clean | |
| 16:36:03 | dansmith | ktibi: yes, because I set it to 10 after discussion in the room with a bunch of operators | |
| 16:36:16 | matrohon | mriedem: don't you think? | |
| 16:36:45 | mriedem | matrohon: i agree that None != unset and there are differences in client side behavior potentially, yes | |
| 16:36:46 | dansmith | ktibi: what's the method? might be something we need to exclude from the disable logic if it's not really a fault | |
| 16:36:56 | mriedem | matrohon: but i don't really know how that's all used on the neutron side | |
| 16:37:04 | ktibi | ok, 10 is good but for now, you can fool nova for disable all compute | |
| 16:37:29 | openstackgerrit | Matthew Booth proposed openstack/nova master: Don't connect volumes in _get_guest_storage_config https://review.openstack.org/531852 | |
| 16:37:32 | jaypipes | efried: yeah, that sounds about right to me, too. | |
| 16:37:36 | mdbooth | lyarwood: ^^^ | |
| 16:37:50 | efried | jaypipes Dangit, that still leaves us with orphan-able shared providers. | |
| 16:37:54 | mdbooth | lyarwood: Lets see how that does against tempest, but that change would be extremely backportable | |
| 16:38:04 | mriedem | ktibi: perhaps you'd like to share how users can do that | |
| 16:38:05 | efried | (aside from being asymmetrical) | |
| 16:38:09 | ktibi | dansmith if you spawn multi instances in a network with 0 free IP, nova fail to spawn and increase the trigger. if you deploy ~60 instances, you disable 6 computes | |