| opendevreview | jayaanand borra proposed openstack/cinder-tempest-plugin master: Add image-cache cross-pool tempest tests https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/989987 | 03:50 |
|---|---|---|
| opendevreview | Siddharth Kumar proposed openstack/cinder stable/2026.1: Dell PowerFlex : Fix multiple broken functions https://review.opendev.org/c/openstack/cinder/+/993565 | 06:40 |
| opendevreview | Merged openstack/cinder stable/2025.2: Fix _is_encrypted comparison in volume api https://review.opendev.org/c/openstack/cinder/+/983620 | 08:35 |
| opendevreview | jayaanand borra proposed openstack/cinder master: NetApp: FlexVol autosize, auto-provisioning and discover https://review.opendev.org/c/openstack/cinder/+/1000307 | 10:13 |
| opendevreview | Thomas Goirand proposed openstack/cinder master: Remove disabled services from state_map https://review.opendev.org/c/openstack/cinder/+/979641 | 10:21 |
| damiandabrowski | Hey folks! I have a few patches that have been waiting for review since February/March. I'd really appreciate it if anyone could take a look when they have a chance. Thanks a lot! | 11:14 |
| damiandabrowski | https://review.opendev.org/q/owner:damian@dabrowski.cloud+and+project:+openstack/cinder+and+status:+open | 11:14 |
| stephenfin | simondodsley: I think there's something off with https://review.opendev.org/c/openstack/openstacksdk/+/997562 | 11:33 |
| stephenfin | the sdk functional job has gone from ~100% passing to very unstable since that landed https://zuul.opendev.org/t/openstack/builds?job_name=openstacksdk-functional-devstack&branch=master&skip=300&limit=100 | 11:34 |
| stephenfin | Would you have time to investigate? I'm proposing a revert in the interim | 11:35 |
| stephenfin | (tbc, I realise the test could be wrong and we've just exposed that, but it's flaky enough atm that we either need the revert or we disable the test entirely) | 11:37 |
| simondodsley | stephenfin: https://review.opendev.org/c/openstack/cinder/+/1000097 is up to fix the underlying issue | 12:25 |
| rosmaita | stephenfin: would be good to get your input on https://review.opendev.org/c/openstack/cinder/+/1000097 | 12:33 |
| stephenfin | simondodsley: Okay, I've pulled the revert from the gate for now. We'd want to get that fix in this week though since that job is pretty widely used (nova, requirements, cinder, sdk itself...) | 13:07 |
| stephenfin | I've reviewed it now too, rosmaita | 13:07 |
| rosmaita | stephenfin: thanks, can you take a look at my comment in cinder/db/migrations/versions/921e1a36b076_initial.py on that patch? | 13:08 |
| * stephenfin looks | 13:09 | |
| stephenfin | rosmaita: I've to drop for a bit, but I'd like to check the old sqlalchemy-migrate migrations to see if *they* set a default of 0 | 13:10 |
| stephenfin | If they did, you're correct and the model is simply out-of-sync. If they didn't, then we mistakenly added the default durin the migration to alembic and some very old deployments may still have NULL (rather than 0) based defaults | 13:11 |
| rosmaita | stephenfin: ack, i will take a look | 13:12 |
| stephenfin | Now if those ancient columns have never been incremented before now (otherwise they'd no longer be set to NULL) then they're probably not going to be incremented now... | 13:12 |
| stephenfin | and the coalesce should in theory make all this a no-op | 13:12 |
| stephenfin | s/no-op/non-issue/ | 13:13 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: Add NVMe/FC support to the LVM/nvmet target https://review.opendev.org/c/openstack/cinder/+/1000326 | 13:21 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 13:22 |
| rosmaita | stephenfin: looks like default=0 was in the legacy migrations: https://opendev.org/openstack/cinder/src/tag/xena-eol/cinder/db/legacy_migrations/versions/135_cinder_init.py#L539 | 13:36 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: backup: count dependent backups in the database https://review.opendev.org/c/openstack/cinder/+/1000097 | 13:59 |
| stephenfin | simondodsley: Based on what rosmaita observed, I suspect you can drop that `coalesce` and associated comment/TODO? | 14:03 |
| stephenfin | left comments. I could fix but I suspect there's enough touching that patch already | 14:07 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: backup: count dependent backups in the database https://review.opendev.org/c/openstack/cinder/+/1000097 | 14:22 |
| simondodsley | stephenfin: rosmaita PS5 is up for this patch. Please see if I have covered everything you commented on | 14:23 |
| rosmaita | simondodsley: looking now | 14:23 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 14:31 |
| stephenfin | simondodsley: I still see the coalesce and associated comments? | 14:35 |
| rosmaita | stephenfin: see Simon's comment on PS3 at 10:07am | 14:36 |
| rosmaita | well, 10:07 my time i guess | 14:37 |
| rosmaita | i am utc-4 | 14:37 |
| rosmaita | (i can't figure out how to get a direct link to the comment) | 14:37 |
| rosmaita | line 574 in migrations/versions/921...initial.py | 14:38 |
| simondodsley | stephenfin: PS5 does take your suggestion — models.py default=0 is gone, so the patch touches no model or schema at all and can backport clean. coalesce + TODO stay; the follow-up on master does migration → backfill → server default → remove coalesce → drop the upgrade note. | 14:40 |
| stephenfin | Oh, I thought we'd decided the models didn't match the reality in the database? | 14:41 |
| stephenfin | actually let me find and read that comment before I say any more | 14:41 |
| stephenfin | okay, good thing I did :) | 14:42 |
| rosmaita | stephenfin: simondodsley: ok, so i don't understand why we want to drop the default=0 in the models.py | 14:48 |
| simondodsley | rosmaita: I think I over-corrected. dropping it was me taking stephenf's "punt this to a follow-up" literally, but the backport argument doesn't apply here — default=0 on the model is a client-side default, no DDL, no migration, so it backports fine. schema changes are what's forbidden on stable, and this isn't one. | 14:51 |
| simondodsley | Do you want me to add it back in? | 14:51 |
| rosmaita | i think so ... i'm not convinced we need a server_default, if we have the default=0 in the model, it should get set on the first insert into the row | 14:52 |
| rosmaita | my reading of your comment is that it's the missing default=0 in the model that is the source of the NULL value | 14:53 |
| simondodsley | correct - i'll do a PS6 | 14:54 |
| rosmaita | ok, cool | 14:55 |
| rosmaita | thanks! | 14:55 |
| simondodsley | the coalese has to stay for the time being - i can look at a patch to fix that as a follow up if you want | 14:57 |
| rosmaita | yes, i agree coalesce has to stay | 14:57 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: backup: count dependent backups in the database https://review.opendev.org/c/openstack/cinder/+/1000097 | 15:01 |
| simondodsley | stephenfin: romaita: PS6 up - hopefully that is it now... | 15:05 |
| rosmaita | LGTM! | 15:05 |
| simondodsley | once in this will clear the openstacksdk-functional-devstack job to free up the gate | 15:06 |
| simondodsley | rosmaita: can you prod another core on this? | 15:08 |
| rosmaita | sure, looking to see who's online now | 15:09 |
| rosmaita | jbernard: eharney: agalica: jungleboyj: hemna: need another +2 on https://review.opendev.org/c/openstack/cinder/+/1000097 to unblock the gates | 15:10 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 15:44 |
| simondodsley | rosmaita: openstacksdk-functional-devstack job is passing for this patch - this is the whole point... just need that second +2 and +W | 15:54 |
| rosmaita | simondodsley: let's wait for zuul to go green on PS 6, and if we don't have a secont +2 by then, I will ninja-approve it with stephenfin's +1 since he is an SME on the db stuff | 16:01 |
| opendevreview | Merged openstack/cinder master: mypy: Correct futurist.GreenThreadPoolExecutor name https://review.opendev.org/c/openstack/cinder/+/999821 | 16:06 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 16:46 |
| simondodsley | rosmaita: Zuul is green... | 17:54 |
| rosmaita | simondodsley: ack | 17:57 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 19:00 |
| opendevreview | Merged openstack/cinder master: backup: count dependent backups in the database https://review.opendev.org/c/openstack/cinder/+/1000097 | 19:35 |
| simondodsley | rosmaita: stephenfin: 100097 has merged so the revert patch can be abandoned now | 19:44 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: privsep/nvmet: drop root arg from Port.setup for new nvmetcli https://review.opendev.org/c/openstack/cinder/+/1000373 | 19:58 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000374 | 19:59 |
| opendevreview | Simon Dodsley proposed openstack/cinder-tempest-plugin master: Add NVMe/FC (nvmet_fc) LVM CI job using nvme_fcloop https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/1000327 | 19:59 |
| opendevreview | Merged openstack/cinder master: Fix temp volume project id for Tatlin storage https://review.opendev.org/c/openstack/cinder/+/858293 | 20:27 |
| opendevreview | Merged openstack/cinder master: Fix: Unset default type for non-existent project https://review.opendev.org/c/openstack/cinder/+/930009 | 20:27 |
| opendevreview | Brian Rosmaita proposed openstack/cinder master: Run Cinder's Backup service in native threads https://review.opendev.org/c/openstack/cinder/+/977851 | 20:49 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!