Tuesday, 2023-06-20

opendevreviewMerged openstack/ironic master: Add test timeout to tox config  https://review.opendev.org/c/openstack/ironic/+/88537204:15
rpittau1 merged, 2 to go06:47
rpittaugood morning ironic! o/06:47
opendevreviewVerification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583706:54
opendevreviewVerification of a change to openstack/ironic master failed: Use tox env variables in coverage tests  https://review.opendev.org/c/openstack/ironic/+/88550707:37
opendevreviewVerification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583707:37
opendevreviewVerification of a change to openstack/ironic master failed: Use tox env variables in coverage tests  https://review.opendev.org/c/openstack/ironic/+/88550707:43
opendevreviewRiccardo Pittau proposed openstack/ironic master: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583708:05
rpittau^ we need this before the rest08:05
opendevreviewRiccardo Pittau proposed openstack/ironic master: Use tox env variables in coverage tests  https://review.opendev.org/c/openstack/ironic/+/88550708:05
opendevreviewVerification of a change to openstack/ironic master failed: Use jammy for base jobs  https://review.opendev.org/c/openstack/ironic/+/86905208:48
opendevreviewVerification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583711:12
*** Continuity__ is now known as Continuity11:24
opendevreviewIury Gregory Melo Ferreira proposed openstack/ironic master: Add DB API for Firmware and Object  https://review.opendev.org/c/openstack/ironic/+/88306211:51
iurygregorygood morning Ironic11:51
iurygregoryrpittau, tks for the review o/ I've updated11:51
rpittauiurygregory: no problem! and welcome back :)11:57
iurygregorytks!11:58
iurygregoryI'm planning to sue American Airlines after all the problems I had in this travel XD11:58
rpittaulol12:10
iurygregoryXD12:13
iurygregoryrpittau, tks for the review12:58
rpittauyw :)12:59
dtantsurfolks, looking for a 2nd +2 on https://review.opendev.org/c/openstack/ironic/+/87594412:59
dtantsurI know it's pretty large, but we need to move on with it (and it will probably conflict with other work, e.g. firwmare)13:00
iurygregorydtantsur, i will check today13:04
dtantsurthx!13:46
iurygregoryrpittau, my bad the patch had a pep8 issue, so I'm fixing and including the nits you mentioned in the review o/13:48
rpittauack no worries13:48
opendevreviewIury Gregory Melo Ferreira proposed openstack/ironic master: Add DB API for Firmware and Object  https://review.opendev.org/c/openstack/ironic/+/88306213:50
iurygregorydtantsur, I've added a +2, will wait till today to see if TheJulia has more thoughts on the patch before approving right now (but later today I will +w)14:02
rpittaulooks like the MIGRATIONS_TIMEOUT is not being respected by the migrations tests https://review.opendev.org/c/openstack/ironic/+/885837 or there something else going on that causes the timeout:/15:00
iurygregory.-.15:19
iurygregoryouch15:19
iurygregoryrpittau, at least looking in codesearch for MIGRATIONS_TIMEOUT I only see the variable in test_migrations.py15:24
rpittauiurygregory: because we're the only project to use it15:24
rpittauit's an override15:25
iurygregoryos.getenv seems correct to me15:25
rpittauyeah, it is, but I think we need a gentle=False entry instead of True15:25
rpittaudoing some tests now15:25
iurygregoryack15:25
rpittaulooks like that's the way, it will abruptly crash but should give us the list of what's failing15:31
opendevreviewRiccardo Pittau proposed openstack/ironic master: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583715:34
rpittaualright, time to start packing, see ya next week! o/15:39
iurygregorygentle=False, makes sense15:39
iurygregoryenjoy the time off rpittau o/15:39
opendevreviewStephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x)  https://review.opendev.org/c/openstack/ironic/+/88602015:56
opendevreviewJay Faulkner proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x)  https://review.opendev.org/c/openstack/ironic/+/88602016:01
opendevreviewJay Faulkner proposed openstack/ironic master: Fix SQLAlchemy engine connection listener  https://review.opendev.org/c/openstack/ironic/+/88579716:04
opendevreviewStephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x)  https://review.opendev.org/c/openstack/ironic/+/88602016:06
opendevreviewStephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x)  https://review.opendev.org/c/openstack/ironic/+/88602016:07
stephenfinJayF: Just rebased the job change on top of the fix16:07
stephenfinI didn't know that was already done16:07
JayFah, hopefully you got my pep8 fix on the fix then :)16:07
JayFyeah you did good stuff16:07
stephenfinJust about did \o/16:08
JayFstephenfin: as the neighborhood sqlalchemy magician, has anyone clued you into our current unit test issues?16:08
stephenfinthey have not16:08
JayFso for about a week or two, we've had intermittant unit test failures16:08
JayFthings seem to be hanging in the db migration tests16:09
stephenfinI saw there were timeouts. I'm going to guess they're related16:09
stephenfinAh16:09
JayFhttps://github.com/openstack/ironic/blob/master/ironic/tests/unit/db/sqlalchemy/test_migrations.py#L1342 was the worst offender16:09
JayFand was disabled to allow anything at all to pass16:09
JayFbut it's happening intermittantly on the gate16:09
JayFand afaik, rpittau is the only person to reproduce it locally16:09
JayFwe're working to improve timeouts around tests to create better errors, but I think there's still something getting hung (probably DB-deadlock-hung, but that's a guess)16:10
JayFso if you have any insight or thoughts, would love to hear them16:10
stephenfinI'm guessing you don't know when it started? There will have been changes in APIs used but I'm guessing most of that should be syntax. We're still not actually using SQLAlchemy 2.x anywhere16:11
JayFwe could not get it to behave differently under sqla 1.4 v 2.x16:12
JayFI think it's probably in the oslo db fixtures, we have a bad migration, or something like that16:12
JayFthe failures do not directly map to any patch in Ironic or obvious related library release16:12
stephenfinYeah, I was going to say you're not using the most recent versions of oslo.db (or alembic??) either since those are also capped by sqlalchemy 2.x16:13
stephenfin*blocked by16:13
Nisha_AgarwalJayF, Regarding bug https://bugs.launchpad.net/proliantutils/+bug/2021995 , could you help me to get the iLO firmware version installed on the system at the minimum? This will help to initiate the discussion with iLO firmware team.16:13
JayFNisha_Agarwal: I asked the folks in the UK to update that with the info in our sync meeting like, 15 minutes ago16:14
JayFNisha_Agarwal: IDK when they will take that action but they're aware16:14
Nisha_AgarwalOk. Thanks16:14
JayFhttps://github.com/openstack/ironic/blob/master/ironic/db/sqlalchemy/alembic/versions/163040c5513f_add_firmware_information.py#L44 why is there a trailing comma there?16:41
JayFiurygregory: ^ intentional?16:41
JayFyeah it's that way in a few places16:46
* JayF just looking to see if he can ID anything funky with the migrations16:46
iurygregoryseems like you found the answer while I was still at lunch =)16:58
JayFwell, I would've preferred it be the problem16:58
JayFthan to be explainable lol16:58
iurygregoryagree16:58
iurygregoryit would make our life easier16:58
JayFI just... dunno how to approach things anymore tbh16:59
JayFstarting to run out of things to check16:59
JayFnot being able to repro it locally is brutal16:59
iurygregoryyeah =(17:00
iurygregoryanyone noticed that since yesterday metal3-integration is reporting POST_FAILURE?20:04
JayFI saw it, and stuffed it deep down until I figure out the OTHER failures in ci 20:07
JayF:| 20:07
iurygregoryseems like is trying to use a wrong version of CAPIRELEASE20:16
iurygregorybut also lib/releases.sh jq command not found is not something good .-.20:17
JayFYeah, it's -nv for a reason. We should ensure it gets fixed but we only control some of the moving parts there20:17
iurygregoryI'm raising this in the k8s slack20:20
iurygregoryok first time I see ironic-cross-sushy with TIMED_OUT (CI, why you are doing this?)21:03
JayFI'm reasonably sure we might have a real bug21:08
JayFbut I'm close to the end of my rope21:08
JayFI've read so much code21:08
JayFtried to understand it21:08
JayFbut without a local reproduction I feel a bit stuck21:08
iurygregoryyeah, same .-.21:16
iurygregoryJayF, want to +W to see if https://review.opendev.org/c/openstack/ironic/+/885837 help us?21:18
JayFdone21:18
opendevreviewMerged openstack/ironic master: Allow setting migrations timeout value from tox  https://review.opendev.org/c/openstack/ironic/+/88583721:38
iurygregory\o/21:38
opendevreviewIury Gregory Melo Ferreira proposed openstack/ironic master: Use tox env variables in coverage tests  https://review.opendev.org/c/openstack/ironic/+/88550721:42
iurygregoryhad to rebase because gerrit was showing merge conflict21:42
JayFo/ 21:45
iurygregorygoing to grab some dinner and go to the gym, bbl21:47
JayFhave a good one 21:47
iurygregoryty21:48
opendevreviewIury Gregory Melo Ferreira proposed openstack/ironic master: metal3-integration NV  https://review.opendev.org/c/openstack/ironic/+/88653322:07
JayFiurygregory: +2A22:08
JayFiurygregory: I forgot we had made that voting or I would've pushed that, sorry22:08
iurygregoryJayF, np o/22:17
iurygregorynow time to go to the gym22:18

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