| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-02 | |||
| 15:02:26 | dansmith | mnaser: yeah, I'm guessing that is what they're saying, that we should provide our own implementation of the client (html) if we're going to do the auth part | |
| 15:02:56 | mnaser | dansmith: the weird thing is that the auth part, they have their own "token" stuff, including something called token_plugins which you can implement | |
| 15:03:10 | mnaser | but even if you implement a token plugin, you can't even use novnc without rewriting things.. I don't get it. | |
| 15:04:00 | dansmith | without rewriting an html file you mean right? | |
| 15:06:47 | dansmith | passing the token in path means we just get the token at the websocket url, right? | |
| 15:06:59 | dansmith | that'd be the right place to do the auth, so why is that a problem? | |
| 15:08:54 | dansmith | (looks further) yeah, that's where we're getting the token for our server side websocket | |
| 15:09:05 | dansmith | so I'm not sure why that's not a solution | |
| 15:10:49 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Add functional regression test for bug 1669054 https://review.openstack.org/649362 | |
| 15:10:50 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Do not persist RequestSpec.ignore_hosts https://review.openstack.org/649363 | |
| 15:10:50 | openstack | bug 1669054 in OpenStack Compute (nova) stein "RequestSpec.ignore_hosts from resize is reused in subsequent evacuate" [Medium,In progress] https://launchpad.net/bugs/1669054 - Assigned to Matt Riedemann (mriedem) | |
| 15:13:07 | kashyap | efried++ | |
| 15:13:24 | kashyap | (Hmm, should probably get a karma bot in here.) | |
| 15:17:35 | dansmith | no, we shouldn't. | |
| 15:19:49 | mnaser | dansmith: what if we did ?path=token -- thoughts? | |
| 15:20:01 | mnaser | then we can drop that whole path parsing thing and just grab all query params | |
| 15:20:32 | dansmith | mnaser: you mean path=$token instead of path=token=$token? | |
| 15:20:37 | mnaser | yeah | |
| 15:20:55 | kashyap | dansmith: Was only joking; where is your characteristic sense of humor... | |
| 15:20:56 | dansmith | you don't want to defeat the ability to actually use a path as part of that and have a proxy in between do you? | |
| 15:21:32 | dansmith | path=$token seems far more hacky than actually providing a legit url as the path | |
| 15:21:38 | mnaser | yeah, that is valid | |
| 15:22:00 | mnaser | the only concern I have now is *hopefully* that works for both old and new novnc | |
| 15:22:47 | dansmith | not sure why that matters, since we control our package versions (and it's up to the distro to match), but as far as I can tell, they didn't change the path behavior | |
| 15:22:50 | dansmith | or claim not to | |
| 15:25:23 | mnaser | dansmith: nova's CI actually installs novnc from distro pkgs which is probably why this wasn't caught | |
| 15:25:33 | mnaser | and yeah, the path behavior seems to always have been there and continued to be there | |
| 15:25:44 | dansmith | oh? I thought we had an u-c for it | |
| 15:25:55 | mnaser | nope, discovered this yesterday | |
| 15:26:17 | mnaser | we have u-c for Websockify probably | |
| 15:26:21 | mnaser | but novnc isn't even a python package so yeah | |
| 15:26:41 | mnaser | dansmith: https://github.com/openstack-dev/devstack/blob/358cc122c3a6d30bf043b3e478790fd2773e9a88/.zuul.yaml#L220 | |
| 15:27:22 | dansmith | okay | |
| 15:27:29 | dansmith | yeah I guess that makes sense actually | |
| 15:27:31 | sean-k-mooney | mnaser: oh your right we set NOVNC_FROM_PACKAGE=True | |
| 15:35:30 | openstackgerrit | Mohammed Naser proposed openstack/nova master: wip: start using ?path=%3Ftoken%3D= |
|
| 15:35:44 | mnaser | I don't have time to follow up on this too much but I guess its a start for someone to go through and start a discussion | |
| 15:36:26 | dansmith | seems like melwitt has some interest at least | |
| 15:37:14 | mnaser | yeah, so maybe that's the initial copy pasta which will probably fail | |
| 15:39:21 | cfriesen | sean-k-mooney: do you know if nova is supposed to handle PCI passthrough for Intel QAT devices? | |
| 15:39:28 | sean-k-mooney | yes | |
| 15:39:37 | sean-k-mooney | it has worked since before icehouse | |
| 15:39:49 | sean-k-mooney | you can do both pf and vf passhtough | |
| 15:40:48 | sean-k-mooney | i think intel QAT device were perhaps the first usecase that pci passhtough was enabeld for. i know nic came after it | |
| 15:41:00 | cfriesen | sean-k-mooney: we're hitting a weird issue where they're hittting the SRIOV_VF clause in LibvirtDriver._get_device_type() and failing in pci_utils.get_ifname_by_pci_address(). Are we configuring something wrong? | |
| 15:42:08 | sean-k-mooney | cfriesen: do you have a physnet set in the pci whitelist for that device | |
| 15:42:27 | cfriesen | sean-k-mooney: not sure, can check. Should it have one? | |
| 15:42:34 | sean-k-mooney | no | |
| 15:42:43 | sean-k-mooney | that should only be set on nics | |
| 15:43:17 | sean-k-mooney | QAT we expect the type of the device to be PCI for both pf and vf in the pcimanager | |
| 15:43:19 | openstackgerrit | Colleen Murphy proposed openstack/nova stable/stein: Move create of ComputeAPI object in websocketproxy https://review.openstack.org/649374 | |
| 15:43:37 | openstackgerrit | Colleen Murphy proposed openstack/nova stable/rocky: Move create of ComputeAPI object in websocketproxy https://review.openstack.org/649375 | |
| 15:43:44 | sean-k-mooney | but its possible that the code we added for the bandwith based schduing is causing issues | |
| 15:44:49 | sean-k-mooney | cfriesen: https://github.com/openstack/nova/blob/master/nova/pci/request.py#L16-L39 | |
| 15:45:59 | cfriesen | sean-k-mooney: sweet, thanks | |
| 15:46:10 | sean-k-mooney | "product_id": "0443" is the VF and "product_id": "0442" is the pf | |
| 15:47:41 | sean-k-mooney | cfriesen: yep QAT was the thing that intoduce pci passthough https://github.com/openstack/nova/commit/fe67148234dba42468793f33c2ca83ce0616e824 so it really should work still | |
| 15:48:15 | cfriesen | sean-k-mooney: I expect it's a config issue. | |
| 15:49:02 | sean-k-mooney | cfriesen: its posible but the pci_utils.get_ifname_by_pci_address call was intoduce for stien for band with based schudling so there could be a bug. i dont have qat devices to test with | |
| 15:49:37 | sean-k-mooney | well the fucntion existed before we jsut use it in more places now | |
| 15:50:48 | efried | sean-k-mooney: this look kosher to you: https://review.openstack.org/#/c/635533/ | |
| 15:51:00 | efried | dansmith: ^ if you please? | |
| 15:51:35 | sean-k-mooney | xen is not really my thing but ill take a look | |
| 15:52:22 | efried | sean-k-mooney: it's not a xen thing, really an ssl thing. | |
| 15:54:07 | dansmith | it's not an ssl thing, it's a python/oslo thing AFAICT | |
| 15:55:21 | sean-k-mooney | ya its processutils exit code checking | |
| 15:55:34 | sean-k-mooney | but yes on second look it looks sane to me | |
| 15:56:32 | sean-k-mooney | efried: is there any change that the password could be loged to standard error if openssl did not like it | |
| 15:57:05 | openstackgerrit | Eric Fried proposed openstack/nova master: Add minimum value in max_concurrent_live_migrations https://review.openstack.org/648302 | |
| 15:58:07 | efried | sean-k-mooney: I would seriously hope there is no ssl command in existence that will echo back a password to you :) | |
| 15:58:34 | efried | dansmith: only peripherally. They're just changing from "anything on stderr means we should fail" to "nonzero return code means we should fail". | |
| 15:58:48 | dansmith | ...right | |
| 16:00:25 | sean-k-mooney | efried: yes | |
| 16:00:27 | dansmith | efried: the patch asserts that before, writing anything to stderr would cause it to raise, even if it exited with zero, rght? | |
| 16:00:35 | dansmith | I don't see where in oslo that behavior happens | |
| 16:00:43 | dansmith | (it'd be pretty dumb, which is why I'm curious) | |
| 16:01:41 | sean-k-mooney | dansmith: https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L414-L424 | |
| 16:02:13 | efried | dansmith: It wasn't in oslo, they were triggering the exception here (in nova) based on stderr being nonempty. | |
| 16:02:16 | dansmith | sean-k-mooney: that's not what I'm asking about | |
| 16:02:40 | dansmith | efried: oh? I thought they were asserting that was oslo... | |
| 16:02:55 | dansmith | oh RuntImeError, I see | |
| 16:03:35 | sean-k-mooney | they are assertign that if openssl wrote anything to stderr then nova would raise the runtime error | |
| 16:04:15 | dansmith | efried: that makes it much more openssl-related than I thought, I just zeroed in on the process handling.. so I dunno, probably need someone much more familiar with openssl to validate those semantics | |
| 16:04:23 | sean-k-mooney | im guessing that if openssl ever decied to add a deperaction warning or somehting this caused issues for them | |
| 16:04:49 | dansmith | right, I definitely get that | |
| 16:05:37 | sean-k-mooney | i looks liek they jsut want to be a little more graceful and assume openssl follow the standard unix thing of if it returned and exit code of 0 it succeded | |
| 16:05:43 | sean-k-mooney | which i think is resonable | |
| 16:05:56 | sean-k-mooney | did the bug have an explcit example | |
| 16:06:25 | dansmith | sean-k-mooney: yes, that's all obvious :) | |
| 16:07:08 | sean-k-mooney | yes so the patch is sane yes to efried original quetion | |
| 16:07:24 | sean-k-mooney | the bug was reported as a deprecation warning as i gueesed | |
| 16:07:34 | sean-k-mooney | |RuntimeError: OpenSSL error: *** WARNING : deprecated key derivation used. | |
| 16:07:36 | sean-k-mooney | |Using -iter or -pbkdf2 would be better. | |
| 16:07:36 | dansmith | the original code didn't look at the return code for something security-related and so changing that behavior is potentially pretty impactful | |
| 16:08:40 | sean-k-mooney | that is possibly true yes in this specifc instance othe Runtime error that was raise i think its safe but you are concerend that there coudl be other case where it would not be | |
| 16:09:21 | dansmith | I'm saying they used to fail if anything was written to stderr and now they won't | |
| 16:09:26 | sean-k-mooney | i woudl hope openssl would not exit with code 0 for anything other then sucess but i dont know that for certine even if i belive it very likely | |
| 16:09:33 | dansmith | and depending on what is going on here, that could be, you know, a big deal | |
| 16:09:35 | sean-k-mooney | yes | |
| 16:09:42 | dansmith | depends on the command and what is going on | |