14:00:09 <rosmaita> #startmeeting cinder
14:00:09 <opendevmeet> Meeting started Wed Jun 23 14:00:09 2021 UTC and is due to finish in 60 minutes.  The chair is rosmaita. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:09 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:09 <opendevmeet> The meeting name has been set to 'cinder'
14:00:15 <rosmaita> #topic roll call
14:00:17 <whoami-rajat> Hi
14:00:24 <simondodsley> Hello
14:00:37 <rosmaita> #action rosmaita - find out why rbd-iscsi-client is not mirrored on github
14:00:41 <fabiooliveira> hi
14:00:59 <sfernand> hi
14:01:11 <tbarron> hi
14:01:15 <eharney> hi
14:01:19 <jungleboyj> Hello.
14:01:23 <abishop> o/
14:01:31 <enriquetaso> hi
14:02:29 <rosmaita> looks like a good turnout, let's get started
14:02:31 <rosmaita> #link https://etherpad.openstack.org/p/cinder-xena-meetings
14:02:39 <rosmaita> #topic announcements
14:03:04 <rosmaita> since next week's meeting is the final meeting of June, it will be held in both video and irc
14:03:31 <rosmaita> also, if you missed it on the mailing list, lseki has stepped down as a cinder core
14:03:42 <rosmaita> #link http://lists.openstack.org/pipermail/openstack-discuss/2021-June/023206.html
14:04:09 <rosmaita> and, spec freeze is on Friday
14:04:33 <rosmaita> #link https://releases.openstack.org/xena/schedule.html#x-cinder-spec-freeze
14:04:34 <sfernand> :(
14:04:53 <e0ne> hi
14:04:59 <rosmaita> hello ivan
14:05:23 <hemna> hey
14:05:37 <rosmaita> i'm going to change the order of stuff on the agenda
14:05:39 <rosmaita> we will discuss
14:05:49 <rosmaita> sqlite (should take 2 minutes)
14:06:04 <rosmaita> 10 minutes on simon's black linter proposal
14:06:12 <rosmaita> and then the rest on spec reviews
14:06:24 <rosmaita> #topic Drop support for SQLite < 3.7
14:06:34 <rosmaita> #link https://review.opendev.org/c/openstack/cinder/+/788871
14:07:30 <rosmaita> basically, all the currently supported platforms for openstack (centos-8-stream and ubuntu-focal) support sqlite >= 3.7
14:07:51 <rosmaita> we have a check in the code for the case where someone is using an older version
14:08:02 <rosmaita> proposal is to remove it
14:08:38 <e0ne> sqlite is used only got tests, so it's ok to use any version which doesn't make our tests more complicated
14:08:41 <eharney> this only affects unit tests, so, not much risk
14:08:48 <e0ne> eharney: +1
14:08:50 <rosmaita> but i was thinking maybe someone working with like queens, say, might be using an older sqlite on their dev box
14:09:04 <hemna> Train ?
14:09:22 <rosmaita> i think queens is still in -em mode
14:09:46 <rosmaita> i believe train's supported platforms have sqlite >= 3.7
14:09:51 <rosmaita> but i am not 100% sure
14:10:25 <hemna> I think train was ubuntu 18.0x > ?
14:11:09 <rosmaita> ok, let's do this ... if anyone can think of a reason *not* to do this, please leave a comment on the patch before Friday 1200 UTC
14:11:23 <rosmaita> otherwise, i am inclined to go with e0ne and eharney on this
14:11:27 <rosmaita> thanks!
14:11:29 <hemna> +1
14:11:50 <rosmaita> ok, it is almost 14:12 now
14:11:54 <e0ne> #link https://governance.openstack.org/tc/reference/runtimes/train.html
14:12:04 <rosmaita> #topic discuss allowing `black` to be used as an auto-linter
14:12:11 <rosmaita> simondodsley: that's you
14:12:29 <e0ne> it's good to have it but we'll lost history
14:12:47 <simondodsley> i like to use `black` as an auto-linter - it makes life super easy  especially for seeing future changes
14:13:10 <simondodsley> e0ne: I agree you loose history - how much of an issue is that?
14:13:32 <e0ne> git blame is usesul, but I prefer to have black :)
14:13:33 <simondodsley> It isn't 100% perfect, but it makes linting somuch quicker
14:13:51 <rosmaita> i think for the main cinder code, losing history is definitely an issue
14:14:06 <simondodsley> I have found a couple of weird things that `tox -e pep8` fails with when using `black` but they are very trivial
14:14:14 <e0ne> if we decide to use it, let's adopt it's settings to keep current code style
14:14:48 <eharney> i thought black was opinionated and didn't really have settings to adjust
14:14:54 <hemna> I've used black in other projects, and it doesn't always produce the most readable code IMHO
14:15:01 <simondodsley> I'm not saying it should be mandatory - yes it is opionated
14:15:05 <hemna> eharney it is very opinionated
14:15:09 <e0ne> eharney: it has some settings and/or ignore list
14:15:15 <rosmaita> so we can see the differnces:
14:15:28 <rosmaita> #link https://review.opendev.org/c/openstack/cinder/+/792462/
14:15:53 <hemna> It can produce stuff like this: https://github.com/craigerl/aprsd/blob/master/aprsd/messaging.py#L561-L573
14:16:00 <hemna> which I think sux0rs
14:16:01 <eharney> see, about the first thing it changed on line 57 there is annoying
14:16:24 <eharney> (breaking our standard formatting of help text on a new line after the option name)
14:16:32 <hemna> I use black in my ham radio python project called aprsd ^^ link above, as a mechanism to ensure that non python people can lint format their code before commit.
14:17:36 <eharney> also not sure why it wants to change a lot of single-quoted strings to double quotes, weird
14:17:49 <simondodsley> For new code is it a godsend, but true it does break some of the current standards, but are they critical?
14:17:51 <rosmaita> yes, it definitely has its quirks
14:18:15 <hemna> I think the function definition 'formatting' really sucks
14:18:16 <eharney> i don't really see a benefit when pep8+hacking gets us quite consistent on style already
14:18:23 <rosmaita> i like some of the stuff it does, but hate other things
14:18:31 <jungleboyj> hemna:  ++
14:18:38 <rosmaita> simondodsley: we really want a consistent style to make it easy to read any of the code files
14:18:43 <jungleboyj> eharney: ++
14:18:55 <simondodsley> but using `pep8` requires lots of manual intervention and that is a waste of productive time
14:19:40 <eharney> i just ran "black" on cinder/volume/manager.py and it blew up and told me to report a bug
14:19:42 <simondodsley> I can `blacken` code in 5 seconds whereas manally editing for `pep8` compliance takes ages.
14:19:49 <eharney> (was hoping to see what it did with type annotations)
14:19:51 <simondodsley> `black` is a superset of `pep8`
14:20:22 <geguileo> I don't care as much about the changing of standards as of losing history
14:20:27 <hemna> another example of sux0rs...same problem though: https://github.com/craigerl/aprsd/blob/master/aprsd/main.py#L286-L301
14:20:31 <simondodsley> eharney: then you should :)
14:20:33 <rosmaita> geguileo: ++
14:20:56 <eharney> i'd rather just stick to pep8/hacking style-wise
14:20:57 <geguileo> eharney: and now you've seen what it does  };-)
14:21:10 <eharney> geguileo: indeed
14:21:15 <rosmaita> i don't see the advantage of using this on the main cinder code because of the history issue
14:21:29 <rosmaita> maybe for drivers? how do people feel about that?
14:21:39 <simondodsley> would it be acceptable for new code?
14:21:40 <rosmaita> or do we need to be completely consistent?
14:21:41 <geguileo> maybe also for tests?
14:21:44 <whoami-rajat> L#289-299 doesn't look good...
14:21:46 <sfernand> git blame is critical for new devs, specially when trying to understand critical changes between os releases
14:21:52 <e0ne> it doesn't make sense to have different code styles in the same repo
14:21:53 <eharney> does black formatted code pass pep8/hacking most of the time?
14:22:07 <rosmaita> sfernand: i agree
14:22:11 <hemna> I think if a driver dev wants to run it on their driver and is ok with losing history that's one thing, still sucks for other people that touch those drivers, but running it on cinder core I think would not be good.
14:22:17 <simondodsley> the style it creates make seeing code changes much simpler and makes for easier coding of changes going forward
14:22:32 <geguileo> sfernand: and for old devs (like me) as well ;-)
14:22:43 <simondodsley> I'm OK with loosing history for my driver code
14:23:05 <hemna> also, backporting fixes to previous releases where black wasn't run.....sux0rs
14:23:07 <rosmaita> i have been seeing some black-esque stylings in some patches recently
14:23:18 <rosmaita> hemna: that is a really good point
14:23:22 <geguileo> hemna: right!!! backports!!!
14:23:59 <simondodsley> True - backports will break a lot.  OK - I'll drop my thoughts on `black` for cinder
14:24:00 <geguileo> I think we agree not to do it for core code and let driver maintainers choose whether to do it or not
14:24:06 <jungleboyj> hemna:  There is a huge issue.  Backports.
14:24:06 <e0ne> we know how to deal with backports. we have such rules for python3 only code
14:24:32 <whoami-rajat> geguileo: but only on patches that they don't want to backport?
14:24:38 <jungleboyj> I would prefer people don't use it.  Definitely do not want it on core code.
14:24:59 <e0ne> it's time for out-of-tree drivers discussion
14:25:00 <jungleboyj> When I did the review I thought it sucked but went down the route of, 'Well, it is their driver'
14:25:04 <geguileo> whoami-rajat: well, if they want to backport them they'll have to fix the merge conflicts
14:25:15 * jungleboyj does a hard eye roll
14:25:28 <rosmaita> ok, so it's looking like the consensus is definitely no black reformatting in the main code
14:25:28 <tosky> do I understand it correctly the whole issue is due to black not being configurable to be tuned to produce the same style we use today?
14:25:36 <hemna> e0ne lolz
14:26:02 <rosmaita> ok, we are out of time on this
14:26:18 <simondodsley> ok - let's say No - I'll refactor my patch
14:26:20 <rosmaita> does someone want to take an action item to investigate black configuration?
14:26:31 <rosmaita> so, for now ... no black
14:26:47 <rosmaita> we can revisit if someone comes up with a way to address the points made above
14:26:52 <rosmaita> thanks, simon
14:27:12 <rosmaita> #topic xena specs review
14:27:23 <rosmaita> well, it's spec freeze time again
14:27:37 <jungleboyj> rosmaita:  ++
14:27:40 <rosmaita> #link https://review.opendev.org/q/project:openstack%252Fcinder-specs+status:open
14:27:47 <rosmaita> let' see what we got
14:28:23 <hemna> hrmm user visible extra specs
14:28:24 <rosmaita> i think the user-visible extra specs is in good shape, a revision was just pushed
14:28:30 <hemna> I'm still not terribly sold on that
14:28:42 <rosmaita> hemna: you need to act fast and review
14:29:12 <hemna> ok will do.   I don't think I'd want my customers to see my volume type extra specs.
14:29:18 <hemna> it's very terse and driver specific
14:29:31 <hemna> details that end users shouldn't see IMHO
14:29:40 <rosmaita> well, it's going to be limited to a specific subset of extra-specs keys
14:29:46 <rosmaita> defined in code, not configurable
14:29:48 <abishop> hemna: driver specific specs are absolutely excluded from the feature
14:29:49 <tbarron> hemna: they wont see any backend specific extra specs
14:29:51 <hemna> I'll read through it
14:29:52 <whoami-rajat> hemna: driver specific extra specs won't be visible
14:30:24 <tbarron> backend-revealing
14:30:26 <hemna> I'm just saying it's problematic for my customers
14:30:45 <rosmaita> well, it will be governed by policy, so you can always turn it off for end-users
14:30:46 <tbarron> and your customers can set policy to not show any if they want
14:31:09 <hemna> wait, customers can set policy ?!  or admins?
14:31:14 <tbarron> admins
14:31:16 <hemna> nm, I'll read the spec.
14:31:24 <tbarron> i thought your customers were admins
14:31:28 <hemna> no
14:31:33 <hemna> I'm the admin :)
14:31:34 <tbarron> if you are the admin then you do it if you want
14:32:00 <rosmaita> anyway, give the spec a read, i think most of the issues have been worked out
14:32:16 <rosmaita> nmve-of connection agent
14:32:25 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/796365
14:32:44 <rosmaita> i think the spec captures the discussion at the PTG
14:33:08 <rosmaita> so i think we're in agreement with the general direction
14:33:28 <rosmaita> it will be useful for people to look it over for anything that needs to be spelled out more clearly
14:33:56 <rosmaita> Spec for "Consistent and Secure RBAC" in Cinder
14:34:05 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/797655
14:34:24 <rosmaita> we already agreed to do this, but i realized that we didn't have a spec to track it
14:34:31 <rosmaita> so there it is
14:34:54 <hemna> nice
14:35:20 <rosmaita> eric: did your remove force from snapshots spec merge already?
14:35:34 <hemna> I +A'd it this morning
14:35:41 <rosmaita> cool
14:35:45 <eharney> yes
14:35:52 <rosmaita> i was worried there was a problem with my gerrit query
14:36:03 <rosmaita> ok, the rest have -1s
14:36:34 <hemna> I +A'd the temorary resource tracking spec as well
14:36:48 <rosmaita> good
14:36:54 <rosmaita> you have been busy this morning
14:36:58 <hemna> yup :)
14:37:14 <rosmaita> Support revert any snapshot
14:37:22 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/736111
14:37:34 <rosmaita> there seems to be a failure of communication on this one
14:38:02 <rosmaita> i asked the author to read the midcycle discussion
14:38:16 <rosmaita> which stressed the importance of adding test scenarios to explain how this will work
14:38:33 <rosmaita> but that hasn't happened
14:39:02 <eharney> i think the spec is still missing info about safely handling operations that fail half-way through
14:40:12 <rosmaita> ok, i will add a comment explaining more explicitly what needs to be addressed
14:41:00 <rosmaita> it would be helpful if the proposer attended the cinder meetings so we could discuss
14:41:22 <jungleboyj> rosmaita:  ++
14:41:32 <rosmaita> Migration support for a volume with replication status enabled
14:41:41 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/766130
14:41:49 <rosmaita> looks like geguileo has some concerns on this one
14:42:47 <rosmaita> and it looks like the author is not willing to provide the details requested
14:43:01 <geguileo> rosmaita: I don't know if I'm failing to explain what's missing
14:43:04 <rosmaita> and those details are not unreasonable, they are key to understanding how this whole thing should work
14:43:29 <eharney> the main concern is about migration would interfere with the replication, right?
14:44:15 <rosmaita> this is another one that would have benefitted from discussion during the weekly meetings
14:44:16 <geguileo> eharney: that is the primary, yes
14:45:05 <geguileo> eharney: and iirc you also mentioned it before
14:45:23 <rosmaita> Update original volume az
14:45:31 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/778437
14:45:49 <rosmaita> this hasn't been updated since my midcycle comment, so not looking good
14:47:37 <rosmaita> Allow volumes to be part of multiple volume groups
14:47:49 <rosmaita> #link https://review.opendev.org/c/openstack/cinder-specs/+/792722
14:48:55 <rosmaita> i would've sworn that we had discussed it, but maybe not
14:49:32 <geguileo> rosmaita: we did discuss it
14:49:46 <geguileo> at least twice...
14:49:57 <rosmaita> ok, i thought maybe i was dreaming about cinder again
14:50:09 <geguileo> lol
14:50:17 <rosmaita> ok, it hasn't been updated since 24 may, so i guess the proposer has lost interest
14:50:19 <eharney> i'm concerned that this one is lacking some detail
14:50:40 <whoami-rajat> rosmaita: I've left some comments on your srbac patch
14:50:45 <geguileo> iirc the first time someone explained the use cases where this was useful
14:50:47 <rosmaita> whoami-rajat: ty
14:50:52 <eharney> it's not obvious what all the various things are that can happen when moving volumes in and out of consistency/replication groups
14:51:01 <eharney> but i suspect it's more than just updating markers to say it's in those groups...
14:51:44 <eharney> i.e. what happens with a chain of cgsnapshots when each snapshot has different volumes in it
14:54:39 <rosmaita> ok, i'll send emails out about 736111, 766130, 778437, 792722
14:55:00 <rosmaita> the aim of the email being that they need to work with the team to address the issues
14:55:17 <rosmaita> and if it's a TZ problem for the weekly meeting, we'll have to work something out
14:55:31 <rosmaita> but i don't realistically see any of those being ready for Xena
14:56:00 <rosmaita> ok, i guess that's it for specs
14:56:17 <rosmaita> please review the ones mentioned at the beginning of this part of the meeting
14:56:32 <rosmaita> they need to merge by 23:59 UTC or thereabouts on friday
14:56:38 <rosmaita> any questions, comments?
14:56:51 <jungleboyj> Question on the NVMe-oF
14:57:02 <jungleboyj> https://review.opendev.org/c/openstack/cinder-specs/+/796365
14:57:15 <jungleboyj> So, There are +1s on it.  No +2s .
14:57:42 <jungleboyj> What do we want to do about that?
14:58:09 <whoami-rajat> I've left some queries and suggestions (personal) to make it more readable
14:58:16 <hemna> there is some good feedback on that review that's not been updated
14:59:21 <rosmaita> i agree with Jay's comment that a more thorough os-brick person than me needs to give it a +2
14:59:25 <jungleboyj> hemna:  Ok.  That was why I wasn't +2.  Just trying to interpret the other +1s
14:59:51 <hemna> we had a discussion about this agent ages ago no?
15:00:03 <hemna> I don't remember all of it
15:00:04 <rosmaita> yes, at the PTG
15:00:16 <rosmaita> #link https://wiki.openstack.org/wiki/CinderXenaPTGSummary#NVMe-oF_and_MDRAID_replication_approach_-_next_steps_for_connector_and_agent
15:00:18 <hemna> I'll read through it
15:00:29 <rosmaita> ok, thanks
15:00:40 <rosmaita> like i said, i think it captures what we talked about
15:01:13 <rosmaita> also, i am worried that the spdk CI has disappeared
15:01:27 <rosmaita> although i guess that doesn't matter for this spec
15:01:58 <rosmaita> ok, we are out of time ... thanks everyone, and please complete your spec reviews as soon as you can
15:02:10 <rosmaita> #endmeeting