Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-02-27
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
15:44:21 mordred it has to create a new adapter - because an adapter is a container of session+auth+endpoint-config - you can't use the old adapter if you have new auth, you need a new adapter
15:44:36 mordred but it does keep/share the things that are safe to keep/share
15:44:36 sean-k-mooney is so then we just need to change "return _CYBORG_CLIENT" to "return _CYBORG_CLIENT.connect_as(context)
15:44:43 mordred yeah
15:45:13 mordred with a little hand wavey, because we'll want to make sure we're doing the right things with context ... but pseudo-code-wide yes
15:45:36 sean-k-mooney ok so ill swap to the sdk adapter and add the connect_as
15:46:49 sean-k-mooney mordred: ya, this isnt my code and i havent realy review it im just testing it but i can relyably repoduce that the current code is borked so tring to fine the minimal correct fix and test that in a follow up patch
15:47:09 sean-k-mooney if it fixes the issue then we can squash it in and move on to the next thing
15:47:41 mordred ++
15:48:00 mordred let me know if you have issues and want me to dive in further
15:48:11 mordred I'm sure I could find some lovely new ways to curse
15:56:00 efried If we have to build a new adapter anyway, then I suppose we might as well just do as dansmith suggests and un-singleton for now. We'll want to do the SDK cutover later, but there's no way we'll forget to do that.
15:56:14 dansmith ++
15:57:48 sean-k-mooney im not sure if the connect_as is cheaper then building the adapter from scratch but ya i just want to see this working and then iterate on it later to not loose the momentem we have while people are around to ask questions
15:58:15 sean-k-mooney *answer
15:58:49 openstackgerrit Stephen Finucane proposed openstack/os-resource-classes master: Switch to hacking 2.x https://review.opendev.org/710276
16:34:55 openstackgerrit Merged openstack/os-traits master: setup.cfg: Configure 'python-requires' https://review.opendev.org/704534
16:34:56 openstackgerrit Merged openstack/os-traits master: setup.cfg: Remove unused 'build_sphinx' configuration https://review.opendev.org/704535
16:40:46 gibi sean-k-mooney: re: provider config. I have to take tomorrow off so I will look at your update earliest on Monday
16:48:04 Sundar dansmith, gibi, sean-k-mooney, efried: On the rescheduling front, I don't have a multi-node env to test it. It is not going to be easy for me to get the lab resources, even without FPGAs, to set that up. Plus, I have other things I need to focus on. Any ideas on bringing this to a timely closure?
16:48:27 dansmith Sundar: functional tests?
16:48:42 efried Sundar: Also, sean-k-mooney set up a multinode CI job.
16:49:21 dansmith multinode ci might be hard to trigger a reschedule though
16:49:54 dansmith Sundar: tangential to this part, did you see my comment about moving the event to the front of the queue, below the -2'd ksa patch?
16:50:52 Sundar dansmith: Yes, I saw that. I have moved it before the ksa patch in my local env. The next commit should have it.
16:50:59 dansmith Sundar: cool

Earlier   Later