Wednesday, 2022-04-20

*** dviroel|out is now known as dviroel00:20
*** dviroel is now known as dviroel|out00:32
*** dviroel|out is now known as dviroel11:23
whoami-rajat#startmeeting cinder14:00
opendevmeetMeeting started Wed Apr 20 14:00:22 2022 UTC and is due to finish in 60 minutes.  The chair is whoami-rajat. Information about MeetBot at http://wiki.debian.org/MeetBot.14:00
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.14:00
opendevmeetThe meeting name has been set to 'cinder'14:00
whoami-rajat#topic roll call14:00
simondodsleyo/14:00
toskyo/14:00
felipe_rodrigueso/14:00
eharneyhi14:01
jbernardo/14:01
jungleboyjo/14:01
nahimsouza[m]o/14:01
caiquemello[m]o/14:01
whoami-rajat#link https://etherpad.openstack.org/p/cinder-zed-meetings14:01
rosmaitao/14:02
lucasmoliveira059o/14:02
whoami-rajatwe've a good turnout14:03
whoami-rajatso let's start14:03
whoami-rajat#topic announcements14:03
*** dasm|off is now known as dasm14:03
whoami-rajatfirst, Cinder mid cycle dates14:03
enriquetasohi14:03
whoami-rajat#link https://de46dad8d0940c6cc81e-27618e467ab9b983fdfb7a89cc8b48ca.ssl.cf2.rackcdn.com/837494/3/check/openstack-tox-docs/e70ce68/docs/zed/schedule.html14:03
e0ne_hi14:03
whoami-rajatI've reproposed the Zed schedule for cinder to include midcycle 1 (at R-18) and midcycle 2 (at R-9)14:04
whoami-rajatas a placeholder, I've marked the dates, 1st June (Wednesday) and 3rd August (Wednesday) 1400-1600 UTC as the proposed dates14:04
fabiooliveirahi14:04
whoami-rajatwould like to know if there are any conflicts of the above dates with other events or country specific holiday14:04
whoami-rajatalso would like to know if the date/time is not comfortable and I should do a poll instead14:05
rosmaitai won't be available for Aug 3, but that's just me14:05
whoami-rajatI've proposed the timing overlapping with the cinder meeting (+ one extra hour) as it works for everyone 14:05
whoami-rajatoh, the idea is to have the midcycle at R-9 so we can schedule any day of the week14:06
rosmaitai will just watch the video when i get back14:07
simondodsleysame for me 14:07
whoami-rajatsure, or we can do a poll as well, if others also feel more comfortable on another day14:08
whoami-rajatok, i guess we're going to do a poll for R-9 then14:08
whoami-rajatany conflicts for R-18?14:08
rosmaitano conflict for me14:09
simondodsleyno14:10
sfernandboth dates works fine for the netapp folks14:10
whoami-rajatcool, I will change the release schedule to exclude R-9 for now and prepare a poll14:10
whoami-rajat#action whoami-rajat to prepare a poll for R-9 midcycle14:10
whoami-rajatgreat14:10
whoami-rajatmoving on14:10
whoami-rajatsecond, Monkeypatching FIPS14:11
whoami-rajat#link http://lists.openstack.org/pipermail/openstack-discuss/2022-April/028250.html14:11
whoami-rajatAde lee sent out a mail regarding monkeypatching paramiko as paramiko uses md5 which makes it FIPS non complaint14:11
whoami-rajat(the mail also mentions cinder)14:11
whoami-rajatI still haven't looked at the cinder code for any occurrence but just for awareness, we need to wait for the fix to land in paramiko or adopt any other lib, until then projects are planning to monkeypatch it14:11
e0ne_#link https://github.com/openstack/cinder/search?q=paramiko14:11
eharneywe have some drivers that use paramiko14:12
whoami-rajatthanks e0ne_ , so we are using paramiko14:12
e0ne_whoami-rajat: np14:12
whoami-rajatso someone (probably me) will reply to the mail about cinder using paramiko14:13
eharneyAFAIK it is only used for a few drivers to talk to the backend, so one option is that those drivers won't work in FIPS mode until it's fixed14:13
whoami-rajatok14:13
simondodsleysome of those backends may not be FIPS compliant anyway so wouldn't be used in a FIPS environment14:13
whoami-rajati guess then it shouldn't be much of an issue since the FIPS jobs we will be running are mostly lvm, ceph, nfs related14:14
e0ne_should we add t FIPS support requirement for new drivers?14:14
whoami-rajatand third party CI still doesn't run on centOS14:14
whoami-rajatso we need to wait for FIPS support in ubuntu to make it a requirement for drivers14:15
eharneyright14:15
rosmaitawe can strongly suggest that drivers keep it in mind14:15
whoami-rajatok, that's a relief14:15
simondodsleysomething in the new drivers documentation?14:16
rosmaitaor the review checklist14:16
rosmaitaor both14:16
rosmaita:)14:16
rosmaitabasically, don't use md5 and only use well-respected crypto libraries14:16
whoami-rajatboth sounds good, one cinder wide and one driver specific14:17
rosmaitaand don't implement your own crypto under any circumstances14:17
eharneyyeah, a review checklist for such things seems like a good idea14:17
e0ne_rosmaita: :)14:17
rosmaitawhoami-rajat: you can give me an action item on this14:17
whoami-rajatrosmaita, regarding documentation or replying to the email thread?14:18
whoami-rajator both :)14:18
rosmaitadocs14:18
whoami-rajatok14:18
whoami-rajati will reply to the email with our discussion14:18
whoami-rajat#action rosmaita to include FIPS related documentation for new drivers14:18
whoami-rajat#action whoami-rajat to respond to the mail thread with our discussion14:19
rosmaitamy personal opinion is that we should use something other than paramiko in ssh_utils14:19
whoami-rajatmaybe Ade has some idea about good alternatives, can discuss with him14:20
whoami-rajatthat's all for my announcements, does anyone have any other announcement?14:20
whoami-rajatguess not, so let's move to topics14:22
whoami-rajat#topic vote on NetApp proposal to backport ONTAP REST API support14:22
whoami-rajatFernando suggested that we should do this in the video + IRC meeting (that is next week) so everyone can express their opinions about it and then we can vote14:22
whoami-rajate0ne_, will you be available in next week's video + IRC meeting?14:22
e0ne_whoami-rajat: I really hope so14:23
jungleboyj:-)14:23
e0ne_if I won't join, my current vote is to don't allow such backports14:23
geguileook14:23
sfernande0ne_:  ok14:24
whoami-rajatok, so my personal opinion is it would make sense to have a discussion on it before voting else we will end up upvoting or downvoting it without any concrete conclusion14:24
eharneyyes, i think more discussion is needed14:24
simondodsleyI can't make the next meeting either and my vote would be to not allow these backports - specify supported ONTAP versions that drivers support instead14:24
rosmaitaright now, i'm with e0ne_ on this as well14:25
whoami-rajatok, so let's discuss it next week and maybe we can drag the discussion till mid cycle to properly finalize it, till then the netapp team can work on their patch for master14:26
sfernandwhoami-rajat: ++14:27
whoami-rajatcool, anything else on this topic?14:27
whoami-rajatguess not, moving on then14:28
whoami-rajat#topic release notes with driver patches14:28
whoami-rajatrosmaita, that's you14:28
rosmaitabasically, just what i said on the agenda14:28
geguileorosmaita: copy/paste then  };-)14:29
rosmaita(which i will paste in here as soon as i find it)14:29
rosmaitanote to reviewers and driver maintainers: any driver fix that will be backported should have a release note14:29
whoami-rajatnote to reviewers and driver maintainers: any driver fix that will be backported should have a release note14:29
rosmaitaok, we agree on this :)14:29
whoami-rajat+114:30
geguileorosmaita: in general ANY driver fix should have a release note14:30
geguileoregardless of the backporting aspect14:30
jungleboyj++14:30
simondodsley+114:30
whoami-rajatusually allowed backports are mostly bugfixes so it's rare I've seen one without a releasenote14:30
rosmaitawell, i was reviewing the other day, and this came up14:31
geguileorosmaita: and we should also say:  driver bug fixes MUST have a launchpad bug explaining what's fixing14:31
rosmaitaso maybe this is more for reviewers to keep in mind14:31
simondodsleythe huawei patches referenced later in this meeting are missing them14:31
e0ne_geguileo: +114:31
rosmaitageguileo: and the commit message should make sense14:31
rosmaitaand the bug report should also make sense14:32
geguileorosmaita: oooooh, that's a good one ;-)14:32
rosmaitaand good engineering practices in the code14:32
whoami-rajatso we already have an example14:32
e0ne_rosmaita: and not include 'proprietary crypto algorithm for a driver'14:32
geguileorosmaita: that's sometimes subjective  ;-P14:32
geguileoe0ne_: +114:32
geguileowhoami-rajat: you were working on a reviewer doc patch, these points can be added there14:33
e0ne_oops... I missed 'implementation' word in my previous message:(14:33
whoami-rajatgeguileo, sure, these are some good ideas14:34
rosmaitaok, for now, reviewers, please keep the above in mind14:34
e0ne_rosmaita: +114:34
rosmaitaand if you see a driver patch with no release note, that's an easy -1 !!!14:35
whoami-rajatI'm happy rosmaita is providing lots of points for my review doc, probably rosmaita can co-author it as well!14:35
rosmaitawhoami-rajat: you can have those points for free :)14:36
whoami-rajat:D14:36
whoami-rajatok, so i think the point is pretty straightforward here14:36
whoami-rajatwe can move on to the next topic then14:36
whoami-rajat#topic fixing some DB API function signature inconsistencies14:37
whoami-rajatrosmaita, that's you again14:37
rosmaitai will be real quick (for a change)14:37
rosmaita#link https://review.opendev.org/c/openstack/cinder/+/837542/314:37
rosmaitathe question is, how far do we want to go in fixing this on this patch14:37
rosmaitaplease take a look, i have comments and stephen has responses14:38
rosmaitai have an opinion, but we should probably get some other people in on the action14:38
rosmaitaso if you are interested in consistency between the db api interface and the sqlalchemy implementation, take a look14:38
rosmaitaand if you don't care, something will happen14:38
rosmaitathat's it14:38
whoami-rajati can see 10 review comments, is there any one in specific or all are pointing to a general idea ?14:39
rosmaitathere are maybe 3 general issues14:40
geguileoam I the only one that thinks we should completely drop that cinder/db/api.py file?14:41
whoami-rajatok, so we can take a look at rosmaita's comments14:41
eharneygeguileo: it's a reasonable idea14:41
geguileoor is there somebody that thinks we'll ever support something different than SQLA?14:41
rosmaitai suspect not14:42
eharneygeguileo: Nova did it here: https://review.opendev.org/c/openstack/nova/+/79952414:43
whoami-rajatthat sounds like a good idea for cinder midcycle14:43
geguileoa pain to review, but makes sense to me14:43
whoami-rajatthat's a long list of file changes...14:44
rosmaitathen i guess it doesn't matter what happens in that patch14:44
rosmaitabecause we will be making changes later anyway14:44
rosmaitalet's get rid of the legacy enginefacade first though14:44
rosmaitai.e., do not stop reviewing the current patches!!!14:45
whoami-rajatyep, that's a good reminder for a priority item ^^14:45
rosmaita#link http://tiny.cc/cinder-legacyfacade14:45
whoami-rajatanyway, we can discuss the merging of db/api and db/sqlalchemy/api later14:46
rosmaita^^ that's a handy review dashboard14:46
geguileoit kills me a bit when somebody reorders methods in a file...14:46
rosmaitayeah, but it is kind of nice to have the related ones together14:46
rosmaitasaves my control key from getting worn out in emacs14:47
whoami-rajatso let's move on as we've other topics to cover as well (and less time)14:47
whoami-rajat#topic EM release for Victoria is April 2714:47
whoami-rajatjbernard, that's you14:47
jbernardheya, last victoria release is next week14:48
jbernardwe have 6 outstanding patches: https://review.opendev.org/q/branch:stable/victoria+project:openstack/cinder+status:open14:48
whoami-rajatso i forgot this in my announcement, jbernard has stepped up to take up stable release maintenance!14:48
enriquetaso\o/14:48
rosmaitajbernard: ++14:48
simondodsleycongrats14:49
eharneyexcellent14:49
rosmaitai guess i should drop my -2 on https://review.opendev.org/c/openstack/cinder/+/822939 ?14:49
whoami-rajatjbernard++ thanks!14:49
jungleboyj\o/14:49
jungleboyjThank you jbernard !14:49
rosmaitanot sure the issue has been settled yet, but i will leave it up to other reviewers14:49
jbernardif im understanding the process correctly, I need some time for the release team to process the request, so we need the patches we care about merged by Monday (Apr 25) at the latest (i estimate)14:49
whoami-rajatso stable cores can take a look at the open patches as we're nearing the final victoria release (mostly have votes as of now)14:50
simondodsleyrosmaita that one is abandoned anyway14:50
rosmaitasimondodsley: ty!14:50
eharneyrosmaita: i'll go fix up that whole thing after i fix master14:50
rosmaitaok, cool14:50
whoami-rajatrosmaita, eharney has abandoned it, so you can remove it when he restores?14:50
rosmaitasure14:51
jbernardok, we're already down to 5, making progress ;)14:51
rosmaitajbernard: you are already killing it as release manager!14:51
eharneyhttps://review.opendev.org/c/openstack/cinder/+/831188 is ineligible currently, so easy to skip that one14:51
whoami-rajatjbernard, that's right, we should do our release before 27th so release team can get time to make the transition easier for victoria to EM14:51
whoami-rajateharney, yep, most probably it won't make to victoria in time14:52
whoami-rajatso we can skip patches that are currently open in master14:53
whoami-rajator are far away from making it to victoria14:53
whoami-rajatunless people want that change in14:53
whoami-rajatand working towards getting it in14:53
whoami-rajatso, stable cores, take a look at the open patches in victoria14:54
whoami-rajatmoving on14:54
whoami-rajat#topic Replace distutils in drivers14:54
whoami-rajatsimondodsley, that's you14:54
simondodsleyas mentioned in the agenda I have a patch up to replace driver usage of distutils (https://review.opendev.org/c/openstack/cinder/+/832130) to stop any future bugs, but some of the affected drivers don't have CIs - can we just merge this patch without those CI, or does anyone know who owns these CI systems?14:54
eharneyunit tests should be sufficient for this change14:56
whoami-rajati can see there are 6 drivers using it14:56
eharneyCI results are not really interesting14:56
whoami-rajatif those driver maintainers can take a look, would be good14:56
jbernardthe nature of the changes do not impact the driver logic itself, i would vote to merge personally14:56
simondodsleyfinmding the CI maintainers is the biggest issue14:56
simondodsleywe need a list of current maintainers14:57
rosmaitai think a message to the ML and then we mark all those as unsupported (if they aren't already)14:57
whoami-rajatrosmaita, that sounds like a good idea14:57
rosmaitawell, the 3rd party CIs are supposed to keep their info in the wiki up to date14:57
simondodsleybut if there isn't even a CI...14:58
simondodsleyeg I cannot find an IBM DS8k CI 14:58
rosmaitai actually thought that driver had been removed14:58
simondodsleyor a powerflex14:58
whoami-rajatthis seems to be a concern with re-occurring changes that modify driver code14:58
eharneyi'm not sure why it's a concern with this change14:59
simondodsleyDS8k is still listed as supported as is powerflex14:59
whoami-rajateharney, not specifically this change but in general driver maintainers not being active15:00
whoami-rajati will send out a mail to the ML mentioning the drivers in question here, in the meantime if the team thinks this change is good with unit tests only then we can merge as well15:00
simondodsleythis change just highlights the lack of visibility on 3rd party CIs15:00
rosmaitaeharney: it's just a general erosion of standards15:00
rosmaitabut eharney has a good point, we shouldn't let it hold up this patch, i guess15:01
whoami-rajatrosmaita, yep, as stated above, if people think the change is good with UTs then we can go ahead and merge15:01
whoami-rajatbut i will send out the mail anyway15:01
whoami-rajatand we're out of time15:01
whoami-rajatlast topic15:02
whoami-rajat#topic request for review of backports  -- i think we already got reviews on them15:02
whoami-rajatto add releasenote15:02
simondodsleyOK - I guess I will have to work out how to do a UT for this - BTW I hate writing in mock :)15:02
whoami-rajatso ganso you can make those changes and team can review again15:02
whoami-rajatsimondodsley, i think eharney means the current UTs should validate your change (or maybe i understood it wrong)15:03
simondodsleyand add a statement that this has been tested on the stable branch15:03
whoami-rajatlet's continue discussion in cinder channel (after bugsquad meeting)15:03
eharneyi don't know if the current ones do or not, they might, or coverage might need to be added15:03
simondodsleywhoami-rajat - ah, ok15:03
whoami-rajatthanks everyone!15:03
whoami-rajat#endmeeting15:03
opendevmeetMeeting ended Wed Apr 20 15:03:52 2022 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)15:03
opendevmeetMinutes:        https://meetings.opendev.org/meetings/cinder/2022/cinder.2022-04-20-14.00.html15:03
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/cinder/2022/cinder.2022-04-20-14.00.txt15:03
opendevmeetLog:            https://meetings.opendev.org/meetings/cinder/2022/cinder.2022-04-20-14.00.log.html15:03
jungleboyjThank you!15:04
*** dviroel is now known as dviroel|lunch15:31
*** dviroel|lunch is now known as dviroel16:31
*** dasm is now known as dasm|off20:53
*** dviroel is now known as dviroel|out22:27

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