| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-07-30 | |||
| 09:01:07 | bauzas | stephenfin: sean-k-mooney[m]: lovely reviews for my generic mdevs implementation, thanks !: | |
| 09:03:05 | artom_ | bauzas, have time to hit https://review.opendev.org/c/openstack/nova/+/802697? | |
| 09:21:27 | artom | bauzas, thanks for the review - so, I did consider doing a functional test, but given that this is 1. niche and 2. straightforward, any functional test I'd write would be either 1. a lot of groundwork to use the Ironic virt driver in the test or 2. have so much stuff mocked out, it'd be functionally (heh) identical to the existing unit test | |
| 09:29:25 | bauzas | artom: you don't need to use the ironic driver | |
| 09:29:49 | bauzas | artom: just use the standard fake driver and mock out (in the func test) the return you want | |
| 09:30:02 | artom | bauzas, that's what the unit test is doing :P | |
| 09:30:27 | artom | I mean OK, it's not using any virt driver | |
| 09:30:28 | bauzas | artom: I know but you wouldn't verify the output of a method call, right? | |
| 09:30:36 | bauzas | for the functest | |
| 09:30:51 | bauzas | you would verify the expected behaviour | |
| 09:30:54 | stephenfin | bauzas: sometimes "good enough" is okay :) | |
| 09:31:35 | bauzas | stephenfin: i know and that's why I asked whether there was urgency | |
| 09:32:04 | bauzas | the bug itself isn't about Ironic | |
| 09:32:14 | artom | bauzas, err, yes it is? | |
| 09:32:19 | artom | How else would you hit this besides Ironic? | |
| 09:32:30 | bauzas | the problem is that we raise up to the API an exception when we don't find compute nodes for a service deletion | |
| 09:32:41 | artom | ... which can only happen with Ironic :) | |
| 09:32:44 | bauzas | sure | |
| 09:33:03 | bauzas | but the regression should verify that we get an API exception | |
| 09:33:16 | bauzas | when calling the API service delete | |
| 09:33:36 | bauzas | while the fix should just modify the api return | |
| 09:33:36 | artom | self.assertRaises( | |
| 09:33:37 | artom | webob.exc.HTTPInternalServerError | |
| 09:35:23 | bauzas | I saw | |
| 09:35:25 | bauzas | anyway | |
| 09:36:11 | bauzas | I'm just asking to use a design framework | |
| 09:36:50 | bauzas | because we're an opensource community and we try to use the same frameworks in general as we want contributors to know about them | |
| 09:36:58 | bauzas | we can do this other way | |
| 09:37:07 | bauzas | and for sure this will work | |
| 09:37:29 | artom | bauzas, I mean, I could just squash the patches and avoid the reproducer alltogether :P | |
| 09:37:48 | artom | I wanted to highlight the broken behaviour first | |
| 09:37:57 | artom | But the fix itself doens't need a func test, methinks | |
| 09:38:12 | bauzas | honestly, I don't know what to say, I wasn't thinking my comment would be a concern | |
| 09:38:17 | bauzas | so, meh | |
| 09:38:38 | bauzas | +Wd | |
| 09:38:52 | bauzas | I just don't wanna take time discussing | |
| 09:48:17 | bauzas | artom: about the log level i'm asking | |
| 09:48:34 | bauzas | well, in theory, unless ironic, a service *has* a compute node, right? | |
| 09:48:54 | bauzas | and even with ironic, that's a normal situation | |
| 09:48:57 | artom | Not really, IIUC, but I'm not an expert | |
| 09:49:10 | artom | A service without nodes can be a part of normal operation | |
| 09:49:26 | artom | For instance, adding a service before adding any nodes to it, replacing all nodes associated with a service, that kind of stuff | |
| 09:49:33 | artom | In my understanding, anyways | |
| 09:50:28 | artom | So to quote pawnstars, "debug is the best I can do" | |
| 09:50:58 | artom | Actually, would you be offended if I did it in a follow-up? I need to fix the dat base typo as well | |
| 09:51:52 | artom | 'dat base: https://i.kym-cdn.com/entries/icons/original/000/000/228/DATASS.jpg | |
| 09:52:06 | artom | Err, sorry about the URL :( | |
| 09:55:26 | bauzas | artom: no, a service without a node isn't "normal" | |
| 09:55:38 | bauzas | we create the CN entry when we create the service | |
| 09:55:43 | bauzas | or CNs | |
| 09:56:07 | bauzas | artom: it's just if you rebalance ironic nodes that you end up with trampling nova | |
| 09:56:22 | bauzas | because ironic didn't manage the service/node relationship we have | |
| 09:56:41 | bauzas | so, unless you rebalance your ironic nodes, you shouldn't expect such things | |
| 09:56:53 | bauzas | hence an INFO at least | |
| 09:58:06 | artom | Fair enough, I can do INFO | |
| 09:58:14 | artom | Do you accept my FUP bargain? :) | |
| 09:59:07 | bauzas | if so | |
| 09:59:48 | artom | Hol'up though - for my own education, Ironic managed actualy physical nodes | |
| 09:59:51 | artom | *manages | |
| 10:00:11 | artom | So... how does "we create CN entry when we create the service" work if there are not physical nodes existing? | |
| 10:02:50 | jkulik | a started ironic compute driver will just write over and over again "No compute node record for host nova-compute-ironic" if there are no physical nodes existing. at least on rocky. | |
| 10:03:57 | opendevreview | Merged openstack/nova master: Reproducer unit test for bug 1860312 https://review.opendev.org/c/openstack/nova/+/802697 | |
| 10:04:41 | bauzas | artom: the compute manager gets the list of cns from the virt driver in https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/compute/manager.py#L1433 | |
| 10:04:52 | artom | jkulik, cheers! Confirms how I understood things | |
| 10:05:28 | artom | bauzas, I feel like that could be None/empty list though | |
| 10:06:24 | bauzas | artom: https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/compute/manager.py#L583 | |
| 10:06:29 | artom | Empty list, based on https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/virt/ironic/driver.py#L823 | |
| 10:06:34 | bauzas | here we instantiate a RT per compute node | |
| 10:07:56 | bauzas | and here we create the CN entry https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/compute/resource_tracker.py#L918 | |
| 10:08:02 | bauzas | every 60 secs | |
| 10:11:29 | artom | bauzas, that's called from the periodic through, not at host init | |
| 10:13:03 | bauzas | yup, my bad | |
| 10:13:08 | bauzas | but | |
| 10:13:23 | bauzas | we eventually call u_r_p at the end of the init, IIRC | |
| 10:13:59 | bauzas | it's just a post hook call and not straighlty from init_host (or something like that, can't exactly remember) | |
| 10:39:49 | opendevreview | Artom Lifshitz proposed openstack/nova master: I2f9ad3df25306e070c8c3538bfed1212d6d8682f fup: add log https://review.opendev.org/c/openstack/nova/+/803001 | |
| 11:13:35 | opendevreview | Sylvain Bauza proposed openstack/nova master: DNM (yet) : Expose the mdev class https://review.opendev.org/c/openstack/nova/+/801743 | |
| 11:13:35 | opendevreview | Sylvain Bauza proposed openstack/nova master: Rename vgpu options to mdev https://review.opendev.org/c/openstack/nova/+/801607 | |
| 11:13:36 | opendevreview | Sylvain Bauza proposed openstack/nova master: WIP Provide the mdev class for every PCI device https://review.opendev.org/c/openstack/nova/+/802918 | |
| 11:16:13 | opendevreview | Sylvain Bauza proposed openstack/nova master: WIP Provide the mdev class for every PCI device https://review.opendev.org/c/openstack/nova/+/802918 | |
| 11:16:14 | opendevreview | Sylvain Bauza proposed openstack/nova master: DNM (yet) : Expose the mdev class https://review.opendev.org/c/openstack/nova/+/801743 | |
| 11:23:09 | songwenping__ | bauzas, now we cannot batch create vgpu vms, right? | |
| 11:23:37 | bauzas | I need to go out for lunch but wdym for "batch create" ? | |
| 11:23:42 | bauzas | multi-create ? | |
| 11:23:50 | songwenping__ | yes multi-create | |
| 11:24:18 | bauzas | if so, https://bugs.launchpad.net/nova/+bug/1874664 | |
| 11:25:18 | sean-k-mooney[m] | we do not support multi create with any request that uses nested resource providers today | |
| 11:26:36 | bauzas | yup, but this doesn't impact virtual GPUs | |
| 11:26:37 | songwenping__ | there is a different exception with the bug | |
| 11:27:03 | songwenping__ | yes, the scheduler is ok | |
| 11:27:15 | songwenping__ | compute raise exception | |
| 11:28:07 | sean-k-mooney[m] | well that bug does affect vgpu | |
| 11:28:13 | bauzas | songwenping__: look at the two different tests we have for vgpus https://review.opendev.org/c/openstack/nova/+/723858/3/nova/tests/functional/libvirt/test_vgpu.py#237 | |
| 11:28:29 | bauzas | sean-k-mooney[m]: depending on your cloud capacity | |
| 11:28:56 | songwenping__ | tbe exception in my env is: mediated device /sys/bus/mdev/devices/58c9a61a-874f-414c-a8e8-91892a9fc6dd is in use by driver QEMU, domain instance-00000008\n' | |
| 11:29:19 | sean-k-mooney[m] | in any case we don’t officially support multi create with vgpu today | |
| 11:30:21 | sean-k-mooney[m] | that looks like there is a uuid collision on the host or similar reuse of the same mdev | |
| 11:30:23 | bauzas | songwenping__: oh, that's another nvidia issue https://bugs.launchpad.net/nova/+bug/1758086 | |
| 11:30:37 | bauzas | because of IOMMU | |
| 11:31:20 | bauzas | you can't bind 2 vGPUs of the same pGPU on the same guest | |