| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 13:17:05 | Spazmotic | Doesn't really feel like it follows any of the standards of how we handle data sets generally and doesn't allow for granular level of control over allocations without touching entire consumers data sets | |
| 13:17:07 | Spazmotic | Feels dirty | |
| 13:18:06 | fried_rice | Spazmotic You mean because you have to set/replace an entire consumer_uuid's allocations all at once? | |
| 13:18:12 | Spazmotic | Yeah | |
| 13:18:23 | Spazmotic | Is that really the elegant solution? | |
| 13:18:35 | Spazmotic | I guess i'm not sure what we gain by clean sweeping | |
| 13:19:29 | fried_rice | Spazmotic I can see where that's going to be suboptimal in the long game of placement, where we could have multiple control points managing resources for a single consumer. Each one would have to GET the current state, make its changes, PUT back the changed set, and deal with 409s if a concurrent update beat them to it. | |
| 13:20:17 | fried_rice | Spazmotic I'm guessing it was designed this way as the most expeditious and convenient for the initial use case, which is nova compute host as single resource provider, nova instance as consumer. | |
| 13:21:05 | Spazmotic | It defiantely could get racey, it also allows for less control of allocations except for directly outside of consumers which may make it a little more unwiedy in a larger multi-control point environment for same tenant | |
| 13:22:05 | fried_rice | Spazmotic Actually, yeah, I don't see a consistency marker like we have for traits & inventories. | |
| 13:22:54 | fried_rice | leakypipes Has this been considered ^ ? | |
| 13:23:32 | mdbooth | lyarwood: Hey, found a test problem. Still reviewing but I'm going to drop what I've got right now as I think you need to fix it. | |
| 13:24:49 | lyarwood | mdbooth: the py35 failures? | |
| 13:25:03 | mdbooth | I hadn't even seen those. | |
| 13:25:19 | lyarwood | mdbooth: awesome, so more issues to fix :) | |
| 13:25:20 | mdbooth | The problem in test_migration. I don't think that test is right. | |
| 13:25:28 | lyarwood | mdbooth: kk | |
| 13:27:14 | lyarwood | mdbooth: right, the old secret should be replaced by the new secret but we aren't creating a new secret UUID here, that's just passed in via migrate_data | |
| 13:27:44 | lyarwood | the device replace is odd and something copied over from the above test | |
| 13:28:01 | mdbooth | Yeah, I saw that. It's weird there, too. | |
| 13:28:15 | Spazmotic | Would be very nice if we could extend that API a bit for more functionality and ease of use. | |
| 13:28:38 | mdbooth | lyarwood: The pattern of those tests is xml=old xml | |
| 13:29:01 | mdbooth | new_xml = s/old thing/thing which should be changed upon migration in this test/ | |
| 13:29:17 | mdbooth | assert(we got new_xml) | |
| 13:30:34 | mdbooth | I didn't look hard at why the test above would expect device name to change on live migration. I can't imagine why we'd ever want that. However, it's SEP right now. | |
| 13:31:11 | lyarwood | mdbooth: ah, the sdb / sdc thing is also a migrate_data thing | |
| 13:31:26 | mdbooth | Yes. It's passed in. | |
| 13:31:32 | lyarwood | mdbooth: so it's the target device changing | |
| 13:31:32 | mdbooth | Still looks bogus. | |
| 13:31:36 | lyarwood | mdbooth: not in the instance | |
| 13:31:50 | lyarwood | mdbooth: so on the host, the block device is just wired up under /dev/sdc | |
| 13:31:53 | lyarwood | mdbooth: that's valid | |
| 13:31:59 | lyarwood | mdbooth: just confusing as the test is using rbd | |
| 13:32:04 | mdbooth | Ah... you looked harder :) | |
| 13:32:17 | mdbooth | Ok, that makes sense | |
| 13:32:47 | Spazmotic | fried_rice, it also feels a little bit like they intended to use the zeroing out of the resources in the allocation for something but forgot. If no allocations are set for a tenant it assumes you want a clean sweep so it goes through and sets them all to 0, but then goes through and deletes them anyway from the dB after that | |
| 13:33:13 | mdbooth | lyarwood: Anyway, following the pattern of those tests, the search/replace we want to see there is old secret for new secret | |
| 13:33:16 | Spazmotic | Kinda wierd.. i'll need to continue looking but it's almost time for me to head to bed and dream about the process hehe | |
| 13:33:23 | lyarwood | mdbooth: ack, done | |
| 13:33:26 | fried_rice | Spazmotic That may have been for the original migration case | |
| 13:34:11 | fried_rice | Spazmotic Which was found to have some pretty hairy implications (read: bugs). Which is why we introduced the POST API there, to allow us to "move" the allocations from one consumer to another in a single atomic operation. | |
| 13:34:34 | fried_rice | dansmith and cdent (neither of whom is here at the moment) ought to be able to shed more light on what happened there. | |
| 13:35:11 | lyarwood | mdbooth: re https://review.openstack.org/#/c/523958/15/nova/tests/unit/virt/libvirt/test_driver.py@10526 - the only values of src_supports_native_luks are True and None, testing both of these above | |
| 13:35:28 | Spazmotic | Sounds good sir.. i'll continue to poke it and will read scrollback when I wake up to see if anything new :) | |
| 13:35:43 | mdbooth | lyarwood: Is it never set to False? | |
| 13:35:56 | Spazmotic | I see now what you mean about the migrations.. that would be useful in that case. | |
| 13:36:02 | fried_rice | Spazmotic But I can definitely see the need for concurrency management of allocations; and also the usefulness of a more granular API to add/remove/update individual allocations - but concurrency management would have to be a prereq of that. | |
| 13:36:09 | lyarwood | mdbooth: https://review.openstack.org/#/c/523958/15/nova/virt/libvirt/driver.py@6253 - nope | |
| 13:36:19 | lyarwood | mdbooth: it's really checking that n-cpu is >= Queens | |
| 13:36:33 | lyarwood | mdbooth: not that the src host can actually decrypt LUKS via QEMU | |
| 13:36:41 | lyarwood | mdbooth: as we are only creating the volume config on the src | |
| 13:36:52 | Spazmotic | Alrighty, i'll think on it man.. thanks for helping me clarity what was happening | |
| 13:37:09 | fried_rice | Spazmotic Glad to bounce around ideas, for what it's worth :) | |
| 13:37:23 | mdbooth | lyarwood: Yep, I'd forgotten that detail. Ignore me. | |
| 13:38:15 | mdbooth | lyarwood: Ah, reading on... I didn't forget, you changed it :) | |
| 13:38:23 | mdbooth | But that's cool | |
| 13:38:51 | lyarwood | mdbooth: ^_^ yeah I think it did check the installed versions in a different PS | |
| 13:39:39 | Spazmotic | Going ot be another night laying in bed thinking about this damn API hehe.. night everyone. Tonight I shall think of Semaphores for this system | |
| 13:40:16 | Spazmotic | I will need to look at how Nova handles its semaphores.. not something i've messed with too much | |
| 13:40:48 | mdbooth | lyarwood: Do we have a test for adding native encryption to xml which doesn't currently have it? | |
| 13:40:56 | mdbooth | lyarwood: I think the answer's no. | |
| 13:41:52 | lyarwood | mdbooth: during LM? No. | |
| 13:42:31 | mdbooth | lyarwood: Should be mostly cut/paste hopefully | |
| 13:44:41 | mdbooth | lyarwood: Couple more nits. If you're respinning, you probably ought to remove the unrelated whitespace change. | |
| 13:45:32 | Spazmotic | And before I goto bed will ask again for good luck, if anyone knows XenAPI well and has some free cycles away from their important reviews, feel free to take a look at https://review.openstack.org/#/c/533168/ :) | |
| 13:46:10 | lyarwood | mdbooth: ack, already have :) | |
| 13:47:00 | Spazmotic | I think this eventaully needs to be changed to use nova.objects.block_device.is_volume() but that's something that can be resolved a bit later to pull in CinderV3 into VMOps | |
| 13:47:04 | Spazmotic | Have a good night everyone :) | |
| 13:59:41 | rabel | is the default quota-class deprecated by now? it seems not to be considered by nova in any way | |
| 14:04:03 | claudiub | lyarwood: sorry, i was in a meeting. did you need any help with mock autospec? | |
| 14:04:44 | lyarwood | claudiub: no sorry, I'm fine, I was just talking to mdbooth about them earlier and I fried_rice was offering help :) | |
| 14:04:56 | lyarwood | I think* | |
| 14:05:05 | claudiub | cool. :) | |
| 14:05:36 | claudiub | anyways, it's been a lingering issue since last year. recently, we've merged a patch to oslotest which should help with it. you can take a look here: https://github.com/openstack/oslotest/blob/master/doc/source/user/mock-autospec.rst | |
| 14:05:56 | claudiub | and this is the 1st patch addressing autospec issues in nova: https://review.openstack.org/#/c/447505/37 | |
| 14:06:14 | mdbooth | lyarwood: It's not a high priority given that we almost never use autospec, btw. I have had autospec catch a couple of bugs for me in the past, though. | |
| 14:06:51 | mdbooth | lyarwood: I was thinking of it as I thought I'd seen you using it. | |
| 14:08:43 | claudiub | i do recommend using autospecs whenever possible, especially when you're using external libraries which might change over time. mocking them normally will make your unit tests pass even if the method signatures changed, which is not ok. :) | |
| 14:10:40 | fried_rice | mdbooth Yeah, one of the reasons we don't use autospec as much as we could/should is because there were bugs in it. claudiub just fixed some of those, so let the floodgates open! | |
| 14:11:13 | artom_ | bauzas, https://bugs.launchpad.net/nova/+bug/1744325 | |
| 14:11:15 | openstack | Launchpad bug 1744325 in OpenStack Compute (nova) "If a rebuild is refused by the scheduler, the instance's imageref is not rolled back" [Undecided,New] | |
| 14:11:39 | artom | mdbooth, ^^ if you care | |
| 14:11:41 | bauzas | artom: yup, I saw your internal discussion | |
| 14:11:46 | mdbooth | artom: I do, thanks | |
| 14:12:45 | TheJulia | Greetings nova folks! Over in the land of ironic, we've been encountering a condition in our multinode grenade job where while underlying libraries are being upgraded and nova is not upgraded from pike to master. What we're seeing is the nova conductor exiting with SEGV, and then looping causing all sorts of other issues. Has anyone seen anything like this? | |
| 14:14:32 | leakypipes | Spazmotic: we don't "zero out the resources" in allocations. | |
| 14:14:33 | TheJulia | Likewise :\ | |
| 14:14:43 | leakypipes | fried_rice: the consistency marker is the resource provider's generation. | |
| 14:15:13 | fried_rice | leakypipes Is it updated when allocations are made? | |
| 14:15:20 | leakypipes | fried_rice: of course. | |
| 14:15:34 | openstackgerrit | David Rabel proposed openstack/nova master: Fix format in flavors.rst https://review.openstack.org/535777 | |
| 14:15:55 | fried_rice | leakypipes But the documentation says the RP generation is ignored in alloc requests. | |
| 14:16:45 | fried_rice | leakypipes ...for PUT. And it appears to be entirely absent for POST. | |
| 14:17:18 | fried_rice | leakypipes Bug? | |
| 14:17:32 | leakypipes | fried_rice: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2085 | |
| 14:17:34 | artom | I can't even begin to think about the fix | |
| 14:17:47 | artom | We're setting a whole bunch of instance attributes in the compute API | |
| 14:17:54 | artom | And *then* doing the rebuild | |
| 14:17:55 | leakypipes | fried_rice: there is no POST. | |
| 14:18:02 | artom | With no checks whether it passed the scheduler or not | |