| opendevreview | Simon Li proposed openstack/nova-specs master: support ironic instance change host while it's host not up https://review.opendev.org/c/openstack/nova-specs/+/802991 | 07:04 |
|---|---|---|
| opendevreview | Simon Li proposed openstack/nova-specs master: support ironic instance change host while it's host not up https://review.opendev.org/c/openstack/nova-specs/+/802991 | 07:08 |
| *** rpittau|afk is now known as rpittau | 07:21 | |
| bauzas | stephenfin: sean-k-mooney[m]: lovely reviews for my generic mdevs implementation, thanks !: | 09:01 |
| artom_ | bauzas, have time to hit https://review.opendev.org/c/openstack/nova/+/802697? | 09:03 |
| *** artom_ is now known as artom | 09:03 | |
| 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:21 |
| bauzas | artom: you don't need to use the ironic driver | 09:29 |
| bauzas | artom: just use the standard fake driver and mock out (in the func test) the return you want | 09:29 |
| artom | bauzas, that's what the unit test is doing :P | 09:30 |
| artom | I mean OK, it's not using any virt driver | 09:30 |
| bauzas | artom: I know but you wouldn't verify the output of a method call, right? | 09:30 |
| bauzas | for the functest | 09:30 |
| bauzas | you would verify the expected behaviour | 09:30 |
| stephenfin | bauzas: sometimes "good enough" is okay :) | 09:30 |
| bauzas | stephenfin: i know and that's why I asked whether there was urgency | 09:31 |
| bauzas | the bug itself isn't about Ironic | 09:32 |
| artom | bauzas, err, yes it is? | 09:32 |
| artom | How else would you hit this besides Ironic? | 09:32 |
| 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 |
| artom | ... which can only happen with Ironic :) | 09:32 |
| bauzas | sure | 09:32 |
| bauzas | but the regression should verify that we get an API exception | 09:33 |
| bauzas | when calling the API service delete | 09:33 |
| artom | self.assertRaises( | 09:33 |
| bauzas | while the fix should just modify the api return | 09:33 |
| artom | webob.exc.HTTPInternalServerError | 09:33 |
| bauzas | I saw | 09:35 |
| bauzas | anyway | 09:35 |
| bauzas | I'm just asking to use a design framework | 09:36 |
| 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 |
| bauzas | we can do this other way | 09:36 |
| bauzas | and for sure this will work | 09:37 |
| artom | bauzas, I mean, I could just squash the patches and avoid the reproducer alltogether :P | 09:37 |
| artom | I wanted to highlight the broken behaviour first | 09:37 |
| artom | But the fix itself doens't need a func test, methinks | 09:37 |
| bauzas | honestly, I don't know what to say, I wasn't thinking my comment would be a concern | 09:38 |
| bauzas | so, meh | 09:38 |
| bauzas | +Wd | 09:38 |
| bauzas | I just don't wanna take time discussing | 09:38 |
| bauzas | artom: about the log level i'm asking | 09:48 |
| bauzas | well, in theory, unless ironic, a service *has* a compute node, right? | 09:48 |
| bauzas | and even with ironic, that's a normal situation | 09:48 |
| artom | Not really, IIUC, but I'm not an expert | 09:48 |
| artom | A service without nodes can be a part of normal operation | 09:49 |
| 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 |
| artom | In my understanding, anyways | 09:49 |
| artom | So to quote pawnstars, "debug is the best I can do" | 09:50 |
| 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:50 |
| artom | 'dat base: https://i.kym-cdn.com/entries/icons/original/000/000/228/DATASS.jpg | 09:51 |
| artom | Err, sorry about the URL :( | 09:52 |
| bauzas | artom: no, a service without a node isn't "normal" | 09:55 |
| bauzas | we create the CN entry when we create the service | 09:55 |
| bauzas | or CNs | 09:55 |
| bauzas | artom: it's just if you rebalance ironic nodes that you end up with trampling nova | 09:56 |
| bauzas | because ironic didn't manage the service/node relationship we have | 09:56 |
| bauzas | so, unless you rebalance your ironic nodes, you shouldn't expect such things | 09:56 |
| bauzas | hence an INFO at least | 09:56 |
| artom | Fair enough, I can do INFO | 09:58 |
| artom | Do you accept my FUP bargain? :) | 09:58 |
| bauzas | if so | 09:59 |
| artom | Hol'up though - for my own education, Ironic managed actualy physical nodes | 09:59 |
| artom | *manages | 09:59 |
| artom | So... how does "we create CN entry when we create the service" work if there are not physical nodes existing? | 10:00 |
| 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:02 |
| opendevreview | Merged openstack/nova master: Reproducer unit test for bug 1860312 https://review.opendev.org/c/openstack/nova/+/802697 | 10:03 |
| 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 |
| artom | jkulik, cheers! Confirms how I understood things | 10:04 |
| artom | bauzas, I feel like that could be None/empty list though | 10:05 |
| bauzas | artom: https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/compute/manager.py#L583 | 10:06 |
| artom | Empty list, based on https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/virt/ironic/driver.py#L823 | 10:06 |
| bauzas | here we instantiate a RT per compute node | 10:06 |
| bauzas | and here we create the CN entry https://github.com/openstack/nova/blob/97e1a6bece29e383f55bb969c69983153df9ffc7/nova/compute/resource_tracker.py#L918 | 10:07 |
| bauzas | every 60 secs | 10:08 |
| artom | bauzas, that's called from the periodic through, not at host init | 10:11 |
| bauzas | yup, my bad | 10:13 |
| bauzas | but | 10:13 |
| bauzas | we eventually call u_r_p at the end of the init, IIRC | 10:13 |
| bauzas | it's just a post hook call and not straighlty from init_host (or something like that, can't exactly remember) | 10:13 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: I2f9ad3df25306e070c8c3538bfed1212d6d8682f fup: add log https://review.opendev.org/c/openstack/nova/+/803001 | 10:39 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Rename vgpu options to mdev https://review.opendev.org/c/openstack/nova/+/801607 | 11:13 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: DNM (yet) : Expose the mdev class https://review.opendev.org/c/openstack/nova/+/801743 | 11: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: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 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: DNM (yet) : Expose the mdev class https://review.opendev.org/c/openstack/nova/+/801743 | 11:16 |
| songwenping__ | bauzas, now we cannot batch create vgpu vms, right? | 11:23 |
| bauzas | I need to go out for lunch but wdym for "batch create" ? | 11:23 |
| bauzas | multi-create ? | 11:23 |
| songwenping__ | yes multi-create | 11:23 |
| bauzas | if so, https://bugs.launchpad.net/nova/+bug/1874664 | 11:24 |
| sean-k-mooney[m] | we do not support multi create with any request that uses nested resource providers today | 11:25 |
| bauzas | yup, but this doesn't impact virtual GPUs | 11:26 |
| songwenping__ | there is a different exception with the bug | 11:26 |
| songwenping__ | yes, the scheduler is ok | 11:27 |
| songwenping__ | compute raise exception | 11:27 |
| sean-k-mooney[m] | well that bug does affect vgpu | 11:28 |
| 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 |
| bauzas | sean-k-mooney[m]: depending on your cloud capacity | 11:28 |
| 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:28 |
| sean-k-mooney[m] | in any case we don’t officially support multi create with vgpu today | 11:29 |
| sean-k-mooney[m] | that looks like there is a uuid collision on the host or similar reuse of the same mdev | 11:30 |
| bauzas | songwenping__: oh, that's another nvidia issue https://bugs.launchpad.net/nova/+bug/1758086 | 11:30 |
| bauzas | because of IOMMU | 11:30 |
| bauzas | you can't bind 2 vGPUs of the same pGPU on the same guest | 11:31 |
| bauzas | 2 or more vGPUs tbc | 11:31 |
| sean-k-mooney[m] | oh right ya. that is noted in our docs | 11:31 |
| bauzas | you can bind 2 or more vGPUs to the same guest if and only if each of those vGPU is actually provided by a different pGPU | 11:32 |
| bauzas | songwenping__: oh, this exception | 11:32 |
| bauzas | sorry, I misread | 11:32 |
| sean-k-mooney[m] | i’m not sure that limitation applies to intel guys by the way. | 11:32 |
| bauzas | that means we tried to allocate a mdev which was already assigned to another instance | 11:33 |
| bauzas | sean-k-mooney[m]: nope | 11:33 |
| bauzas | anyway, I'm starving | 11:33 |
| sean-k-mooney[m] | right that is what i meant by collision | 11:33 |
| sean-k-mooney[m] | hehe and i’m off today but i get notification on my ipad. was looking a coffee machine reviews :) | 11:34 |
| sean-k-mooney[m] | ill be back on tuesday | 11:34 |
| stephenfin | artom: what's going on with the commit message here? https://review.opendev.org/c/openstack/nova/+/803001 | 12:21 |
| artom | stephenfin, no idea | 13:10 |
| * artom fix | 13:10 | |
| opendevreview | Artom Lifshitz proposed openstack/nova master: I2f9ad3df25306e070c8c3538bfed1212d6d8682f fup: add log https://review.opendev.org/c/openstack/nova/+/803001 | 13:11 |
| opendevreview | Merged openstack/nova master: Fix request path to query a resource provider by uuid https://review.opendev.org/c/openstack/nova/+/800855 | 13:25 |
| *** rpittau is now known as rpittau|afk | 14:07 | |
| -opendevstatus- NOTICE: There will be a brief outage of the Gerrit service on review.opendev.org starting at 15:00 UTC today as part of a routine project rename maintenance: http://lists.opendev.org/pipermail/service-announce/2021-July/000023.html | 14:12 | |
| bauzas | argh, forgot this ^ | 14:48 |
| bauzas | a Friday 5pm outage ? man, does it mean I should stop then ? :) | 14:49 |
| bauzas | melwitt: sorry I had no time reviewing your series but I haven't forgotten, will do it hopefully on Monday morning so you'd get feedback when you're back from time off | 14:50 |
| bauzas | I'll tho try to start now unti Gerrit shutdowns | 14:50 |
| bauzas | stephenfin: around ? how can I verify the revisions uuids for https://review.opendev.org/c/openstack/placement/+/669170/19/placement/db/sqlalchemy/alembic/versions/422ece571366_add_consumer_types_table.py#1 ? | 14:52 |
| bauzas | L25-26 | 14:52 |
| stephenfin | looking | 14:52 |
| stephenfin | bauzas: the revision UUID should be unique | 14:53 |
| stephenfin | so if you grep for that in the migrations directory, it shouldn't find anything | 14:53 |
| bauzas | ok | 14:53 |
| bauzas | do we have a test running it ? | 14:53 |
| stephenfin | the down_revision on the other hand should be the most recent migration | 14:53 |
| stephenfin | I suspect there are migrations tests | 14:53 |
| bauzas | ok | 14:53 |
| stephenfin | probably in placement.tests.db.test_migrations or similar | 14:54 |
| bauzas | lemme look | 14:54 |
| bauzas | stephenfin: do we need to run some command in some venv for creating a new alembic migration ? | 14:55 |
| bauzas | like reno, I mean | 14:55 |
| stephenfin | yes, 'alembic revision' | 14:55 |
| stephenfin | I've documented it for nova https://review.opendev.org/c/openstack/nova/+/800078/3 | 14:56 |
| -opendevstatus- NOTICE: There will be a brief outage of the Gerrit service on review.opendev.org in the next few minutes as part of a routine project rename maintenance: http://lists.opendev.org/pipermail/service-announce/2021-July/000023.html | 15:01 | |
| bauzas | stephenfin: I see, thanks ! | 15:13 |
| opendevreview | Takashi Kajinami proposed openstack/nova stable/wallaby: Fix request path to query a resource provider by uuid https://review.opendev.org/c/openstack/nova/+/803070 | 16:40 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Rename vgpu options to mdev https://review.opendev.org/c/openstack/nova/+/801607 | 16:40 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Provide the mdev class for every PCI device https://review.opendev.org/c/openstack/nova/+/802918 | 16:40 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: DNM (yet) : Expose the mdev class https://review.opendev.org/c/openstack/nova/+/801743 | 16:40 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage db purge to nova-audit https://review.opendev.org/c/openstack/nova/+/708783 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage db archive_deleted_rows to nova-audit https://review.opendev.org/c/openstack/nova/+/708784 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage cell_v2 discover_hosts to nova-manage https://review.opendev.org/c/openstack/nova/+/708785 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage cell_v2 map_instances to nova-audit https://review.opendev.org/c/openstack/nova/+/708786 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage placement sync_aggregates to nova-audit https://review.opendev.org/c/openstack/nova/+/708787 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: Move nova-manage placement heal_allocations to nova-audit https://review.opendev.org/c/openstack/nova/+/708788 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/nova master: WIP: nova-audit: Use cliff instead of homegrown argparse bleh https://review.opendev.org/c/openstack/nova/+/803059 | 16:54 |
| opendevreview | Stephen Finucane proposed openstack/placement master: placement-status: check only consumers in allocation table https://review.opendev.org/c/openstack/placement/+/762987 | 17:04 |
| melwitt | bauzas: ok no worry, thanks! | 17:11 |
| *** melwitt is now known as jgwentworth | 17:55 | |
| opendevreview | Merged openstack/nova master: db: Use module-level imports for sqlalchemy (for real) https://review.opendev.org/c/openstack/nova/+/796519 | 17:57 |
| opendevreview | Merged openstack/nova master: db: Move db.sqalchemy.migration to db.migration https://review.opendev.org/c/openstack/nova/+/799518 | 17:57 |
| opendevreview | Merged openstack/placement master: Remove unused test helper https://review.opendev.org/c/openstack/placement/+/763958 | 19:05 |
| opendevreview | Samuel proposed openstack/nova-specs master: Migrate Instance Between Projects https://review.opendev.org/c/openstack/nova-specs/+/802034 | 19:30 |
| opendevreview | Merged openstack/placement master: placement-status: check only consumers in allocation table https://review.opendev.org/c/openstack/placement/+/762987 | 19:48 |
| opendevreview | melanie witt proposed openstack/nova stable/stein: Reject open redirection in the console proxy https://review.opendev.org/c/openstack/nova/+/802935 | 20:33 |
| opendevreview | Elod Illes proposed openstack/nova stable/rocky: [stable-only] Fix lower-constraints job https://review.opendev.org/c/openstack/nova/+/769910 | 21:15 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
