opendevreview | melanie witt proposed openstack/nova master: Add hw_tpm_secret_security image property https://review.opendev.org/c/openstack/nova/+/940196 | 01:03 |
---|---|---|
opendevreview | melanie witt proposed openstack/nova master: Add hw:tpm_secret_security extra spec validation https://review.opendev.org/c/openstack/nova/+/940197 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: migrate legacy instances to new security policy https://review.opendev.org/c/openstack/nova/+/942501 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `user` secret security https://review.opendev.org/c/openstack/nova/+/942502 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `host` secret security https://review.opendev.org/c/openstack/nova/+/941795 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: Add vtpm_secret_(uuid|value) to LiveMigrateData https://review.opendev.org/c/openstack/nova/+/952628 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support live migration of `host` secret security https://review.opendev.org/c/openstack/nova/+/941483 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `deployment` secret security https://review.opendev.org/c/openstack/nova/+/942021 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support live migration of `deployment` secret security https://review.opendev.org/c/openstack/nova/+/925771 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: test live migration between hosts with different security https://review.opendev.org/c/openstack/nova/+/952629 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: update instance request_spec with secret security https://review.opendev.org/c/openstack/nova/+/952630 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: confirm secret security via hard reboot https://review.opendev.org/c/openstack/nova/+/955847 | 01:03 |
opendevreview | melanie witt proposed openstack/nova master: TPM: migrate legacy instances to new security policy https://review.opendev.org/c/openstack/nova/+/942501 | 01:12 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `user` secret security https://review.opendev.org/c/openstack/nova/+/942502 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `host` secret security https://review.opendev.org/c/openstack/nova/+/941795 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: Add vtpm_secret_(uuid|value) to LiveMigrateData https://review.opendev.org/c/openstack/nova/+/952628 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support live migration of `host` secret security https://review.opendev.org/c/openstack/nova/+/941483 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support instances with `deployment` secret security https://review.opendev.org/c/openstack/nova/+/942021 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: support live migration of `deployment` secret security https://review.opendev.org/c/openstack/nova/+/925771 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: test live migration between hosts with different security https://review.opendev.org/c/openstack/nova/+/952629 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: update instance request_spec with secret security https://review.opendev.org/c/openstack/nova/+/952630 | 01:13 |
opendevreview | melanie witt proposed openstack/nova master: TPM: confirm secret security via hard reboot https://review.opendev.org/c/openstack/nova/+/955847 | 01:13 |
sean-k-mooney | melwitt: im actully going to go to sleep now instead of saying i will and not but do you have tempest test for that and or a devstack config you can share. i dont think i have any locally with barbican but the rest is pretty simple. | 01:17 |
sean-k-mooney | oh you have whitebox https://review.opendev.org/c/openstack/whitebox-tempest-plugin/+/955969 | 01:17 |
melwitt | sean-k-mooney: yes whitebox so far but I'm thinking of writing up some plain tempest tests also. thanks :) | 01:18 |
sean-k-mooney | https://zuul.opendev.org/t/openstack/build/c0bce36dbd29472bb743b4e883d0dfc0/log/controller/logs/local_conf.txt i can prbably steal the bit i will need form ther | 01:19 |
sean-k-mooney | ack ya whitebox is nice but not strictly needed for this | 01:19 |
sean-k-mooney | its useful for checkign the libvirt secrete flags in the deployment one | 01:20 |
sean-k-mooney | but the base functionalty can be done in core tempest | 01:20 |
melwitt | yeah the main configs you need are swtpm_enabled = True and supported_tpm_secret_security = <list>. there is also default_tpm_secret_security = <string> that you specify if you want | 01:21 |
sean-k-mooney | that and deploying barbican :) | 01:21 |
melwitt | also with barbican, the thing to know is if you have enforce_new_defaults = False then you need the 'creator' role to create secrets | 01:21 |
sean-k-mooney | which is the main thing missing form my config i think btu the rest is useful | 01:21 |
melwitt | if you have enforce_new_defaults = True then being a project member is good enough for creating secrets | 01:22 |
sean-k-mooney | melwitt: so i need to deploy my 3 node devstack with watcher ot test some other stuff but ill add barbican to it when i do and then probly next week ill checkout your tpm serise and try and use it | 01:22 |
sean-k-mooney | the new defautl shoudl be the default now | 01:23 |
sean-k-mooney | so ya member shoudl be fine or admin i guess fo rthe migration | 01:23 |
melwitt | sean-k-mooney: that would be super :) thanks | 01:23 |
melwitt | yeah admin for live migration unless you change the policy | 01:23 |
melwitt | also in my experience barbican by default got deployed with enforce_new_defaults = False. dunno if it's a devstack thing or what | 01:24 |
sean-k-mooney | in the whitebox test can you check that the libvirt secret is defiend a snot epmeral or private | 01:24 |
sean-k-mooney | https://review.opendev.org/c/openstack/whitebox-tempest-plugin/+/955969/8/whitebox_tempest_plugin/api/compute/test_vtpm.py#198 | 01:24 |
sean-k-mooney | basicly for deployment you shoudl check that we its readable via virsh or just check the flag on the secret | 01:25 |
melwitt | yes I can add that. I have been wondering how to add more stuff to the tests | 01:25 |
melwitt | yeah I was thinking the xml | 01:26 |
sean-k-mooney | i think either is fine that the main thing that core tempest cant asssert | 01:26 |
sean-k-mooney | so that where whitebox can add coverage. i knwo we will have unit/functional tests too | 01:26 |
melwitt | yeah core tempest won't be able to | 01:26 |
sean-k-mooney | so im not that concerend | 01:26 |
sean-k-mooney | before i go, unrealted to live mgiration specificly | 01:27 |
sean-k-mooney | we may want to add some vtpm test ot show data is preserved | 01:27 |
sean-k-mooney | i.e. ssh into the vm and add somethign to the tpm | 01:27 |
sean-k-mooney | then live migrate it or hard reboot it and show its still there | 01:28 |
melwitt | that would be good if we can. I was thinking about that yesterday with regard to the reboot bug/launchpad | 01:28 |
sean-k-mooney | i dotn actuly know how to do that | 01:28 |
sean-k-mooney | i assume there is a tool for that but its probaly not in busy box | 01:28 |
melwitt | I don't either. will have to do some research | 01:28 |
sean-k-mooney | i dont knwo enought about tpms to know if there is a /sys interface that we can jsut echo too or something adn read back with cat | 01:29 |
sean-k-mooney | that woudl be the best option but future us problem | 01:29 |
melwitt | yeah. I hope it could be that easy | 01:29 |
gmaan | sean-k-mooney: for your tomorrow. did you get chance to look into manager role tests, stephenfin is +2 on that now, waiting for your review https://review.opendev.org/q/topic:%22bp/policy-manager-role-default%22+status:open | 01:30 |
sean-k-mooney | partly but that actully what im hoping to do in the morning. so all going well ill add +2w later tomorrow | 01:31 |
gmaan | thanks | 01:31 |
sean-k-mooney | i was happy with the production code i just did not fuly look at the test code before | 01:31 |
gmaan | k, in summary for tests coverage: 1. I moved existing tests to use member token 2. added a few new tests | 01:32 |
sean-k-mooney | cool night all o/ | 01:34 |
gmaan | GN | 01:34 |
melwitt | gnight o/ | 01:35 |
*** mhen_ is now known as mhen | 01:37 | |
gmaan | stephenfin: finished the openapi series, some comments for follow up and some for fixing. | 03:03 |
*** mhen_ is now known as mhen | 08:22 | |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Address issues with images APIs https://review.opendev.org/c/openstack/nova/+/956102 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Separate volume, snapshot and volume attachments https://review.opendev.org/c/openstack/nova/+/952347 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: tests: Use valid UUIDs for cinder resources https://review.opendev.org/c/openstack/nova/+/952935 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Only apply "soft" additionalProperties validation to requests https://review.opendev.org/c/openstack/nova/+/952936 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for volumes APIs https://review.opendev.org/c/openstack/nova/+/952348 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for snapshots APIs https://review.opendev.org/c/openstack/nova/+/952349 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for volume attachments APIs https://review.opendev.org/c/openstack/nova/+/952350 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for floating IP APIs https://review.opendev.org/c/openstack/nova/+/952972 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for security group APIs https://review.opendev.org/c/openstack/nova/+/952973 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for services APIs https://review.opendev.org/c/openstack/nova/+/953196 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server usage audit log APIs https://review.opendev.org/c/openstack/nova/+/953209 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for tenant network APIs https://review.opendev.org/c/openstack/nova/+/956088 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for simple tenant usage APIs https://review.opendev.org/c/openstack/nova/+/956096 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for limits API https://review.opendev.org/c/openstack/nova/+/956139 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for quota class sets API https://review.opendev.org/c/openstack/nova/+/956140 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for quota sets API https://review.opendev.org/c/openstack/nova/+/956141 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for migrations API https://review.opendev.org/c/openstack/nova/+/956142 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server migrations API https://review.opendev.org/c/openstack/nova/+/956143 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server tags API https://review.opendev.org/c/openstack/nova/+/956144 | 09:45 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server metadata APIs https://review.opendev.org/c/openstack/nova/+/956145 | 09:46 |
opendevreview | Ian Watson proposed openstack/nova master: Changing how placement updates are triggered in RT https://review.opendev.org/c/openstack/nova/+/955687 | 09:47 |
opendevreview | Ian Watson proposed openstack/nova master: Changing how placement updates are triggered in RT https://review.opendev.org/c/openstack/nova/+/955687 | 09:48 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 10:00 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 10:12 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 10:30 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 10:58 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:00 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:02 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:02 |
opendevreview | Ian Watson proposed openstack/nova master: Add migrations_supported methed for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:04 |
opendevreview | Ian Watson proposed openstack/nova master: Add migration conditional for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:04 |
opendevreview | Ian Watson proposed openstack/nova master: Add migration conditional for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 11:06 |
opendevreview | Merged openstack/nova master: [test]Speed up fs retry tests by mocking sleep https://review.opendev.org/c/openstack/nova/+/955904 | 11:49 |
opendevreview | Merged openstack/nova master: [test]Speed up ironic console test by decreasing timeout https://review.opendev.org/c/openstack/nova/+/955905 | 11:50 |
opendevreview | Merged openstack/nova master: [test]Speed up RBD test by decreasing retry interval https://review.opendev.org/c/openstack/nova/+/955906 | 11:51 |
opendevreview | Merged openstack/nova master: [test]Speed up qemu announce test by mocking sleep https://review.opendev.org/c/openstack/nova/+/955907 | 11:57 |
stephenfin | gibi: Picking this up from last week: it appears the device detach libvirt event stuff is broken on every release since Caracal and it always times out https://github.com/kubernetes/cloud-provider-openstack/pull/2742#issuecomment-3135940269 | 11:59 |
stephenfin | I'm still looking so jfyi for now | 11:59 |
sean-k-mooney | i mean its not optional and i dont think we have see that in devstack in our jobs | 11:59 |
sean-k-mooney | is this only happing in that ci env | 12:00 |
sean-k-mooney | or did you hit it locally | 12:00 |
stephenfin | no, I can reproduce in CI and across multiple PSI instances | 12:00 |
stephenfin | s/multiple PSI instances/multiple local VMs/ (for the non-RHers) | 12:01 |
sean-k-mooney | what exactly doe this look like in the nova logs | 12:01 |
sean-k-mooney | i dont see that in your comment above | 12:01 |
sean-k-mooney | oh its in drivers | 12:03 |
stephenfin | sean-k-mooney: it manifests as a 20 second "stall" while detaching volumes https://paste.opendev.org/show/b6K2tt0kBpOO2liORP7k/ | 12:03 |
sean-k-mooney | https://github.com/kubernetes/cloud-provider-openstack/pull/2742#issuecomment-3114089612 | 12:03 |
sean-k-mooney | its blocking the comptue agent for 20 seconds | 12:03 |
sean-k-mooney | or there was jsut nothing else happeing at that time on that host | 12:04 |
stephenfin | sean-k-mooney, gibi: It's also happening upstream: our CI logs are full of those warnings | 12:04 |
sean-k-mooney | stephenfin: im going to check the ovs-hybrid plug job | 12:05 |
sean-k-mooney | i want to see if this is a ubuntu bug or if it show up in debian | 12:05 |
stephenfin | For example, the tempest-integrated-compute job from a patch in melwitt's vTPM series https://review.opendev.org/c/openstack/nova/+/942501 | 12:05 |
sean-k-mooney | or rhel | 12:05 |
stephenfin | https://d6e5655f0b1ee2f8df74-e35c415f7cc7f78f33337c81de121e72.ssl.cf2.rackcdn.com/openstack/52b57c77f805400888762a87e08196f1/controller/logs/screen-n-cpu.txt | 12:05 |
sean-k-mooney | ya but that also using ubuntu | 12:05 |
sean-k-mooney | i want to check a non ubuntu job to see if tis a bug in ther elibvirt or if its a nova issue | 12:05 |
stephenfin | I see 27 hits grepping for 'Waiting for libvirt event', which is over 9 minutes of wasted time (though not really since I think tempest runs in parallel?) | 12:07 |
stephenfin | sean-k-mooney: ack | 12:07 |
sean-k-mooney | oh i tured off cinder in that job | 12:07 |
sean-k-mooney | let me see if i can check the ceph job i just proposed moving to debian | 12:07 |
stephenfin | sean-k-mooney: do you know how DevStack works on CentOS Stream currently? I can trivially reproduce this locally if needed too | 12:10 |
sean-k-mooney | Waiting for libvirt event about the detach of device vdb with device alias ua-88d8994a-f5be-42d8-9a2f-5e030b1f4ce8 from instance 621ab0d3-61a0-437f-8d31-e9c3d7ae3eae is timed out. | 12:10 |
sean-k-mooney | stephenfin: it does | 12:11 |
sean-k-mooney | so its happenign on debian too | 12:11 |
sean-k-mooney | with ceph in this case you were using lvm right | 12:11 |
sean-k-mooney | the backend should not really matter | 12:11 |
stephenfin | yes (pretty much a "stock" DevStack configuration) | 12:12 |
sean-k-mooney | but that is both local block and network type covered | 12:12 |
sean-k-mooney | ack | 12:12 |
sean-k-mooney | let me see if i still have a c9s devstack somewhere | 12:12 |
sean-k-mooney | that woudl be no but i can check our watcher ci job | 12:14 |
sean-k-mooney | actully no | 12:14 |
sean-k-mooney | we have master jobs that trigger with our new installer on cenetos butthsoe are disbaling cinder right now | 12:15 |
sean-k-mooney | i think neutorn and maybe cinder have centos josb | 12:15 |
sean-k-mooney | we might have an experminatal one too | 12:15 |
stephenfin | I'll just create one locally | 12:16 |
stephenfin | However, I see a few related changed that I'm going to try reverting out of caracal first | 12:16 |
gibi | I did the last round of fixes in that codepath so I remember it was working at that time to wait for the events. If the event does not come we still have code to check if the actual detach happened and move on. But as you noted it might be excessive waiting time | 12:22 |
gibi | if this is visible in upstream CI then please file an upstream bug with the links and I will try to look at it | 12:23 |
gibi | In https://d6e5655f0b1ee2f8df74-e35c415f7cc7f78f33337c81de121e72.ssl.cf2.rackcdn.com/openstack/52b57c77f805400888762a87e08196f1/controller/logs/screen-n-cpu.txt I see cases when it is on 20 sec wait and we move one | 12:27 |
gibi | Jul 30 01:36:30.229367 npa183b57ac85f4 nova-compute[75854]: DEBUG nova.virt.libvirt.guest [None req-5859b1ac-1198-4297-845b-2c4ef82e635b tempest-VolumesAssistedSnapshotsTest-257024579 tempest-VolumesAssistedSnapshotsTest-257024579-project-admin] detach device xml: <disk type="block" device="disk"> | 12:27 |
gibi | Jul 30 01:36:50.849840 npa183b57ac85f4 nova-compute[75854]: WARNING nova.virt.libvirt.driver [None req-5859b1ac-1198-4297-845b-2c4ef82e635b tempest-VolumesAssistedSnapshotsTest-257024579 tempest-VolumesAssistedSnapshotsTest-257024579-project-admin] Waiting for libvirt event about the detach of device vdb with device alias ua-50261be4-1b93-406a-ad22-095d53ab27bb from instance | 12:27 |
gibi | 8b434db2-72e7-45a7-a946-a43b7e5b04de is timed out. | 12:27 |
gibi | Jul 30 01:36:50.862330 npa183b57ac85f4 nova-compute[75854]: INFO nova.virt.libvirt.driver [None req-5859b1ac-1198-4297-845b-2c4ef82e635b tempest-VolumesAssistedSnapshotsTest-257024579 tempest-VolumesAssistedSnapshotsTest-257024579-project-admin] Successfully detached device vdb from instance 8b434db2-72e7-45a7-a946-a43b7e5b04de from the live domain config. | 12:27 |
gibi | obviously it is fishy that the event is never recevied even though the detach happened successfully on libvirt side | 12:29 |
gibi | so either libvirt does not send the event or nova does not receive it | 12:29 |
sean-k-mooney | gibi: stephen and i were just lookign at it inhis env | 12:33 |
sean-k-mooney | the is a virsh command to pretty print the events | 12:33 |
sean-k-mooney | and we are seeing the evnet form the qemu monitor | 12:33 |
sean-k-mooney | but nova does not for some reason | 12:34 |
sean-k-mooney | gibi: stephen goign to head to lunch in a bit but it looks liek somethign broke along the way | 12:34 |
gibi | cool nice troubleshooting | 12:34 |
sean-k-mooney | stephens theory was maybe the alisa did not match or it was someithng related to how we were mathcing the event | 12:35 |
sean-k-mooney | but i think we likely need to add extra loging to print all the libvirt events and confirm if nova even gets it | 12:35 |
sean-k-mooney | it could be something funky with the sepeate tread we have for that but its definly and upstream bug | 12:36 |
stephenfin | Yeah, what sean-k-mooney said. The reason it's such a big deal for cloud-provider-openstack is that the tests (a) create and delete a lot of PVCs (which translates to a lot of attach/detach calls), and (b) run serially, so things compound | 12:39 |
* stephenfin actually goes for lunch | 12:39 | |
gibi | https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L312-L319 | 12:40 |
gibi | this is the code that finds which event waiter to unblock based on incoming events | 12:40 |
gibi | called from here https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L385 | 12:41 |
gibi | https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L2512-L2542 and this is the place where we get the events from libvirt | 12:41 |
gibi | it has logs | 12:41 |
gibi | so if these logs are not visible it means nova is not receiving any events | 12:41 |
sean-k-mooney | im not sure if that loggin actully works, which thrad is that runign on | 12:47 |
sean-k-mooney | specificly https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L2512-L2542 | 12:47 |
sean-k-mooney | is that on the libvirt pthread or on the nova main thread | 12:47 |
sean-k-mooney | there is some issue with mixing real thread and oslo.log | 12:48 |
sean-k-mooney | for what its worht the matching code looks corect | 12:50 |
gibi | yeah that is likely running on the libvirt pthread | 12:50 |
sean-k-mooney | https://github.com/openstack/nova/blob/3a08a557df7b9a52c8faefe010df0ba9994c63a9/nova/virt/libvirt/driver.py#L316-L319 | 12:50 |
gibi | I thought we can still log from the libvirt pthread | 12:51 |
sean-k-mooney | it was "fixed" by adding a pipemutex in oslo logs | 12:51 |
sean-k-mooney | but im not sure i really trust that entilry | 12:52 |
gibi | :) | 12:52 |
sean-k-mooney | im thinkging we add some good old prints or perhaps writes to a file that we can tail | 12:52 |
sean-k-mooney | although not right now. | 12:52 |
gibi | yeah that is always an option locally to troubleshoot it further | 12:56 |
* gibi did a lot of printing to file when troubleshooted hanging unit test recently | 12:57 | |
stephenfin | https://bugs.launchpad.net/nova/+bug/2119114 | 12:58 |
stephenfin | I marked it as confirmed since eharney also took a look at this and I've reproduced it across multiple environments. I did not set a priority though | 12:59 |
stephenfin | eharney and sean-k-mooney, naturally | 12:59 |
gibi | stephenfin: thank you! | 13:00 |
gibi | Uggla: ^^ for visibility | 13:00 |
stephenfin | I will dump all this is the bug, but fwiw, I see no entries for `Received event` messages in Caracal or later. I do see them on Bobcat | 13:01 |
stephenfin | (in the n-cpu logs, that is) | 13:01 |
Uggla | gibi, thanks, I'm not sure. Does it relates to the "gate" bug we discussed in the meeting yesterday ? | 13:04 |
sean-k-mooney | stephenfin: so its not actully breaking anything its just slow but i think tis in the medium to high range | 13:08 |
sean-k-mooney | i woudl proably set it to high but its been 3 release so medium woudl work for me too | 13:09 |
gibi | Uggla: no it is a new one, it does not block the gate | 13:19 |
Uggla | gibi, oh ok. | 13:19 |
opendevreview | Takashi Natsume proposed openstack/nova-specs master: Create specs directory for 2026.1 Gazpacho https://review.opendev.org/c/openstack/nova-specs/+/956166 | 13:20 |
opendevreview | Takashi Natsume proposed openstack/nova master: Update contributor guide for 2025.2 Flamingo https://review.opendev.org/c/openstack/nova/+/944603 | 13:22 |
stephenfin | So this is interesting: restarting the n-cpu services fixes things... 🤔 | 13:42 |
gibi | stephenfin: maybe something blocks the libvirt thread receiving the notifications? | 14:11 |
gibi | and it gets unblocked by the restart until that something hits again and blocks it again | 14:12 |
opendevreview | Wesley Hershberger proposed openstack/nova stable/2024.1: Add ServersViewBuilderTestV296 unit test class https://review.opendev.org/c/openstack/nova/+/955304 | 14:13 |
opendevreview | Wesley Hershberger proposed openstack/nova stable/2024.1: Use dict object for request_specs_dict in the _list_view https://review.opendev.org/c/openstack/nova/+/955305 | 14:13 |
dansmith | bauzas: did you see my comments on the tpm series about the confirm flag? | 14:17 |
dansmith | you seemed to ask why they were changed.. I haven't looked at the change yet, but I asked to do it differently to avoid creating a mess we have to clean up or live with forever | 14:18 |
opendevreview | Merged openstack/nova master: Update contributor guide for 2025.2 Flamingo https://review.opendev.org/c/openstack/nova/+/944603 | 14:18 |
andrewbonney | hi all. if there are any cores around I'd appreciate a review of https://review.opendev.org/c/openstack/nova/+/919961 | 14:21 |
andrewbonney | that's a small bug fix which has been hanging around for 1 year+ | 14:21 |
gibi | eventlet removal sync in 4 minutes https://meet.google.com/bcy-uqoz-hje | 14:26 |
gibi | andrewbonney: you are today's lucky one :) I approved the fix. Thanks for fixing that bug. | 14:28 |
andrewbonney | Thank you! | 14:28 |
stephenfin | gibi: Yeah, sean-k-mooney and I were thinking it might be a race, but it's weird that its happening consistently on every branch since bobcat | 14:32 |
stephenfin | restarting libvirtd didn't help either | 14:32 |
opendevreview | Ian Watson proposed openstack/nova master: Add migration conditional for resource tracker https://review.opendev.org/c/openstack/nova/+/955690 | 15:03 |
dansmith | gibi: andrewbonney I had commented about some missing test coverage | 15:09 |
dansmith | looks like we raced and I lost | 15:09 |
gibi | I'm fine pulling it from the gate or looking at a fup with the test coverage | 15:14 |
dansmith | maybe andrewbonney can at least respond to my question and do a FUP if we just need an extra assert in an adjacent test | 15:15 |
andrewbonney | I'll take a look, but I've been away for a few months so need to context switch back in | 15:16 |
sean-k-mooney | oh the dns patch | 15:18 |
sean-k-mooney | dansmith: your asking is the a testcase where we do resest the dns name? | 15:19 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for quota sets API https://review.opendev.org/c/openstack/nova/+/956141 | 15:20 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for migrations API https://review.opendev.org/c/openstack/nova/+/956142 | 15:20 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server migrations API https://review.opendev.org/c/openstack/nova/+/956143 | 15:20 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server tags API https://review.opendev.org/c/openstack/nova/+/956144 | 15:20 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server metadata APIs https://review.opendev.org/c/openstack/nova/+/956145 | 15:20 |
dansmith | sean-k-mooney: no, | 15:22 |
sean-k-mooney | oh then i did not understand your question in the review | 15:23 |
dansmith | sean-k-mooney: the new test case added an assertion that we don't call reset when we're not supposed to, but I don't see an adjacent similar test case that asserts that we do call it when expected | 15:23 |
dansmith | I'm assuming there just needs to be an assert_called_once() somewhere in one of the nearby tests | 15:23 |
dansmith | point being, the patch is disabling calling that function in some situations, but I don't know that anything would break if it incorrectly disables the call in _all_ situations | 15:24 |
sean-k-mooney | oh ok am its only requrie if a prot is detached so the next test i think could have that mocked and assert its called | 15:26 |
sean-k-mooney | although in the detach flow | 15:27 |
sean-k-mooney | im not sure we need to do that | 15:27 |
sean-k-mooney | so we might be able to remove _reset_port_dns_name entirly | 15:28 |
dansmith | doesn't that make this patch totally different then? | 15:28 |
sean-k-mooney | i woudl have to lod context again | 15:28 |
dansmith | that's removing a line we were always calling.. this patch makes it conditional, but never checks the was-default-before case | 15:28 |
dansmith | gibi: andrewbonney ^ | 15:28 |
sean-k-mooney | si the _reset_port_dns_name call if if we are using extenreal dns integration | 15:29 |
sean-k-mooney | if we are uisng neutron internl dns the its cleared vai port update on detact. | 15:29 |
sean-k-mooney | i think the code is still correct | 15:29 |
dansmith | that has nothing to do with my concern that we don't know if we're doing it or not | 15:29 |
sean-k-mooney | but we maybe jsut missing a test for that edge case | 15:29 |
dansmith | it's not an edge case, it's what we were *always* doing before | 15:30 |
sean-k-mooney | yes which was incorect | 15:30 |
dansmith | and if it wasn't problematic for andrewbonney's situation, this patch wouldn't be needed | 15:30 |
sean-k-mooney | we shoudl never have been doing it on unbind | 15:30 |
sean-k-mooney | only on detach | 15:30 |
sean-k-mooney | in both the internal and external dns case we were privously doing it unconditionally | 15:31 |
sean-k-mooney | now both path only do it if its a detach | 15:31 |
dansmith | none of that has anything to do with the tests I'm quoting.. I'm talking about the _unit_ test that checks that this method does what it should based on the various inputs, and there's now a conditional that we don't (AFAIK) verify does the right thing | 15:32 |
sean-k-mooney | detach defautls to true. so test_unbind_ports_reset_dns_name_by_admin whih is the next test calls unbiend iwth it set to ture | 15:34 |
dansmith | right, and where's the assert that reset_dns was called? | 15:34 |
sean-k-mooney | and the new test passes detach=false | 15:34 |
sean-k-mooney | it never existed before | 15:34 |
dansmith | I KNOW THAT | 15:34 |
sean-k-mooney | my point is the lack fo that assert is not related to this patch | 15:35 |
sean-k-mooney | we can add it | 15:35 |
dansmith | before, there was no question.. it was clearly a gap in the assertion, but at least there was no way for it to not be run.. now there *is* a way for it not to be run, and the test doesn't assert that it is called when we expect it to | 15:35 |
sean-k-mooney | sure we can add the assert. i have no issue with that | 15:37 |
sean-k-mooney | we can proably check this in the funtional test as well. we jsut need to replace shelve with detach | 15:39 |
dansmith | that's probably something we should be doing anyway.. making sure that this gets called properly in the high-level op that it purports to change | 15:43 |
sean-k-mooney | we had very minimal test coveage wehn thsi was added to nova in the first place https://github.com/openstack/nova/commit/997d8f516cee99b4e16429d13ca5cf7fc05166aa | 15:43 |
sean-k-mooney | so its as good a time to improve that as any | 15:43 |
sean-k-mooney | well actully this funciton was added in https://github.com/openstack/nova/commit/1b797f6f7e99fdef380340d6fe29e4004be48781 later | 15:46 |
opendevreview | Dan Smith proposed openstack/nova master: Don't reset port dns_name when shelving instances https://review.opendev.org/c/openstack/nova/+/919961 | 15:53 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server migrations API https://review.opendev.org/c/openstack/nova/+/956143 | 15:54 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server tags API https://review.opendev.org/c/openstack/nova/+/956144 | 15:54 |
opendevreview | Stephen Finucane proposed openstack/nova master: api: Add response body schemas for server metadata APIs https://review.opendev.org/c/openstack/nova/+/956145 | 15:54 |
sean-k-mooney | gmaan: i added my +2 and some other comments. i think im fine to refiene the docs/release not in a follow up but holding +w so you can read the comments | 18:18 |
sean-k-mooney | if no one else provides feedback ill add +w tomorrow or others are freee to do so | 18:18 |
gmaan | sean-k-mooney: checking | 18:18 |
sean-k-mooney | gibi: dan adressed there testing comment in https://review.opendev.org/c/openstack/nova/+/919961 woudl you mind looking at that again tomorow when your aroudn and reappoving it if your still happy. i just recheked it because of a kernel panic so hopefully it will be green in the morning | 18:23 |
gmaan | sean-k-mooney: replied, 'i think im fine to refiene the docs/release not in a follow up' - > you mean to you are ok to do it in follow up | 18:29 |
gmaan | ohk I think 'release not' -> 'release note' got it now | 18:29 |
gmaan | fixing those in follow up | 18:29 |
sean-k-mooney | ya im fine with a follow up | 18:29 |
gmaan | cool | 18:29 |
sean-k-mooney | you can respin if you want but i think tis udnerstandable just the wording could be clearer | 18:29 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Document native threading mode and tuneables https://review.opendev.org/c/openstack/nova/+/949364 | 19:27 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Allow services to start with threading https://review.opendev.org/c/openstack/nova/+/948311 | 19:27 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450 | 19:27 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run nova-api and -metadata in threaded mode https://review.opendev.org/c/openstack/nova/+/951957 | 19:27 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Do not yield in threading mode https://review.opendev.org/c/openstack/nova/+/950994 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Allow to start unit test without eventlet https://review.opendev.org/c/openstack/nova/+/953436 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run unit test with threading mode https://review.opendev.org/c/openstack/nova/+/953475 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [test]RPC using threading or eventlet selectively https://review.opendev.org/c/openstack/nova/+/953815 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [CI]Make nova-tox-py312-threading voting https://review.opendev.org/c/openstack/nova/+/955791 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [vncproxy]Handle ssl.wrap_socket removal in py312 https://review.opendev.org/c/openstack/nova/+/955915 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Make RBD Tpool usage conditional https://review.opendev.org/c/openstack/nova/+/956089 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Make libvirt Tpool proxying conditional https://review.opendev.org/c/openstack/nova/+/956090 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Fix ProviderTree copying with threading Lock https://review.opendev.org/c/openstack/nova/+/956091 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [test]Further categorization of disabled unit tests https://review.opendev.org/c/openstack/nova/+/956092 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Warn on long task wait time for executor https://review.opendev.org/c/openstack/nova/+/952666 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Document native threading mode and tuneables https://review.opendev.org/c/openstack/nova/+/949364 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Allow services to start with threading https://review.opendev.org/c/openstack/nova/+/948311 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run nova-api and -metadata in threaded mode https://review.opendev.org/c/openstack/nova/+/951957 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Do not yield in threading mode https://review.opendev.org/c/openstack/nova/+/950994 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Allow to start unit test without eventlet https://review.opendev.org/c/openstack/nova/+/953436 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Run unit test with threading mode https://review.opendev.org/c/openstack/nova/+/953475 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [test]RPC using threading or eventlet selectively https://review.opendev.org/c/openstack/nova/+/953815 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [CI]Make nova-tox-py312-threading voting https://review.opendev.org/c/openstack/nova/+/955791 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [vncproxy]Handle ssl.wrap_socket removal in py312 https://review.opendev.org/c/openstack/nova/+/955915 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Make RBD Tpool usage conditional https://review.opendev.org/c/openstack/nova/+/956089 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Make libvirt Tpool proxying conditional https://review.opendev.org/c/openstack/nova/+/956090 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Fix ProviderTree copying with threading Lock https://review.opendev.org/c/openstack/nova/+/956091 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: [test]Further categorization of disabled unit tests https://review.opendev.org/c/openstack/nova/+/956092 | 19:28 |
opendevreview | Balazs Gibizer proposed openstack/nova master: Warn on long task wait time for executor https://review.opendev.org/c/openstack/nova/+/952666 | 19:28 |
gibi | dansmith: sean-k-mooney: ^^ fixed the doc patch and rebased on recent master | 19:29 |
opendevreview | Ghanshyam proposed openstack/nova master: Fix doc comment on manager role change https://review.opendev.org/c/openstack/nova/+/956189 | 19:31 |
gmaan | sean-k-mooney: stephenfin ^^ fixing doc comments | 19:33 |
sean-k-mooney | gibi: cool ill take a look soon | 19:33 |
gibi | thanks | 19:33 |
sean-k-mooney | im going to try an wrap up soon but if i dont get to it before i finsih ill start with it in the morning | 19:34 |
dansmith | hoo boy | 19:48 |
sean-k-mooney | gibi: i left a question on https://review.opendev.org/c/openstack/nova/+/953436 but up to thre im +2 im goign to swap to gmaan's doc followup and call it a day | 19:55 |
opendevreview | Ghanshyam proposed openstack/nova master: Fix doc comment on manager role change https://review.opendev.org/c/openstack/nova/+/956189 | 20:56 |
opendevreview | Merged openstack/nova master: Add project manager role in Nova API policy rule https://review.opendev.org/c/openstack/nova/+/953063 | 21:28 |
opendevreview | Merged openstack/nova master: Don't reset port dns_name when shelving instances https://review.opendev.org/c/openstack/nova/+/919961 | 22:02 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!