| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-18 | |||
| 12:52:21 | sean-k-mooney | technially yes to the sart of the propsoed changes | |
| 12:52:27 | sean-k-mooney | the usecases section is correct | |
| 12:52:37 | mikal | Done. | |
| 12:52:38 | opendevreview | Michael Still proposed openstack/nova-specs master: Propose a new API microversion for a spice-direct console. https://review.opendev.org/c/openstack/nova-specs/+/915190 | |
| 12:53:19 | sean-k-mooney | cool ill contineu reviewing the new version | |
| 12:53:34 | sean-k-mooney | sofar the content looks fine to me | |
| 12:56:42 | sean-k-mooney | mikal: by the way if you have not seen this https://review.opendev.org/c/openstack/nova-specs/+/920687 i think that woudl also be very useful to you | |
| 12:56:58 | sean-k-mooney | we may not have time to fully review and approve that todya as well | |
| 12:57:12 | sean-k-mooney | but i think both your fature and that could work nicely togetter going forward | |
| 13:03:52 | mikal | Oh that's interesting. SPICE tunnels USBredir over its transport, and that already works in my prototype | |
| 13:04:17 | mikal | That spec presumably is also proposing to add the same USB devices that I need to add for that to work in SPICE? I shall read the spec. | |
| 13:04:52 | sean-k-mooney | yes and adding image/flavor properies to say how many | |
| 13:05:36 | sean-k-mooney | there is a lot of overlap in some respect although they were happy with the html5 proxy for there usecase | |
| 13:06:04 | sean-k-mooney | form the naming im pretty sure this is for https://shadow.tech/ | |
| 13:06:10 | mikal | The HTML5 proxy supports USB redirection? I find that surprising. | |
| 13:06:53 | sean-k-mooney | mikal: why? it a javascript applcation hosted on a webserver that tunnes the raw spice protocal over a webseocket | |
| 13:07:40 | sean-k-mooney | we are litrally just wrapping the spice tcp connetcion into a websocket and providign that to the browser | |
| 13:07:45 | mikal | So I haven't looked at the websocket proxy much, but the javascript app is definitely missing stuff -- it doesn't support all the compression options that the protocol does for example. Its missing GLZ IIRC. | |
| 13:08:11 | sean-k-mooney | mikal: right but that not a limitaiton fo openstack | |
| 13:08:23 | mikal | That's fair. | |
| 13:08:55 | sean-k-mooney | showdow likely have there own client that they are using with the websocket directly | |
| 13:09:36 | sean-k-mooney | they are defeitly not using horizon's or skylines | |
| 13:10:38 | mikal | Their site definitely mentions apps, but also says they have browser support. | |
| 13:10:59 | mikal | But yeah, they seem to be proposed a relatively overlapping subset of what I am proposing. | |
| 13:11:28 | sean-k-mooney | yep but you can do a lot in in a moddern browser :) | |
| 13:14:08 | sean-k-mooney | anyway if we can enabel both yoru use cases i would be happy too | |
| 13:18:49 | sean-k-mooney | mikal: +1 https://review.opendev.org/c/openstack/nova-specs/+/915190 minor line lenght issue on line 79 and you still need to menation addding the tls_port in the respocne | |
| 13:19:12 | sean-k-mooney | mikal: but if you fix those i think im +2 on the spec | |
| 13:20:14 | sean-k-mooney | i have also resolved teh irrelevent comments form older version so it shoudl be simpler to review now | |
| 13:20:44 | sean-k-mooney | bauzas: gibi can ye review ^ if you have time | |
| 13:20:59 | bauzas | I'll do | |
| 13:31:04 | opendevreview | Michael Still proposed openstack/nova-specs master: Propose a new API microversion for a spice-direct console. https://review.opendev.org/c/openstack/nova-specs/+/915190 | |
| 13:31:04 | mikal | sean-k-mooney: I've done both of those. | |
| 13:33:04 | mikal | sean-k-mooney: all of your comments about docs and so forth seem reasonable to me, but I am not passionate about updating the spec to include them right now given I worry that will derail the review process. | |
| 13:35:07 | mikal | sean-k-mooney: thanks for taking the time to work through the spec with me by the way, I appreciate it. | |
| 13:44:36 | sean-k-mooney | mikal: +2, no worries, happy to collaberate on thigns like this | |
| 13:44:53 | sean-k-mooney | i may or may not be able to comit to reviewing it before feature freeze but ill try as time allows | |
| 13:45:03 | mikal | pep8 hates me. | |
| 13:45:19 | sean-k-mooney | you know you can run that locally via docs right :P | |
| 13:45:59 | mikal | It only reported one error last time, so I thought if I fixed that it wouldn't be angry any more... | |
| 13:46:22 | sean-k-mooney | ah | |
| 13:46:38 | mikal | I have learnt my lesson | |
| 13:46:40 | sean-k-mooney | ya we normaly config it to keep going but that does not alwasy work depending on the tool we use | |
| 13:47:28 | opendevreview | Michael Still proposed openstack/nova-specs master: Propose a new API microversion for a spice-direct console. https://review.opendev.org/c/openstack/nova-specs/+/915190 | |
| 13:47:30 | mikal | Well, it passes locally now so let's see what Zuul thinks. | |
| 13:48:15 | mikal | I really should go to bed, its nearly midnight here. I guess I'll see if this thing is approved in the morning. Thanks again for your help. | |
| 13:57:33 | mikal | sean-k-mooney: it passes CI now, so that's nice. On that note I'm off to bed. | |
| 14:49:45 | bauzas | sean-k-mooney: mikal : I have two comments in https://review.opendev.org/c/openstack/nova-specs/+/915190 | |
| 14:49:55 | bauzas | one about testing at least | |
| 14:50:12 | bauzas | I'm okay with accepting this spec, but only provided we're clear on the testing side | |
| 14:51:06 | sean-k-mooney | bauzas: well i suggested tempest testign at the ptg and we said we shoudl not make that a requirement | |
| 14:51:13 | sean-k-mooney | so the expection is unit and fucntional tests only | |
| 14:51:27 | sean-k-mooney | wiht no devstack based tempest job | |
| 14:52:06 | dansmith | did we? | |
| 14:52:07 | sean-k-mooney | i origally asksed for a devstack plugin and to enabel it in one of our jobs but dansmith and other feel that was two high a bar to require in the spec | |
| 14:52:30 | dansmith | I really thought I brought up that we at least sniff test the RFB banner for our existing vnc stuff | |
| 14:52:39 | bauzas | sean-k-mooney: see my comment | |
| 14:53:02 | sean-k-mooney | dansmith: i susggested a temept test like that but i tought you said no to that | |
| 14:53:04 | bauzas | I proposed a fake object in the functional tests that would reproduce some external call | |
| 14:53:14 | sean-k-mooney | dansmith: im not agaisn doing that just liek we do for vnc | |
| 14:53:32 | dansmith | sean-k-mooney: okay I thought the opposite, but it's been a while | |
| 14:53:37 | bauzas | but I'm afraid unittests and API functional tests aren't enough | |
| 14:56:05 | sean-k-mooney | bauzas: sure so i think we can approve the spec as is if you have no other obejctsion and work on a trivial devstack plugin with mikal as part fo the implemnation and add this into one of our exsitign jobs | |
| 14:56:31 | bauzas | sean-k-mooney: I'm not really asking for a devstack plugin | |
| 14:56:35 | sean-k-mooney | the plugin would resided in the kerbside repo or a new dedeicated repo | |
| 14:56:47 | sean-k-mooney | bauzas: well something need to be able to deploy kerbside | |
| 14:56:48 | bauzas | I'm just asking for a fake object that would simulate the HTTP calls | |
| 14:57:00 | bauzas | so we just call we correctly call it | |
| 14:57:14 | sean-k-mooney | we dont call kerbside | |
| 14:57:16 | sean-k-mooney | it calls us | |
| 14:57:43 | sean-k-mooney | we shoudl have tempest coverage for the new api changes | |
| 14:57:48 | sean-k-mooney | or could | |
| 14:58:18 | sean-k-mooney | but im not sure that give use anything over the api sample tests | |
| 14:58:36 | sean-k-mooney | unless we actully deploy with kerbside as well in a tempest job | |
| 14:58:47 | bauzas | hmmmmmmmmm | |
| 14:59:04 | bauzas | the whole spec is about passing a configurable URL to our API | |
| 14:59:16 | bauzas | that's it | |
| 14:59:20 | sean-k-mooney | not entirly | |
| 14:59:29 | sean-k-mooney | it about that + adding new devices to the xml | |
| 14:59:55 | sean-k-mooney | and 2 addtion tional api chagnes | |
| 15:00:00 | sean-k-mooney | 1 to define the console type | |
| 15:00:18 | sean-k-mooney | and another to expsoe the tls_port via the existing admin only console token api | |
| 15:00:24 | bauzas | lemme be clear | |
| 15:00:29 | bauzas | I was confusing | |
| 15:00:37 | bauzas | " | |
| 15:00:42 | bauzas | "The user then fetches that URL, and Kerbside delivers a .vv file with the | |
| 15:00:44 | bauzas | connection information for a SPICE client. Kerbside uses a call to | |
| 15:00:46 | bauzas | `/os-console-auth-tokens/bf2e6883-...` to determine the validity of the | |
| 15:00:48 | bauzas | console authentication token, and the connection information for the console." | |
| 15:01:31 | bauzas | here, I think we can have a functional test that would reproduce the kerbside server by calling the os-console-auth-tokens | |
| 15:01:39 | bauzas | as a mock object | |
| 15:01:42 | sean-k-mooney | sure | |
| 15:01:53 | sean-k-mooney | but thats just a standar fucntional test | |
| 15:01:55 | bauzas | this way, we wouldn't need to depend on kerbside | |
| 15:02:07 | sean-k-mooney | sure but i was edxpecting that already | |
| 15:02:14 | bauzas | right, that's what I'm asking in https://review.opendev.org/c/openstack/nova-specs/+/915190/13/specs/2024.2/approved/libvirt-spice-direct-consoles.rst | |
| 15:02:44 | bauzas | from what I just read, mikal only proposed manual testing | |
| 15:03:07 | sean-k-mooney | we can make that more explit but i was starting form a baseline of "all new feature should have unit and functional tests as our minium baseline" | |
| 15:03:15 | sean-k-mooney | no | |
| 15:03:28 | bauzas | for functional tests, quoted " However, a test for the | |