| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-02 | |||
| 15:00:02 | jaypipes | mnaser: that's what I was afraid of... thanks. | |
| 15:01:26 | mnaser | dansmith: yeah, im not sure how we "do something on our end" because of the design architecture, it doesn't give you a way to pass that info, unless we implement our own vnc.html and store it in repo and serve it.. overlaid on top of the vnc code, it starts to be iffy | |
| 15:01:37 | mnaser | I pinned the novnc release in openstack ansible to avoid this but yeah. | |
| 15:01:41 | mnaser | jaypipes: :< you're welcome | |
| 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 | 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:10:50 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Do not persist RequestSpec.ignore_hosts https://review.openstack.org/649363 | |
| 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 | 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:07:36 | sean-k-mooney | |Using -iter or -pbkdf2 would be better. | |
| 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 | |