| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-02 | |||
| 18:31:02 | dansmith | efried: Sundar: is there a testing patch somewhere in the series that wires this all up with a fake cyborg driver? | |
| 18:31:22 | dansmith | I assume that would be the one that needs to depends-on that cyborg change | |
| 18:31:26 | efried | the CI job is working last I checked | |
| 18:31:58 | sean-k-mooney | there is a patch to add a tempest job againd nova nad i think the job is now running on all cyborg patches | |
| 18:31:59 | dansmith | there are a lot of patches that have various confused histories | |
| 18:32:05 | efried | dansmith: https://review.opendev.org/#/c/670999/20 | |
| 18:32:10 | efried | passing atm | |
| 18:32:11 | dansmith | ack just found that | |
| 18:32:34 | sean-k-mooney | same was going to link it | |
| 18:33:15 | efried | there was a time when I looked and it wasn't doing anything, but I thought the last time I checked it was actually running through the spawn flow with a real (pseudo) device | |
| 18:33:29 | sean-k-mooney | it is doing two things | |
| 18:33:31 | efried | still very basic though | |
| 18:33:35 | efried | green path only | |
| 18:33:51 | sean-k-mooney | its cratein a device profile and then a super minima test of booting a vm | |
| 18:33:55 | sean-k-mooney | althogh that one is not correct | |
| 18:34:09 | sean-k-mooney | its booting a vm but not waiting for ti to go active before deleting it | |
| 18:34:35 | sean-k-mooney | at least the last time i looked but everythin is in place to add proper lifecycle tests | |
| 18:34:37 | dansmith | seems like a lot of vm_state=error in the compute logs for that, I'll have to track it down closer to see if it's related | |
| 18:35:17 | sean-k-mooney | https://opendev.org/openstack/cyborg-tempest-plugin/src/branch/master/cyborg_tempest_plugin/tests/scenario/test_accelerator_basic_ops.py#L45-L60 | |
| 18:35:46 | dansmith | does that even wait for it to become running? | |
| 18:36:03 | sean-k-mooney | no which i mentioned before | |
| 18:36:22 | sean-k-mooney | to the cyborg folks | |
| 18:36:32 | sean-k-mooney | create server does https://opendev.org/openstack/cyborg-tempest-plugin/src/branch/master/cyborg_tempest_plugin/tests/scenario/manager.py#L245-L257 | |
| 18:36:33 | dansmith | heh okay | |
| 18:36:38 | dansmith | oh okay | |
| 18:36:47 | dansmith | also, the name of the test makes it hard to track down those instances in the logs | |
| 18:36:57 | dansmith | would be better if it was called test_server_ops_with_accel or something | |
| 18:37:41 | sean-k-mooney | ya with that said its an easy change as is extenidn it to do more really tests | |
| 18:37:55 | Sundar | sean-k-mooney: The tempest test waits till the server becomes active: https://review.opendev.org/#/c/667231/11/cyborg_tempest_plugin/tests/scenario/manager.py@155 | |
| 18:39:08 | sean-k-mooney | oh you are using the common tempest function ok | |
| 18:39:08 | dansmith | I don't see that the cyborg instance ever made it to the compute node | |
| 18:40:06 | Sundar | dansmith: The tempest CI test in cyborg-tempest-plugin is called test_server_basic_ops '' https://review.opendev.org/#/c/667231/11/cyborg_tempest_plugin/tests/scenario/test_accelerator_basic_ops.py@45 | |
| 18:40:24 | dansmith | Sundar: right, which makes it hard to distinguish from the other test_server_basic_ops | |
| 18:40:41 | dansmith | I don't see it actually running though: https://8664d62e69400bd89796-9cb1d5e035819d8d5535734f80756cd4.ssl.cf1.rackcdn.com/670999/20/check/tempest-integrated-compute/cdfc846/testr_results.html.gz | |
| 18:40:48 | sean-k-mooney | Sundar: that is an existing class name in tempest | |
| 18:41:00 | dansmith | oh wait, that's the wrong log, ignore me | |
| 18:41:46 | sean-k-mooney | although the module path should be different | |
| 18:41:55 | Sundar | If you just want to mention 'accel' somewhere in the name, that's not a problem. Will do. | |
| 18:42:06 | dansmith | sean-k-mooney: right but the test name is what drives the tempest tenant name I think | |
| 18:42:13 | sean-k-mooney | https://2371b0492dbe3a0c56c0-5469ab4f5c2741453cb8b95135b2c449.ssl.cf2.rackcdn.com/670999/20/check/cyborg-tempest/2124885/testr_results.html.gz | |
| 18:42:19 | sean-k-mooney | that is the result you were looking for | |
| 18:42:28 | dansmith | but they're hardcoding the server name, which helps | |
| 18:42:29 | dansmith | yeah I know | |
| 18:42:58 | dansmith | so for this fake test, we don't actually provide any device to libvirt to attach to the guest right: | |
| 18:43:00 | dansmith | ? | |
| 18:43:17 | Sundar | That's right | |
| 18:43:21 | sean-k-mooney | correct | |
| 18:43:36 | sean-k-mooney | the arq has type fake and its ignored | |
| 18:43:43 | Sundar | The path till creating/binding.getting ARQs is the same as for a real driver | |
| 18:43:49 | dansmith | will there be a real job running on intel ci or something that can actually do this? | |
| 18:44:03 | dansmith | Sundar: sure, it just doesn't cover actually producing a device and getting libvirt to attach it | |
| 18:44:09 | Sundar | dansmith: Yes, we are planning a 3rd party CI for Intel FPGAs | |
| 18:44:22 | sean-k-mooney | we could maybe use some other virtual device for testing in a more robust way too | |
| 18:45:14 | dansmith | sean-k-mooney: yeah I was going to ask if we could pass some harmless pci device through, but wasn't sure if we can do that in the gate without nested | |
| 18:45:19 | Sundar | sean-k-mooney: We have plans to enhance the fake driver to simulate programming -- it will just respons to the programming API within Cyborg with a success/failure | |
| 18:45:35 | sean-k-mooney | dansmith: i was thinking of creating a file on disk and passing it to the guest | |
| 18:45:45 | dansmith | Sundar: yeah, that's good, I just want to see the actual "here's the pci device" stuff tested | |
| 18:45:48 | Sundar | That would cover more paths, including Glance interaction within Cyborg. But it would still not attach a PCI device to the VM | |
| 18:46:02 | dansmith | sean-k-mooney: how does a file on disk simulate a pci device/ | |
| 18:46:42 | dansmith | okay anyway, this log looks good for what we have now and can test now, so that's cool | |
| 18:46:57 | dansmith | I'll want to look at this one Sundar retools the current patches per the pending feedback | |
| 18:47:05 | sean-k-mooney | it does not but it will simulate genarting xml. i was really thinking of a lvm driver for local caching so we could have cyborg provide local storage to a vm. | |
| 18:47:20 | dansmith | Sundar: FWIW, I have this week and next week before I turn to a pumpkin for the rest of the year | |
| 18:47:38 | sean-k-mooney | e.g. have a driver that could be useful in producion but has no hardware depency | |
| 18:47:43 | Sundar | dansmith: I am going to squash https://review.opendev.org/631245 into https://review.opendev.org/631244, for the reasons I mentioned above | |
| 18:47:46 | dansmith | sean-k-mooney: is that a thing? I thought we were focused on pci devices for accelerators right now | |
| 18:48:27 | sean-k-mooney | we are but form a livirt driver point of there is very littel difference between generating the xml for a pci device vs a block device | |
| 18:48:32 | dansmith | Sundar: okay I skimmed over that.. IMHO, keeping the new event definition in its own patch makes sense just because of how much api stuff has to change | |
| 18:49:08 | dansmith | sean-k-mooney: seems fairly different to me.. I understand it's just a chunk of xml, but it's not the same kind of xml we really care about | |
| 18:49:11 | Sundar | dansmith: Agreed. The event definition patch https://review.opendev.org/692707 will stay as is. | |
| 18:49:36 | sean-k-mooney | if we just want a way to create a device (lvm volume) program it (dd image to volume) and validate it in the guset it was a way to do it. | |
| 18:49:41 | dansmith | sean-k-mooney: it's something, it's just not enough | |
| 18:49:52 | dansmith | Sundar: oh sorry I was looking at the wrong two tabs, lemme re-read | |
| 18:50:24 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Request and store the decrypted path of an attached encrypted volume https://review.opendev.org/696693 | |
| 18:50:40 | sean-k-mooney | ya i was also looking at ways to fake pci devices in linux and there are some ways but its not trivial | |
| 18:50:47 | Sundar | sean-k-mooney: The code paths within the virt driver are quite different for block devices and PCI devices. So, how does this help? | |
| 18:51:03 | dansmith | Sundar: 631234 all but goes away after you address my feedback | |
| 18:51:22 | dansmith | Sundar: so assuming you mean just bringing forward the cyborg client method, then sure | |
| 18:51:46 | dansmith | Sundar: it would be attaching a block device as an accelerator, so no typical nova block device handling for it | |
| 18:51:50 | sean-k-mooney | its a more complete end to end test that is all. also im not sure how https://review.opendev.org/#/c/689070/ is going to progress and its another way to enable local caching | |
| 18:52:16 | dansmith | sean-k-mooney: I don't think cyborg for block devices is a good way to address the local caching thing | |
| 18:52:24 | dansmith | sean-k-mooney: how is that not going to confuse people? | |
| 18:52:54 | sean-k-mooney | well given samsung are now supporting sriov on there new ssds it might be but sure ill let you get back to it | |
| 18:53:18 | dansmith | well, I guess we can have that discussion, but I definitely don't want to merge that with this effort at the moment | |
| 18:53:34 | dansmith | and since PCI is different enough, I still want to see PCI working for this | |
| 18:53:41 | sean-k-mooney | oh sure. its totally seperate | |
| 18:53:48 | sean-k-mooney | ya | |
| 18:53:57 | sean-k-mooney | i just have not figured out how to fake it yet | |
| 18:54:23 | sean-k-mooney | the kernel netdevsim module was the close i came up with in the past | |
| 18:54:25 | dansmith | yeah, well, as long as we'll have intel ci so we can see it working, that's something | |
| 18:54:30 | Sundar | dansmith: 631234 is a typo, right? Quoting myself from above: I think we need to squash https://review.opendev.org/631245 into the previous patch https://review.opendev.org/631244. Because once the get_resolved_arqs() moves to previous patch, the only things left behind are the virt driver changes. It would arguably be easier to understand those | |
| 18:54:31 | Sundar | changes together with the changes to get the arqs in compute/manager.py. | |
| 18:54:55 | dansmith | Sundar: I meant 631245 yes, sorry | |
| 18:55:01 | sean-k-mooney | ya. the fake driver should allow us to test the api flow. the intel ci should validate it works with real hardware | |
| 18:55:09 | Sundar | Cool, np | |
| 18:55:24 | dansmith | Sundar: the virt changes will be insulated from the cyborg client method is my point | |
| 18:56:08 | dansmith | Sundar: if I were you I would have one patch that adds the support into the manager, all the build_resources changes to capture the ARQs and pass them to the virt driver, | |
| 18:56:17 | Sundar | dansmith: Sure, will try to get back ASAP with the changes | |
| 18:56:17 | dansmith | Sundar: then a separate patch to implement things for libvirt | |
| 18:56:33 | Sundar | Oh, separate patch for libvirt? | |