| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-18 | |||
| 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 | |
| 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 | |
| 15:03:30 | bauzas | API microversion will be added" | |
| 15:03:56 | bauzas | which is different from having a mock object that would simulate kerbside external call to Nova | |
| 15:04:20 | sean-k-mooney | so i expect the testing section of the spec to call out any special testing behiound our baselien | |
| 15:04:45 | sean-k-mooney | i dont in genrall expect to say "we will add unit and fucntional tests" but if you want that to be updated to included that im fine with that | |
| 15:05:22 | sean-k-mooney | the sepcific thing you asked for however i dont think makes sense for us to test | |
| 15:05:41 | sean-k-mooney | nova has 2 existign apis both fo which have test coverage | |
| 15:06:12 | sean-k-mooney | the console url show rpvoded a url that is templated based on `spice.kerbside_base_url` | |
| 15:06:25 | bauzas_ | okay, then I'll accept the spec but asking for a FUP | |
| 15:06:48 | sean-k-mooney | the url provided to the use rwhen they do show has a console token which kebside will use to call /os-console-auth-tokens | |
| 15:06:58 | sean-k-mooney | so im expeicting 2 seperte tests | |
| 15:07:16 | sean-k-mooney | one to cover the new console type for console url show | |
| 15:07:35 | sean-k-mooney | and one to test the addtion of a tls_port for spice-direct in /os-console-auth-tokens | |
| 15:53:01 | opendevreview | Merged openstack/nova-specs master: Re-propose using extend volume completion action for 2024.2 https://review.opendev.org/c/openstack/nova-specs/+/917133 | |
| 16:05:43 | bauzas | sean-k-mooney: dansmith: gibi: I know this is late, but could you please look at https://review.opendev.org/c/openstack/nova-specs/+/922201 if you have time today ? | |
| 16:05:58 | bauzas | I can also try to look at it before leaving | |
| 16:09:35 | dansmith | there's a ton of unresolved comments on that? | |
| 16:12:25 | bauzas_ | I think fwiesel replied on sean's comments | |
| 16:12:29 | bauzas_ | but sure, I can look at those | |