| opendevreview | minwoo seo proposed openstack/ironic master: Add `api-call` action for ironic inspection rule https://review.opendev.org/c/openstack/ironic/+/946741 | 00:50 |
|---|---|---|
| dtantsur | TheJulia: the patch got in merge conflict by my morning | 06:35 |
| opendevreview | Queensly Kyerewaa Acheampongmaa proposed openstack/sushy-tools master: Add PATCH support for Redfish DateTime fields in Manager resource https://review.opendev.org/c/openstack/sushy-tools/+/950925 | 10:13 |
| dtantsur | Could I get some reviews on the bifrost CI fix please? https://review.opendev.org/c/openstack/bifrost/+/951849 | 10:41 |
| * TheJulia cries a single tear | 12:55 | |
| TheJulia | I guess I'm rebasing once I'm awake | 13:02 |
| * TheJulia caffinates | 13:08 | |
| TheJulia | dtantsur: sure, please revisit your -1 on https://review.opendev.org/c/openstack/ironic/+/950363 too ;) | 13:10 |
| iurygregory | coffee! | 13:10 |
| TheJulia | yay keystone is simpler... kind of.... now | 13:10 |
| TheJulia | dtantsur: done | 13:11 |
| TheJulia | coffee coffee coffee | 13:11 |
| opendevreview | Julia Kreger proposed openstack/ironic master: Patch configdrive metadata https://review.opendev.org/c/openstack/ironic/+/946677 | 13:32 |
| opendevreview | Julia Kreger proposed openstack/ironic master: Consider missing MTU invalid metadata https://review.opendev.org/c/openstack/ironic/+/949385 | 13:32 |
| opendevreview | Julia Kreger proposed openstack/ironic master: Inject network config when configdrive is empty https://review.opendev.org/c/openstack/ironic/+/951572 | 13:32 |
| TheJulia | is there a song about rebasing? | 13:32 |
| iurygregory | not that I'm aware, but there is a nice hello world song https://www.youtube.com/watch?v=yup8gIXxWDU | 13:39 |
| opendevreview | Queensly Kyerewaa Acheampongmaa proposed openstack/sushy-tools master: Add PATCH support for Redfish DateTime fields in Manager resource https://review.opendev.org/c/openstack/sushy-tools/+/950925 | 14:14 |
| opendevreview | Abhishek Bongale proposed openstack/ironic-tempest-plugin master: Add Tempest tests for inspection rules in Ironic https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/951761 | 14:30 |
| cardoe | Just need 5 more patches to be +W to achieve my goal of only 25 items in ironic-week-prio | 14:33 |
| cardoe | speaking of rebases.... https://review.opendev.org/c/openstack/ironic/+/898010 | 14:34 |
| opendevreview | Merged openstack/bifrost master: Fix Keystone after their migration from WSGI scripts https://review.opendev.org/c/openstack/bifrost/+/951849 | 14:35 |
| TheJulia | ugh, yeah | 14:39 |
| TheJulia | that one | 14:39 |
| opendevreview | Dmitry Tantsur proposed openstack/bifrost stable/2025.1: Fix Keystone after their migration from WSGI scripts https://review.opendev.org/c/openstack/bifrost/+/951963 | 14:52 |
| dtantsur | we don't *need* to backport ^^^ but I guess it's a good hardening | 14:53 |
| dtantsur | meanwhile, -1 revisited and cleared :) | 14:53 |
| TheJulia | dtantsur: thanks | 14:54 |
| TheJulia | cardoe: so, looking more about what your describing name issue wise, it appears to be around vlans specifically. Is that your understanding as well? | 14:55 |
| TheJulia | cardoe: I do love how they sort of imply they took the bad implication as the understanding in the cloud-init conversion script | 14:55 |
| cardoe | yeah that's the issue for me with vlans | 14:57 |
| TheJulia | okay | 14:59 |
| TheJulia | I think the issue, based upon different points of view, the mistake was putting an example of "eth0" in the metadata example as the example | 14:59 |
| TheJulia | so then when cloud-init folks parsed that they went "oh, its a name!" | 15:00 |
| TheJulia | and sort of locked on to it | 15:00 |
| TheJulia | :\ | 15:00 |
| cardoe | So basically the default name for a bunch of physical NICs is like 12 or 14 characters. The kernel or udev renames them to shorter aliases. Unless you've got vlans going... then they append the vlan ID to the long name and it's too long and boom. | 15:00 |
| TheJulia | cardoe: so I don't think we really, at least afaik, support vlan metadata generation, but it might be easiest to post-process the links down to uniqueness and just go "link1", "link2", etc | 15:00 |
| cardoe | Yes. So my ask to the nova folks is to take that example out and be more clear that its anything or maybe suggest that UUID is best. | 15:00 |
| cardoe | But I've been told that the value can be used in os-vif as an interface name | 15:01 |
| TheJulia | If you could create a verbose bug so we could track it, I think that would help because it was never really entirely clear it was just vlans until I went and tried to write a bug myself | 15:01 |
| cardoe | Which project? | 15:02 |
| cardoe | I need a good way to tie bugs together too. | 15:02 |
| TheJulia | I guess lets create one in ironic since it seems we might need to sanity check vlan use overall in our metadata stuffs | 15:02 |
| TheJulia | and it could cross reference "oh, this is related to x,y,z in these other projects) | 15:03 |
| cardoe | launchpad doesn't have a "relates to" option | 15:03 |
| TheJulia | I know :( | 15:03 |
| TheJulia | .... I know plenty of folks who would start to build guiletenes if we were to suggest moving openstack to jira ;) | 15:03 |
| TheJulia | unfortunately | 15:03 |
| JayF | you're right; I'm only going to consent to a move to RT | 15:04 |
| * TheJulia facepalms | 15:04 | |
| * TheJulia goes and walks the corgi overlord | 15:05 | |
| * TheJulia dares not make RT or Jira suggestions to the corgi... the response will be much like "I can see the neighbor's red golf cart" | 15:05 | |
| opendevreview | Merged openstack/ironic master: doc: Make port binding failure configurably fatal https://review.opendev.org/c/openstack/ironic/+/950363 | 15:09 |
| dtantsur | TheJulia: a whole bunch of minor comments on https://review.opendev.org/c/openstack/ironic/+/946677. I'm fine with W+1, just want to make sure you've seen them before I do so. | 15:13 |
| cardoe | I get enough pain and papercuts from jira. | 15:14 |
| dtantsur | don't we all... | 15:14 |
| JayF | I don't do that tight of tracking downstream for my team right now. IDK how much longer I can avoid it but I'm trying :D | 15:19 |
| cardoe | I try not to. But leadership loves them some jira | 15:21 |
| TheJulia | dtantsur: I'd really just like to make some progress to eventually get it as-is off my board, its easier to do follow-up issue items than continuing to carry a story sprint to sprint | 15:25 |
| * JayF just died a little inside | 15:28 | |
| JayF | I understand completely though :) | 15:28 |
| TheJulia | looks like some of your comments make sense, some others I think are a little misplaced based upon the data/contents/structure because we're not dealing with lists, but dicts, and if we just need the key value, then it makes sense to grab keys to keep it clean | 15:30 |
| TheJulia | I'm going to make a follow-up item for my next sprint to address some of the follow-up items and sanity check vlan stuffs in it | 15:31 |
| dtantsur | TheJulia: are you referring to the fact that list(dict) is the same thing as list(dict.keys())? | 15:32 |
| dtantsur | because it is | 15:32 |
| TheJulia | I think it gives you each object with the attached values, but now I feel the need to check | 15:33 |
| TheJulia | oh, it seems to | 15:33 |
| TheJulia | okay I stand corrected! | 15:33 |
| dtantsur | Yep. Iterating dicts yields keys. I see your logic, but that's the way Python is. | 15:33 |
| dtantsur | But since it's not obvious, maybe leave keys() in for clarity.. | 15:34 |
| TheJulia | There was a time, and maybe I'm off my rocker (and it could have been py2...) but I felt like at one point you could yield keys and values at the same time | 15:34 |
| dtantsur | TheJulia: it was always itervalues (py2) / values (py3) | 15:34 |
| dtantsur | or wait, I'm saying nonsense | 15:35 |
| TheJulia | but together, there was a syntax to do it as for k,v in blah | 15:35 |
| dtantsur | iteritems() / items() | 15:35 |
| TheJulia | it works when your handed tuples though which is maddening :( | 15:35 |
| dtantsur | yeah, this one | 15:35 |
| TheJulia | yeah | 15:35 |
| TheJulia | at the end of the day it is all a blur | 15:35 |
| dtantsur | `for k, v := range m` is Go | 15:36 |
| JayF | for k, v in myDict.items() | 15:36 |
| dtantsur | yep | 15:36 |
| JayF | used to be iterItems() in 2.x | 15:36 |
| JayF | or just for tupl in myDict.items() ... | 15:36 |
| TheJulia | It is literally all blending together in my head at this point | 15:37 |
| dtantsur | Just wait a bit, I'll finally start forgetting Python and stop being such a nitpicking asshole in reviews :-P (when it comes to Python syntax, not in other cases) | 15:37 |
| JayF | I appreciate your detailed reviews... when they come in within the first handful of patchsets on a change ;) | 15:37 |
| dtantsur | lol, I hear ya | 15:37 |
| JayF | otherwise you get whammied like cid just did to me, my automated cleaning via runbook change, 100% functional, but not arranged well | 15:38 |
| cardoe | how's https://bugs.launchpad.net/ironic/+bug/2112655 ? | 15:38 |
| JayF | (good feedback and I'm glad I got it; just sucks to go from 'it's done' to lol no in one email) | 15:38 |
| dtantsur | TheJulia: I've +W'ed the patch now that you've seen my comments | 15:39 |
| dtantsur | I think the ones around logging and a few around edge cases are the only really valuable ones | 15:39 |
| TheJulia | dtantsur: much appreciated! I'm creating a new jira item now | 15:40 |
| dtantsur | we need "no jira fridays" | 15:40 |
| dtantsur | like "no meetings fridays" that someone of us already follow (with a varying degree of success) | 15:41 |
| opendevreview | Abhishek Bongale proposed openstack/ironic-tempest-plugin master: Add Tempest tests for inspection rules in Ironic https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/951761 | 15:45 |
| TheJulia | heh | 15:46 |
| TheJulia | cardoe: so I've also referened your bug link, because it looks like we don't support vlan sub interfaces, but sounds like it wouldn't work today at all and we're likely going to need to address that, so I'll try to pick that up next sprint | 15:51 |
| TheJulia | or at least ,try to move the ball forward a little more | 15:51 |
| TheJulia | I was originally thinking of backporting all this downstream, but more and more I'm leaning towards fix it in master, wait/see, and only backport when someone screams | 15:52 |
| * TheJulia closes out downstream items... which upstream already addressed. | 15:58 | |
| cardoe | So what do ya mean vlan sub interfaces? | 16:18 |
| cardoe | https://review.opendev.org/c/openstack/nova-specs/+/471815 | 16:18 |
| cardoe | https://review.opendev.org/c/openstack/nova/+/941227 | 16:19 |
| cardoe | Vasyl also added some tests for ironic | 16:25 |
| TheJulia | well, each vlan interface is technically a sub interface | 16:51 |
| TheJulia | oh, heh, its not even in nova yet, woot! | 17:08 |
| TheJulia | one less thing to worry about at the moment | 17:08 |
| dtantsur | JFYI folks, a public holiday on Monday, so see you on Tuesday | 17:30 |
| JayF | o/ | 17:33 |
| opendevreview | Jay Faulkner proposed openstack/ironic-python-agent master: Remove non-abstract methods from HWM https://review.opendev.org/c/openstack/ironic-python-agent/+/951977 | 17:49 |
| opendevreview | Julia Kreger proposed openstack/ironic master: Advanced vmedia deployment test ops https://review.opendev.org/c/openstack/ironic/+/898010 | 17:53 |
| TheJulia | o/ | 17:53 |
| JayF | \o | 17:55 |
| TheJulia | I know Steve is doing some work in NGS, I think he would appreciate another set of eyes even if it is just glancing. He is trying to start to get security group programming to be a thing in it. | 18:01 |
| JayF | Is it done enough that it's likely to land in a week or two? | 18:03 |
| JayF | part of why I've avoided it is to not load up all that context to disappear before it merges :D | 18:03 |
| TheJulia | unlikely, but some longer term input stuff might be helpful | 18:03 |
| TheJulia | like in particular, he is trying to muse his way through how to approach some of the problems and.. yeah | 18:03 |
| JayF | I may point clif at it next week as an opportunity to onboard to some of the networking code. NGS is actually kinda well-contained so can potentially be helpful with extremely-dusty openstack context. I won't get to it today and honestly my list for the next 2 weeks is probably longer than my ability to deliver anyway :/ | 18:05 |
| opendevreview | Merged openstack/ironic bugfix/28.0: Fix agent get_XXX_steps retries from being treated as not fresh agents https://review.opendev.org/c/openstack/ironic/+/951525 | 18:15 |
| opendevreview | Verification of a change to openstack/ironic master failed: Patch configdrive metadata https://review.opendev.org/c/openstack/ironic/+/946677 | 18:20 |
| TheJulia | ack ack | 18:33 |
| TheJulia | ... pondering clean steps stuff. Thinking logging the failed step, and all steps at start | 18:33 |
| TheJulia | does that seem... reasonable? | 18:33 |
| JayF | I would suggest you not look at that until my automated cleaning by runbook is revised -- I suspect I may end up with a good, single place to do it all | 18:37 |
| TheJulia | well, It looks like it would largely be on error handling | 18:38 |
| TheJulia | fwiw, but I see your point | 18:38 |
| JayF | I will say that I had interest in especially a "cleaning was completed on node X with steps Y" | 18:38 |
| JayF | as an audit trail | 18:38 |
| TheJulia | I guess logging the last step and logging the steps out of the gate as an history event make it pretty clean | 18:38 |
| TheJulia | and I *think* both points are clean, but it will be intersting to see what you end up with the runbook stuffs | 18:39 |
| JayF | basically my use case is | 18:39 |
| JayF | there's some security breech, node X is accessed | 18:39 |
| JayF | I want to be able to show them in a simple, concise way: we got the data off using the process you signed off on | 18:39 |
| TheJulia | okay, then I think we need to log step start, step end | 18:41 |
| * TheJulia will need to ponder a little more then | 18:41 | |
| TheJulia | in error handling cases, error on x step is what we need | 18:41 |
| TheJulia | (which is easy enough, we just put that into error handlers) | 18:42 |
| JayF | I was thinking more clean start [step list], clean end [step list] and a set of steps | 18:45 |
| JayF | s/and a set of steps/and not put a history item per-step/ | 18:45 |
| JayF | but I guess we don't persist that | 18:45 |
| JayF | (the full list of steps for a run) | 18:45 |
| JayF | BTW; this entire story is some of why automated cleaning via runbook was desirable; an API contract that says "cleaning will always do X" | 18:46 |
| TheJulia | I'm not sure we inject steps as we *really* get going | 18:46 |
| TheJulia | but, there is a lot there | 18:47 |
| TheJulia | so, I definitely want to log on failure the step we were executing just so it is loud and clear when there was an issue | 18:47 |
| JayF | yeah so I guess log steps at start, log successful completion, and log if anything fails | 18:47 |
| JayF | you can use 2 history entries to get full story in any case | 18:48 |
| TheJulia | otherwise, you must dig up the stone tablets of translation and then consult with the winds of destiny while powering the stargate with the stationary bicycle | 18:48 |
| JayF | I was just going to stuff more crap into one of our freeform json fields /s | 18:48 |
| JayF | oh, just as an aside: do you think there'd be any interest in moving up instance_info/display_name to a top level field; like instance_name? Or alternatively structuring instance_info in another; more queryable way? I was tossing around ideas about this yesterday with adamcarthur5 and I can't get the idea of node.instance_name outta my head | 18:49 |
| TheJulia | I think that kind of makes sense to do | 18:50 |
| TheJulia | or at least enable | 18:50 |
| TheJulia | I can see folks who use it | 18:50 |
| TheJulia | there has always been this weird tension between operators who want to name the hardware, and then independnetly have a separate name on the node, and then confusion when ironic node name doesn't match the thing | 18:50 |
| JayF | I would probably hook up whatever patch nova sends us today to set a display name to automatically set node.instance_name as a backwards compat | 18:50 |
| TheJulia | reasonable, super resasonable | 18:50 |
| JayF | and then update the nova virt driver to send the new correct thing later | 18:50 |
| TheJulia | yah | 18:51 |
| JayF | adamcarthur5: ^ that's the easy way to solve the instance_name querying problem for big red button; let me tackle making instance name queryable, and you can just worry about the DR API features | 18:51 |
| TheJulia | .... There was something else I was thinking of, but I've lost my train of thought completely | 18:52 |
| JayF | I'll point my coding robot at it during lunch and see if it can come up with something plausible enough that I can get a thing shipped quickly | 18:52 |
| TheJulia | I also have my 1-on-1 soon | 18:52 |
| JayF | clean steps history logging was the original topci | 18:52 |
| TheJulia | oh yeah, I'm good there | 18:52 |
| TheJulia | I was thinking about use | 18:52 |
| adamcarthur5 | Ack JayF | 18:53 |
| JayF | exposing instance_name is such a nice UX enhancement for the "lessee manages their own nodes" story \o/ | 18:53 |
| opendevreview | Merged openstack/ironic stable/2024.2: Fix agent get_XXX_steps retries from being treated as not fresh agents https://review.opendev.org/c/openstack/ironic/+/950309 | 19:03 |
| opendevreview | Verification of a change to openstack/ironic stable/2024.1 failed: Fix agent get_XXX_steps retries from being treated as not fresh agents https://review.opendev.org/c/openstack/ironic/+/950310 | 19:03 |
| opendevreview | Merged openstack/ironic bugfix/28.0: Control port updates with update_pxe_enabled flag https://review.opendev.org/c/openstack/ironic/+/951718 | 19:03 |
| opendevreview | Merged openstack/ironic stable/2024.2: Control port updates with update_pxe_enabled flag https://review.opendev.org/c/openstack/ironic/+/951716 | 19:08 |
| cardoe | So so close... we're at 28 | 19:19 |
| opendevreview | Verification of a change to openstack/ironic stable/2024.1 failed: Fix agent get_XXX_steps retries from being treated as not fresh agents https://review.opendev.org/c/openstack/ironic/+/950310 | 19:32 |
| opendevreview | Verification of a change to openstack/ironic master failed: Patch configdrive metadata https://review.opendev.org/c/openstack/ironic/+/946677 | 19:43 |
| cardoe | So I'm looking at the docs around runbooks I again see that system-scope is required for doing stuff with runbooks. | 20:42 |
| cardoe | I'm gonna pitch this out there... can we define another role like "manager"? | 20:42 |
| cardoe | I understand the history around system scope and I understand the history around Ironic. | 20:43 |
| cardoe | system scope just is big and wide and we don't really want to use it. | 20:44 |
| cardoe | Hoping to tailor some of this and I know I've asked a bunch of times around owner having access and there's been some teeth gnashing about it. | 20:45 |
| JayF | cardoe: no it's not | 21:03 |
| JayF | cardoe: it's required to have system-scope to create *public* runbooks. It's basically an either or: runbook has an owner and belongs to that project OR runbook is public and has no owner | 21:04 |
| JayF | but essentially we did not want to permit cross-project runbook shenanigans which would post an obvious issue in multitenant cases | 21:04 |
| cardoe | oh | 21:05 |
| JayF | this is to prevent people from doing things like, project x using a runbook created by project y, then project y updates the runbook to say "and give me all your data" | 21:05 |
| JayF | so created in project, for project, or created at a system level, for the whole system | 21:06 |
| JayF | and this is per-runbook, kinda inspired a little by image visibility | 21:06 |
| cardoe | So here's what I'm mentally wanting... I want a runbook called "latest_dell_r7615_bios" let's say. It'll be owned by the same project that owns all my ironic baremetal nodes. We can use that during cleaning or whatever. BUT then I want the people that have leased the nodes from me to be able to run that same runbook as a service. | 21:07 |
| JayF | I think in that case, it'll have to be a system-scope runbook | 21:07 |
| JayF | mainly because we didn't want to go down the route of a full-on sharing model | 21:07 |
| JayF | I wouldn't be opposed to one existing; but it'd have to be well designed and it'd be a lot of work | 21:07 |
| cardoe | Since there's no way to do an api-call in the runbook to lookup what the actual latest bios is. I would need members of my ironic baremetal node project (the baremetal admins if you will) which are the owners of my baremetal nodes to be able to update that runbook. | 21:07 |
| cardoe | I mean I'm fine saying gitops has to update it and gitops uses a better scoped user. | 21:08 |
| cardoe | But I do see 3 classes of users | 21:08 |
| JayF | I'm going to disconnect now, I have something personal to take care of. I'm happy to continue this conversation later I just don't have the capacity for it now. | 21:09 |
| cardoe | absolutely | 21:09 |
| JayF | We have a sick animal in the hospital in very bad shape | 21:09 |
| cardoe | oh no. I'm sorry to hear. Go be there | 21:09 |
| JayF | we just were, and I tried to pick up work and failed, so now I'm just going o/ thanks for the kind words | 21:10 |
| opendevreview | Merged openstack/ironic stable/2024.1: Fix agent get_XXX_steps retries from being treated as not fresh agents https://review.opendev.org/c/openstack/ironic/+/950310 | 22:12 |
| cardoe | So https://review.opendev.org/c/openstack/ironic/+/951719?usp=search is no longer a clean backport. Are we okay with closing that and not backporting it there? | 23:09 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!