| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-27 | |||
| 14:34:17 | sean-k-mooney | efried: no | |
| 14:34:43 | sean-k-mooney | i was logged off ill look at irc logs unless you want to summerise | |
| 14:34:54 | efried | sean-k-mooney: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2020-02-26.log.html#t2020-02-26T23:11:23 | |
| 14:35:00 | efried | not too much to summarize | |
| 14:35:59 | sean-k-mooney | ah i see | |
| 14:36:21 | efried | I don't like the idea of un-singleton-ing, which would mean we do the whole Adapter setup with every API call, versus putting in something that will keep the token alive for us. If that thing is converting to sdk, win/win. | |
| 14:36:28 | sean-k-mooney | so ya my theory is that by making it a singolton in the current patch we are not refrshing the token | |
| 14:36:45 | sean-k-mooney | so the fact we were always createing the adapters in other case was masking that | |
| 14:36:53 | efried | yup | |
| 14:37:28 | sean-k-mooney | so unless we do the swich i think the singolton is wrong or at least we would need to add an expiry/token check to it if we kept it | |
| 14:38:18 | sean-k-mooney | i have been using https://review.opendev.org/#/c/679914/ | |
| 14:38:21 | sean-k-mooney | in my testing | |
| 14:38:30 | sean-k-mooney | oh which is now merged cool | |
| 14:38:45 | sean-k-mooney | so that seams to work fine | |
| 14:39:15 | sean-k-mooney | if that has all the feature we need then ya using the sdk i think make sense if it will handel the tokens for us | |
| 14:44:07 | sean-k-mooney | efried: do you know if an update to the series is in the works | |
| 14:44:44 | efried | I don't know, no. | |
| 14:44:52 | sean-k-mooney | ok | |
| 14:45:33 | efried | If there's now real sdk impl (rather than just REST primitive passthrough) that we could switch to, so much the better, though I would argue that should be done later, since it doesn't involve a problem we need to solve immediately. | |
| 14:45:40 | sean-k-mooney | in that case i might just put a testing patch on the top of the chain that swaps the function calls and runs my multi node job to see if it fixes it | |
| 14:46:22 | sean-k-mooney | ya i was thinkig of just doint the s/get_ksa_adapter/get_sdk_adapter/ | |
| 14:46:32 | efried | yeah | |
| 14:46:36 | efried | worth a try | |
| 14:46:36 | sean-k-mooney | and see if that works | |
| 14:46:46 | efried | I don't remember for sure whether that's all it takes... | |
| 14:47:03 | efried | but easy enough to sniff out. | |
| 14:47:18 | sean-k-mooney | if we are basically doing raw request i think it is | |
| 14:47:20 | efried | If you have a live env handy it might make sense to try it out to winnow out any easy problems. | |
| 14:47:30 | sean-k-mooney | if we wanted to use the new proxy function that would be more involed | |
| 14:47:32 | efried | yeah, it wouldn't be wrt the requests, it would be with the setup. | |
| 14:49:17 | sean-k-mooney | did you see i updated https://review.opendev.org/#/c/673341/ yesterday by the way | |
| 14:49:33 | sean-k-mooney | im going to try and do the rest of the cain later today | |
| 14:51:16 | efried | sean-k-mooney: yeah, I was going to wait until all updated to start reviewing, unless you think I should look sooner. | |
| 14:51:48 | sean-k-mooney | am whatever works for you. most of the comments were gibis so i think he is the one that is more pressing | |
| 14:52:03 | sean-k-mooney | that said waiting till they are all done is proably for the best | |
| 14:52:27 | sean-k-mooney | that way you will have less context switching | |
| 14:53:02 | efried | mordred: I think there's not too much context to be missed here. In proposed patch series, nova is talking to cyborg using a raw ksa adapter. I insisted we make that client a singleton, and we're seeing tokens expire after a while (I guess ksa doesn't take care of keeping them fresh). We want to cut over to using sdk anyway, so if sdk handles expiration under the covers, two birds. | |
| 14:53:08 | sean-k-mooney | so ya im just going to grab lunch. when i get back ill quickly do the test patch for the cyborg series and then ill start on the provider.yaml patches | |
| 14:53:18 | efried | ++ | |
| 14:57:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Always provide the size in bytes when calling virDomainBlockResize https://review.opendev.org/707590 | |
| 14:58:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: images: Remove Libvirt specific configurable use from qemu_img_info https://review.opendev.org/707591 | |
| 15:03:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Remove MIN_LIBVIRT_MULTIATTACH https://review.opendev.org/710238 | |
| 15:03:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Remove QEMU_VERSION_REQ_SHARED https://review.opendev.org/710239 | |
| 15:05:53 | openstackgerrit | Stephen Finucane proposed openstack/os-traits master: setup.cfg: Configure 'python-requires' https://review.opendev.org/704534 | |
| 15:05:54 | openstackgerrit | Stephen Finucane proposed openstack/os-traits master: setup.cfg: Remove unused 'build_sphinx' configuration https://review.opendev.org/704535 | |
| 15:05:55 | openstackgerrit | Stephen Finucane proposed openstack/os-traits master: Remove use of six https://review.opendev.org/710274 | |
| 15:05:56 | openstackgerrit | Stephen Finucane proposed openstack/os-traits master: Switch to hacking 2.x https://review.opendev.org/710275 | |
| 15:08:38 | stephenfin | lyarwood: I've a question left at https://review.opendev.org/#/c/706898/ btw | |
| 15:08:56 | stephenfin | regarding how we're going to backport this fix if depends on a new oslo.utils feature | |
| 15:09:07 | stephenfin | not sure if you've thought through that yet or no | |
| 15:12:38 | mordred | efried: ok. that's something I think we should still look in to ... ksa should be auto-renewing tokens ... but sdk should _certainly_ be doing it | |
| 15:13:03 | mordred | efried: and I'd consider it a bug in sdk if it's not - even if it's something we should be getting ksa to do for us - so that sounds like a good plan | |
| 15:13:20 | efried | ack, thx | |
| 15:13:44 | mordred | efried: are these tokens created from credentials nova has? or tokens received from the user that nova is re-using? | |
| 15:14:13 | efried | mordred: from conf, via our soon-to-be-famous oslo.config loader | |
| 15:14:25 | efried | um, I think | |
| 15:14:45 | efried | Now I can't remember whether we decided to do cyborg-ness as user or admin... | |
| 15:15:20 | efried | ...user | |
| 15:15:45 | efried | mordred: so you can conf the session & adapter opts, but the auth should be the nova user's. | |
| 15:15:54 | efried | Which means it makes even less sense that the token is expiring. | |
| 15:16:38 | lyarwood | stephenfin: sorry just back from a call | |
| 15:17:19 | lyarwood | stephenfin: yeah I was talking of backporting this more in a downstream context where we control everything within a given container | |
| 15:17:36 | lyarwood | stephenfin: and then only to stable/train | |
| 15:17:48 | lyarwood | stephenfin: for upstream, what do you mean in terms of vendoring? | |
| 15:20:07 | stephenfin | I mean copying the newly updated contents of 'oslo_utils.imageutils' into the nova tree | |
| 15:21:17 | openstackgerrit | GEET JAIN proposed openstack/nova master: Fix bug where aggregate api works with invalid id https://review.opendev.org/710290 | |
| 15:22:33 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Remove QEMU_VERSION_REQ_SHARED https://review.opendev.org/710239 | |
| 15:22:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Remove MIN_LIBVIRT_MULTIATTACH https://review.opendev.org/710238 | |
| 15:22:56 | lyarwood | stephenfin: ah wait, so in the context of the bugfix I just need to bump requirements in https://review.opendev.org/#/c/706900/ | |
| 15:24:01 | stephenfin | For the master patch, yup | |
| 15:24:11 | stephenfin | Assuming a new oslo.utils patch has been cut since | |
| 15:25:15 | lyarwood | https://review.opendev.org/#/c/710078/ - cut but not released | |
| 15:29:50 | mordred | efried: yeah - it should *really* be renewing | |
| 15:30:47 | openstackgerrit | Lee Yarwood proposed openstack/nova master: workarounds: Add option to disable native LUKSv1 decryption by QEMU https://review.opendev.org/708030 | |
| 15:30:48 | openstackgerrit | Lee Yarwood proposed openstack/nova master: workarounds: Connect RBD volumes to the compute host as block devices https://review.opendev.org/708029 | |
| 15:31:01 | sean-k-mooney | efried: we should be useing the token form the api request not form the config | |
| 15:31:42 | sean-k-mooney | we are not adding setting up the service user in the config and we are not adding a cyborg section to the nova.conf | |
| 15:32:15 | sean-k-mooney | so i dont think the issue is with renewing the token in the nova code | |
| 15:32:43 | sean-k-mooney | i think with the singelton we create the client onece capturing the context and token at that point | |
| 15:32:51 | sean-k-mooney | and then it never gets updated again | |
| 15:34:35 | sean-k-mooney | mordred: ^ that is also relevent to you i guess | |
| 15:37:34 | mordred | yeah. that - if we're using the api request token, then when we get the singleton sdk connection object, we need to call conn.connect_as on it to get a new connection with the credentials we've been given, otherwise we're making all of the requests with the creds of the original user :) | |
| 15:38:09 | sean-k-mooney | ya we are not doing that | |
| 15:38:19 | sean-k-mooney | so that is likely the bug | |
| 15:38:35 | mordred | conn.connect_as lets us keep discovery cache appropriately and all of that - but will construct a new underlying Auth construct and a new Adapter | |
| 15:38:59 | mordred | sean-k-mooney: computers are hard | |
| 15:39:18 | sean-k-mooney | mordred: we would do that here right https://review.opendev.org/#/c/631242/51/nova/accelerator/cyborg.py@32 just before the retrun? | |
| 15:39:32 | sean-k-mooney | although im not sure that would be safe | |
| 15:39:47 | sean-k-mooney | we could context switch at any point so we would have to take the lock | |
| 15:39:58 | sean-k-mooney | and do it before making any call | |
| 15:41:49 | mordred | sean-k-mooney: sdk is threadsafe | |
| 15:41:55 | mordred | you shouldn't need a lock | |
| 15:42:12 | sean-k-mooney | is it eventlet safe | |
| 15:42:22 | mordred | _should_ be | |
| 15:42:25 | mordred | if it's not it's a bug | |
| 15:42:38 | sean-k-mooney | if the con.connect_as returns a new object that we then make new calls on it should work | |
| 15:42:42 | mordred | I mean - but that said - with eventlet who the hell knows | |
| 15:42:53 | mordred | sean-k-mooney: yes - connect_as returns a new object | |
| 15:43:29 | mordred | it returns a whole new Connection with all the settings of the old one but with just the auth bits replaced | |
| 15:43:32 | sean-k-mooney | and that object basicaly poxies the requeqes on it to the adapter it was created form but with the update creds | |