| *** mhen_ is now known as mhen | 01:53 | |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: Adds regression test for bug LP#2044235 https://review.opendev.org/c/openstack/nova/+/960349 | 07:37 |
|---|---|---|
| opendevreview | Julien LE JEUNE proposed openstack/nova master: Adds regression test for bug LP#2044235 https://review.opendev.org/c/openstack/nova/+/960349 | 08:21 |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: nova-conductor puts instance in error state https://review.opendev.org/c/openstack/nova/+/901655 | 09:01 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Support multiple allocations for vGPUs https://review.opendev.org/c/openstack/nova/+/845757 | 09:47 |
| sean-k-mooney | ivveh so the current state is as follows, when we were adding the feature to nova orginally the qemu feature to supprot attach was not complete so it was put out of scope of the nova feature wit the intent to support atach in the future. the nova support for this took longer then intended and since then libvirt and qemu have released supprot for it but nova has not been | 10:40 |
| sean-k-mooney | updated to supprot it | 10:40 |
| sean-k-mooney | ivveh so the tl;dr is it used ot be a qemu limiation and not its a nova one as we have not updated to use the new qemu fatures. for partcial reasons however the limitation si still in effefct for openstack but its something we would like to eventually adress. | 10:41 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix string format specifier https://review.opendev.org/c/openstack/nova/+/961211 | 10:49 |
| ratailor | sean-k-mooney, gibi bauzas could you please review clean backports, one more +2 is required. https://review.opendev.org/q/project:openstack/nova+branch:stable/2024.2+owner:ratailor@redhat.com | 11:31 |
| sean-k-mooney | gibi's on pto until wednesday i belvie but i can take a look at the latter two now. it looks like elod and gibi have already approved the first 2 | 11:48 |
| sean-k-mooney | oh elod already didi it | 11:48 |
| sean-k-mooney | since elod has appvoed the 2024.2 version ill quickly pre review the 2024.1 backports | 11:49 |
| jlejeune | hello sean-k-mooney, did you have time to check my regression test: https://review.opendev.org/c/openstack/nova/+/960349 ? | 11:51 |
| sean-k-mooney | i saw that you change to a plain Excption you could also have used a TestEXcption but my main concern with that is you likely can just catch every excption in the main code. | 11:52 |
| sean-k-mooney | this might be fine https://review.opendev.org/c/openstack/nova/+/901655/8/nova/compute/manager.py#9095 because its in pre live migraiton | 11:53 |
| sean-k-mooney | but the futher we get into the migraiton tbe more clean up that is required and the more careful and specific we need to be with what we catch | 11:54 |
| sean-k-mooney | you are at least converting the generic error to a specific on in that expcption handeler which is good | 11:55 |
| jlejeune | yes it's because it happens in pre live migration check that I allowed myself to do it | 11:56 |
| sean-k-mooney | can you rebase your fix patch on the latest revsion of the repoducer so we can see them working togehter | 11:57 |
| jlejeune | sure, doing | 11:58 |
| sean-k-mooney | your reusign the exsitng exception.MigrationPreCheckError that we have for the late tenatn anti affinity check which imples this exception case shoudl already be handeld and it shoudl rollback any work done so i think that is a good chioce and it is also another precheck failure so its semanticly valid | 12:00 |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: nova-conductor puts instance in error state https://review.opendev.org/c/openstack/nova/+/901655 | 12:00 |
| sean-k-mooney | jlejeune: by the way, whle we prefer functional repoduces if wee cant simulate the exact condtions in the fucntional suite we also do use unit test repodcues form time to time. | 12:02 |
| jlejeune | hm ok good to know | 12:03 |
| sean-k-mooney | so if other want to see more explicte testign with a timeout excption, we could add one that directly calls check_can_live_migrate_destination and mke it mock check_can_live_migrate_source to raise the timeout excption you orgianlly hit | 12:04 |
| sean-k-mooney | then you can show yoru fix convet it to a MigrationPreCheckError | 12:04 |
| sean-k-mooney | the functional test you already have will continue to assert the overall instnace state stays active | 12:05 |
| jlejeune | it was what I have done at first on the unit test :) | 12:05 |
| sean-k-mooney | and the unit test will assert that timeouts are properly handeled | 12:05 |
| sean-k-mooney | yep | 12:05 |
| jlejeune | I can replace the test.Exception by the olso message timeout if you prefer | 12:05 |
| jlejeune | in the unit tets | 12:06 |
| sean-k-mooney | im not sure that is requird. | 12:06 |
| sean-k-mooney | im more saying f other think we should have a more explcit repoducer | 12:06 |
| sean-k-mooney | i woudl proably do that via pulling in part fo the unit test you wote | 12:07 |
| sean-k-mooney | but instead assert that the excption is not handled properly locally. | 12:07 |
| ratailor | sean-k-mooney, do we need to backport this once it merges on master. https://review.opendev.org/c/openstack/nova/+/961211 | 12:08 |
| sean-k-mooney | i.e. that it is not properly converted to the MigrationPreCheckError error by check_can_live_migrate_destination | 12:08 |
| sean-k-mooney | it depend on what the side effect is. | 12:09 |
| sean-k-mooney | we proably should backprot it because its trivial | 12:09 |
| sean-k-mooney | but i dont see a bug for it | 12:09 |
| sean-k-mooney | im guessign the error is not incldued in the message correct | 12:09 |
| sean-k-mooney | so it prints litrally $(error)s | 12:10 |
| sean-k-mooney | instead of interpolating the strign | 12:10 |
| sean-k-mooney | that is not critical to backport as long as it does not break anything but it will improve debugablity so its good to backport it | 12:10 |
| ratailor | sean-k-mooney, should I file a bug for it then ? | 12:11 |
| sean-k-mooney | sure, feel free to triage it and asign it to your self | 12:12 |
| ratailor | sean-k-mooney, ack. Thanks! | 12:12 |
| jlejeune | not sure to understand what do you mean sean-k-mooney :) | 12:15 |
| sean-k-mooney | jlejeune: i mean your current unit test is fine in the fix patch, you might consider copying it to the repodcuer patch and have it assert it retruns the timeout or test expction instead fo converting it to the pre migration expction | 12:17 |
| sean-k-mooney | jlejeune: that woudl show that orginally the code did not have any sepcial handelign for timeouts and after your change it does | 12:17 |
| sean-k-mooney | where as the functional test shows that before that ment the instnace went to error and after the instance stays active | 12:17 |
| sean-k-mooney | jlejeune: that makes it very clear what you fixed and how | 12:18 |
| sean-k-mooney | what: error -> active how: Timeout/generic excption -> MigrationPreCheckError | 12:19 |
| sean-k-mooney | jlejeune: you dont have to do that but it would be nice to have that level fo clarity in the repoducer and fix. | 12:20 |
| jlejeune | hm ok I get it | 12:20 |
| jlejeune | so we can move on with that for now ? and I will know it for the next time :) | 12:42 |
| sean-k-mooney | yes i think so, and if other would like more testing in the repoducer you knwo what to add | 12:43 |
| jlejeune | ok ! | 12:46 |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: nova-conductor puts instance in error state https://review.opendev.org/c/openstack/nova/+/901655 | 12:48 |
| jlejeune | what do you think about that (https://review.opendev.org/c/openstack/nova/+/960349) gibi ? | 12:50 |
| opendevreview | Stephen Finucane proposed openstack/nova-specs master: Add compute-volume-az-mapping spec https://review.opendev.org/c/openstack/nova-specs/+/961233 | 13:06 |
| opendevreview | Stephen Finucane proposed openstack/nova-specs master: Add compute-volume-az-mapping spec https://review.opendev.org/c/openstack/nova-specs/+/961233 | 13:07 |
| sean-k-mooney | stephenfin: is ^ a purly nova spec or also invovles cidner | 13:08 |
| opendevreview | Rajesh Tailor proposed openstack/nova master: Fix string format specifier https://review.opendev.org/c/openstack/nova/+/961211 | 13:28 |
| opendevreview | Merged openstack/nova stable/2024.2: Fix case sensitive comparison https://review.opendev.org/c/openstack/nova/+/959534 | 13:44 |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: Adds regression test for bug LP#2044235 https://review.opendev.org/c/openstack/nova/+/960349 | 14:37 |
| opendevreview | Julien LE JEUNE proposed openstack/nova master: nova-conductor puts instance in error state https://review.opendev.org/c/openstack/nova/+/901655 | 14:41 |
| jlejeune | sean-k-mooney: here are my last changes following your note ^ | 14:42 |
| sean-k-mooney | thanks i have a meeting at the top of the hour but ill see if i can review it before then | 14:46 |
| jlejeune | ok, no pb | 14:47 |
| dansmith | Uggla: rc1 is cut so we're good to merge on master for 2026.1 yeah? | 14:48 |
| Uggla | dansmith, yes RC1 patch was merged so we are good to go. | 14:56 |
| dansmith | Uggla: cool | 14:57 |
| dansmith | bauzas: can you circle back on this? https://review.opendev.org/c/openstack/nova/+/954990 | 14:57 |
| bauzas | ack | 14:57 |
| stephenfin | sean-k-mooney: I think just Nova | 15:03 |
| stephenfin | If we did it the way I planned, it would live in nova because that's where the metadata service lives | 15:04 |
| opendevreview | Merged openstack/nova stable/2024.2: Fix 'nova-manage image_property set' command https://review.opendev.org/c/openstack/nova/+/959429 | 15:13 |
| opendevreview | Merged openstack/nova stable/2024.2: Fix disable memballoon device https://review.opendev.org/c/openstack/nova/+/952960 | 15:14 |
| sean-k-mooney | stephenfin: i havene tread it but ok this is about metadta not about modelign it in palcement | 15:16 |
| stephenfin | yes, purely a host-level config option exposed to the user via metadata | 15:17 |
| stephenfin | I want it so cinder CSI doesn't need to conflate compute AZ with block storage AZs (we've no mechanism currently to say "give me the storage AZ for this instance) | 15:18 |
| opendevreview | Merged openstack/nova master: Remove eventlet timer from multi_cell_list https://review.opendev.org/c/openstack/nova/+/954990 | 17:15 |
| opendevreview | Merged openstack/nova stable/2024.1: Fix disable memballoon device https://review.opendev.org/c/openstack/nova/+/952961 | 17:15 |
| opendevreview | Merged openstack/nova stable/2024.2: Fix case-sensitivity for metadata keys https://review.opendev.org/c/openstack/nova/+/959435 | 20:14 |
| -opendevstatus- NOTICE: The Gerrit service on review.opendev.org will be offline briefly for a quick patch update, but will return within a few minutes | 21:09 | |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!