Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-08-27
17:00:18 gmaan sean-k-mooney: you mean nova change did not work with glance revert?
17:00:28 sean-k-mooney no the cider oen i crfeated
17:00:28 gmaan or glance revert things only
17:00:48 sean-k-mooney i looke at how nova creates our glance client nad tried to do the same in cidner
17:00:50 gmaan ohk, i can check that. cinder to send right roles in nova, I can check for glance
17:01:12 gmaan i checked all those files recently so it will be quick, looking..
17:02:31 sean-k-mooney https://review.opendev.org/c/openstack/cinder/+/958582
17:02:43 gmaan sean-k-mooney: here is issue, cinder does not send the confiugured service user to glance instead send the user token (context) + service token so user token does not have 'service' role - https://github.com/openstack/cinder/blob/89709ef589b74506885c72e42cad57eb8dedfb56/cinder/image/glance.py#L134C45-L134C52
17:03:34 gmaan here cinder should have first try to get the configured service user and that auth plugin (load from keystoneauth) they can send along with service token
17:03:48 opendevreview Balazs Gibizer proposed openstack/nova master: Ask for pre-prod testing for native threading https://review.opendev.org/c/openstack/nova/+/957424
17:03:49 opendevreview Balazs Gibizer proposed openstack/nova master: Run unit test with threading mode https://review.opendev.org/c/openstack/nova/+/953475
17:03:49 opendevreview Balazs Gibizer proposed openstack/nova master: Allow to start unit test without eventlet https://review.opendev.org/c/openstack/nova/+/953436
17:03:50 opendevreview Balazs Gibizer proposed openstack/nova master: [CI]Make nova-tox-py312-threading voting https://review.opendev.org/c/openstack/nova/+/955791
17:03:50 opendevreview Balazs Gibizer proposed openstack/nova master: [test]RPC using threading or eventlet selectively https://review.opendev.org/c/openstack/nova/+/953815
17:03:51 opendevreview Balazs Gibizer proposed openstack/nova master: Make RBD Tpool usage conditional https://review.opendev.org/c/openstack/nova/+/956089
17:03:51 opendevreview Balazs Gibizer proposed openstack/nova master: Do not yield in threading mode https://review.opendev.org/c/openstack/nova/+/950994
17:03:52 opendevreview Balazs Gibizer proposed openstack/nova master: Fix ProviderTree copying with threading Lock https://review.opendev.org/c/openstack/nova/+/956091
17:03:52 opendevreview Balazs Gibizer proposed openstack/nova master: Make libvirt Tpool proxying conditional https://review.opendev.org/c/openstack/nova/+/956090
17:03:54 opendevreview Balazs Gibizer proposed openstack/nova master: [vncproxy]Handle ssl.wrap_socket removal in py312 https://review.opendev.org/c/openstack/nova/+/955915
17:03:54 opendevreview Balazs Gibizer proposed openstack/nova master: [test]Further categorization of disabled unit tests https://review.opendev.org/c/openstack/nova/+/956092
17:03:56 opendevreview Balazs Gibizer proposed openstack/nova master: Warn on long task wait time for executor https://review.opendev.org/c/openstack/nova/+/952666
17:04:14 sean-k-mooney gmaan that what i was tryign to do here https://review.opendev.org/c/openstack/cinder/+/958582/1/cinder/image/glance.py#145
17:04:45 sean-k-mooney let me update that quickly to get rid of the pep8 errors so its readable
17:05:53 gmaan sean-k-mooney: but you need to load service user token from keystoneauth and pass it to https://review.opendev.org/c/openstack/cinder/+/958582/1/cinder/image/glance.py#148
17:05:57 gibi dansmith: fixed the doc pre-prod testing doc patch to ask for a mail.
17:06:06 dansmith gibi: already approved
17:06:07 dansmith thanks
17:06:12 gibi dansmith: thanks!
17:06:47 gibi sean-k-mooney: you threading unit test finding was valid, I missed that in our exclude list in the first patch and only added later. Now it is excluded from the begining
17:06:50 gmaan sean-k-mooney: otherwise auth going to keystone sessions is still user token (with no service role) + service token - https://review.opendev.org/c/openstack/cinder/+/958582/1/cinder/image/glance.py#154
17:07:54 sean-k-mooney gmaan: that what i was tryign to do by having the session object constucted form teh config
17:08:16 sean-k-mooney im obvioulsy missing a step but im tring to use the user token form the config not the actual user
17:09:15 gmaan I think keystonemiddleware do fetch the headers from auth plugin and populate the context with those data but let me check
17:10:13 opendevreview melanie witt proposed openstack/nova stable/2025.1: libvirt: Get info with abs path, rebase with rel path https://review.opendev.org/c/openstack/nova/+/958674
17:10:16 gmaan sean-k-mooney: but yes if you can cleanup those pep8, i can read it more clearly
17:10:37 sean-k-mooney im doing that as we speak
17:10:58 sean-k-mooney gibi: ack cool i was hoping it was just that
17:11:26 sean-k-mooney gibi: so its still exclude and or passing in the later patches ya?
17:12:16 opendevreview Balazs Gibizer proposed openstack/nova master: Switch nova-conductor to use ThreadPoolExecutor https://review.opendev.org/c/openstack/nova/+/957088
17:12:17 opendevreview Balazs Gibizer proposed openstack/nova master: Run nova-conductor in native threading mode https://review.opendev.org/c/openstack/nova/+/958575
17:12:36 gibi sean-k-mooney: yepp it was excluded later, but now it is excluded from the first patch :) thanks for catching it
17:13:01 gibi the conductor series is now move top of master out from the unit test series
17:13:18 sean-k-mooney ack ill loop back over that shortly
17:13:37 gibi thanks. I'm filing the futurist bug now and the end my day
17:13:39 sean-k-mooney i need to actully finish at leat the first patch in the sev series
17:14:16 sean-k-mooney im going to be aroudn for amybe another hour doign review possible a little more
17:14:47 sean-k-mooney gmaan: i didnt have a chance to revew the dnms but did you get clean runs on the service role the last time
17:15:47 gmaan sean-k-mooney: yes, all good. stable job on tempest change was failing in experimental pipeline so I fixed tests to continue using admin on stable and service on master
17:15:50 sean-k-mooney i was mostly happy with your nova change last time i looked so i think we can likely land that today or tomorrow
17:16:11 sean-k-mooney did you see my quetion on the tempest change
17:16:14 gmaan I fixed your doc/release notes comment too
17:16:21 gmaan oh, not yet, checking
17:16:28 sean-k-mooney baout wither the primary role could be service?
17:16:32 sean-k-mooney instead of admin
17:16:45 gmaan sean-k-mooney: oh, that one i replied yes
17:17:16 sean-k-mooney https://review.opendev.org/c/openstack/cinder/+/958582 now passes pep8
17:17:26 gmaan its not admin. actually 'credential' var in tests is little weird and unreadable
17:18:13 gmaan credentials = ['primary', 'admin', ['service_user', 'admin', 'service']] means: it create three users - 1. user with member role (primary), 2. user with admin (admin), 3. user with admin and saervice role (service_user)
17:18:24 gibi here is the futurist bug https://bugs.launchpad.net/futurist/+bug/2121545
17:18:36 sean-k-mooney this was a case of "i dont understand how to read that list" so i was just asking why primary was admin for what its worth
17:19:10 sean-k-mooney oh
17:19:18 sean-k-mooney thats how you read it
17:19:42 sean-k-mooney gmaan: ok that not what i was expectign at all ok
17:19:44 gmaan yeah, it take, normal predefined users (primary, admin, reader, alt_admin etc), users with coustom name and roles so it is not so easy to read.
17:20:20 gmaan and with historic alias in tempest, primary == member etc make it more unreadable
17:20:35 gmaan I think we need to get rid of primary and use member consistently
17:20:44 sean-k-mooney ya a dict of username to list of roels would be simpler also
17:21:14 sean-k-mooney ya the indirection though primary also was not somethign i was aware of
17:21:31 gmaan it keep becoming complex with more personas and we need to keep old predefined personas for compatibility
17:21:42 sean-k-mooney i assme that was to deall with "member" vs "_member_"
17:22:34 gmaan self.os is another name of promary/member but we do not use that in many tests :)
17:22:56 gmaan but I have that cleanup in my list
17:23:39 gmaan at least use latest personas name in tests and keep old one just for compatibility if tempest plugin are using
17:23:57 sean-k-mooney ya that woudl help
17:24:14 sean-k-mooney so i need to finsh the sev revew but yuou thin the issue is https://review.opendev.org/c/openstack/cinder/+/958582/2/cinder/image/glance.py#148
17:24:47 sean-k-mooney evne though i am creating the session form the config now its still not useing tha tfor the user toekn an pulling that form the context
17:27:00 sean-k-mooney nova does https://github.com/openstack/nova/blob/master/nova/service_auth.py#L33-L55 cidner does almost the same https://github.com/openstack/cinder/blob/master/cinder/service_auth.py#L80-L91
17:27:42 gmaan sean-k-mooney: you are creating the sessions from conf but passing the auth plugin to keystonauth https://review.opendev.org/c/openstack/cinder/+/958582/2/cinder/image/glance.py#145
17:28:10 sean-k-mooney if you have na idea of how to properly udpat that patch or can comment i can respien it later
17:28:18 sean-k-mooney but i see i can pass auth as a second parmater
17:28:31 sean-k-mooney so i just need to find an incanation to copy past for that
17:28:35 gmaan sean-k-mooney: that is all ok but in that method nova pass the loaded conf service user and cinder just context so user aith plugin loaded from user context
17:28:39 gmaan yes
17:29:37 gmaan for nova case this is here cinder load the internal service user from conf https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L98-L100
17:29:57 gmaan and wrap that with service token in get_auth_plugin
17:30:43 gmaan and then keystonmiddlware gets the right user_token and service_token headres and populate the requext context which is passed to services
17:31:18 sean-k-mooney oh so i need basiclly all of this right https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L98-L118
17:32:19 gmaan yes, you can do all. so like cinder call nova based on what they call they can with pass privileged_user to true or not
17:32:22 sean-k-mooney and ideally cinder would have a glance section like the nova one but i could steal the nova option or the keystone_auth ones just to who it works
17:33:14 sean-k-mooney what im trying to show is that with a change to how cidner calls glance the revert is valid to show that its a cidner bug now a glance one
17:33:22 gmaan sean-k-mooney: that can be wrong but that is why I am checking in cinder code if they register the keystoneauth options for glance section also like they do for nova
17:33:26 sean-k-mooney if i can demonstrate tha then we coudl fix it properly
17:33:53 sean-k-mooney gmaan: no not that i can see form there config ref
17:36:38 gmaan sean-k-mooney: I checked what all method cinder should pass user token (like they do currently) and I think these are the two call cinder need to pass config service user to glance - https://github.com/openstack/cinder/blob/master/cinder/image/glance.py#L345-L370
17:37:15 gmaan so they can pass privileged_user =True in common mehtod you mentioned to copy from nova file to glance
17:38:13 gmaan those are two glance call have service role for their policy chgeck https://github.com/search?q=repo%3Aopenstack%2Fglance+base.SERVICE&type=code
17:39:51 gmaan rest all other call can continue using the user token. for example, this is nova case where cinder get servers volume from nova and just call it with user token (not configured service user) + service token https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L243-L246
17:39:58 sean-k-mooney ok i was hopign thiw woudl take me a few second to poc
17:40:07 sean-k-mooney but it will take me more time then i have
17:40:24 sean-k-mooney gmaan: is this somethign you could try?

Earlier   Later