rpittau | good morning ironic! o/ | 07:47 |
---|---|---|
*** jroll01 is now known as jroll0 | 08:45 | |
*** tosky_ is now known as tosky | 09:33 | |
rpittau | TheJulia, JayF, dtantsur, iurygregory: I'm out on Monday, if someone could take care of the meeting would be great, thanks :) | 09:42 |
*** diablo_rojo_phone is now known as Guest5496 | 09:51 | |
opendevreview | Merged openstack/ironic bugfix/27.0: Calculate missing checksum for file:// based images https://review.opendev.org/c/openstack/ironic/+/938674 | 09:55 |
opendevreview | Merged openstack/ironic bugfix/25.0: [bugfix only] Pin upper-constraints https://review.opendev.org/c/openstack/ironic/+/938661 | 10:16 |
opendevreview | Verification of a change to openstack/ironic bugfix/26.0 failed: Calculate missing checksum for file:// based images https://review.opendev.org/c/openstack/ironic/+/938628 | 10:20 |
*** Guest5496 is now known as diablo_rojo_phone | 10:40 | |
opendevreview | Merged openstack/ironic bugfix/24.0: Calculate missing checksum for file:// based images https://review.opendev.org/c/openstack/ironic/+/938627 | 10:46 |
iurygregory | rpittau, ack | 11:01 |
opendevreview | Merged openstack/ironic bugfix/26.0: Calculate missing checksum for file:// based images https://review.opendev.org/c/openstack/ironic/+/938628 | 11:37 |
opendevreview | Merged openstack/ironic bugfix/25.0: Calculate missing checksum for file:// based images https://review.opendev.org/c/openstack/ironic/+/938629 | 11:42 |
TheJulia | good morning | 14:24 |
rpittau | hey TheJulia :) | 14:26 |
opendevreview | Julia Kreger proposed openstack/ironic master: WIP OCI container adjacent artifact support https://review.opendev.org/c/openstack/ironic/+/937896 | 14:29 |
opendevreview | Julia Kreger proposed openstack/ironic master: WIP: Automatic zstd detection and decompression... https://review.opendev.org/c/openstack/ironic/+/938904 | 14:29 |
TheJulia | doh, meant to upload that last night | 14:31 |
rpittau | TheJulia: if you have a moment please check my comment here https://review.opendev.org/c/openstack/metalsmith/+/933154 | 14:32 |
rpittau | I'd like to merge that before releasing metalsmith | 14:32 |
TheJulia | rpittau: why not shutter it? | 14:47 |
TheJulia | I'm not a fan of expending resources on it at this point | 14:48 |
TheJulia | I have no capacity for it, and no business mandate for it at this point | 14:48 |
rpittau | sounds good to me | 14:51 |
JayF | We should send out a call for maintainers to the mailing list or something before we completely shutter it | 15:01 |
rpittau | Bye everyone see you on Tuesday have a great weekend o/ | 15:37 |
* TheJulia watches bytes transfer which started on a container registry | 17:38 | |
* TheJulia parties.... Fedora CoreOS just booted | 17:42 | |
TheJulia | pulled from podman's machine-os image | 17:42 |
TheJulia | #success Got ironic to deploy from an OCI container registry! | 17:43 |
opendevstatus | TheJulia: Added success to Success page (https://wiki.openstack.org/wiki/Successes) | 17:43 |
JayF | nice job | 18:14 |
TheJulia | Lots of rough edges since we're so focused on glance and the OCI model is just so drastically different | 18:20 |
TheJulia | and... we're sort of leaving the door open to lazy definition and precise definition | 18:20 |
TheJulia | and as such, that adds a little more complexity, blend in the fact files may need additional work... like uncompressing from Zstandard format... its gets a bit more "fun" | 18:21 |
JayF | I mean, getting it to work at all is a meaningful milestone anyway :) | 18:24 |
TheJulia | oh absolutely | 18:24 |
TheJulia | ... and honestly, if I were to upload a raw disk image which was not Zstandard compressed to quay *today*, IPA could totally directly download it with the code today as-is | 18:25 |
TheJulia | Speaking of, I suspect we ought to do some stream content checking in IPA's http download class at some point | 18:25 |
TheJulia | next step is likely getting an local container registry online and doing authy things with it. | 18:27 |
JayF | btw, you all still in the clear from all the fire down there? | 18:28 |
TheJulia | yeah | 18:29 |
opendevreview | Jay Faulkner proposed openstack/ironic master: SCIENCE: Test against oslo.utils image format inspector changes https://review.opendev.org/c/openstack/ironic/+/938956 | 19:13 |
JayF | https://review.opendev.org/c/openstack/ironic/+/928920 is a good change and has been waiting on another core review for a good long time | 19:17 |
JayF | we are a bit behind on review in general as a project tbh | 19:17 |
JayF | lots of big changes and post-holiday though so makes sense | 19:17 |
TheJulia | Also, very heads down to try and get items in a good shape in time to land some things | 19:57 |
JayF | yeah, but also like re: the API schema stuff, folks like adamcarthur5 are blocked a bit on review and getting things landed, we're usually good about finding a balance so I'm sure it's a temporary thing | 20:02 |
TheJulia | JayF: got a good starting point to unblock adamcarthur5 ? | 20:10 |
JayF | the one linked at 11:17, the stephenfin change, he's taking over that project on the ironic side | 20:12 |
JayF | schema refactors + tempest tests for microversion handling | 20:12 |
JayF | those and the i-t-p changes with em | 20:12 |
adamcarthur5 | I can re-send the links, just let me know TheJulia | 20:13 |
JayF | that Ironic patch 928920 is the first of the ironic chain | 20:13 |
JayF | https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/937206 is the one in the itp chain | 20:13 |
JayF | https://review.opendev.org/c/openstack/ironic/+/928920 | 20:13 |
JayF | there's the ironic link again | 20:13 |
JayF | our hope is if we can get the pattern established, future changes can go the pattern of: 1) add tempest test to validate behavior 2) do schema refactor, with those tests as a safety net 3) goto 1 with the next endpoint | 20:14 |
JayF | and once it's all done, we're in line with the way schemas are handled elsewhere and setup for openapi generation stuff | 20:15 |
TheJulia | wait? we're forcing tempest plugin changes? | 20:18 |
TheJulia | ... thats a bad sign | 20:18 |
JayF | no no, think of it the opposite direction | 20:19 |
JayF | we're ensuring tempest test coverage exists | 20:19 |
JayF | before we do any refactoring | 20:19 |
JayF | we're not changing existing tests; we're adding coverage for microversion handling where it is missing (which is almost everywhere) | 20:20 |
TheJulia | Ahh, okay | 20:20 |
JayF | as long as the test passes with both old and new code, the code is good (assuming the test is good) | 20:20 |
opendevreview | Julia Kreger proposed openstack/ironic master: WIP OCI container adjacent artifact support https://review.opendev.org/c/openstack/ironic/+/937896 | 20:20 |
opendevreview | Julia Kreger proposed openstack/ironic master: Automatic zstd detection and decompression... https://review.opendev.org/c/openstack/ironic/+/938904 | 20:20 |
TheJulia | FWIW, I could use a sanity check pass on the wip ^ change. Steve gave me a quick pass last week | 20:21 |
JayF | ack; I'll do that now | 20:21 |
TheJulia | cool, thanks | 20:21 |
TheJulia | at least, before I go writing more unit tests :) | 20:22 |
TheJulia | do keep in mind, there is quite a bit there to also try and keep everyone happy | 20:24 |
TheJulia | and walk that weird middle ground | 20:24 |
TheJulia | ... like... conductor side configuration authentication should be *easy* to wire in after the fact | 20:24 |
TheJulia | adamcarthur5: o/ question for you! | 20:27 |
TheJulia | adamcarthur5: Regarding https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/937213/7/ironic_tempest_plugin/tests/api/admin/test_microversion_enforcement.py#30 | 20:27 |
TheJulia | Does that mean the tests are not going to work on versions which don't support that microversion, version | 20:27 |
adamcarthur5 | Yes - but to be honest, me and Jay had a really hard time (mostly me) trying to predict these edge cases in advance, but I will probably keep ownership of doing this code for a while, so the hope is that get this solution passed for now, knowing it probably has to change later? | 20:28 |
TheJulia | Yeah, I just confirmed that as well | 20:29 |
JayF | Is there a guard that needs to be in place for that which we're missing? | 20:29 |
adamcarthur5 | I think in general most features support the latest microversion? | 20:29 |
TheJulia | ... the important test point is *really* master branch anyhow | 20:29 |
TheJulia | so, its likely okay that it doesn't run them on older versions | 20:29 |
adamcarthur5 | Ah - you mean branch issues. | 20:29 |
JayF | yeah | 20:29 |
TheJulia | yup, exactly | 20:29 |
adamcarthur5 | I tried to do something dynamic, but didn't get anywhere with it. | 20:29 |
JayF | ITP is branchless? | 20:29 |
JayF | I guess that makes sense | 20:30 |
JayF | we run every job in the universe on it | 20:30 |
TheJulia | yes, and expects to be able to be run against n-2 versions if not oler... | 20:30 |
TheJulia | s/oler/older/ | 20:30 |
JayF | they can't get thru the gate if they don't pass back to I think 2023.1 | 20:30 |
TheJulia | JayF: but the new tests don't run where the microversion defined on line 30 is not available | 20:30 |
adamcarthur5 | Hmmm. I could just specify a local microversion per-test. It's not ideal, since we now state the microversion in two places, but if we need to I can do it | 20:30 |
TheJulia | the issue is min_microversion triggers skipping logic | 20:31 |
TheJulia | so if you go look at 2024.1, no microversion testsing | 20:31 |
TheJulia | am I making sense? | 20:31 |
JayF | you're saying if, for a given tempest run | 20:31 |
JayF | the latest microversion is < min_microversion | 20:31 |
JayF | we won't run the test | 20:32 |
JayF | that sounds like a good thing? | 20:32 |
TheJulia | exactly | 20:32 |
TheJulia | Well, it won't even setup the test classes | 20:32 |
JayF | oooh, so we can't use that to do negative testing | 20:32 |
TheJulia | so if your deleting tests which were around for xyz versions, shifting them to a new micoversoin test class | 20:32 |
TheJulia | then that class no longer runs... | 20:32 |
adamcarthur5 | Hmmm. But right now, that microversion is global for all tests, regardless of the microversion of everything individual API part | 20:32 |
TheJulia | eh, not actually | 20:33 |
JayF | min_microversion is set at the top of the test class | 20:33 |
JayF | I think you'd just need a different class for positive vs negative testing | 20:33 |
JayF | and for the negative testing, min_microversion is effectively null since you wanna be able to check that those methods aren't available on the older version | 20:33 |
JayF | e.g. to ensure we don't regress and expose /v1/shards on an Ironic microversion before it should exist | 20:33 |
* JayF not 100% sure he's following right | 20:34 | |
adamcarthur5 | Okay, so basically we think when I run into that case (my current work doesn't?) we'll need a second class? | 20:34 |
adamcarthur5 | I'm with Jay, not 100% following | 20:34 |
TheJulia | JayF: your focusing on negative testing, I'm speaking in general of older versions | 20:34 |
TheJulia | would the spoken word be faster? | 20:34 |
JayF | I can jump into a zoom if adam can | 20:35 |
adamcarthur5 | Sorry, it's 8:30pm here now and I'm away from a laptop | 20:35 |
JayF | well, I'll jump in with ya TheJulia and maybe I can summarize | 20:35 |
JayF | or convey it on Monday or something | 20:35 |
TheJulia | that works | 20:35 |
adamcarthur5 | Yeah - I think that works me and Jay are pretty in sync now. Thanks folks | 20:36 |
JayF | moving to my desktop; will post a zoom link or click a meet.google link when I get there :D | 20:36 |
TheJulia | https://meet.google.com/rwy-jzcw-bwf | 20:36 |
TheJulia | dtantsur: thanks for the replies on the container registry spec re auth, I think it might help to get on a call and discuss and even team up a little if you have a couple spare cycles | 21:04 |
TheJulia | I'm not trying to bake anything beyond the simplest case in the patch but leaving the door open to do more. Maybe get people to fight over config file formats ;) | 21:09 |
TheJulia | But seriously, a higher bandwidth discussion would be good | 21:09 |
JayF | combined with a demo that'd be neat | 21:09 |
JayF | and I'd wanna attend | 21:09 |
JayF | We should do more live demos with these chats when things are working, makes the chats more exciting :D | 21:10 |
TheJulia | I could totally do a quick demo... well, it won't be *that* quick | 21:10 |
TheJulia | .... and I'll need to forward ssh to that VM, but its all good :) | 21:10 |
TheJulia | I *do* like just being able to set image_source to oci:///quay.io/podman/machine-os:5.3 and things just work | 21:11 |
JayF | I am excited to be compatible with that ecosystem; less excited about that ecosystem in general | 21:12 |
TheJulia | There is a nice side-effect | 21:13 |
JayF | I think imagetype:latest (or any :tag that can remain the same while the file behind changes) is a scary antipattern | 21:13 |
JayF | but I can't move the entire industry just because I dislike something so 🤷♂️ | 21:13 |
TheJulia | that container registry can have arm and x86 images (actually... does today), and i just set the same registry as image_source regardless of cpu type | 21:13 |
JayF | yeah, that's in the same category of the gross stuff I don't like :) | 21:15 |
JayF | even if I can see why that's an ergonomic interface | 21:15 |
TheJulia | structual data model is... painful | 21:18 |
JayF | Comments on those OCI changes. I didn't review unit tests | 21:26 |
TheJulia | a... lot of work is still needed on unit tests, but it is getting there | 21:33 |
JayF | honestly when something is WIP, I rarely review unit tests, I assume it's still a development tool | 21:34 |
JayF | hard to test behavior that's still a bit in flux | 21:34 |
TheJulia | Yeah, good comments, and a good suggestion regarding "unknown" | 21:41 |
JayF | like, at least how the IPA code is written | 21:41 |
JayF | image_disk_format of None being passed sets off klaxons | 21:42 |
JayF | but Ironic has stronger guards -- the convert to raw always runs the security check -- so I shouldn't need to be as paranoid | 21:42 |
opendevreview | Merged openstack/ironic-tempest-plugin master: Microversion Test Generator https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/937206 | 21:52 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!