Wednesday, 2021-08-25

NobodyCamsean-k-mooney: I clarify that there is a reserve inventory instance that is tripping up our readiness checks.. 01:36
opendevreviewMerged openstack/nova stable/stein: Move 'check-cherry-picks' test to gate, n-v check  https://review.opendev.org/c/openstack/nova/+/80461501:46
opendevreviewMerged openstack/nova stable/stein: Prevent archiving of pci_devices records because of 'instance_uuid'  https://review.opendev.org/c/openstack/nova/+/76098402:25
opendevreviewMerged openstack/nova master: fup: Remove unused legacy block_device_info format  https://review.opendev.org/c/openstack/nova/+/80428602:25
opendevreviewMerged openstack/nova master: fup: Increase service_down_time beyond INITIAL_REPORTING_DELAY in test  https://review.opendev.org/c/openstack/nova/+/80566702:25
*** tamas_erdei is now known as terdei06:57
*** rpittau|afk is now known as rpittau07:06
*** mgoddard- is now known as mgoddard07:47
*** akekane_ is now known as abhishekk08:19
opendevreviewtest proposed openstack/nova master: Add some missing parameters in docs of os-cells  https://review.opendev.org/c/openstack/nova/+/80597208:56
opendevreviewLee Yarwood proposed openstack/nova master: nova-manage: Introduce volume show, refresh, get_connector commands  https://review.opendev.org/c/openstack/nova/+/80063409:26
opendevreviewBalazs Gibizer proposed openstack/placement master: Restrict amqp indirect dep to speed up pip resolution  https://review.opendev.org/c/openstack/placement/+/80597909:51
opendevreviewBalazs Gibizer proposed openstack/placement master: Bump os-traits to latest 2.6.0  https://review.opendev.org/c/openstack/placement/+/80583009:51
lyarwoodgibi: https://review.opendev.org/q/topic:remove_luks_workarounds some simple cleanups here if you have time09:55
elodilleslyarwood: I've commented on the stable/train's CI patch. I'm not completely against it, but it is not necessary in Train so I wouldn't merge it in all cost, plus I see one difference that would reduce coverage if I'm not mistaken: py2 grenade09:56
gibilyarwood: ack, lookgin09:56
lyarwoodelodilles: ack I'll look 09:56
lyarwoodgibi: thanks, I'm trying to get to your qos series btw, almost there :)09:56
gibilyarwood: thanks in advance :)09:57
opendevreviewMerged openstack/nova master: fup: Fix os-volume_attachments api-ref parameters  https://review.opendev.org/c/openstack/nova/+/80587610:16
opendevreviewStephen Finucane proposed openstack/nova master: api: Add support for 'hostname' parameter  https://review.opendev.org/c/openstack/nova/+/77855010:24
opendevreviewStephen Finucane proposed openstack/nova master: tests: Speed up 'servers' API tests  https://review.opendev.org/c/openstack/nova/+/77873210:24
opendevreviewLee Yarwood proposed openstack/placement master: Bump os-traits to latest 2.6.0  https://review.opendev.org/c/openstack/placement/+/80583011:14
opendevreviewLee Yarwood proposed openstack/placement master: Restrict amqp indirect dep to speed up pip resolution  https://review.opendev.org/c/openstack/placement/+/80597911:14
lyarwoodgibi: ^ just had to reorder these to avoid the 2.6.0 test failure11:14
opendevreviewMerged openstack/nova master: Reproduce bug 1941005  https://review.opendev.org/c/openstack/nova/+/80588211:30
kevinzkashyap: sbauza: Hi, could you help to review this live migration patch? https://review.opendev.org/c/openstack/nova/+/763928, the comments has been addressed.11:39
kevinzkashyap: sbauza: live migration on arm64 patch, really appreciated!11:40
sean-k-mooneykevinz: they are both on vaction this week12:04
gibilyarwood: right that is the correct order. I knew I wanted to do something with them but I got distracted12:04
sean-k-mooneyill try and take a look at it12:04
gibithanks12:04
gibilyarwood: wait, now the 2.6.0 is the first patch, but that will fail due to timeout without the amqp restriciton12:05
gibido we have a deadlock then12:06
gibi?12:06
gibithe global req was bumped causing that we need the gabbit update, but that is failing as lower constraints is timing out without the amqp change, but the amqp patch alone will fail due to the gabbit failures 12:07
opendevreviewMerged openstack/nova master: compute: Query the service group API within check_instance_host  https://review.opendev.org/c/openstack/nova/+/80231712:14
lyarwoodgibi: yeah sorry I didn't think the amqp change was causing a failure12:17
lyarwoodgibi: in that case we should squash both changes into one I guess12:17
gibiyes, I don't see any other way around12:17
gibiI will squash them in a minute12:17
opendevreviewBalazs Gibizer proposed openstack/placement master: Bump os-traits to latest 2.6.0  https://review.opendev.org/c/openstack/placement/+/80583012:24
gibilyarwood: this should be green now ^^12:24
opendevreviewMerged openstack/python-novaclient master: Microversion 2.89 - os-volume_attachments  https://review.opendev.org/c/openstack/python-novaclient/+/80502213:09
opendevreviewlmercl proposed openstack/nova master: nova-api add flavorid value to server's flavor part  https://review.opendev.org/c/openstack/nova/+/80599513:25
opendevreviewlmercl proposed openstack/nova stable/wallaby: nova-api add flavorid value to server's flavor part  https://review.opendev.org/c/openstack/nova/+/80599713:29
opendevreviewlmercl proposed openstack/nova master: nova-api add flavorid value to server's flavor part  https://review.opendev.org/c/openstack/nova/+/80599513:40
opendevreviewRodrigo Barbieri proposed openstack/nova stable/victoria: Fix 1vcpu error with multiqueue and vif_type=tap  https://review.opendev.org/c/openstack/nova/+/80600414:20
gmannstephenfin: gibi replied on hostname policy chekcs https://review.opendev.org/c/openstack/nova/+/778550/10/nova/api/openstack/compute/views/servers.py#41414:25
gmannwe can chat to conclude it quickly 14:26
gmannmy point is, this is policy change and should not be controlled with microversion instead just deprecate and remove in next cycle for old microversion also14:26
gmannthat is how other policy changes we do14:26
gmannold microversion in this case i mean >2.3 for GET and >2.75 for PUT/Rebuild as this attribute was added for them. 14:27
*** lbragstad_ is now known as lbragstad14:39
gibigmann: does this mean we are not implementing the POST /servers change to allow passing hostname in Xena? If we do implement that then delaying the policy change (at least of the new microversion) with deprecation does not make sense from usability perspective14:40
gibiI want see what I passed in 14:41
gibiso what if in Xena we deprecate the policy BUT also in xena in the microversion that allows passing hostname in server create we introduce the hostname in the responses. Then in Yoga we remove the policy from the hostname attribute and that results that hostname will be shown in every microversion14:43
gibithis allows that the current POST change be usable in Xena and also follows deprecation policy14:44
gmanngibi: in that case we have to mention that policy is controlled with microversion. for >=2.90 you can see this attribute even previously you have restricted it for non-admin and even non-admin does not pass hostname in POST request.14:52
gibigmann: purely from policy perspective yes, this would be a microversion that temporarily changes the policy. After Yoga where the policy is removed this microversion controlls policy situation would be resolved14:55
gibihonestly I don't want to delay the possibility to set hostnames 14:55
gibibut only allow to set them in 2.90 but not allow to see what you set feels bad14:56
gmanngibi: they can see with policy change, but yes with default policy they would not be able to see15:02
gmann*policy override15:02
gibigmann: is there a way to only override the policy for the hostname attribute only?15:03
gmanngibi: no, it is with other server extended attributes 15:06
gibibut we don't want to show all the exteneded attribute as there are sensitive informations there like the hypervisor_hostname15:06
gmannhumm15:06
gibibut the hostname of the VM is harmless 15:06
gibithat does not reveal any infra informatiuon15:07
gmannyeah15:07
*** abhishekk is now known as akekane|home15:08
*** akekane|home is now known as abhishekk15:08
gmanngibi: I think in this exceptional case when there is related API change, I am getting your point. 15:09
gmannlet me rethink if any other way it can create inconsistency other seems ok to me15:10
gmannotherwise15:10
gibigmann: for me it is the case when our strict rules (microversion and policy deprecation) get in our way to provide a meaningful and (for me) safe change for the end user15:11
gmannyeah. 15:12
gibiand I support our strict rules in general as it is a safety net to avoid breaking users, but in this specific case I think we can allow an exception for the rules15:13
gmanni agree. especially with the point of using 2.90 changes completely (request and see the hostname by end users) 15:14
gmannI think I am convinced now :). thanks 15:15
opendevreviewMerged openstack/placement master: Bump os-traits to latest 2.6.0  https://review.opendev.org/c/openstack/placement/+/80583015:17
gibigmann: thank you for the discussion15:24
melwittlyarwood: thanks for noting that I need to rebase the consumer types set, going to do that now and will need +W reapplied after15:29
lyarwoodmelwitt: I think we can get away without a rebase now15:29
lyarwoodmelwitt: zuul should do it for us once it's rechecked15:30
* lyarwood was about to do it before this call15:30
melwitto rly15:30
melwittok15:30
melwittI'm glad I said something. thanks I can do that15:30
stephenfingmann: gibi: Sorry, I had meetings. It _sounds_ like you've come to a conclusion and are ultimately happy with what I've done?15:30
gmannstephenfin: yeah, it looks good to me, reviewing that patch...15:31
gibiohh consumer_types are going in \o/15:33
melwittyes \o/15:36
lyarwoodassuming it's rebased by zuul ;)15:36
lyarwoodcool looks like it did15:39
lyarwoodbrb15:39
*** rpittau is now known as rpittau|afk16:04
gmannstephenfin: left few comments on test and documentation 16:07
artomdef _do_old_style_instance_list_for_poor_cellsv1_users()16:36
artomRight, who's the joker responsible for that?16:36
artomMr Smith.16:37
sean-k-mooneysomeone who took pitty on the cell v1 users16:37
sean-k-mooneythen need all the care we can give them16:37
artom(Yes, this is very old code - looking at a Queens bug report)16:37
sean-k-mooney*********16:39
sean-k-mooneythey are mising url quoting16:39
sean-k-mooneyhttps://github.com/sqlalchemy/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py#L54216:40
artom?16:43
sean-k-mooneythe query args secotion fo the url is enccoded using quote_plus16:44
sean-k-mooneywhere spaces are replaced with +16:44
sean-k-mooneybut the protocol seciont of the url has a + in it that is not a spec16:44
sean-k-mooney*space16:45
sean-k-mooneyso if i decode it with unqoute technically the queary args wont be decoded properly16:45
artomOhhh16:45
sean-k-mooneywhich means if i want to properly decode it i need to hande eveything beofre the ? one way and everything after the other way16:46
sean-k-mooneyi htink im just going to skip that and replcae % with %% 16:46
sean-k-mooneyand hope that config parser is ok with the url encoded value16:47
artomYeah16:47
artomWe'll get the tripleo job to run with the fix as a dependency16:47
sean-k-mooneyya i can add a depends on16:48
sean-k-mooneythat works with ooo right16:48
sean-k-mooneyvia a dnm patch16:48
artomIt should16:50
artomI was thinking the other way around, have an ooo DNM patch depending on the nova one, but whatever works16:51
*** dprince is now known as Guest538916:52
sean-k-mooneyartom: that is what im going to do16:53
lbragstadgmann so - i'm working through a test where a system-admin creates a server in a project (specifically trying to find what needs to change in KSA to make that possible)17:01
lbragstadand it looks like the first failure is https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L265 because neutron is using a novaclient with a project-scoped token (instead of a system-scoped one)17:02
sean-k-mooneythat nova client should be using the token set in the neutron.conf17:03
sean-k-mooneywhich woudl normally be the nova user or a service user17:03
lbragstadyeah - exaclty17:03
lbragstadand it does17:04
lbragstadso - that's good17:04
sean-k-mooneyso the nova user would have to be a project member or if it was a system_memeber we need a new parmater to specifcy a project id i ugess17:04
sean-k-mooney*guess17:04
sean-k-mooneythe api its calling however is admin only and not really proejct specfic17:04
sean-k-mooneyim surpised that is failing17:05
lbragstadin https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L265 specifically - the nova user would need to be a system-admin https://github.com/openstack/nova/blob/master/nova/policies/server_external_events.py#L2717:05
lbragstadiiuc 17:05
sean-k-mooneyyes17:05
sean-k-mooneyso the nova user woul dhave to be a system_admin17:06
lbragstadright17:06
lbragstadso that neutron can use that user with a client to create external events in nova17:07
sean-k-mooneyyes17:07
sean-k-mooneyits calling this endpoint https://docs.openstack.org/api-ref/compute/?expanded=run-events-detail#run-events17:07
lbragstadin addition to that - we'd need a way to make sure neutron is using the right scope when building the auth request17:08
lbragstadhttps://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L6117:08
sean-k-mooneyyep we woudl need to add a new config parmater ofor the scop to use17:09
sean-k-mooneyhttps://github.com/openstack/nova/blob/master/nova/policies/server_external_events.py#L24-L3517:09
sean-k-mooneyin this case system scope17:09
lbragstadand i imagine that's going to be a pattern we need else where?17:10
lbragstadi'm expecting this is just the first time i'm hitting this issue, but there are other layers to the onion17:10
sean-k-mooneywell its going to happen but worse for volume resize17:11
lbragstadsure - that makes sense17:11
sean-k-mooneycinder currently has bug where it calls that endpoing with the users token17:11
sean-k-mooneyinstead of actully using an admin one17:11
sean-k-mooneybut yes i expect the pattern to be the same or similar17:12
dansmithsean-k-mooney: is that a bug, or just how it used to work?17:12
sean-k-mooneyim surprised that it got that far by the way17:12
sean-k-mooneydansmith: no that is a bug17:12
sean-k-mooneywhich ooo worked around by hardcodeing cidner to call nova admin api endpoint directly17:12
dansmithis this the swap volume api or something else?17:12
lbragstadfwiw - this is what i'm doing to recreate this https://review.opendev.org/c/openstack/tempest/+/80545217:13
sean-k-mooneyi tought it was resize for nfs but ill have to go look it up17:13
sean-k-mooneylbragstad: by the way i was expecting a failure before that when nova called neutron to bind the port17:13
lbragstadyeah - me too17:13
sean-k-mooneywe do that as a admin17:13
dansmithokay just saying.. the swap volume api has always been user-visible, but problematic.. people have used it for non-cinder tasks before and we discussed whether or not they should be able to.. but they are (or were)17:13
sean-k-mooneydansmith: im pretty sure the issue i was refering too was for online volume resize whne cinder calls nova back to tell it its done17:14
sean-k-mooneydansmith: ill see if i can find it17:14
dansmithokay, there's some volume op that uses swap_volume for that sort of thing (cinder calling to nova)17:15
dansmithbut might be different17:15
sean-k-mooneydansmith: i dont have the ooo one to hand but this is the osa dup https://bugs.launchpad.net/openstack-ansible/+bug/1902914 and redhat kcs...https://access.redhat.com/solutions/367599117:16
dansmithokay I expect this is different17:17
sean-k-mooneyyes lbragstad current issue is different and expected17:17
sean-k-mooneywell expected in that each of  the serviceis will need to know how to call each of the other services with the correct scopes17:18
sean-k-mooneylbragstad: you could try booting an instance with no network as a simpler starting point17:19
sean-k-mooneylbragstad: that would limit the interaction ot nova, keystone, glance and placment17:20
sean-k-mooneyplacment would be the only thing that used any admin creds in that flwo also17:20
sean-k-mooneywe would use the user token for the glance image17:20
sean-k-mooneyim not 100% shoure what you would need to add but i think it would be something like network=none to your self.create_test_server( call17:22
lbragstadyeah - that makes sense17:22
gmannlbragstad: for system scope enabled. tempest does not  create network17:24
gmannlbragstad: that is skipped when create network was project scoped in neutron but that is changed i think17:24
lbragstadah - interesting 17:24
gmannlbragstad: 17:25
gmannlbragstad: https://review.opendev.org/c/openstack/tempest/+/79813017:25
lbragstadgmann is that still the case with https://review.opendev.org/c/openstack/tempest/+/805452/4/tempest/api/compute/admin/test_servers.py ?17:26
gmannlbragstad: for network created for network, we need to modify that also to pass the project_id in neutron request  for system scope17:26
gmannlbragstad: yes. we have not changed that for ^^. and no network for system scoped cred17:26
gmannI can work on that tomorrow to pass projetc_id as slaweq mentioned in that review17:28
lbragstadgmann ok - that sounds good, i'm trying to track all of this stuff as a find it, but it's likely going to be a lot of sprawl17:42
lbragstadstepping out for about 30 minutes17:43
opendevreviewmelanie witt proposed openstack/placement master: Refactor consumer type methods for readability  https://review.opendev.org/c/openstack/placement/+/80603517:45
melwittsean-k-mooney, lyarwood: fixups for consumer types ^17:46
sean-k-mooneymelwitt: im kind of  +0.5 https://review.opendev.org/c/openstack/placement/+/806035/1/placement/objects/usage.py#9118:12
melwittsean-k-mooney: thanks, I will reword it18:14
opendevreviewmelanie witt proposed openstack/placement master: Refactor consumer type methods for readability  https://review.opendev.org/c/openstack/placement/+/80603518:22
sean-k-mooneymelwitt: by the way im just looking at the db tests  i assume somehting in here https://github.com/openstack/nova/blob/master/nova/tests/unit/db/main/test_migrations.py that uses the OpportunisticDBTestMixin would be what you want for the alembic urls18:26
melwittsean-k-mooney: ah, cool. I have used that mixin once before I think. fwiw I wasn't clear on whether we need a database to cover some test inputs or if we only need the config parser behind it. whatever works18:30
melwittwant to be able to do at least a couple of cases, one with the encoded chars and one with a password for example with a literal "%" in it18:31
melwittor I guess that could just be one case by putting both together in one example18:31
sean-k-mooneyok im just going thought the different fixture we have aviable to us18:32
sean-k-mooneyhttps://github.com/openstack/oslo.db/blob/22b44ee18b1585bc2943a29ddaa18051cb1344ed/oslo_db/sqlalchemy/test_fixtures.py#L39918:32
sean-k-mooneywe have the fix ture and the mixins18:32
melwittah ok18:32
sean-k-mooneythe adhock db fixture  allows us to pass a url https://github.com/openstack/oslo.db/blob/22b44ee18b1585bc2943a29ddaa18051cb1344ed/oslo_db/sqlalchemy/test_fixtures.py#L27318:33
sean-k-mooneythe opertunistic mixin can be combind with any of the fixture i think18:34
sean-k-mooneywe need to modify the engin url if we want to test this right before tuning the db sync18:35
melwittmeanwhile, the consumer types patches are failing on that dang server delete fail for AllocationDeleteFailed conflict 😑18:36
sean-k-mooneymelwitt: i have updated stephens exisitng test case already just trying to see if theyer is a good way to also add the extra tests you where hoping for18:37
sean-k-mooneyif only there was a patch we could merge for that :)18:37
sean-k-mooneythats furstrating18:37
melwittsean-k-mooney: ok, cool. lmk if you need a hand with it, I can try to help18:38
opendevreviewsean mooney proposed openstack/nova master: db: Handle parameters in DB strings  https://review.opendev.org/c/openstack/nova/+/80566320:01
sean-k-mooneymelwitt: ok that did not work but i found another way to test this20:01
sean-k-mooneyi realised after i fot the adhoc fixture working that would not test any of our config loading ectra20:02
sean-k-mooneywhich is what we actully wanted to test20:02
sean-k-mooneywell initalising the alembic config form our config20:02
opendevreviewArtom Lifshitz proposed openstack/nova master: WIP: Update PCI requests in request spec on resize  https://review.opendev.org/c/openstack/nova/+/80604920:10
melwittsean-k-mooney: sweet! just added some comments20:15
opendevreviewMerged openstack/nova stable/wallaby: Allow X-OpenStack-Nova-API-Version header in CORS  https://review.opendev.org/c/openstack/nova/+/79686022:18
opendevreviewMerged openstack/placement master: Add consumer_types migration, database and object changes  https://review.opendev.org/c/openstack/placement/+/66917022:18
opendevreviewMerged openstack/placement master: Microversion 1.38: API support for consumer types  https://review.opendev.org/c/openstack/placement/+/67944122:18
opendevreviewMerged openstack/placement master: Switch ConsumerType to use an AttributeCache  https://review.opendev.org/c/openstack/placement/+/67948622:18

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!