| vanou | good morning Ironic o/ | 00:28 |
|---|---|---|
| opendevreview | Merged openstack/ironic stable/victoria: Do not reboot into nowhere after BIOS settings with fast-track https://review.opendev.org/c/openstack/ironic/+/859411 | 01:23 |
| lecris[m] | hjensas: about the ironic vs neutron dhcp issue [1], any ideas of how we can add the null character? | 05:48 |
| lecris[m] | [1] https://bugs.launchpad.net/tripleo/+bug/1990028 | 05:48 |
| lecris[m] | Though it might be better to contact them at tripleo channel? | 05:49 |
| vanou | JayF: Sorry for delay. I had been away for a while... I'll check it and try it on local lab. | 05:53 |
| vanou | ASAP | 05:53 |
| opendevreview | Kirill proposed openstack/ironic stable/wallaby: add vnc console https://review.opendev.org/c/openstack/ironic/+/860552 | 11:28 |
| janders | ajya dtantsur I got the SD resource from a Nokia machine: https://paste.openstack.org/show/bOGB1mJ3D2ZrStf2siUE/ | 11:29 |
| janders | it does have the attribute we are manipulating, so I suppose we could use the presence of the relevant attributes as a decision making factor | 11:29 |
| dtantsur | janders: this is awesome, let's proceed then? | 11:51 |
| janders | yeah I think so | 11:51 |
| ajya | janders: sounds good. To reiterate, iDRAC has only BootSourceOverrideMode in /Settings resource if Settings are present at all. | 11:53 |
| iurygregory | good morning Ironic | 11:57 |
| janders | dtantsur ajya so - will go with the approach that if SettingsURI is specified by the BMC AND an attribute is listed under that SettingsURI, then we attempt to PATCH this attribute against the SettingsURI, alternatively we just patch the System resource, correct? | 12:11 |
| janders | also, on failure against SettingsURI, do we fail out or retry against the System resource (as per the current direction of the patch)? | 12:12 |
| dtantsur | janders: do we have any hardware that needs retry? | 12:12 |
| dtantsur | after what we've discovered? | 12:13 |
| ajya | janders: I don't think retry will be necessary, at least for known systems | 12:14 |
| janders | I wasn't thinking specifically about a particular server, just thought perhaps that would help resiliency against future brokenness | 12:15 |
| janders | ajya it wouldn't be necessary for the Nokia and Lenovo, agreed, was thinking hypothetical other systems | 12:16 |
| janders | (retry shouldn't break anything as we're not doing anything unusual to start withg) | 12:16 |
| janders | but I don't feel strongly about the retry part, if you folks reckon it's better to drop it I'm happy to do that | 12:17 |
| ajya | janders: I rather think that such failures should be investigated as that would mean that new logic is not correct or BMC has a bug. | 12:21 |
| janders | ajya fair enough | 12:22 |
| ajya | also note that iDRAC returns different error id (PropertyUnknown) as Redfish spec isn't very specific about it. Unknown systems could return something else too | 12:22 |
| janders | yeah when I was thinking about the retry idea I figured we'd need to dillute the error checking (we used one specific to Lenovo failure mode, don't think it would apply to others) | 12:25 |
| opendevreview | Kirill proposed openstack/python-ironicclient stable/wallaby: Add graphical console interface https://review.opendev.org/c/openstack/python-ironicclient/+/860568 | 12:39 |
| * TheJulia wipes sleep from eyes | 13:27 | |
| TheJulia | good morning | 13:27 |
| dtantsur | morning TheJulia | 13:30 |
| JayF | vanou: thanks; I hope you enjoyed your time away o/ | 13:38 |
| JayF | I'm on openinfra live this morning, talking about Zed Ironic in ~22 minutes | 13:38 |
| JayF | if anyone wants to tune in :) | 13:38 |
| * dtantsur is debugging some kubernetes insanity | 13:42 | |
| TheJulia | I get to have a meeting in a little bit :( | 13:43 |
| TheJulia | thursday meetings should be banned | 13:43 |
| *** iurygregory_ is now known as iurygregory | 14:04 | |
| iurygregory | JayF, tks for doing this! | 14:10 |
| iurygregory | I hate the fact my parents are doing renovations in the house for the next 2 weeks -.-' | 14:10 |
| TheJulia | is there value in keeping the inspection data storage location migration tooling. It existed to allow data to be migrated from swift into a local database, but wondering if it is just cruft at this point | 14:17 |
| dtantsur | TheJulia: are there any issues with it? | 14:22 |
| iurygregory | probably worth an email in the list to see if people are using it... not sure | 14:22 |
| dtantsur | it's not horribly useful, but it's also not complete cruft | 14:22 |
| TheJulia | just looking at the db cruft | 14:22 |
| dtantsur | (people may decide that the database no longer suits them and decide to migrate to swift, for example) | 14:22 |
| TheJulia | I think it only allows swift to db, but I don't remember offhand | 14:22 |
| TheJulia | lots of the migration stuff feels like magic | 14:28 |
| * JayF nuked his keyboard 5 minutes before the presentation when he yeeted his tea everywhere :( | 14:53 | |
| JayF | at least I recovered in time to present lol | 14:53 |
| iurygregory | jesus, is so complicated to figure out why the rpm build is failing only because of some driver tests .-. | 15:04 |
| TheJulia | iurygregory: would another set of eyes help? | 15:22 |
| iurygregory | TheJulia, I can send you the link if you have some time | 15:30 |
| iurygregory | I think it's probably a broken dependency.. but couldn't reproduce the failure | 15:31 |
| opendevreview | Julia Kreger proposed openstack/ironic-inspector master: WIP: Use declarative reader/writer separation https://review.opendev.org/c/openstack/ironic-inspector/+/463768 | 15:44 |
| TheJulia | hmm.. functional tests seem broken, but progress() | 15:54 |
| TheJulia | and they work if your local | 16:02 |
| TheJulia | lovely! | 16:02 |
| opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic bugfix/20.2: [DNM] Testing something https://review.opendev.org/c/openstack/ironic/+/860587 | 16:03 |
| kubajj | dtantsur: TheJulia: I was looking at how the inspection works and figured that the periodic check result function waits until check status changes, right? This then needs to be stored in the database, but I have no clue how (I can't find where the value is used and written to the db). | 16:15 |
| opendevreview | Julia Kreger proposed openstack/ironic-inspector master: WIP: Use declarative reader/writer separation https://review.opendev.org/c/openstack/ironic-inspector/+/463768 | 16:17 |
| TheJulia | kubajj: do you have an example I can look at? | 16:17 |
| TheJulia | Also, you may want to look at the patch I just uploaded. Making good progress :) | 16:17 |
| kubajj | TheJulia: https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/inspector.py#L305 is the periodic function, check status is slightly below it | 16:18 |
| kubajj | Will have a look | 16:18 |
| TheJulia | kubajj: so... what actually changes states internally in ironic is https://github.com/openstack/ironic/blob/3b1504f4abeac357298ea967d63787f53aa977f6/ironic/drivers/modules/inspector.py#L363 | 16:21 |
| TheJulia | and it uses helpers | 16:21 |
| TheJulia | so tasks can self change, the process_event() calls, and the error handlers use that | 16:22 |
| opendevreview | Merged openstack/ironic bugfix/20.2: [iRMC] Add SNMPv3 authentication functionality https://review.opendev.org/c/openstack/ironic/+/859216 | 16:26 |
| kubajj | TheJulia: but what happens if everything goes right? | 16:27 |
| kubajj | Found it, it is the process_event('done'), right? | 16:28 |
| TheJulia | yup | 16:32 |
| TheJulia | process event then moves the state to the successful completion state in the state machine | 16:32 |
| TheJulia | I suspect the existing state machine is sufficiently modeled, as compared to inspector's own internal state machine | 16:34 |
| kubajj | is this state machine somewhere on conductor? (or who keeps track of it) | 16:34 |
| TheJulia | conductor keeps track of it in ironic | 16:34 |
| TheJulia | ironic/common/states.py defines the state machine and ironic/conductor, specifically the task_manager stuffs handle interaction and movement through the state machine | 16:35 |
| kubajj | Thanks, will have a look | 16:35 |
| iurygregory | TheJulia, ok, we are not running py36 in bugfix/20.2.. yay? | 17:13 |
| TheJulia | targetted to support 3.8.... | 17:14 |
| TheJulia | and 3.7 | 17:14 |
| iurygregory | yeah, going to try to add the job to see the results XD | 17:14 |
| opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic bugfix/20.2: [DNM] Testing something https://review.opendev.org/c/openstack/ironic/+/860587 | 17:20 |
| opendevreview | Julia Kreger proposed openstack/ironic-inspector master: WIP: Use declarative reader/writer separation https://review.opendev.org/c/openstack/ironic-inspector/+/463768 | 18:46 |
| opendevreview | Julia Kreger proposed openstack/ironic-inspector master: WIP: Use declarative reader/writer separation https://review.opendev.org/c/openstack/ironic-inspector/+/463768 | 21:34 |
| TheJulia | yeah, I think I'm going to excise the internal version_id stuffs | 21:35 |
| JayF | TheJulia: have you gotten commitments from anyone else on reviewing the sqla 2.0 stuff? Or should I workflow them as I go through? | 21:56 |
| JayF | and/or do they need to land atomically or semi-atomically | 21:56 |
| TheJulia | no explicit committments, they need to land in stacked order | 21:57 |
| JayF | OK; that's ambiguous for what I wanted to know: | 21:57 |
| JayF | should I review all 3, and workflow all 3 only after I know they're all good to land | 21:58 |
| JayF | or as long as they land 1->2->3 (even with stuff between) it's OK | 21:58 |
| * JayF suspects the latter given tests pass on each patch | 21:58 | |
| TheJulia | oh! so the allocations table one should likely go first | 22:13 |
| TheJulia | then 1->2->3 as it makes sense | 22:13 |
| JayF | allocations table one? | 22:15 |
| JayF | can you link it up, I'm like 7/16 pages on phase 1 | 22:15 |
| JayF | or I'll find it if you don't | 22:16 |
| JayF | it wasn't marked prio iirc | 22:16 |
| JayF | https://review.opendev.org/c/openstack/ironic/+/860198 | 22:16 |
| JayF | nope it was in my tabs list | 22:16 |
| JayF | GOT IT | 22:16 |
| JayF | TheJulia: ... that change is extremely problematic | 22:19 |
| JayF | TheJulia: can we chat about it sync? I think it'll be smoother if we do | 22:20 |
| * JayF is unsure if you're aware of the ... insanity that is mysql variants and what `utf8` means | 22:20 | |
| JayF | this change is basically the opposite of what I expected :| | 22:21 |
| JayF | the more I think about this, the more bad the problem space and all the possible fixes are | 22:22 |
| * JayF looks at https://design215.com/toolbox/utf8-4byte-characters.php to see if the 4th byte is worth fighting for | 22:24 | |
| JayF | I think I'm coming to the conclusion that TheJulia's change is actually the least-terrible choice amung an array of terrible choices | 22:26 |
| JayF | TheJulia: I put a comment on it. I think we cannot get out from under the allocations table problem "quietly", and trying to fix it directly out from under the operator is likely to create more severe complications, unless I misunderstand something | 22:31 |
| JayF | rpittau: https://review.opendev.org/c/openstack/ironic/+/859250 can you look at my comment there when you get a chance? | 22:37 |
| JayF | dtantsur: https://review.opendev.org/c/openstack/ironic/+/836966 you previously had comments on this; I think they are well enough out of date they are no longer relevent; but I'd rather not land this without your explicit thumbs-up that your concerns are resolved | 22:38 |
| JayF | dtantsur: (tl;dr: if you think it's OK now, just land it :D) | 22:39 |
| TheJulia | JayF: I would be happy to chat now if you want | 22:42 |
| TheJulia | we can't, it is mysql's architecture | 22:42 |
| JayF | Did you read the specific scenario I commented in the PR? | 22:42 |
| TheJulia | not yet, dealing with a board thing at the moment | 22:42 |
| JayF | TheJulia: then I know it's close to eod for both of us, why not read and respond async; I'll dedicate some time tomorrow afternoon to look at it together if your response doesn't set me straight :) | 22:43 |
| JayF | TheJulia: it all hovers around the "what-if" of a ironic operator already being on a mysql-like DB that believes utf8 means utf8mb4 | 22:43 |
| TheJulia | but now I want to use hieroglyphs in server names... | 22:44 |
| JayF | TheJulia: like I said, last time I stared down this issue was from using my standard MB4 test string (a poop emoji) in object names at like, 3 jobs ago | 22:44 |
| TheJulia | heh | 22:44 |
| TheJulia | yeah, honestly... allocations being optional to use, I suspect either people are going to want to do a planned db change *or* ignore it if they know they are not ising it | 22:45 |
| JayF | that is basically what I think the ideal solution is | 22:45 |
| TheJulia | but come to think about it, we should likely issue an upgrade warning and link to docs | 22:45 |
| JayF | do not try to paper over this quietly | 22:45 |
| TheJulia | which indicate how to fix it | 22:46 |
| JayF | We cannot fix this quietly because we cannot assume all existing operator DBs are in a similar state | 22:46 |
| TheJulia | oh, so... an operator explicitly can't use mysql in utf8mb4 | 22:46 |
| TheJulia | it won't work, our table creation points to utf8 being utf8mb3 today | 22:46 |
| JayF | I need to find the citations for this, but I believe mysql *variants* (which we may not officially support but are used) did that migration earlier | 22:47 |
| TheJulia | the dump/load has what the prior was so it would re-create as is | 22:47 |
| TheJulia | unfortunately alembic also has no primitives to support alter table charset/engine | 22:47 |
| JayF | AIUI no such primitives exist *at all* in mysql | 22:47 |
| JayF | you have to dump and recreate table to change charset | 22:48 |
| JayF | at least that's what the state was last time I had to deal with a bug like this | 22:48 |
| TheJulia | I know you can change the engine | 22:48 |
| TheJulia | err | 22:48 |
| TheJulia | charset | 22:48 |
| TheJulia | the engine is dump/load | 22:48 |
| TheJulia | and modify the load file | 22:48 |
| JayF | I don't wanna say I don't believe you; but I absolutely have to see what release that support was added in | 22:48 |
| TheJulia | but realistically,, it shouldn't be a big deal. Things should just work and we can check what and warn I guess | 22:48 |
| TheJulia | there are examples on changing charsets in the mysql docs, fwiw | 22:49 |
| JayF | ack; I'll take a look | 22:49 |
| TheJulia | but yeah, I'd rather just highlight, warn, and provide reference into how to fix it | 22:49 |
| TheJulia | upgrade status being ideal for that actually | 22:49 |
| JayF | Honestly, reading that change | 22:50 |
| JayF | all I thought about is the places I've worked where staging and prod are /slightly different/ | 22:50 |
| JayF | and char encoding being just a little off in one or the other, I'd be willing to guarantee has happened somewhere I worked before | 22:50 |
| TheJulia | "warning: your thing was created with [MyIASM|Latin1], you should like fix it if you actually use allocations" | 22:50 |
| JayF | so I have this story playing out of someone being ready to kick off their upgrade and getting screamed at by alembic :| | 22:50 |
| TheJulia | yeah, which is likely why warning makes sense and if we detect where it might be a huge issue, we can also just change the warnign to an error | 22:51 |
| TheJulia | so they don't go to the next version | 22:51 |
| JayF | I think I'd be more keen on that kind of approach | 22:51 |
| TheJulia | fwiw | 22:51 |
| JayF | when I've run Ironic in the past; I absolutely would rather a "stop and check this" in the release notes to trying to do it quietly | 22:51 |
| TheJulia | it is actually just a matter of taking what I posted, and adding a upgrade status check in | 22:51 |
| JayF | You still think we should utf8->utf8mb3 in the migrations ? | 22:52 |
| TheJulia | since there is not an actual alembic upgrade which would be impacted | 22:52 |
| TheJulia | its a label for the creation aiui | 22:52 |
| TheJulia | as a hint for sqlalchemy's dialect, nothing more | 22:52 |
| TheJulia | so that is more for "when someone does a new install" so we don't create a 4 byte utf problem for themselves later | 22:53 |
| JayF | I think the scenario I want to test is a half-upgraded Ironic DB in utf8 | 22:53 |
| JayF | and see if it tries to change charset if I run a utf8mb3-explicit migration on it | 22:54 |
| TheJulia | the mysql/mariadb pages I ran across highlighted utf8 is presently an alias for utf8mb3 | 22:54 |
| TheJulia | fwiw | 22:54 |
| JayF | I owe, for this discussion, finding proof that is not completely true for every mysql variant. | 22:54 |
| JayF | Which may just be wrong and my memory is bad, that's an extremely likely possiblity :) | 22:54 |
| TheJulia | if you want, debian ships the hardcoded config to use utf8mb4 by default | 22:55 |
| TheJulia | but notes they are the only folks that do that | 22:55 |
| JayF | That may be exactly what I was thinking of | 22:55 |
| JayF | or something like dazzle (?) | 22:55 |
| TheJulia | and if you create a table as "utf8" on there, it uses "utf8mb3" | 22:55 |
| JayF | drizzle? | 22:55 |
| TheJulia | dunno about drizzle | 22:56 |
| JayF | it's abandoned per the product page, as of 2012 | 22:56 |
| JayF | so that doesn't matter | 22:56 |
| JayF | ack; I think if we put a warning it's fine, and I'll make time to do some testing to give myself some confidence too | 22:56 |
| JayF | I am extremely careful about DB migrations b/c like I said before; I think in most environments it's the one place in the code that never /really/ gets tested until it's run against someone's prod DB | 22:57 |
| TheJulia | I completely agree, being someone who has written custom DB data migration tooling | 22:58 |
| JayF | +2 on all the SQLA2.0 stuff from me, btw | 22:59 |
| JayF | so if we can find a happy place here, we can bring your 747 full of work in to runway 2023 | 22:59 |
| * JayF -> EOD | 23:02 | |
| TheJulia | lol | 23:31 |
| TheJulia | JayF: I likely will be starting on inspector itself beyond updating it's overall interaction early next week. I'm likely going to kill an internal data model feature in the process which explains some of the headaches encountered in the past (and will make it simpler for the eventual migration into ironic. | 23:33 |
| JayF | I have a shallow understanding of that problem, basically just building up the DB scaffolding that it's needed for a while, but b/c it grew organically never got? That old patch from years ago? | 23:38 |
| opendevreview | Verification of a change to openstack/ironic-python-agent bugfix/8.6 failed: Pin bugfix 8.6 ci jobs to zed https://review.opendev.org/c/openstack/ironic-python-agent/+/859253 | 23:39 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!