| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-cyborg - 2026-06-02 | |||
| 14:17:44 | jgilaber | ack | |
| 14:17:53 | chandankumar | I alsoanual recovery:** Operators retry failed cleanups using | |
| 14:17:55 | chandankumar | ``cyborg-nvme-cleanup --device <uuid>`` (requires admin credentials). The CLI | |
| 14:17:57 | chandankumar | tool re-triggers cleanup and resets ``cleanup_failed=False`` on success. | |
| 14:18:06 | chandankumar | that I have proposed in the spec | |
| 14:18:31 | chandankumar | anyway let me address the current comments and new design based on above discussion | |
| 14:18:38 | chandankumar | we can follow up on spec | |
| 14:19:41 | sean-k-mooney | im not really a fan of provideing a cil for the cleaning | |
| 14:20:10 | sean-k-mooney | but ack i have some pednign coeemt form when i first lookd but i stoped at the problem description after my inital skim pass | |
| 14:20:30 | sean-k-mooney | ill review it in detail this week ideally today or tomrorow | |
| 14:20:54 | sean-k-mooney | to be clear im not really a fan of having any dirver speciric clis | |
| 14:21:18 | chandankumar | sean-k-mooney: I will ping you tomorrow for review once I update it based on above design, | |
| 14:22:02 | sean-k-mooney | cyborgs core role is too provied a hardware indepent common api over the acclerator it manges so you shoudl nto need nvme specific clis but a generic way to triger cleaning or programmign is ok | |
| 14:22:43 | sean-k-mooney | ack | |
| 14:23:48 | jgilaber | ack, is that all for this topic? | |
| 14:23:49 | chandankumar | sure | |
| 14:23:54 | chandankumar | thank you jgilaber sean-k-mooney ! | |
| 14:24:10 | jgilaber | thanks Chandan, let's move to reviews | |
| 14:24:17 | jgilaber | #topic Reviews | |
| 14:24:23 | jgilaber | we have one | |
| 14:24:29 | jgilaber | #link https://review.opendev.org/c/openstack/cyborg/+/991027 | |
| 14:24:37 | jgilaber | with tempest tests https://review.opendev.org/c/openstack/cyborg-tempest-plugin/+/991081 | |
| 14:24:56 | sean-k-mooney | i have 2 to add later | |
| 14:24:57 | chandankumar | I was working on dropping the image verification code for device program functionality | |
| 14:25:52 | sean-k-mooney | so the fake driver already supprot program | |
| 14:25:53 | chandankumar | It is dropping the code and marking existing verify_glance_signatures config as deprecated | |
| 14:26:00 | sean-k-mooney | https://review.opendev.org/c/openstack/cyborg/+/991027/2/cyborg/accelerator/drivers/fake.py#127 | |
| 14:26:03 | chandankumar | it has a update method not program one | |
| 14:26:04 | sean-k-mooney | that what update is | |
| 14:26:14 | chandankumar | will I rename that? | |
| 14:26:17 | sean-k-mooney | no update is what prgram is called in teh drier interface | |
| 14:26:24 | chandankumar | ok | |
| 14:26:36 | sean-k-mooney | so that a sperat question | |
| 14:26:45 | sean-k-mooney | we could perhas rename it but that is a large change | |
| 14:26:56 | sean-k-mooney | sicne it woudl be changing the public api of the drivers | |
| 14:27:02 | chandankumar | program interface is currently used in fpga driver only not in generic one | |
| 14:27:16 | sean-k-mooney | again that is but true and untrue | |
| 14:27:41 | sean-k-mooney | so form an api perspective we shoudl not have any driver specifc apis | |
| 14:28:11 | sean-k-mooney | so its more correct to say that the update api si a noop for driver other then the fpg driver | |
| 14:28:24 | sean-k-mooney | rahter then teh progarm api is only for fpga | |
| 14:28:43 | sean-k-mooney | the ablity to program or update a device with a glance image | |
| 14:29:05 | sean-k-mooney | is a genereic capablity that is not used by other drivers | |
| 14:29:18 | sean-k-mooney | but the nvme driver coudl supprot it as we have dicussed in the past | |
| 14:29:24 | chandankumar | yes | |
| 14:30:27 | chandankumar | https://github.com/openstack/cyborg/blob/master/cyborg/accelerator/drivers/driver.py#L27 with respect to current patch, How we do want to proceed? Reusing update at all places a seperate patch and calling update flow via profram cli? | |
| 14:30:30 | jgilaber | I'm looking quickly but the program api can't call the fake driver right? | |
| 14:30:42 | chandankumar | jgilaber: yes | |
| 14:31:28 | sean-k-mooney | so the current patch is incorrect | |
| 14:31:30 | sean-k-mooney | https://github.com/openstack/cyborg/blob/master/cyborg/accelerator/drivers/driver.py#L26-L35 | |
| 14:31:31 | jgilaber | and is there any endpoint for update? | |
| 14:31:38 | sean-k-mooney | udpate is part of the base driver interface | |
| 14:31:44 | sean-k-mooney | and that is how programing shoudl be invoked | |
| 14:32:09 | sean-k-mooney | at least that is muy current understandign but ill need to review that in more detail to confirm | |
| 14:32:19 | chandankumar | there is no endpoint for update, we currently invoke program https://bugs.launchpad.net/openstack-cyborg/+bug/2144308/comments/1 | |
| 14:32:29 | jgilaber | so the fpga implemented it incorrectly? | |
| 14:32:41 | sean-k-mooney | so there are 2 ways to program the ptgs | |
| 14:32:42 | chandankumar | $CYBORG_URL/deployables/$DEPLOYABLE_UUID/program - was the api interface | |
| 14:32:54 | sean-k-mooney | you an refence the image via the device profile | |
| 14:33:01 | sean-k-mooney | or you can manually do it via the deployable api | |
| 14:33:54 | jgilaber | but to test chandankumar's patch we need to call the api iirc | |
| 14:34:03 | sean-k-mooney | if we look at progrma its curenly using PATCH which i want to fix in the future but PATCH is an "update" the same way put is | |
| 14:34:13 | jgilaber | the image verification was only in the program endpoint path | |
| 14:34:46 | chandankumar | https://review.opendev.org/c/openstack/cyborg-tempest-plugin/+/991081/1/cyborg_tempest_plugin/services/cyborg_rest_client.py#154 is this how i am calling in tempest | |
| 14:34:56 | sean-k-mooney | internally its calling fpga_program over the rpc bus | |
| 14:35:39 | sean-k-mooney | which looks like you are right is callign driver.prgram | |
| 14:35:44 | sean-k-mooney | which is an internal api | |
| 14:36:13 | sean-k-mooney | that shoudl nto be called by the manger | |
| 14:36:49 | sean-k-mooney | so i think we need to fix that first | |
| 14:36:49 | chandankumar | ah, yes correct, that needs to be fixed on tempest patch side | |
| 14:37:17 | chandankumar | I also need to add a flag to disable it on older release | |
| 14:37:51 | sean-k-mooney | yes you shoudl add a supprot_program flag or similar | |
| 14:38:30 | sean-k-mooney | we shoudl have a accelorator_features configre group or similar config sction ile the compute_enabeld_feature tempest section | |
| 14:38:56 | sean-k-mooney | we can add fake_program True|False to that to guard the supprot | |
| 14:39:35 | sean-k-mooney | so looping back | |
| 14:39:35 | chandankumar | sure, I will update both the patches based on above suggestion | |
| 14:39:47 | sean-k-mooney | https://github.com/openstack/cyborg/blob/2875d3c12d4484e9336ba5084f32f2acf83a2366/cyborg/accelerator/drivers/fpga/base.py | |
| 14:39:57 | chandankumar | first one I need to re-read the conversaiton | |
| 14:40:03 | sean-k-mooney | shoudl likely be updated to add an implamntion fo update that calle program | |
| 14:40:21 | sean-k-mooney | and program should be updated to _programe or just removed from the fpga drivers | |
| 14:40:43 | sean-k-mooney | alternitivly we coudl do the reverse and make program the public method and remove update | |
| 14:41:15 | sean-k-mooney | we shoudl not really ahve both and the manager shoudl never call a method on a driver that is not part of https://github.com/openstack/cyborg/blob/2875d3c12d4484e9336ba5084f32f2acf83a2366/cyborg/accelerator/drivers/driver.py | |
| 14:42:20 | jgilaber | I don't understand how the current version of chandankumar patches passed in ci with the new test | |
| 14:42:47 | jgilaber | the agent manager calls explicitely the fpga driver program method | |
| 14:42:51 | chandankumar | jgilaber: it is just faking the device path, there is no real check | |
| 14:42:55 | sean-k-mooney | jgilaber: yes it does | |
| 14:43:30 | sean-k-mooney | jgilaber: well technially it doesnt | |
| 14:43:38 | sean-k-mooney | it called the program funciton on the driver object | |
| 14:43:49 | sean-k-mooney | jgilaber: by inheriting form the genic fgpa class | |
| 14:43:56 | sean-k-mooney | the fake driver gained a program funciton | |
| 14:44:12 | sean-k-mooney | which is why its workign but that not really the correct way this should work | |
| 14:44:39 | sean-k-mooney | jgilaber: does that make sense | |
| 14:45:00 | jgilaber | sort of, but I need to stare at it for a bit | |
| 14:45:07 | jgilaber | I'll do that offline though | |
| 14:45:12 | jgilaber | we can continue | |
| 14:45:26 | sean-k-mooney | https://review.opendev.org/c/openstack/cyborg/+/991027/2/cyborg/accelerator/drivers/fake.py | |
| 14:45:43 | sean-k-mooney | so progam call update and update redutr True | |
| 14:46:04 | sean-k-mooney | and because that added program the agened didnt explode when it did driver.program | |
| 14:46:24 | sean-k-mooney | here https://github.com/openstack/cyborg/blob/2875d3c12d4484e9336ba5084f32f2acf83a2366/cyborg/agent/manager.py#L184 | |
| 14:46:30 | sean-k-mooney | but cool lets move on | |
| 14:47:07 | jgilaber | oh I misread the variable, I get it now thanks! | |