15:00:08 <bswartz> #startmeeting manila
15:00:09 <openstack> Meeting started Thu Aug 18 15:00:08 2016 UTC and is due to finish in 60 minutes.  The chair is bswartz. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:00:10 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:00:12 <openstack> The meeting name has been set to 'manila'
15:00:17 <cknight> Hi
15:00:19 <rraja> hi
15:00:19 <ganso> hello
15:00:20 <tbarron|afk> hi
15:00:21 <gouthamr> hey o/
15:00:22 <tovchinnikova> o/
15:00:25 <bswartz> hello all
15:00:25 <dsariel> hi
15:00:28 <aovchinnikov> hi
15:00:38 <xyang2> hi
15:00:56 <bswartz> #topic announcements
15:01:14 <bswartz> feature proposal freeze is TODAY!
15:01:43 <bswartz> new features proposed after 2359 UTC will get -2 for newton
15:02:08 <bswartz> everything for newton should be in gerrit and with +1 from jenkins by tonight
15:03:26 <bswartz> feature freeze is R-5, between Aug 29-Sep 02 (basically as soon as we can merge all the features)
15:04:00 <bswartz> #agenda https://wiki.openstack.org/wiki/Manila/Meetings
15:04:08 <bswartz> #topic Driver private data admin API
15:04:15 <bswartz> no name next to this...
15:04:25 <cknight> bswartz: I can lead it
15:04:33 * bswartz grumbles nad looks into the wiki history
15:04:40 <cknight> This is the new API to retrieve private driver share data
15:04:52 <bswartz> thanks cknight
15:04:52 <cknight> The first question is whether this should be in the REST API or in manila-manage
15:04:55 <toabctl> hi
15:05:04 <cknight> It's admin only, for debugging purposes.
15:05:19 <bswartz> there are testing use cases for it too
15:05:22 <xyang2> cknight: we already talked about that question before
15:05:31 <cknight> bswartz: True, and that argues for a REST API
15:05:34 <gouthamr> debugging and testing doesn't seem like a valid need for an API
15:05:46 <cknight> gouthamr: why not?
15:06:01 <gouthamr> this particular API defeats the point of abstraction imho
15:06:24 <tbarron> the counterarg is that it is needed for tempest?
15:06:24 <gouthamr> if you start being able to see what drivers do how they do, what's the point? :s
15:06:34 <bswartz> well the same testing use cases could be addressed with database queries
15:06:35 <toabctl> gouthamr, I agree. for testing and debugging, you can access the db directly
15:06:55 <tbarron> for debug & manual testing one doesn't need rest, right?
15:06:58 <gouthamr> toabctl: +1
15:07:27 <gouthamr> if we really do need to validate, maybe provide a way to access the database.. not an API, not even admin facing..
15:07:34 <bswartz> xyang2: I know we've covered this topic before but I don't remember the conclusion, do you?
15:07:56 <ganso> gouthamr: +1
15:07:58 <xyang2> last time when we talked about this, only Valeriy had strong objections
15:08:01 <gouthamr> cknight xyang2: any reservations using manila-manage?
15:08:14 <xyang2> he has since changed his vote so it allows us to move forward with REST API
15:08:18 <ganso> bswartz: I remember we decided to use manila-manage
15:08:22 <bswartz> I can support manila-manage for this
15:08:27 <cknight> gouthamr: would that still support white-box Tempest tests?
15:08:38 <xyang2> why after so many rounds of addressing review comments, go back to the beginning now
15:08:40 <tbarron> cknight: yeah, that's the question i think
15:08:41 <gouthamr> cknight: we need a way to invoke manila-manage via tempest..
15:08:45 <vkmc> o/
15:08:58 <bswartz> cknight: if we do manila-manage then there's no tempest test because there's no API
15:09:03 <gouthamr> xyang2: every round of my review has the question in it
15:09:05 <gouthamr> :)
15:09:09 <toabctl> xyang2, the process is not ideal, I agree
15:09:10 <bswartz> we'd have to cover it with "functional" tests
15:09:18 <toabctl> but it's also not an argument to add it.
15:09:48 <bswartz> xyang2: you're still in favor of REST API for this?
15:09:49 <toabctl> bswartz, why do you want a tempest test for driver private data? what would that test do?
15:10:02 <xyang2> bswartz: yes
15:10:08 <tbarron> so we're saying use manila-manage instead and get coverage in functional tests rather than tempest, right?
15:10:21 <cknight> toabctl: it would enable white-box tests to ensure the driver was storing what it should
15:10:24 <gouthamr> yes.. i'm not opposed to testing this, thoroughly, but REST API isn't an answer...
15:10:26 <bswartz> toabctl: vendor black box testing would want to know what the driver set for the private metadata
15:10:48 <bswartz> cknight: s/white/black/
15:10:58 <gouthamr> you can easily write a script to invoke manila-manage from your white box "tempest" tests if that's your goal
15:11:18 <tbarron> s/white/black/ and tempest should be black, right?
15:11:24 <toabctl> bswartz, cknight then we should make every DB model accessable via a rest api
15:11:34 <bswartz> tbarron: tempest is emphatically white box tests
15:12:02 <bswartz> tempest knows nothing of the driver or what it does under the covers -- tempest only tests the external API surface
15:12:21 <tbarron> bswartz: that's what i meant to be saying too, may have inverted my colors
15:12:40 <bswartz> there's a legitimate issue with testing of this feature if we do add a REST API which is that you can't write a reasonable test without knowing what the driver is doing
15:12:51 <bswartz> oh wait
15:12:53 <bswartz> DAMNIT
15:12:58 <tbarron> :)
15:13:01 <bswartz> I'm getting my white and black mixed up
15:13:07 * bswartz headdesk
15:13:28 <tbarron> anyways, tempest deals w/ behavior and not internal mechanisms
15:13:28 <ganso> bswartz: got to apply a negative filter
15:13:37 <gouthamr> lol
15:13:43 <bswartz> yes please reverse all my white/black comments
15:13:57 <cknight> bswartz: black tests matter
15:14:04 <bswartz> blackbox is what tempest does, which is only testing the outside API and not knowing the inside
15:14:33 <bswartz> whitebox is what downstream vendors do, where they test the internals of their driver and storage controller
15:14:47 <gouthamr> and tempest is just the vehicle
15:15:08 <bswartz> gouthamr: if I get my way tempest won't be the vehicle much longer
15:15:35 <gouthamr> bswartz: yes.. :P
15:15:39 <bswartz> but let's avoid that rathole
15:15:43 * gouthamr continues to write tempest tests
15:15:45 <tbarron> xyang2: could you get the coverage you need with functional teests and manila-manage?
15:15:47 <bswartz> okay so back to the topic at hand
15:16:06 <xyang2> tbarron: functional tests?
15:16:26 <tbarron> yeah
15:16:30 <bswartz> the proposal on the table is to implement the driver private share data as a manila-manage command, and not to add any REST APIs or tempest tests
15:16:34 <xyang2> tbarron: manila functional tests are tempest tests
15:16:52 <xyang2> tbarron: sorry I didn't get what you meant
15:16:56 <bswartz> we've discussed this before, and it seems that support has grown for the manila-manage approach
15:17:38 <bswartz> who has a link to the current patch?
15:17:52 <bswartz> #link https://review.openstack.org/#/c/315346/
15:17:53 <xyang2> bswartz: manila-manage should be just for db migrations
15:18:06 <xyang2> bswartz: at least in Cinder that is what we are trying to do
15:18:23 <tbarron> xyang2: sorry, learning my way around, i thought we were going to do something between unit and tempest, but I was in cinder context I guess
15:18:28 <bswartz> well cinder doesn't have this feature, so it's not a great comparison
15:18:52 <xyang2> tbarron: np, we are using different terms
15:19:07 <toabctl> I'm not for manila-manage but against another rest api. xyang2 why can't you just so something via the manila rest api and then access the db with whatever mechanism you want to test the result?
15:19:10 <xyang2> bswartz: Cinder has other commands that are not db migrate
15:19:20 <bswartz> you could just as well argue that cinder-manage/manila-manage are just for admin-only-db-things
15:19:20 <xyang2> bswartz: we talked about they should go away
15:19:25 <toabctl> s/so/do/
15:20:19 <toabctl> xyang2, to access the db during your tests, you can use sqlalchemy or even the mysql http plugin to have a DB rest api.
15:20:42 <xyang2> toabctl: well, tempests test should use REST API
15:20:47 <bswartz> toabctl: you're arguing for dropping this whole patch and blueprint
15:20:55 <toabctl> bswartz, hm. I think yes
15:21:09 <xyang2> toabctl: for tempest to access DB directly is wrong
15:21:17 <bswartz> xyang2: no, proper tempest test would never look at the values returned by the API because tempest doesn't know driver specific stuff
15:21:37 <toabctl> xyang2, see bswartz answer
15:21:41 <xyang2> bswartz: so proper tempest should check db directly?
15:21:50 <xyang2> bswartz: how does it know what to check
15:21:53 <bswartz> if we added an API we would need to cover it with tempest tests, but that puts us in a bind because we don't know how to write those
15:22:50 <toabctl> xyang2, if that is for downstream tempest tests (as written in the commit message) you can create an extra tempest plugin and do whatever you want there (also access the db directly)
15:23:13 <bswartz> I feel bad now because xyang and accelazh have been working on this since May and we're still arguing about the right way to do it
15:23:41 <bswartz> however I'm inclined to agree with toabctl
15:23:52 * toabctl feels bad, too
15:24:06 <xyang2> I wish Valeriy has not removed -2 or someone else has put -2 on it earlier, rather than say it now
15:24:12 <toabctl> but adding another API for testing sound just wrong.
15:24:16 <bswartz> yeah
15:24:44 <xyang2> done,  abandoned
15:25:15 <xyang2> If anyone has such a strong opinion, please put -2 on it earlier
15:25:29 <bswartz> xyang2: sorry this didn't get addressed earlier
15:25:34 <gouthamr> i'm sorry, i thought my -1s were not addressed..
15:25:38 <toabctl> xyang2, yes. sorry for that.
15:25:38 <xyang2> that's what -2 is for
15:26:15 <bswartz> I suspect we have a lot of problem of this type where ideas are proposed and someone is opposed but doesn't provide feedback
15:26:28 <bswartz> it's something we should work on as a community
15:27:04 <bswartz> because the result is wasted time for the contributor and reviewers
15:27:08 <xyang2> there is difference between -1 and -2.  If you think it can't be merged at all, please use your core right
15:27:34 <bswartz> your point is well taken xyang2
15:27:46 <bswartz> we have 2 other topics though so I'll move on
15:28:00 <bswartz> #topic Scenario tests are broken
15:28:09 <bswartz> #link http://logs.openstack.org/86/309286/34/check/gate-manila-tempest-dsvm-scenario/610f96e/console.html
15:28:20 <bswartz> no name again
15:28:28 <ganso> bswartz: me
15:28:29 <bswartz> wiki history says ganso
15:28:53 <ganso> just pointing out that something broke scenario tests. I tried to look at recently merged gerrit changes, but haven't found a suspect
15:29:06 <bswartz> ganso: how recently?
15:29:07 <ganso> so it could be other project's commit
15:29:15 <bswartz> and is it consistent or random?
15:29:22 <ganso> bswartz: consistent
15:29:29 <ganso> bswartz: Aug 16th it was working
15:29:39 <bswartz> okay
15:29:40 <ganso> I submitted a patch on Aug 16th, and it ran ok
15:29:48 <bswartz> ganso are you looking at it?
15:29:57 <bswartz> do we need others to jump in an help too?
15:30:13 <ganso> I need it running to validate migration patches
15:30:15 <bswartz> if the gate is broken it will make it hard to achieve the deadline of +1 from jenkins by tonight
15:30:22 <ganso> scenario job has a migration tempest tests
15:30:31 <ganso> bswartz: gate is not broken, just scenario job
15:30:37 <ganso> it is non-voting
15:31:25 <ganso> I am not investing effort to implement a fix myself right now
15:31:36 <bswartz> okay well it should be voting
15:31:37 <ganso> I still have to update my patches
15:31:42 <bswartz> that's something else to fix
15:31:59 <ganso> bswartz: Valeriy was trying to improve its run time before making it voting
15:32:02 <bswartz> anyone have bandwidth to investigate failing scenario tests?
15:32:07 <ganso> bswartz: it hasn't improved much
15:32:13 <ganso> bswartz: and still fails from time to time
15:32:20 <bswartz> ganso: I'm working on that too, but it's on backburner
15:33:12 <bswartz> okay I get that everyone is working on stuff for the feature freeze, but failing tests are important
15:33:36 <bswartz> if this is caused by some other project's commit, then we can expect more of the same kind of problems in the next 2 weeks
15:33:50 <bswartz> if we want any features to get in we need to be quick about tracking down tests failures and fixing them
15:34:10 <ganso> after I update my patches, I may sacrifice review time to try to debug and fix it, which if nobody does I will have to do it in the end.
15:34:39 <bswartz> normally vponomaryov is our hero on these kinds of problems, but he's out for another 11 days
15:35:25 <bswartz> okay last topic
15:35:28 <ganso> the Joker usually wreaks havoc while Batman is on vacation
15:35:38 <bswartz> #topic Revert to snapshot
15:35:45 <bswartz> ganso: yep
15:35:52 <markstur> ganso,  Does that make you Robin?
15:35:56 <bswartz> #link https://review.openstack.org/#/c/340502/
15:36:01 <ganso> markstur: lol definitely not
15:36:02 <bswartz> #link https://review.openstack.org/#/c/356682/
15:36:11 <cknight> So we have discussed revert-to-snapshot before, and some review feedback has pointed out that there should be a required extra spec for that to enable us to preserve that attribute on the share and check for it in the API layer.
15:36:23 <cknight> In considering that, we realized that 'snapshot_support' is overloaded.
15:36:31 <cknight> It has always meant that a driver can 1) create snapshots, and 2) create shares from snapshots.
15:36:34 <bswartz> so the revert to snapshot is a "small" "major" feature which has been hanging around waiting for reviews for a while
15:36:45 <cknight> So before we add new snapshot semantics, we have to fix this overload, because there will be drivers that can create snapshots but not create new shares from them.
15:36:56 <cknight> We have code up that does this (#link https://review.openstack.org/#/c/356682/), with associated Tempest work underway.
15:37:09 <cknight> The revert-to-snapshot patch is now atop the aforementioned one (#link https://review.openstack.org/#/c/340502/), again with needed Tempest work in progress.
15:37:26 <bswartz> yeah it was recently we realized nobody had broken apart the snapshot-support and create-share-from-snapshot capabilities which we agreed to in Tokyo
15:37:42 <cknight> Unfortunately, the mountable snapshots patch will also need to be rebased on this.
15:37:55 <bswartz> we didn't NEED to split them until we had either revert-to-snapshot or mountable-snapshots, and neither of those happened in mitaka
15:38:22 <bswartz> so thanks to cknight for going ahead and doing the split now
15:38:32 <bswartz> it's a surprising large change because it affects most existing drivers
15:38:44 <cknight> bswartz: glad to do it, and I had help from gouthamr
15:39:08 <bswartz> it's fairly easy to review though because all of the driver changes are 1 or 2 liners
15:39:36 <bswartz> is tpsilva here?
15:39:47 <tpsilva> yep
15:40:06 <bswartz> tpsilva: you plan to rebase on top of this right?
15:40:14 <tpsilva> bswartz: yes, working on it
15:40:19 <bswartz> okay
15:40:48 <bswartz> only other thing is to support this in first party drivers
15:40:57 <bswartz> vponomaryov plans to add ZFS support for this, I'll do LVM
15:41:06 <cknight> tpsilva: I tried to design the code & unit tests so your new snapshot semantic would slot in pretty easily
15:41:17 <cknight> tpsilva: holler if you have questions
15:41:24 <ganso> bswartz: you mean revert or mountable?
15:41:35 <bswartz> ganso: revert
15:41:35 <tpsilva> cknight: okay, I'm just finishing up some of the -1's before rebasing
15:41:48 <bswartz> ganso: I could add mountable snapshot support to LVM too if nobody is doing it
15:41:55 <ganso> tpsilva: you still have tempest and LVM to go, right?
15:42:03 <bswartz> ganso: no guarantee on a timeline for that though
15:42:08 <tpsilva> ganso: yes
15:42:30 <ganso> bswartz: wouldn't those have to be up by today?
15:42:42 <bswartz> ganso: for revert it will be
15:42:43 <ganso> bswartz: or are you including in the same patch?
15:42:47 <ganso> bswartz: oh, ok
15:42:55 <bswartz> I was going to ammend cknight's patch
15:43:14 <bswartz> it's easier to test as 1 patch
15:44:01 <bswartz> the main reason I put this topic on the agenda is because we want drivers authors to review cknight's patch as it affects all existing drivers
15:44:16 <bswartz> make sure the new capability being advertised is correct, and if not, fix it
15:44:35 <bswartz> it will now be possible to support snapshots without also supporting create-share-from-snapshot
15:45:09 <bswartz> and we know of some drivers which have that restriction, so those will be able to add snapshot_support=True
15:45:38 <bswartz> #topic open discussion
15:45:40 <ganso> bswartz: I suspect some driver require more drastic changes
15:45:40 <tbarron> i've been reading it and it looks good so far, waiting for the outstanding tempest stuff
15:45:49 <ganso> bswartz: like, ceph driver, IIRC
15:46:03 <ganso> bswartz: it had snapshot_support disabled because it could not support create_from_snapshot
15:46:03 * bswartz looks around for jcsp
15:46:22 * bswartz doesn't find him
15:46:42 <bswartz> ganso: yes, but I'd rather have the driver maintainer support those patches rather than cknight
15:46:44 <tbarron> i will follow up with jcsp and rraja
15:46:49 <bswartz> s/support/submit/
15:47:22 <ganso> bswartz: I assume separate patches won't be accept after today
15:47:26 <bswartz> cknight's patch just preserves the existing behavior after the capability split
15:47:30 * tbarron thinks jcsp may be on vacation
15:47:42 <rraja> i'll help out
15:47:45 <bswartz> ganso: we can make exceptions for small things
15:47:48 <tbarron> rraja: ++
15:47:53 <bswartz> if it's a 1 line to correct a capability then it's fine
15:47:56 <bswartz> 1-liner
15:48:11 <bswartz> if it's adding support for snapshots where none existed before, that should wait for ocata
15:48:16 * tbarron thinks it would be a bug that the capability is reported wrongly :)
15:48:21 <ganso> bswartz: 1-liner could be included in cknight's I guess. But "more drastic" as I said, would be left out
15:48:49 <bswartz> any other topics?
15:48:54 <gouthamr> bswartz: yes..
15:49:02 <cknight> ganso: yes, I'm glad to make trivial updates during the review process
15:49:21 <bswartz> cknight: as long as the driver maintainer is involved
15:49:24 <gouthamr> so, if you see the patch - you'll see the capability is called "create_share_from_snapshot_support" - that's a mouthful
15:49:33 <gouthamr> I was hoping someone here has a better name for it
15:50:08 <cknight> gouthamr: +1, the long name is crystal clear, but it makes for some ugly code wrapping
15:50:19 <bswartz> cknight +1
15:50:29 <bswartz> I like the self-documenting name
15:50:42 <tpsilva> maybe we could drop the 'support' from it
15:50:42 <gouthamr> i proposed "clonable" -> but bswartz and cknight say that it is suggestive of a potential feature that we may have down the road
15:50:44 <bswartz> but if it's too long for pep8 then that's a problem
15:51:09 <cknight> *would be very glad to increase the 80-character limit*
15:51:13 <bswartz> tpsilva: I think the _support suffix is essential and fits with the existing snapshot_support capability
15:51:14 <gouthamr> yeah, i'd hate to misspell it.
15:51:31 <bswartz> gouthamr: add bash completion for manila then...
15:51:37 <tbarron> we already have DHSS, how about CSFS
15:51:48 * bswartz barfs
15:51:53 <ganso> tbarron: lol
15:51:53 <gouthamr> tbarron: we don't have DHSS
15:51:53 <gouthamr> :D
15:51:53 <tpsilva> I like clonable, it fits perfectly with mountable and revertable
15:52:01 * tbarron doesn't like acronyms but there are reasons they exist
15:52:08 <ganso> gouthamr: not officially
15:52:15 <tbarron> it's not a clone though
15:52:22 <tbarron> you are makiing one thing from another
15:52:26 <toabctl> can_create_share_from_snapshot
15:52:38 <tbarron> one kind of thing from another
15:52:41 <tpsilva> tbarron: you're right
15:52:49 <bswartz> IMO, "clone" means create-share-from-share
15:52:59 <tbarron> bswartz: +1
15:53:05 <bswartz> and a snapshot may or may not be involved
15:53:22 <bswartz> it's a feature cinder has that I think we should copy, but it's never been very important
15:53:23 <gouthamr> i wish it were "snapshottable", "clonable", "mountable", "revertible"
15:53:27 <tbarron> and if it's involved it should be as behind-the-scences mechanism
15:53:32 <gouthamr> yeah, something better than "clonable" in this case then
15:53:59 <cknight> clonable_from_snapshot ?
15:54:04 <gouthamr> :D
15:54:06 <tpsilva> CSFSble
15:54:12 <bswartz> okay well let's not debate this one forever -- if someone has feedback on this topic please take it to the review
15:54:14 <ganso> cknight: but then are you cloning a snapshot to create another snapshot?
15:54:22 <bswartz> clone_from_snapshot_support?
15:54:40 <ganso> bswartz: same as above ^
15:54:40 * bswartz -1s is own idea
15:54:46 <ganso> bswartz: lol
15:55:38 <bswartz> I suspect we may have to just shake our fists at the PEP8 gods and stick with the long name here
15:56:16 <bswartz> if it's too painful inside the code we could use an abbreviated variable name
15:56:27 <bswartz> but keep the full name for the API
15:56:37 <bswartz> any late things?
15:56:44 <bswartz> we're almost out of time
15:56:57 <bswartz> okay thanks everyone
15:57:03 <bswartz> #endmeeting