Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-02-27
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
16:51:14 Sundar dansmith: With functional tests, I can check if the logs show rescheduling messages, and that the tests pass. Is there anything more you are looking for?
16:51:49 efried You can do better than that to prove a reschedule occurred: use the fixture that makes the initial host predictable and prove that it lands on a different one.
16:52:11 gibi efried ++
16:52:11 dansmith Sundar: "tests passing" isn't a bar to meet, "tests validating something useful" is
16:52:12 efried there should be some good examples
16:52:15 dansmith yep, what efried said
16:52:48 Sundar efried: Thanks, will look for that. If you happen to remember the(rough) name of the fixture, off the top of your head, I'll start with that.
16:53:00 dansmith Sundar: what specifically are you stuck on? there's lots of feedback to work on without much response from you yet that I can see.. is there something specific that you're blocked on?
16:53:46 Sundar dansmith: Not sure that any of my changes will actually work in the real world, without a multinode env. I tried to get a 2nd server but it will take time, if at all.
16:53:53 efried Sundar: HostNameWeigherFixture I think
16:53:58 gibi Sundar: HostNameWeigherFixture makes the host selection peridctable
16:54:11 dansmith Sundar: functional tests man :)
16:54:12 Sundar efried, gibi: Thanks, got it
16:54:50 dansmith Sundar: if you have reasonable functional tests that don't mock out insane levels of things and validate reasonable conditions, then any real world failure we find later is understandable
16:55:53 dansmith Sundar: test_servers.py has a bunch of reschedule-specific tests
17:00:36 Sundar dansmith: Ok, let's just focus on functional tests, then. BTW, https://review.opendev.org/gitweb?p=openstack/nova.git;f=nova/conductor/manager.py;hb=refs/changes/44/631244/63#l602 is specific to the resheduling path AFAICS. is there any other path, like rebuild/resize/whatever, that invokes this?

Earlier   Later