| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-18 | |||
| 12:44:29 | mikal | Oh bugger. I _do_ need a schema change. There is only one port column in the ConsoleAuthToken table and I need two (one for TLS). | |
| 12:44:55 | mikal | I'll tweak the spec now. | |
| 12:45:03 | sean-k-mooney | ok well that is easy to add to the spec i dont think that woudl be contoverial | |
| 12:45:08 | mikal | Look, if I've missed the deadline so be it. Its frustrating but its not the end of the world. | |
| 12:47:01 | 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:48:10 | sean-k-mooney | mikal: i think you have some other issues | |
| 12:48:31 | sean-k-mooney | mikal: im not seeing the propsoe chage header | |
| 12:49:07 | mikal | I'm not sure what a "propose change header" is? | |
| 12:49:15 | mikal | Oh you mean in Gerrit? | |
| 12:49:27 | sean-k-mooney | yep it will fail our ci job | |
| 12:49:34 | sean-k-mooney | https://github.com/openstack/nova-specs/blob/master/specs/2024.2-template.rst?plain=1#L92 | |
| 12:50:08 | sean-k-mooney | oh no | |
| 12:50:23 | sean-k-mooney | you have it but you started descibibg the propsoed change in the problem description section | |
| 12:51:22 | sean-k-mooney | lets ignore that for now but basiclly the "Problem description" shoudl descibe the "what" and "why" not the "how" | |
| 12:51:52 | sean-k-mooney | so the discussion of config option or api shoudl be in the propsoe changes btu that not a big deal | |
| 12:51:53 | mikal | So basically everything except the first para of "problem description" should move down? | |
| 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 | mikal | sean-k-mooney: I've done both of those. | |
| 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: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 | |