15:00:01 <iurygregory> #startmeeting ironic
15:00:02 <openstack> Meeting started Mon May 17 15:00:01 2021 UTC and is due to finish in 60 minutes.  The chair is iurygregory. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:00:04 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:00:06 <openstack> The meeting name has been set to 'ironic'
15:00:09 <iurygregory> o/
15:00:12 <dtantsur> o/
15:00:28 <iurygregory> Hello everyone, welcome to our weekly meeting, you can find our agenda in the wiki
15:00:35 <iurygregory> #link  https://wiki.openstack.org/wiki/Meetings/Ironic#Agenda_for_next_meeting
15:00:49 <iurygregory> #topic Announcements/Reminder
15:01:01 <TheJulia> o/
15:01:10 <bfournie> o/
15:01:17 <rpioso> \o
15:01:22 <iurygregory> Does anyone have anything to announce / reminder us?
15:01:27 <rloo> o/
15:02:08 <dtantsur> I think we're slowly approaching the sprint 1 releases
15:02:25 <dtantsur> in 2 weeks
15:02:25 <TheJulia> we are I believe
15:02:46 <stendulker> o/
15:02:47 <iurygregory> #info sprint 1 release in 2 weeks
15:02:47 <ajya> o/
15:02:50 <TheJulia> I need to try and crank out performance related patches since it will be a major version bump with the removal of iscsi
15:02:53 <erbarr> o/
15:03:39 <iurygregory> yeah that makes sense
15:03:58 <iurygregory> should we move for our next topic?
15:04:09 <TheJulia> ++
15:04:17 <iurygregory> #topic Review action items from previous meeting
15:04:44 <iurygregory> I was looking at the logs, we don't have any action item, moving on
15:04:53 <iurygregory> #topic #Review subteam status reports
15:05:14 <iurygregory> Xena themes is not merged yet https://review.opendev.org/c/openstack/ironic-specs/+/784143
15:05:35 <dtantsur> very close actually, we may even apply lazy consensus
15:05:45 <dtantsur> (I think we wanted it on the last meeting?)
15:05:45 <iurygregory> according to last week meeting, the concensus was that we would merge by EOW
15:05:50 <iurygregory> dtantsur, yeah
15:05:55 <dtantsur> SHIP IT!
15:06:00 <rloo> ++
15:06:11 <iurygregory> Done!
15:06:41 <iurygregory> I will update the ironic board with the information, next week we can have the status, sounds good?
15:06:52 <dtantsur> yep
15:06:56 <rloo> thx iurygregory!
15:07:15 <iurygregory> #topic Deciding on priorities for the coming week
15:07:54 <iurygregory> Anyone has patches that would like to be added to the priorities?
15:08:02 <TheJulia> thanks iurygregory
15:08:02 <dtantsur> I have a bunch of new things related to driver refactoring (kernel_append_params this time): https://review.opendev.org/c/openstack/ironic/+/791755 https://review.opendev.org/c/openstack/ironic/+/791761 https://review.opendev.org/c/openstack/ironic/+/791035
15:08:50 <dtantsur> any objections to adding them? they're not large, just.. annoying?
15:08:56 <iurygregory> dtantsur, ++ to me it makes sense to have them
15:09:09 <TheJulia> no objections from me
15:09:48 <TheJulia> I'd like to get the db indexes added to the priority list for the week https://review.opendev.org/c/openstack/ironic/+/788625
15:10:15 * dtantsur should probably stop generating more patches now that 1/3 of the weekly priority are his :D
15:10:21 <dtantsur> ++ to indexes
15:10:34 <TheJulia> and my patch to cleanup the node santiziation for performance
15:10:46 <TheJulia> https://review.opendev.org/c/openstack/ironic/+/790142 however, it will also need another patch to cleanup objects on selected columns
15:10:49 <iurygregory> TheJulia, I think it's ok, the failures seems to be with tests and cover
15:11:04 <TheJulia> so it is blocked until I get that patch created/uploaded
15:11:35 <TheJulia> iurygregory: yeah, I need to cycle back to them later today, I was MIA all last week
15:11:46 <iurygregory> TheJulia, no worries =)
15:11:54 <bfournie> not sure if its a priority, but would like to get some feedback on new fields in bios api - https://review.opendev.org/c/openstack/ironic/+/786707
15:12:29 <iurygregory> bfournie, can be a priority without problems I would say
15:12:42 <dtantsur> yep, the only problem is its size :)
15:12:44 <bfournie> thanks iurygregory
15:12:49 <iurygregory> dtantsur, yeah =)
15:12:50 <bfournie> yeah, size
15:13:04 <dtantsur> you could avoid a dependency on sushy (which blocks this patch until a release) by splitting it
15:13:14 <dtantsur> into the generic parts and the redfish parts
15:13:20 <iurygregory> humm I think we can release sushy
15:13:29 <iurygregory> we already have the support merged
15:13:36 <dtantsur> ah, have we?
15:13:47 <iurygregory> I can push today a patch in releases
15:13:49 <dtantsur> good, then let's release and replace depends-on with a bump of driver-requirements
15:13:55 <bfournie> yeah the sushy patch merged
15:14:11 <bfournie> k, cool
15:14:12 <bfournie> thanks
15:14:28 <iurygregory> np!
15:15:36 <iurygregory> anyone have more patches?
15:15:42 <iurygregory> or should we move on?
15:16:55 <iurygregory> moving on =)
15:17:04 <iurygregory> #topic Discussion
15:17:21 <iurygregory> we have one topic added by ajya - Failed to inspect node created with inspect-interface as idrac-redfish
15:17:29 <iurygregory> #link https://storyboard.openstack.org/#!/story/2008901
15:18:02 <ajya> mysql has changed how it reports duplicate key error by adding table name to key name. This does not work well with oslo.db. There is bug reported in oslo.db, but not fixed yet
15:18:18 <ajya> I'm thinking it should be fixed there, but if not possible, add workaround in Ironic. Thoughts? Comments?
15:18:44 <dtantsur> we probably rely on this property in a lot of places...
15:18:58 <rpioso> dtantsur: yep
15:19:01 <dtantsur> like, name duplicates, MAC duplicates everywhere.. these all probably raise HTTP 500 now?
15:19:13 <dtantsur> do we have any engagement from oslo.db folks already?
15:19:31 <ajya> not yet, they're having their weekly meeting now
15:19:38 <ajya> I'll raise is there, no response earlier today when I asked
15:19:47 <openstackgerrit> Merged openstack/ironic-specs master: Xena themes  https://review.opendev.org/c/openstack/ironic-specs/+/784143
15:19:50 <iurygregory> doesn't seem according to the
15:19:57 <iurygregory> https://bugs.launchpad.net/oslo.db/+bug/1896916
15:19:57 <openstack> Launchpad bug 1896916 in oslo.db "duplicate key error for mysql >=8.0.19 are not parsed correctly" [Undecided,New]
15:19:57 <rloo> I'd be surprised if this is only an issue with ironic
15:20:06 <TheJulia> Definitely needs to be fixed in oslo.db
15:20:13 <dtantsur> that being said, I'm surprised we're trying to create ports twice in redfish-idrac inspection
15:20:15 <ajya> there is another project that had failing CI and they applied workaround
15:20:26 <dtantsur> sounds like we shouldn't do it on the iDRAC side?
15:20:40 <ajya> there is some logic in iDRAC side why it's done
15:20:49 <iurygregory> I don't see problem in our CI atm, Dell CI is reporting this?
15:21:00 <rpioso> iurygregory: No coverage
15:21:19 <ajya> no problems in CI, I meant this issue - https://bugs.launchpad.net/tacker/+bug/1896867
15:21:19 <openstack> Launchpad bug 1896867 in tacker "functional test failures due to unsuccessfully parsed duplicate key error" [Undecided,Fix released] - Assigned to Koichiro Den (koichiroden)
15:21:30 <dtantsur> ajya: if you insist on creating all ports (which I think may be wrong, but okay), you should overwrite _create_ports instead
15:21:33 <ajya> and yes, there is no coverage for our case
15:22:06 <iurygregory> people can workaround using other release from mysql also, I'm wondering what is the mysql version we have in requirements
15:22:14 <dtantsur> trying to add ports twice is at least sub-optimal
15:22:27 <dtantsur> I think this particular issue can and should be fixed on our side
15:22:41 <rpioso> dtantsur: This bug could affect other uses of inspect_utils.create_ports_if_not_exist
15:22:44 <TheJulia> But it also does follow the "fail and fallback model" as opposed to check first reconcile, then create model
15:23:02 <rpioso> TheJulia: +1
15:23:03 <dtantsur> rpioso: surely. but it doesn't mean that the iDRAC implementation shouldn't be fixed.
15:23:16 <rpioso> dtantsur: Not the topic at hand.
15:23:33 <dtantsur> mmm?
15:23:45 <rpioso> As rloo pointed out, other projects have encountered this, including tacker.
15:23:47 <dtantsur> the bug is triggered by calling create_ports_if_not_exist twice. It should not happen.
15:23:53 <TheJulia> We should have consistency in duplicate key errors because the code path all the way to API consumers doing things is impacted
15:24:01 <dtantsur> I agree that it has to be fixed in oslo.db, but idrac-redfish is also wrong IMO.
15:24:27 <dtantsur> and the fix is very trivial btw, can be landed much faster than oslo.db
15:24:39 <TheJulia> Lets delineate the issues, they are two separate things and we could fall into a disagreement of python programming models easily with the driver and the reliance upon the failure and then fallback.
15:24:50 <TheJulia> in other words, we could bikeshed the driver easily
15:24:52 <rpioso> TheJulia: ty
15:24:55 <dtantsur> we're not touching these topics
15:24:59 <rloo> is this for a new feature in idrac?
15:25:03 <TheJulia> rloo: no
15:25:05 <dtantsur> we're discussing a very trivial issue: calling the same function twice for no reason
15:25:15 <rpioso> rloo: No, it's a regression since Focal was introduced.
15:25:19 <dtantsur> which has been fine all the time, but started triggering the mysql regression now
15:25:26 <TheJulia> indeed
15:25:28 <rloo> if you want to backport it, i suspect it'll have to work with older oslo.db. unless oslo.db's fix is backported too.
15:25:42 <dtantsur> it = ????
15:25:53 <rloo> oh, it is already failing.
15:26:20 <rpioso> dtantsur: All calls to inspect_utils.create_port_if_not_exist could be affected.
15:26:23 <rloo> if the code fix is easy as dtantsur sez, would be worth fixing in both places.
15:26:38 <dtantsur> rpioso: I don't argue with that. Nor does it cancel anything I've said.
15:26:40 <rloo> or wait for oslo.db to fix.
15:26:42 <dtantsur> unless I'm missing something.
15:27:05 <iurygregory> we should probably talk with oslo folks to see about it
15:27:06 <rloo> dtantsur: that idrac code has merged; guessing you are suggesting an improvement here.
15:27:31 <rpioso> dtantsur: https://github.com/openstack/ironic/blob/af94a3da1e3f66c70309bbb889c68dfc5bd67e9f/ironic/drivers/modules/inspect_utils.py#L49
15:27:51 <rpioso> dtantsur: The exception is not caught.
15:28:07 <rloo> So I think we're done? Someone needs to mention to oslo.db that this affects ironic, it'll be up to them to decide priority etc. and if folks choose to enhance idrac code, that is their choice?
15:28:11 <ajya> Talking with oslo now about this issue
15:28:36 <rpioso> rloo: Sounds workable to me :-)
15:28:39 <dtantsur> I don't think that fixing bugs is a choice
15:28:48 * dtantsur just makes a patch
15:29:00 <rpioso> dtantsur: We'll handle it :-)
15:29:13 <dtantsur> Great, thanks!
15:29:21 <rpioso> dtantsur: yw
15:29:24 <dtantsur> It boils down to implementing _create_ports instead of inspect_hardware
15:30:49 <iurygregory> should we move to our next topic?
15:31:00 <dtantsur> likely yes
15:31:12 <iurygregory> #topic Baremetal SIG
15:31:15 <ajya> ok, np, so for oslo we will propose a patch as noone else plans to do it, but they seem ok fixing it
15:31:35 <iurygregory> arne_wiebalck, do you have anything for the Baremetal SIG?
15:31:36 <arne_wiebalck> Record attendance at last week's meeting due to rpioso making publicity, thanks again!
15:31:48 <iurygregory> nice!
15:32:00 <iurygregory> tks rpioso =)
15:32:06 <arne_wiebalck> Video is already up, thanks stevebaker!
15:32:21 <arne_wiebalck> That's it, I think.
15:32:24 <iurygregory> \o/
15:32:26 <iurygregory> cool!
15:32:34 <iurygregory> moving on
15:32:35 <iurygregory> #topic RFE review
15:32:37 <rpioso> arne_wiebalck: Thank you so much for sharing CERN's experience with ironic. Very informative and insightful!
15:32:47 <arne_wiebalck> rpioso: ty :)
15:33:10 <iurygregory> MahnoorAsghar, you added https://storyboard.openstack.org/#!/story/2008866
15:33:45 <MahnoorAsghar> iurygregory: Yes; looking for an approval for the RFE
15:33:49 <MahnoorAsghar> Updated it now
15:34:29 <dtantsur> I don't my concerns about it have been addressed
15:34:46 <dtantsur> I've put a comment re what we previously discussed
15:34:46 <MahnoorAsghar> Last we discussed this, a vendor passthru implementation was considered favorable
15:34:52 <iurygregory> yes
15:35:01 <dtantsur> yep, just keep in mind that I'll object to including it in metal3
15:35:07 <TheJulia> arne_wiebalck: out of curiosity, was the video linked to from the ironicbaremetal.org blog?
15:35:11 <dtantsur> so if you're doing it because of metal3, you may end up in a weird position
15:35:51 <arne_wiebalck> TheJulia: the blog post with the same topic was
15:35:52 <MahnoorAsghar> dtantsur: Ive changed it to vendor passthru now...let me see your comment
15:36:36 <rpioso> Does Redfish RAID work with vendors other than Dell Technologies?
15:36:37 <arne_wiebalck> TheJulia: maybe could amend that link to mention the video as well?
15:36:49 <dtantsur> rpioso: I'm not sure if anyone has tried
15:37:27 <rpioso> Does the ironic RAID schema support controller and disk IDs?
15:37:43 <TheJulia> arne_wiebalck: should enitrely be possible
15:37:46 <MahnoorAsghar> rpioso: It does, as per the documentation
15:37:55 <TheJulia> arne_wiebalck: it is just markdown in a git repo :)
15:38:52 <rpioso> Couldn't a default implementation of the originally proposed API addition return empty or a status that indicates it's not supported?
15:40:10 <dtantsur> shouldn't maybe the driver implementation support simple indexes instead of moving to metal3?
15:40:53 <TheJulia> rpioso: each raid interface has slightly different requirements and I don't believe the ilo interface has as strict requirements for names/data like the idrac driver does, but it has been a couple years since I last looked at that code/docs so it is a bit difficult to speak to at the moment.
15:41:29 <dtantsur> if e.g. the iDRAC impelementation allows numbers in addition to string names, it will solve the metal3 problem without most of complications
15:41:43 <rpioso> dtantsur: The driver IDs indicate where the devices are located in the system. It depends on the system design.
15:42:12 <dtantsur> right. what I'm suggesting is to move the numbering logic from https://github.com/metal3-io/metal3-docs/pull/148 to the ironic driver
15:42:14 <rpioso> dtantsur: Redfish schema include ID props.
15:42:30 <dtantsur> and avoid a new API in ironic completely
15:42:55 <dtantsur> then the feature proposed for metal3 will also work for other ironic consumers
15:42:56 <rpioso> dtantsur: I don't see how the IDs could be made generic.
15:43:08 <dtantsur> rpioso: exactly the way you're proposing it?
15:43:09 <rpioso> dtantsur: Even Redfish did not attempt to normalize them.
15:43:19 <dtantsur> I'm not saying cross-vendor
15:43:40 <dtantsur> I'm saying: rather than inventing a new API to match numbers to names, accept numbers in your RAID implementation
15:43:55 <dtantsur> in the physical_drives field (or how is it called?)
15:44:08 <dtantsur> so pretty much what is proposed for metal3, but do it on ironic level, not in BMO
15:45:31 <rpioso> dtantsur: What would the numbers mean?
15:45:45 <dtantsur> rpioso: please check the metal3 proposal
15:46:09 <dtantsur> they're proposing using numbers instead of names and using the new API to replace numbers with names
15:46:10 <MahnoorAsghar> My laptop suddenly died without warning, sorry
15:46:21 <rpioso> dtantsur:  We could not assure that as new hardware is introduced.
15:46:45 <dtantsur> okay, then the metal3 proposal should probably drop the conversion bit
15:47:15 <dtantsur> that being said, I'm okay with https://storyboard.openstack.org/#!/story/2008866
15:47:27 <dtantsur> just trying to figure out if it's going to be useful for metal3 (seems like no)
15:47:36 <rpioso> dtantsur: The metal3 proposal to generically number controllers and disks seems misguided to me.
15:47:51 <dtantsur> rpioso: could you leave a comment there please? you have more information than me.
15:47:56 <rpioso> dtantsur: 'naive' might be a better choice.
15:47:57 <MahnoorAsghar> I think its an Airship use case
15:48:44 <rpioso> dtantsur: We'll see what we can do.
15:49:55 <rpioso> Regardless, seems like ironic should provide an API which offers controller and disk IDs since the RAID schema supports that.
15:50:14 <dtantsur> also for generic Redfish?
15:50:27 <rpioso> dtantsur: yep
15:50:49 <dtantsur> this is nice. yeah, I agree.
15:50:53 <rpioso> dtantsur: At least for Dell.
15:51:07 <MahnoorAsghar> rpioso: +1
15:51:11 <rpioso> Other vendors are welcome to join to the discussion :)
15:51:54 <MahnoorAsghar> I was looking into the HP docs; I think stendulkar could shed some light on it?
15:51:55 <Qianbiao> rpioso may u give a link to the spec you are disgussing about?
15:52:19 <rpioso> MahnoorAsghar: Do you have the schema handy?
15:52:19 <TheJulia> MahnoorAsghar: he likely could, but I suspect he is asleep at this time. :)
15:52:32 <MahnoorAsghar> TheJulia: I see
15:52:34 <Qianbiao> I know you are talking about raid config of metal3 but not clear about which part.
15:52:43 <TheJulia> MahnoorAsghar: he is based in India
15:53:00 <MahnoorAsghar> TheJulia: My neighbour
15:53:06 <iurygregory> Qianbiao, https://github.com/metal3-io/metal3-docs/pull/148/files
15:53:16 <MahnoorAsghar> rpioso: Let me try and find it
15:53:53 <MahnoorAsghar> rpioso: You were talking about the Redfish spec?
15:54:26 <rpioso> MahnoorAsghar: schemas
15:54:37 <MahnoorAsghar> rpioso: okay
15:54:52 <iurygregory> https://redfish.dmtf.org/redfish/schema_index
15:55:08 <iurygregory> ^ you can probably find in this page
15:55:14 <Qianbiao> rpioso dtantsur may you have a look at https://docs.openstack.org/ironic/latest/admin/drivers/ibmc.html#backing-physical-disks
15:56:13 <Qianbiao> is this implementation matches rpioso proposed?
15:56:44 <rpioso> The ID property in https://redfish.dmtf.org/schemas/v1/Storage.v1_10_0.json
15:56:48 <Qianbiao> i think we should support drive-id too
15:57:18 <MahnoorAsghar> Qianbiao: The implementation matches the proposal
15:57:29 <rpioso> s/ID/Id/
15:57:46 <iurygregory> we are almost out of time =) 2min
15:58:01 <rpioso> Same idea in https://redfish.dmtf.org/schemas/v1/Drive.v1_12_0.json
15:58:05 <Qianbiao> Hmm, i think we should give "id" to vendor.
15:58:41 <Qianbiao> iurygregory no worries, we may end the meeting first :)
15:58:43 <rpioso> Qianbiao: Agree. Those would be returned by the new APIs which the spec originally proposed.
15:58:52 <iurygregory> yeah =)
15:58:58 <iurygregory> #topic Who is going to run the next meeting?
15:59:05 <iurygregory> Do we have any volunteers?
15:59:05 <TheJulia> I can
15:59:11 <iurygregory> tks TheJulia
15:59:19 <iurygregory> #endmeeting