16:02:19 <ildikov> #startmeeting cinder-nova-api-changes
16:02:20 <openstack> Meeting started Thu Oct 19 16:02:19 2017 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:02:21 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:02:25 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
16:02:35 <ildikov> johnthetubaguy jaypipes e0ne jgriffith hemna mriedem patrickeast smcginnis diablo_rojo xyang1 raj_singh lyarwood jungleboyj stvnoyes
16:02:35 <mriedem> o/
16:02:56 <jgriffith> derp
16:02:58 * johnthetubaguy wonders in and waves
16:04:01 <groot> I am groot
16:04:02 <ildikov> sorry for being late
16:04:05 * smcginnis is still glaring at zuul
16:04:42 <ildikov> we have a new version of the multi-attach spec: https://review.openstack.org/499777
16:04:47 <ildikov> thanks to jgriffith
16:05:23 <mriedem> reading the diff now
16:05:30 <ildikov> I need to read through it quickly, but if anyone has concerns with either of the bits in it feel free to raise it
16:06:31 <johnthetubaguy> we say Active/Active is out of scope? I thought that was largely the point of it
16:06:38 <groot> and if you want me to walk through it now would be a good time :)
16:06:50 <groot> johnthetubaguy I just copied what was already said
16:07:02 <groot> put it back in if you want... but that's what was there
16:07:13 <groot> I only wanted to update the process/flow stuff
16:07:14 <mriedem> this is news to me,
16:07:17 <mriedem> "NOTE:  multi-attach checks are all done at attachment-create time, there is no longer a requirement to create a volume with multi-attach flags."
16:07:28 <groot> mriedem yeah, that's what I added
16:07:48 <johnthetubaguy> :S
16:08:02 <mriedem> so nova-api will call attachment-create and if it's not a multiattach volume, the volume has to be 'available' for that to work,
16:08:13 <groot> nope
16:08:17 <groot> oh...
16:08:20 <groot> mriedem YES
16:08:21 <groot> sorry
16:08:33 <groot> mriedem it goes through those 4 checks that I listed
16:08:35 <mriedem> if it's a multiattach volume, attachment-create will work even if the volume is in-use
16:08:53 <groot> mriedem yes, IFF the requirements are met
16:08:56 <mriedem> "The backend providing the volume supports it"
16:09:03 <ildikov> and multi-attach is defined at the first attachment-create
16:09:05 <mriedem> is that going to be an rpc call from cinder api to the backend?
16:09:14 <mriedem> or something set on the volume in the db?
16:09:29 <groot> mriedem we have that in the backend-capabilities already
16:09:32 <mriedem> like a capability of hte type
16:09:34 <mriedem> ok
16:09:59 <groot> I don't want to use types for this EXCEPT for the case of a user explicitly saying they want that capability
16:09:59 <mriedem> side question, but is cinder going to then deprecate the multiattach flag in the volume API?
16:10:06 <groot> YES!!!!!!
16:10:06 <mriedem> if it's not actually used anywhere
16:10:21 <mriedem> ok
16:10:24 <groot> I'd like to just delete it straight away and wash it from my memory forever
16:10:52 <groot> If I could go back in time it would've never merged into the code
16:11:21 <mriedem> so in cinder channel you said, "The idea being that in the end the main thing we need on the Nova side is just to check the multiattach property in the attachment record and use it for libvirt if Nova supports/allows it and that's it"
16:11:29 <mriedem> so there is going to be a multiattach flag on the attachment record now?
16:11:35 <groot> mriedem yes
16:11:50 <johnthetubaguy> hang on...
16:12:08 <mriedem> but it's the volume that's multi-attachable, not the attachment record, right?
16:12:16 <groot> no
16:12:17 <mriedem> the attachment is only for 1 volume + 1 instance
16:12:21 <johnthetubaguy> the first attachment needs to be multi-attach = true, how does that work? its a volume thing
16:12:30 <mriedem> the volume can be attached to multiple instances, which are multiple attachments
16:12:33 <groot> the attachment is the source of all info regarding how to and if you can attach
16:12:47 <mriedem> but that attachment is only per volume/instance combo
16:12:57 <groot> mriedem the point was raised that libvirt MUST set shared on every one of those
16:13:17 <ildikov> it's based on how you attached the first attachment that moved the volume from 'available' to 'in-use'
16:13:18 <groot> groot so the easiest way to track that is the attachment record
16:13:21 <johnthetubaguy> how does cinder know there might be multiple attachments then?
16:13:34 <johnthetubaguy> for the first attachment, in particular
16:13:36 <groot> because even if the volume is "mulit-attach" that doesn't matter
16:13:43 <groot> johnthetubaguy we have a database
16:13:56 <johnthetubaguy> but isn't it a user preference?
16:14:06 <johnthetubaguy> I am asking where the data came from, not where it is stored
16:14:23 <groot> johnthetubaguy and that's why there's an option to attachment-create
16:14:34 <groot> attachment-create --multiattach
16:14:39 <johnthetubaguy> but thats a Nova call, and nova doesn't have an API for that
16:14:52 <groot> ???
16:15:01 <groot> johnthetubaguy attachment-create?
16:15:10 <johnthetubaguy> that doesn't take that flag today
16:15:10 <mriedem> nova does'nt know if it should pass a multiattach flag
16:15:31 <johnthetubaguy> this was all build assuming it was passed by a user when they create the volume
16:15:35 <ildikov> groot: Nova only has the volume-attach call with no extra parameters and the multiattach flag decided what to do
16:15:50 <groot> why wouldn't nova add it as part of this spec?
16:16:04 <groot> the other option then if you want....
16:16:16 <johnthetubaguy> so the nova flow is horrid this way... its not just us being a stick in the mood
16:16:18 <johnthetubaguy> mud
16:16:22 <ildikov> is there any concern on who's allowed to set 'multiattach'?
16:16:22 <groot> if Cinder supports it we ALWAYS send the parameter back as true
16:16:32 <groot> we just auto-set it
16:16:43 <johnthetubaguy> thats worse for volume performance
16:16:54 <johnthetubaguy> we don't want to disable the cache when we don't have to
16:17:01 <johnthetubaguy> user needs to opt into that
16:17:04 <johnthetubaguy> somehow
16:17:11 <groot> Ok
16:17:13 <groot> -2
16:17:14 <johnthetubaguy> so if it goes in Nova, here is the flow
16:17:19 <groot> I'm fine with that
16:17:31 <johnthetubaguy> user attaches volume, passed multi-attach
16:17:52 <johnthetubaguy> user attaches same volume elsewhere, forgets to pass multi-attach, we fail saying you should have said multi-attach = true here
16:17:54 <johnthetubaguy> that seems nuts
16:18:06 <groot> johnthetubaguy just let me say for the record, the flag on the volume at creation is going to be problematic in the future
16:18:26 <groot> johnthetubaguy did you read the spec?
16:18:44 <groot> I realize it was just put up
16:18:53 <johnthetubaguy> not got that far down I guess
16:19:13 <groot> the spec points out that if it was already attached somewhere using multiattach then cinder will auto-set that flag on all subsequent attach calls
16:19:51 <johnthetubaguy> OK
16:20:03 <ildikov> it might be inconvenient as you need to know when you're attaching the first attachment to set it
16:20:26 <groot> ildikov not as inconvenient as knowing to set it at volume-create!!!
16:20:44 <ildikov> like checking whether the volume has attachment or not
16:21:19 <johnthetubaguy> so you are wanting a flow where you create a volume, fill it up, detach it, then flip it to multi-attach, so you don't need to create a snapshot then create a new volume?
16:21:19 <groot> "inconvenient" is sort of a a funny thing to throw out considering the alternative :)
16:21:35 <groot> johnthetubaguy yup, that's one use case
16:22:15 <ildikov> well, the other is not flexible however seems straightforward
16:22:18 <groot> johnthetubaguy also my opinion is that the changes I proposed actually work and are easier to mangae/track things
16:22:35 <groot> ildikov I'd argue that it is NOT straight forward at all
16:22:40 <groot> but that's just me :)
16:23:11 <johnthetubaguy> I think the problem is we are not all thinking through the same set of use cases / flows here
16:23:25 <ildikov> I didn't say not at all, just found that part a little inconvenient :)
16:24:13 <johnthetubaguy> if I think about the data creation, I like the the on-attachment idea. What I hate is its gonna take an age to iron out the Nova API changes, there are many of them implied there.
16:24:58 <groot> johnthetubaguy well then I guess it won't work :(
16:24:59 <groot> bummer
16:25:08 <groot> I don't know why it's so hard though
16:25:39 <groot> "nova volume-attach --multiattach <instance> <volume>"
16:25:45 <groot> What's so difficult there?
16:25:52 <johnthetubaguy> nova boot needs changing too
16:25:59 <johnthetubaguy> BDM v2 needs updates
16:25:59 <groot> Yeah?
16:26:05 <groot> it does in either case
16:26:15 <ildikov> if we like the flexibility of this better, than it would be better to do it now than implementing the whole thing with the current flag
16:26:24 <johnthetubaguy> the old way there are zero changes to the microversion of the API
16:26:46 <ildikov> johnthetubaguy: we would've bumped it anyway as multiattach is a new functionality
16:27:06 <groot> and a train wreck in the cinder code :(
16:27:10 <groot> anyway, I guess it's too hard to do
16:27:26 <johnthetubaguy> not sure about too hard
16:27:34 <ildikov> me neither
16:28:01 <johnthetubaguy> just its harder (Nova side), need to be worth it, seems like it could be
16:28:23 <johnthetubaguy> mriedem: I am curious what you think about an extra multi-attach flag for all volume attachments
16:28:27 <groot> well let's think through it a bit and decide if it "is" worth it I guess
16:28:36 <mriedem> just put my comments in the spec,
16:29:05 <mriedem> i don't see why the multiattach flag makes sense on the attachment record, like i said, the attachment record is per volume/instance, and can't be shared with another instance
16:29:09 <mriedem> so it's not multiattachable
16:29:13 <mriedem> the volume is multiattachable
16:29:21 <mriedem> i think the flag should be modeled on the volume as it always has been
16:29:34 <mriedem> if the backend doesn't support multiattach when the volume is created, cinder should fail the volume create
16:29:54 <johnthetubaguy> so how does the data get into the volume, I think that is the consideration here
16:30:00 <mriedem> that leaves it to the user if they are doing multiattach at volume create time rather than when attaching the volume to an instance, which they might only do once
16:30:06 <groot> mriedem attachment record on the cinder side is per-volume
16:30:11 * smcginnis is glad to not have to be the PTL on stage stating yet again "We've made progress towards supporting mutliattach, but there's more work we're hoping to get done in $RELEASE+1"
16:30:54 <groot> smcginnis nahh... I'll withdraw my suggestions
16:31:03 <groot> I don't want to argue about it or derail anything
16:31:11 <mriedem> groot: the volume has multiple attachments sure
16:31:18 <smcginnis> Not saying that. We should get the right design.
16:31:31 <groot> smcginnis I'm not sure anything will be *right* :)
16:31:44 <mriedem> honestly i'm just really surprised that this is coming up now
16:31:49 <mriedem> like, this seems really left field to me
16:32:04 <groot> mriedem that's apparantly my fault
16:32:11 <ildikov> mriedem: groot: can we say that it gets set at attachment-create if the back end supports it, but it will be added to the volume as opposed to the attachment?
16:32:13 <groot> for some reason I thought I had communicated this all along
16:32:18 <ildikov> kind of the way between?
16:32:21 <groot> but obviously that's not the case
16:32:23 <groot> :(
16:32:47 <mriedem> ildikov: but like johnthetubaguy said, what if the user doesn't want the volume to be attached to multiple instances?
16:32:57 <mriedem> it seems like something the user should be opting into when creating the volume
16:33:16 <ildikov> mriedem: do you mean that someone else is using the volume than who created?
16:33:31 <mriedem> sure, volumes are not owned by a user right? they are owned by the project
16:33:35 <groot> mriedem why does creation instead of attach?
16:33:41 <ildikov> mriedem: as if it's the same user than don't call attachment-create with --multiattach
16:33:42 <mriedem> a user in the same project could attach the volume to another instance in the same project
16:33:46 <groot> I'd argue that attach time makes WAY more sense
16:34:21 <groot> and honestly you could create a type that says explicitly "dont multiattach" if you really wanted to for some odd reason
16:34:54 <groot> honestly it'd make more sense IMO to have a "forbid-multi-attach create-volume option than the other"
16:35:20 <mriedem> ^ is a tihng in the type?
16:35:48 <mriedem> so what happens today if i try to create a volume with multiattach=true, will it fail if the type capability (via the backend) doesn't support multiattach?
16:36:10 <groot> mriedem correct
16:36:39 <groot> which actually none of that code relating to that flag is actually *finished* so it doesn't really do anything today at all
16:37:09 <groot> it shouldn't even be considered/discussed except possibly as an example of a flag
16:37:12 <smcginnis> I can see the use where a volume is created not thinking of multiattach, gets some useful data, then a user decides they could use that ReadOnly on multiple hosts.
16:38:06 <mriedem> snapshot the volume and create a new multiattach=true volume from the snapshot?
16:38:11 <johnthetubaguy> smcginnis: +1 that is what I was saying earlier, but they can snapshot that, and create a multi-attach form the snapshot?
16:38:12 <ildikov> smcginnis: there was a Cinder blueprint earlier to make that flag changeable after creation
16:38:38 <johnthetubaguy> ildikov: actually that is a better option, if no attachments, allow users to tweak it
16:38:56 <groot> well there ya go then.  problem solved
16:39:21 <ildikov> johnthetubaguy: yep, that works too
16:39:25 <groot> ildikov sorry for messing up your spec :)
16:39:50 <ildikov> smcginnis: groot: would that work from your perspective? ^^
16:40:05 <ildikov> groot: would've been a too easy day otherwise... :)
16:40:05 <groot> and Nova owns figuring out BFV etc
16:40:22 <mriedem> changing the multiattach flag on the volume is not something we need to worry about today,
16:40:24 <mriedem> or for this nova spec
16:40:31 <mriedem> it's a later enhancement
16:40:35 <smcginnis> ildikov: Yeah, makes sense.
16:40:36 <mriedem> for now you can snapshot and recreate if needed
16:40:40 <groot> mriedem well wait
16:40:44 <groot> bah
16:40:45 <groot> ok
16:40:46 <ildikov> I just wanted to check on whether it's considered doable
16:40:47 <groot> fine
16:40:48 <groot> I give up
16:40:52 <ildikov> don't want to add it now
16:41:12 <johnthetubaguy> who is gonna recap to see where we have landed?
16:41:38 <groot> not me
16:41:40 <mriedem> it sounds like we're back to previous understanding of how things would work
16:41:40 <johnthetubaguy> no read_only, no bfv, bootable=false and mutliattach=true dissallowed by cinder for the moment
16:41:41 <groot> :)
16:41:53 <johnthetubaguy> volume sets multi-attach = true/false
16:41:57 <mriedem> i thought we were going to have bfv?
16:42:16 <johnthetubaguy> oh, I thought we said no a few weeks back :S
16:42:17 <ildikov> johnthetubaguy: the bootable + multiattach is questionable
16:42:28 <johnthetubaguy> because of no read-only mode
16:43:07 <mriedem> ok i haven't really followed the r/o / r/w stuff
16:43:18 <mriedem> totally confused about policy and all that
16:43:27 <johnthetubaguy> so the way I looked at it was this...
16:43:35 <ildikov> mriedem: we said at a certain point that we wouldn't want to boot multiple instances from a R/W volume
16:43:52 <johnthetubaguy> do the "simple" use case, r/w multi-attach as a first version
16:44:03 <johnthetubaguy> i.e. target the cluster whatnot people
16:44:32 <johnthetubaguy> so policy is on or off in Cinder, i.e. are you allowed to set multi-attach=true or not, period
16:44:47 <johnthetubaguy> if multi-attach=true, do not allow bootable=true, (for now)
16:44:57 <johnthetubaguy> that gives us something we could do in queens
16:44:59 <johnthetubaguy> possibly
16:45:10 <johnthetubaguy> and hits the 80% use case people seem to be asking about
16:45:14 <johnthetubaguy> ... then next time
16:45:22 * smcginnis needs to AFK for a bit, be back later
16:45:35 <johnthetubaguy> we add read-only multi-attach volumes as an option, and allow bootable=true for that combo
16:45:46 <johnthetubaguy> I say we, that is purely a Cinder change at that point
16:45:53 <johnthetubaguy> all of this is enforced by cinder
16:46:02 <groot> sigh
16:46:21 <johnthetubaguy> so over the last few weeks that seems to be where we landed, totally open to other ideas
16:46:56 <johnthetubaguy> the idea is we allow one use case, and come back next cycle to see how we can do better
16:47:03 <mriedem> oh i see from the spec,
16:47:10 <mriedem> "In the first iteration we will have Read/Write volumes and will not control that through policies. Due to this aspect we will not enable to multi-attach a bootable volume for which the check will be on the Cinder side. "
16:47:16 <johnthetubaguy> rather than be really permissive and regret it later
16:47:17 <groot> johnthetubaguy you can toggle a volume from non-bootable to bootable
16:47:35 <mriedem> that sentence totally confused me in PS5
16:47:36 <johnthetubaguy> groot: the policy check should be applied to all those changes, thats fine
16:47:48 <mriedem> but means, you can't have bootable=true and multiattach=true in cinder,
16:47:53 <mriedem> and it's not a policy thing, it's hard-coded
16:47:54 <mriedem> right?
16:47:58 <johnthetubaguy> yeah
16:48:00 <ildikov> mriedem: I wasn't 100% sure what we agreed on regarding to that one, so kept it high level...
16:48:27 <ildikov> mriedem: basically whatever we come up with for the BFV case will be hard coded
16:49:00 <johnthetubaguy> so the problem was we couldn't agree how read-only works, but we though best to only allow BFV for the read-only multi-attach volumes
16:49:05 <mriedem> not sure why nova wouldn't check that honestly, but i do'nt know the details
16:49:10 <johnthetubaguy> nova doesn't
16:49:26 <johnthetubaguy> nova just checkes bootable=true on the volume, cinder enforces the checks
16:49:48 <ildikov> there are multiple options, the current code proposal says not allow to boot from a multi-attach volume
16:50:03 <ildikov> you can identify when Nova tries to boot from a volume and add actions there
16:50:07 <mriedem> so...during bfv in nova, if bootable=true and the volume is r/w and has > 0 attachments, fail
16:50:10 <ildikov> or we play with the flags in Cinder
16:50:10 <mriedem> isn't that what we want?
16:50:17 <johnthetubaguy> so nova only allows bfv is the volume says bootable=true
16:50:43 <mriedem> so you could bfv with a r/w volume once
16:50:49 <johnthetubaguy> cinder can just disallow bootable=true for the cases it thinks that is bad
16:50:57 <ildikov> mriedem: that would be y understanding
16:51:02 <ildikov> *my
16:51:25 <mriedem> i'm thinking longer term interop wise, if cinder is hard-coding this stuff, at what point it starts to allow things and how is that discoverable by a user?
16:51:48 <ildikov> johnthetubaguy: that's another thing, we either say not to boot from multi-attach volume or not to multi-attach a bootable volume
16:51:55 <groot> mriedem eventually you end up back at my proposal FWIW
16:52:01 <ildikov> johnthetubaguy: we need to pick one regardless of any flags or anything
16:53:26 <mriedem> looking at https://developer.openstack.org/api-ref/block-storage/v3/#show-a-volume-s-details
16:53:26 <johnthetubaguy> ildikov: those sound like the same question, I am missing something
16:53:43 <mriedem> the r/w thing is not actually an attribute on the volume itself, it's on the attachment right?
16:53:57 <ildikov> groot: if we do it your way but store the multiattach info on volume level, I'm good :)
16:53:59 <johnthetubaguy> mriedem: that is why I said we ignore that for this cycle
16:54:24 <ildikov> johnthetubaguy: in one case you can boot from the volume in the other you can multi-attach it, but not both
16:54:27 <mriedem> https://developer.openstack.org/api-ref/block-storage/v3/#show-attachment-details
16:54:29 <mriedem> attach_mode
16:54:32 <ildikov> johnthetubaguy: that's not the same for me
16:55:33 <johnthetubaguy> ildikov: but cinder doesn't know what nova has done with the volume, in terms of booting it vs not booting it, Nova would be racey on any checks I believe
16:56:45 <johnthetubaguy> is talking going to make this easier? i.e. a hangout?
16:56:52 <mriedem> just looking at the nova vol attach code, nova always attaches in rw mode unless the connection_info on the attachment says otherwise
16:57:26 <ildikov> johnthetubaguy: never mind, I might've misread one of your lines, so just skip my last comments
16:57:35 <ildikov> johnthetubaguy: hangout would be good
16:57:53 <johnthetubaguy> https://hangouts.google.com/call/kefPGVo77ipoUkuKTMzzAAEI
17:02:50 <ildikov> we switched to the Hangouts ^^
17:03:10 <ildikov> closing this meeting for now as our slot ran out
17:03:15 <ildikov> #endmeeting