Thursday, 2017-01-19

*** david-lyle has quit IRC00:05
openstackgerritMerged openstack/manila: [Unity driver] VLAN enhancement  https://review.openstack.org/41003700:07
*** catintheroof has joined #openstack-manila00:23
openstackgerritGoutham Pacha Ravi proposed openstack/python-manilaclient: [DNM] Testing this against the access rule API refactor  https://review.openstack.org/42230700:24
*** kaisers has joined #openstack-manila00:27
*** harlowja has joined #openstack-manila00:52
*** kaisers has quit IRC00:54
*** catintheroof has quit IRC01:13
*** catintheroof has joined #openstack-manila01:14
*** catintheroof has quit IRC01:14
*** tuanluong has joined #openstack-manila01:23
*** mtanino has quit IRC01:24
*** cknight has quit IRC01:26
*** lgreg1 has quit IRC01:41
*** kaisers has joined #openstack-manila01:50
*** kaisers_ has joined #openstack-manila02:10
*** kaisers has quit IRC02:13
*** gcb has joined #openstack-manila02:14
*** kaisers_ has quit IRC02:18
*** gouthamr_ has joined #openstack-manila02:35
*** darrenc_ has joined #openstack-manila02:35
*** sage_ has joined #openstack-manila02:37
*** gouthamr has quit IRC02:41
*** sage has quit IRC02:41
*** vkmc has quit IRC02:41
*** darrenc has quit IRC02:41
*** tbarron has quit IRC02:41
*** gouthamr_ is now known as gouthamr02:44
*** tbarron has joined #openstack-manila03:02
*** vkmc has joined #openstack-manila03:06
*** lgreg has joined #openstack-manila03:57
*** cknight has joined #openstack-manila04:00
*** kaisers has joined #openstack-manila04:14
*** absubram has joined #openstack-manila04:33
*** kaisers has quit IRC04:42
*** furlongm_ has quit IRC04:44
*** furlongm_ has joined #openstack-manila04:44
*** absubram has quit IRC05:03
*** cknight has quit IRC05:03
*** kaisers has joined #openstack-manila05:39
*** kaisers has quit IRC05:52
*** kaisers has joined #openstack-manila05:52
*** kaisers has joined #openstack-manila05:53
*** kaisers has quit IRC05:58
*** lgreg has quit IRC06:02
*** gouthamr has quit IRC06:04
*** kaisers has joined #openstack-manila06:23
*** kaisers has quit IRC06:28
*** lpetrut has joined #openstack-manila07:08
*** jprovazn has joined #openstack-manila07:17
*** kaisers has joined #openstack-manila07:24
*** kaisers has quit IRC07:26
*** kaisers has joined #openstack-manila07:26
*** lpetrut has quit IRC07:54
*** lpetrut has joined #openstack-manila08:04
*** ociuhandu has quit IRC08:18
*** lpetrut has quit IRC08:26
*** sapcc-bot has quit IRC08:30
*** carthaca_ has joined #openstack-manila08:30
*** sapcc-bot2 has joined #openstack-manila08:30
*** dgonzalez_ has joined #openstack-manila08:30
*** tpatzig_ has joined #openstack-manila08:30
*** mkoderer_ has joined #openstack-manila08:30
*** mkoderer_ has quit IRC08:32
*** dgonzalez_ has quit IRC08:32
*** carthaca_ has quit IRC08:32
*** tpatzig_ has quit IRC08:32
*** openstackgerrit has quit IRC08:33
*** rraja has joined #openstack-manila08:35
*** sapcc-bot has joined #openstack-manila08:42
*** sapcc-bot2 has quit IRC08:42
*** mkoderer_ has joined #openstack-manila08:42
*** dgonzalez_ has joined #openstack-manila08:42
*** tpatzig_ has joined #openstack-manila08:42
*** carthaca_ has joined #openstack-manila08:42
*** mkoderer_ has quit IRC08:44
*** dgonzalez_ has quit IRC08:44
*** tpatzig_ has quit IRC08:44
*** carthaca_ has quit IRC08:44
*** kaisers has quit IRC08:46
*** kaisers has joined #openstack-manila08:46
*** jprovazn has quit IRC08:52
*** jprovazn has joined #openstack-manila09:03
*** ociuhandu has joined #openstack-manila09:08
*** zhonghua has quit IRC09:40
*** zhonghua has joined #openstack-manila09:40
*** a-pugachev has joined #openstack-manila09:52
*** tuanluong has quit IRC09:56
*** yumiriam has joined #openstack-manila10:01
*** lpetrut has joined #openstack-manila10:03
*** ganso has joined #openstack-manila10:04
gansobswartz: pong10:05
*** lpetrut has quit IRC10:55
*** lpetrut has joined #openstack-manila10:55
*** sapcc-bot has quit IRC11:00
*** sapcc-bot has joined #openstack-manila11:00
*** carthaca_ has joined #openstack-manila11:00
*** dgonzalez_ has joined #openstack-manila11:00
*** tpatzig_ has joined #openstack-manila11:00
*** openstackgerrit has joined #openstack-manila11:01
openstackgerritValeriy Ponomaryov proposed openstack/python-manilaclient: Add share group support to Manila client  https://review.openstack.org/33512011:01
*** carthaca_ has quit IRC11:02
*** dgonzalez_ has quit IRC11:02
*** tpatzig_ has quit IRC11:02
openstackgerritValeriy Ponomaryov proposed openstack/manila: Rename consistency group modules to share groups  https://review.openstack.org/40986411:02
*** zhugaoxiao has joined #openstack-manila11:14
openstackgerritValeriy Ponomaryov proposed openstack/manila: Manila Share Groups  https://review.openstack.org/33509311:15
openstackgerritValeriy Ponomaryov proposed openstack/manila: [Tempest] Add functional tests for share groups feature  https://review.openstack.org/35526411:15
*** alyson_ has joined #openstack-manila11:21
openstackgerritAlyson proposed openstack/python-manilaclient: Add mountable snapshots support to manila client  https://review.openstack.org/34562511:24
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552611:27
*** zhugaoxiao has quit IRC11:29
*** zhugaoxiao has joined #openstack-manila11:29
gansovponomaryov, tbarron, markstur: Hello. Could you please review this driver implementation of revert-to-snapshot when you have some review time? https://review.openstack.org/#/c/411923/ Thanks in advance!11:37
openstackgerrityankee proposed openstack/python-manilaclient: Support instance's status of migrating in client  https://review.openstack.org/41482911:45
*** JoseMello has joined #openstack-manila12:01
vponomaryovganso: have you considered "reusing of 'share' tables related to export locations" implementing "mountable snapshots" feature?12:10
gansovponomaryov: I haven't implemented it, tpsilva did12:10
gansovponomaryov: so I don't know if he considered12:11
vponomaryovganso: ok, what do you think about it?12:11
gansovponomaryov: I think the main problem would be the mappings12:11
gansovponomaryov: it is currently mapping to share instances12:12
vponomaryovalso, existing spec https://github.com/openstack/manila-specs/blob/master/specs/ocata/mountable-snapshots.rst omits too many valueable data12:12
vponomaryovrelated to API and DB structure12:13
gansovponomaryov: probably data that was found out later during implementation, but what's in the spec has been agreed on12:13
vponomaryovganso: there is no APi responses12:14
vponomaryovganso: there is no DB table structure12:14
gansovponomaryov: yes, I see it12:14
vponomaryovganso: so, I think we do not have "real" agreement here12:14
vponomaryovat all12:14
gansovponomaryov: indeed, I do not see your +2 on the spec patch12:15
gansovponomaryov: oh wait, I do12:16
gansovponomaryov: was looking at wrong patch12:16
gansovponomaryov: so I guess there was agreement after all12:16
vponomaryovganso: i am saying no one really understands it12:16
vponomaryovganso: valueable parts are just absent12:17
vponomaryovganso: possible approach to reuse 'share' tables is not considered12:17
gansovponomaryov: I can make a spec update, similar to what I am doing for migration, but only after the main patch merges12:17
vponomaryovganso: and not described why it should not be used12:17
vponomaryovganso: only after? ))12:17
vponomaryov^_^12:17
gansovponomaryov: we can decide all this by reviewing the main patch12:17
vponomaryovyes, we can, it is already going this way12:18
gansovponomaryov: at this point, there is no need to debate on the spec, it has been agreed in the past, and possible changes when implementing it are expected12:18
vponomaryovBUT12:18
vponomaryovtime12:18
*** kaisers has quit IRC12:20
*** kaisers has joined #openstack-manila12:21
*** kaisers has quit IRC12:26
*** catintheroof has joined #openstack-manila12:43
gansobswartz: ping12:49
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support to HNAS driver  https://review.openstack.org/41147412:51
openstackgerritRodrigo Barbieri proposed openstack/manila: Mountable snapshots scenario tests  https://review.openstack.org/41200112:51
*** gcb has quit IRC12:59
*** jcsp has joined #openstack-manila13:08
*** gouthamr has joined #openstack-manila13:12
gouthamrganso: ping13:15
gouthamrvponomaryov: ping13:23
*** kaisers has joined #openstack-manila13:27
*** kaisers has quit IRC13:28
*** kaisers has joined #openstack-manila13:29
*** kaisers2 has quit IRC13:31
*** kaisers3 has quit IRC13:31
*** sandanar has joined #openstack-manila13:42
*** tommylikehu_ has joined #openstack-manila13:43
*** kaisers1 has joined #openstack-manila13:43
*** tommylikehu_ has quit IRC13:44
*** tommylikehu_ has joined #openstack-manila13:44
*** tommylikehu_ has quit IRC13:45
*** kaisers2 has joined #openstack-manila13:45
*** tommylikehu_ has joined #openstack-manila13:46
*** eharney has joined #openstack-manila13:49
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Refactor Access Rules APIs  https://review.openstack.org/36966813:58
vponomaryovgouthamr: pong14:04
*** porrua has joined #openstack-manila14:05
*** cknight has joined #openstack-manila14:07
*** porrua has quit IRC14:09
gouthamrvponomaryov: hey! question regarding a bug you raised a long time ago: https://bugs.launchpad.net/manila/+bug/151240314:11
openstackLaunchpad bug 1512403 in Manila "API actions can not be versioned" [Low,Confirmed]14:11
vponomaryovgouthamr: yes?14:12
gouthamrvponomaryov: was wondering what really fails here... i tried using the version decorator to the allow_access methods and running some tests, i get the behavior expected..14:12
vponomaryovgouthamr: tommylikehu says the same14:13
gouthamrvponomaryov: yes, i'm following up after his comment..14:13
vponomaryovgouthamr: looks like it was fixed without regards to this report14:13
vponomaryovgouthamr: but I am not aware about it14:13
vponomaryovtry manila as of that state14:14
vponomaryov2015-11-0214:14
vponomaryovif oyu want to see bug14:14
*** mkoderer has joined #openstack-manila14:14
gouthamrvponomaryov: i don't see a fix committed.. or any changes that stand out in api/openstack/wsgi.py14:15
*** kaisers has quit IRC14:15
*** kaisers has joined #openstack-manila14:16
gouthamrvponomaryov: hmmm, okay will do..14:16
gouthamrvponomaryov: thanks!14:16
gouthamrvponomaryov: though i don't think we want to use the decorator if the code is more readable without it14:19
vponomaryovgouthamr: you mean usage of conditions with microversion comparison?14:19
vponomaryovgouthamr: if yes, then you know why it had "low" priority14:20
gouthamrvponomaryov: yeah makes sense.. i'll confirm that it was fixed as intended and mark that bug appropriately..14:21
vponomaryovgouthamr: also, keeping in mind that it is low priority, I would suggest not spending time on it prior to FF14:21
gouthamrvponomaryov: +114:22
gansogouthamr: pong14:27
gouthamrganso: ^ same question :)14:27
gansogouthamr: ok14:28
gansogouthamr: can you weigh in in the migration patch? regarding hyphens vs a underscore14:28
gansogouthamr: I posted a response there14:28
gouthamrganso: i did...?14:28
bswartzganso: good morning14:29
bswartzganso: I couldn't find you yesterday afternoon14:29
gouthamrganso: yeah, while i like bswartz's suggestion, we can't break consistency.. all of our APIs use _s in parameters.14:29
gansobswartz: morning14:29
gansobswartz: I had left the office14:29
gansogouthamr: we can't break or we should break?14:30
gansobswartz: I posted several responses14:30
gansobswartz: guess we have a discussion ahead of us14:30
bswartzgouthamr: consistency with what?14:31
gouthamrbswartz: consistency with all of manila APIs that send any POST data14:32
bswartzgouthamr: what about other openstack services? is there an official style guide for this kind of thing?14:32
gouthamrbswartz: no i tried to look up one.. maybe we should propose it, afaics, we followed nova closely and nova does the same for API params: http://developer.openstack.org/api-ref/compute/?expanded=create-server-detail14:33
gouthamrbswartz: https://specs.openstack.org/openstack/api-wg/guidelines/naming.html#fields-in-an-api-request-or-response-body14:34
gouthamrbswartz: i referred to this in my email response: "Field names should use the snake_case style, not CamelCase or StUdLyCaPs style."14:35
bswartzyeah I read it14:35
bswartzwell if snake case is preferred I can withdraw my objection14:35
bswartzI just think that snake case is horrifying in HTTP bodies14:35
bswartzI guess my opinion is not shared14:36
vponomaryovgouthamr: export locations for shares violate rules14:37
vponomaryovgouthamr: they use underscore as part of the url14:37
gouthamrvponomaryov: yes, which was the point of your email iirc, we didn't decide if we'd resolve that..14:37
gouthamrvponomaryov: we probably should leave it alone... deprecating an API because we used "_" in the resource name is probably overkill14:38
vponomaryovgouthamr: we should add alias14:39
vponomaryovgouthamr: proper alias14:39
vponomaryovgouthamr: and use it in client14:39
vponomaryovand tempest14:39
gouthamrvponomaryov: +114:39
*** Yogi1 has joined #openstack-manila14:41
vponomaryovtbarron: we already have a lot of big methods14:42
tbarronvponomaryov: so?14:42
tbarronvponomaryov: if you can understand it and are confident it works +214:42
vponomaryovtbarron: yes, my brain hurts when I see this change ))14:43
vponomaryovtbarron: but we have tests14:43
vponomaryovtbarron: functional tests14:43
tbarrontests are not a substitute for being able to understand the code14:43
vponomaryovtbarron: to make sure it works14:43
vponomaryovtbarron: agree14:43
vponomaryovtbarron: in good case, whole this change should be split up to lots of smaller changes14:44
tbarronand they probably won't catch the races that we are fixing here14:44
*** dustins has joined #openstack-manila14:44
tbarronvponomaryov: agree, but I'm not insisting on that14:44
tbarronin general the code in this patch is quite clean14:44
vponomaryovtbarron: clean, just mixing over9000 logical changes )14:44
tbarronbut in probably the most critical part we've made a big ugly hard to understand method even bigger and harder to understand14:45
tbarronI think the general idea is good, I just can't verify that its implementation is correct.14:45
vponomaryovtbarron: that is why you should insist on splitting this up )14:46
vponomaryovtbarron: if you cannot see the whole picture14:46
vponomaryovtbarron: but I guess gouthamr will be upset )14:46
tbarronI am insisting that we split up this method.14:46
tbarronvponomaryov: as you know I don't worry too much about upsetting people.14:47
gouthamrwas upset writing that method, trust me14:47
gouthamrlol14:47
tbarronI have customers14:47
tbarronThey being upsset is more my concern.14:47
tommylikehu_ping zengyingzhe14:48
vponomaryovgouthamr: mission impossible - split it up so everyone is happy to merge it right away =)14:48
gansovponomaryov: Could you please elaborate on why you are requesting this? https://review.openstack.org/#/c/345526/44/manila/api/v2/share_snapshots.py14:48
gansovponomaryov: share export locations is separate from share module14:49
gouthamrvponomaryov: yessir. onnit. :) it's pretty easy to split up.. maybe i just got used to it with all these patchsets.. too bad i was the only one getting used to it :P14:49
gansogouthamr: I got used to it too...14:49
vponomaryovganso: do you have the answer for question raised there?14:49
gouthamrganso: :D i know you did. gave you some inspiration for all that nice beer14:50
*** breitz has quit IRC14:50
gansovponomaryov: answer is because it looks good, as share module14:50
*** breitz has joined #openstack-manila14:50
gansogouthamr: lol xD14:50
vponomaryovganso: not strong enough argument14:50
gansovponomaryov: also, consistency14:50
vponomaryovganso: better to rewrite share's part too14:51
gansovponomaryov: what's the problem with it?14:51
gansovponomaryov: it is better organized this way14:51
gansovponomaryov: and access rules do not have its own controllers, you don't have "get" access rule API actions, just index14:51
*** dustins has quit IRC14:51
*** dustins has joined #openstack-manila14:52
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Refactor Access Rules APIs  https://review.openstack.org/36966814:55
vponomaryovganso: "access rules" are made in the same way as export locations - nested objects14:56
vponomaryovganso: so, export locations could be implemented using completely the same approach14:57
gansovponomaryov: could, but if it is more organized implemented this way, why shove everything under a single module?14:58
*** mtanino has joined #openstack-manila14:58
vponomaryovganso: you handle alike-objects in different ways14:59
vponomaryovganso: it cannot be "more organized"14:59
vponomaryovganso: it is exactly vie versa14:59
vponomaryovs/vie versa/vice versa/14:59
gansovponomaryov: how?15:00
gansovponomaryov: what would be the advantages of merging the share_snapshot_export_locations and instance modules with the share_snapshot module?15:01
vponomaryovlater15:01
*** xyang_ has joined #openstack-manila15:02
*** a-pugachev has quit IRC15:05
*** a-pugachev has joined #openstack-manila15:06
*** a-pugachev has quit IRC15:07
*** rraja has quit IRC15:18
*** makowals has joined #openstack-manila15:21
*** porrua has joined #openstack-manila15:21
*** porrua has quit IRC15:26
*** lgreg has joined #openstack-manila15:29
*** timcl has quit IRC15:45
*** mtanino has quit IRC15:46
*** lpetrut has quit IRC15:50
*** tommylikehu_ has quit IRC15:54
*** timcl has joined #openstack-manila15:59
gouthamrganso: "is there any way to have more than one snapshot instance on a non-replicated share?" -> no16:01
gansogouthamr: great16:01
gouthamrganso: i'm assuming you meant one snapshot instance per snapshot16:01
gansogouthamr: guess I coded that snapshot mapping thing in advance xD16:01
bswartzganso: I don't see where we're disagreeing16:02
bswartzerrors occur rarely, but when they do, they're fatal16:02
gansobswartz: so, right now, for migration, your comment is not valid... but you bring up a valid point where the status of replicated snapshots are not consistent with share replicas16:02
bswartzhold on I need to go back and look at my comment16:03
gouthamrganso: how? we always pick up the active share instance's snapshot instance as the snapshot16:03
gansogouthamr: that's why, the order = (constants.STATUS_MIGRATING, constants.STATUS_AVAILABLE, constants.STATUS_ERROR) does not matter because we only have one16:04
bswartzganso: we will we have during an active migration or after a failed migration?16:05
gansobswartz: that order of status we are talking about is for the active instance, the one that is shown when user does snapshot-list while a migration is in progress16:05
gansobswartz: we should always show MIGRATING in this case, that's why it is on top16:06
bswartzokay my comment was on the model itself -- so it will affect all snapshots that have 2 or more instances16:06
*** david-lyle has joined #openstack-manila16:06
gansobswartz: regarding AVAILABLE vs ERROR, it will relate only to replicated snapshots, which are not applicable to migration at all16:06
bswartzganso: you comment was "does not matter because we only have one" -- won't we have 2 while migrating?16:07
gansobswartz: yes, but we should always show MIGRATING16:07
bswartzmigration should sort higher than available, so it should should migrating normally16:07
gouthamrbswartz: we will have two when we're migrating, but we pick up the one that's 'migrating'16:07
bswartzhowever once an error occurs, should migrating sort higher than error?16:07
bswartzbecause an error should stop the migration IMO16:08
gansobswartz: still should, IMO16:08
gansobswartz: it does16:08
bswartzso what happens?16:08
gouthamrbswartz: once an error occurs where/16:08
gansobswartz: an exception during migration will be raised16:09
gansobswartz: this error could happen in migration-start/continue or migration-complete16:09
gansobswartz: if it happens in start/continue, it is cleaned up, the source snapshot instance is revert back to AVAILABLE, and the destination is deleted (we don't care about the destination in a failed migration that did not reach migration-complete, it can be discarded as everything is preserved in the source)16:10
bswartzokay that's importnat16:10
bswartzis there any case where it doesn't get cleaned up automatically?16:10
gansobswartz: yes, if migration-complete fails during a driver-assisted migration16:11
*** carthaca_ has joined #openstack-manila16:11
*** tpatzig_ has joined #openstack-manila16:11
bswartzin that case you're left with 2 instances?16:11
bswartzand what would their states be?16:11
gansobswartz: no, the source is set back to available, the destination is set to error, but not deleted16:12
gansobswartz: sorry, yes, 2 instances16:12
gansobswartz: the source instance is revert back to available so it can be used16:12
gansobswartz: if stuck in migrating status, it is unusable for the user16:12
gouthamrganso: if the source share is set to migration_error, why can't you set the source snapshots to the same? why 'available'?16:13
*** tpatzig_ has quit IRC16:13
*** carthaca_ has quit IRC16:13
gouthamrganso: i.e, how can you guarantee that migration didn't mess anything up?16:13
gansogouthamr: we can't, that's why we don't delete16:13
gansogouthamr: the source share is set to 'available', not 'error' gouthamr16:13
bswartzbut we want to prevent the user from doing anything to the share until it's cleaned up16:14
gouthamrganso: ? we do16:14
gansogouthamr: /16:14
gansogouthamr: ?16:14
gouthamrganso: sorry i missed that.. we have a migration_error status for the same reason16:14
*** zhonghua2 has joined #openstack-manila16:14
gansogouthamr: we have the task state16:14
gansogouthamr: not status16:14
gouthamrganso: if the share is available, it can be used by others..16:14
bswartzokay so the problem here is that we're discussion code that affects all snapshots in the context of one very specific workflow16:15
gansobswartz: yes16:15
bswartzI think I'm convinced by ganso that nothing bad can happen here with migrations of snapshots16:15
bswartzbut the next feature to use snapshot instances might be different16:15
bswartzand this code is in the code snapshot model16:15
*** zhonghua has quit IRC16:16
gansobswartz: so, there was no order at all before16:17
gansobswartz: an order had to be added for MIGRATING to be on top16:17
gansobswartz: thing is, the current order for shares is available on top of error16:17
bswartzand if we follow my suggestion then what bad thing happens?16:17
gansobswartz: the user will see the snapshot in error state, while the share is in available state, which is inconsistent16:18
gouthamrganso: have you considered the aggregate status below?16:18
gansobswartz: if we are changing the order, the share instances order has to be changed as well16:18
gansogouthamr: that is one confusing field16:19
bswartzganso: not at all16:19
bswartzyou can have error snapshots on normal shares16:19
bswartzany time you fail to create a snapshot it will be in error state16:19
gansogouthamr: I am not sure I understand the difference between aggregate status and the regular snapshot['status'] which already aggregates snapshot instances status (it shouldn't do snapshot.instances[0] at all)16:19
gouthamrganso: aggregate status is what is used in the view builder16:20
gouthamrganso: taht's what the user sees16:20
gansogouthamr: why do we have this?16:20
gansogouthamr: and not snapshot.instance that selects the proper instance?16:20
bswartzbecause snapshots don't have statuses, snapshots instances do16:21
gansobswartz: same as shares'16:21
*** sandanar has quit IRC16:21
gansobswartz: we do not have aggregate statuses on share's, we select the proper instance when one does share.instance16:21
bswartzerrr16:22
*** a-pugachev has joined #openstack-manila16:22
bswartzokay so maybe that needs fixing16:22
bswartzthey should work the same16:22
*** absubram has joined #openstack-manila16:22
bswartzI was only reviewing the new code in ganso's patch16:22
gansobswartz: yes, I was trying to get them to work the same16:23
gansobswartz: I also found it every weird that the aggregate status in snapshots are in opposed order as the share's?16:23
gansobswartz: s/?/16:23
* bswartz sighs16:24
gansobswartz: s/every/very16:24
bswartzlooks like for shares the error instances are sorted last?16:24
gansobswartz: exactly16:24
* bswartz headdesk16:24
gansolol16:24
gouthamrmisses u_glide16:24
bswartzI need my git blame16:24
gansoso, I remember gouthamr and I touched that order last16:25
gansothat's why it is important that gouthamr is here so we can undestand why it is like this16:25
gouthamras was necessary with migration and replication, only uses of share instances so far16:25
bswartzcknight's name is on that order list16:26
bswartzgouthamr's on the rest of that function16:26
gansogouthamr: yes, so if we agreed to that in the past, why would snapshot's order be any different?16:26
bswartzLet me see if I can find out why we made this decision for share instances16:27
gouthamrganso: we use snapshot statuses to determine the health across replicas16:27
gouthamrthe user doesn't have APIs to check the exact snapshot replicas16:27
bswartzit's lunchtime here and I'm not able to argue on an empty stomach16:27
gansogouthamr: what about the health of a share across replicas?16:27
gouthamrthere are share replica APIs16:27
gansoso I guess we are missing an API then16:28
gansoIMO this field is being overloaded and due to that, possibly misused16:28
gouthamryes,  aggregate_status could be bandaid code for some late-breaking design decisions in mitaka.. however, if we haven't changed it since, i don't see how it's broken..16:28
* ganso is referring to the 'aggregate_status'16:28
gouthamrit's not16:29
gouthamrit was built specifically for the view builder on snapshots16:29
gouthamruser lists a snapshot of a share - needs to know if it's truly "available" across all instances of the share16:29
gansogouthamr: why would the snapshot viewbuilder result be different from snapshot.instance?16:30
gansogouthamr: 'status': snapshot.get('aggregate_status'),16:31
gansogouthamr: if it was a separate field that would be awesome and not confusing at all16:32
gouthamri take back what i said about the view builder16:32
gouthamrit's used as the snapshot's status everywhere else (and that is correct)16:32
gouthamrthough, no, i don't have an answer right off the top of my head why this can't be instance that we pick for a snapshot16:33
gansogouthamr: except snapshot.instance, which is mostly used internally for all actions16:33
gansogouthamr: ok, I think we need a separate patch to fix that16:33
gansogouthamr: I guess the code to select the instance in my patch is correct, because snapshot.instance is used for actions. But maybe aggregate status should have MIGRATING on top as well then16:35
gansogouthamr: it is not the solution to the problem, but it also shouldn't be fixed within my patch, I only need to make the changes for migration to work16:35
gouthamrganso: hmmm... i think i see an answer here: aggregate status is used for status checks all over.. and is most important in the fragile exchange between the driver and the share manager in replication operations16:40
gansogouthamr: so, it must not be touched? how can MIGRATING fit in there?16:41
gouthamrganso: you'd add it before 'available'16:42
gansogouthamr: ok, thanks16:43
gouthamrganso: and make sure to create the destination snapshot instance with 'migrating' status as well..16:43
gansogouthamr: it is MIGRATING_TO16:43
gouthamrganso: yes..16:43
*** absubram has quit IRC16:47
gansogouthamr: are there more updates incoming to the refactor access rules patch?16:48
gouthamrganso: no16:48
gansogouthamr: ok, thanks16:48
*** absubram has joined #openstack-manila16:49
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Tooz integration  https://review.openstack.org/31833616:51
*** david-lyle has quit IRC16:55
*** absubram has quit IRC16:55
*** absubram has joined #openstack-manila16:59
*** timcl has quit IRC17:00
*** xyang_ has quit IRC17:01
*** absubram has quit IRC17:08
*** timcl has joined #openstack-manila17:08
openstackgerritVitaliy Levitski proposed openstack/manila: debug  https://review.openstack.org/42276917:10
*** tommylikehu1 has joined #openstack-manila17:15
*** zengyingzhe_ has joined #openstack-manila17:15
*** tommylikehu has quit IRC17:18
*** tommylikehu1 is now known as tommylikehu17:18
*** david-lyle has joined #openstack-manila17:18
*** david-lyle has quit IRC17:18
*** zengyingzhe has quit IRC17:18
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552617:19
*** kaisers has quit IRC17:21
*** lpetrut has joined #openstack-manila17:27
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552617:27
*** david-lyle has joined #openstack-manila17:28
*** mkoderer has quit IRC17:36
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552617:37
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552617:55
*** absubram has joined #openstack-manila17:56
*** xyang_ has joined #openstack-manila17:58
*** lgreg has left #openstack-manila18:11
*** david-lyle has quit IRC18:18
*** catinthe_ has joined #openstack-manila18:18
*** catintheroof has quit IRC18:18
*** ociuhandu has quit IRC18:35
*** absubram has quit IRC18:36
openstackgerritRodrigo Barbieri proposed openstack/manila: Share Migration Ocata Improvements  https://review.openstack.org/40630518:37
openstackgerritRodrigo Barbieri proposed openstack/python-manilaclient: Implement Share Migration Ocata improvements  https://review.openstack.org/40630618:42
bswartzganso: So I dug into the history of the share instance / snapshot instance evolution18:49
bswartzganso: gouthamr pointed out a few things that are important I Think18:49
bswartzganso: would you rather discuss here or in a meeting of some kind?18:49
gansobswartz: we can discuss here18:50
bswartzk18:50
*** a-pugachev has quit IRC18:50
bswartzso our first observation is that the sort order for share instances is probably wrong, and error should come first in that list18:51
bswartzif any instance of a share is in an error state, then the share should be in an error state18:51
*** xyang_ has quit IRC18:51
bswartzthe existing code is just dangerous because it allowed error instances to exist without preventing any further actions18:51
bswartzsecond, when migrations fail, we probably should not be setting the source instance back to "available"18:52
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552618:52
gansobswartz: why not?18:53
bswartzin many cases that might be the right thing to do, but as more vendors implement driver-assisted migraiton it might become a significant burden to always ensure that the source side is always something you can revert back to18:53
gansobswartz: I thought that was the most important requirement18:53
bswartza more generic approach is to allow everything to go into an error state when the migration fails, so the admin can clean it up18:53
gansobswartz: We decided that in a meeting18:53
bswartzare you talking about austin? or earlier or later?18:54
gansoaround newton midcycle probably18:54
bswartzbefore we talked about driver-assisted migration and we were focused on the fallback approach, that was true18:54
bswartzthe fallback approach (which we write and own) should never corrupt your original copy18:55
bswartzbut when drivers are doing this themselves, who knows how things might work internally?18:55
bswartzthey could make state changes which are hard or impossible to revert18:55
bswartzdo we want to say that such approaches are automatically invalid?18:55
gansobswartz: if we follow this road, it may end up that the driver-assisted migration has risks of losing data18:56
*** yumiriam has quit IRC18:56
bswartzganso: I don't think we'd go that far18:56
gansobswartz: if we remove this requirement, I think we woudl18:56
bswartzyou could end up with a driver-assisted migration that results in loss of access to your data until an admin is able to clean up a failure18:56
*** lpetrut has quit IRC18:57
*** xyang_ has joined #openstack-manila18:57
bswartzthe theory is that admins are able to recover when migrations fail18:57
bswartzusing some out-of-band operations and a reset-state on manila18:57
gansobswartz: are you talking about failures in migration-complete or any point in migration?18:57
bswartzmostly the complete case18:57
gansobswartz: ok18:58
bswartzbecause migrations are supposed to be cancel-able before then18:58
gansobswartz: yes, exactly18:58
bswartzbut I haven't thought through all the corner cases18:58
bswartzanyways I think it's worth considering the possibilty of both the migrating_from and the migrating_to instances going to a migration error state when something goes wrong18:59
bswartzthe third issue was the most important18:59
*** ianychoi has quit IRC19:00
gansobswartz: it will be a little bit harder to identify which is the source and which is the destination, only admin will be able to identify through the 'host' field... the code always checks for these statuses19:00
gansobswartz: the code will not be able to distinguish between source and destination anymore19:00
bswartzgouthamr explained how the ShareSnapshot.instance() method works today and has a different idea of what we should do for migrating snapshots19:00
bswartzganso: that's a good point19:01
bswartzganso: but the admin will have the actual backends to look at, and the logs to19:01
bswartzhe'll be able to figure it out19:01
bswartzbesides it might be a moot point -- in the case of a migration, either side might be the "good" side after a failure19:02
bswartzit's equally likely that the admin will want to recover the source vs the destination19:02
gansobswartz: during migration-complete, most likely yes19:02
gansobswartz: what's the idea for ShareSnapshot.instance() ?19:03
openstackgerritVitaliy Levitski proposed openstack/manila-ui: Add MapRFS protocol  https://review.openstack.org/42188419:04
bswartzgouthamr pointed out that for replicated shares, the snapshot instance that should be exposed depends on the share instances states more than the snapshot instance states19:04
bswartzno matter what the status of the snapshots instances are, we want to expose the snapshot instance of the active share instance19:05
bswartzI think the same applies to migrating snapshots19:05
gansobswartz: yes, but at this moment we do not support migration with replicas19:05
bswartzif the share is migrating, we should find the snapshot instance attached to the source instance always19:05
gouthamrin this case, the source of the migration19:05
gansobswartz: yes I already do that19:05
*** dustins has quit IRC19:06
gouthamrganso: can we do that without the sorting?19:06
bswartzbut it should not be based on snapshot status19:06
gansobswartz: within the code, it is no problem, the only concern was what the user sees when issues "manila list" or "manila snapshot-list" commands19:06
bswartzit should be based on share status19:06
gansogouthamr: the sorting is for listing19:06
gouthamrganso: we want to deliberately pick only the migrating instance's snapshot instance19:06
gouthamrganso: listing at the API?19:06
gansogouthamr: yes19:06
openstackgerritAlyson proposed openstack/python-manilaclient: Add mountable snapshots support to manila client  https://review.openstack.org/34562519:06
gansogouthamr: only that19:06
gouthamrganso: hmmm, can we specifically pick the one we're interested in, by just looking at the share instance statuses?19:07
bswartzthe point is that this model code gets used all over the place, not just for the list commands19:07
gansogouthamr: we would need to change snapshot list view logic19:07
gansobswartz: there are other places, not only listing or migration19:08
gansobswartz: like, let's say we are migrating19:08
gansobswartz: and user wants to create a share from the snapshot that is being migrated19:08
gansobswartz: I believe that currently there is nothing preventing that, and why would we right?19:08
gansobswartz: so, create share from snapshot API does snapshot.instance... totally unaware that there are more than one instance19:09
bswartzganso: we probably shouldn't allow that19:09
bswartzthere's a built in race condition there19:09
bswartzthe snapshot creation will race against the migration competing19:09
gansobswartz: well that was an example, but how about access rules19:09
bswartzerr the share creation, not snapshot19:10
bswartzs/competing/completing/19:10
*** dustins has joined #openstack-manila19:10
gouthamrganso: for access rules, we will disallow adding/removing rules if ANY share instance is in an invalid state19:10
gansogouthamr: MIGRATING will be an invalid state?19:11
*** furlongm_ has quit IRC19:12
*** furlongm_ has joined #openstack-manila19:12
bswartzganso: I like the idea of blocking everything by default, until we know it's safe for 2 operations to proceed in parallel19:12
bswartznot being rigorous about this stuff is how the code got riddled with race conditions19:12
gansobswartz: what is the advantage of the share being accessible in readonly or writable if the user cannot add rules?19:14
bswartzganso: there's a difference between the share being accessible for I/O purposes and the share being accessible through manila for management purposes19:15
gouthamrganso: there's a huge advantage in not disrupting existing clients19:15
bswartzall the manila services can be taken offline, and we still expect NFS operation to existing shares to continue without problems19:15
*** xyang_ has quit IRC19:17
*** cdelatte has joined #openstack-manila19:18
*** xyang_ has joined #openstack-manila19:18
gansobswartz: ok so we are agreeing that we will change things again, MIGRATING status will block everything else, is that right?19:19
gansogouthamr: If that's so, I gotta update the cast_to_readonly patch19:19
*** ociuhandu has joined #openstack-manila19:19
*** xyang_ has quit IRC19:20
bswartzganso: that seems like a safer default to me19:20
gansobswartz: ok19:20
bswartzunless we're going to spend the time to look at operations which should be safe to do at the same time and make sure we have all the races covered19:20
bswartzthe interesting cases are parellel operations that occur right before or after the migration starts or completes19:21
gansobswartz: yes19:21
gansobswartz: some operations are ok for the most part19:21
gansobswartz: unless we increase the granularity of the statuses19:21
bswartzI agree a create-share-from-snapshot in the middle of a long migration should be safe19:21
gansowith several -ING statuses19:21
bswartzsimilarly changing access19:22
bswartzbut they're not safe right around the transitions and unless we have good locks we simply shouldn't allow them19:22
gansobswartz: but we don't have time to do that right now :\19:22
bswartzright19:22
gansobswartz: also, our locks are worthless in a HA environment without Tooz19:23
*** timcl has quit IRC19:24
*** mtanino has joined #openstack-manila19:25
bswartzmerge the tooz patch! :-D19:25
gansobswartz: so the less possibility for race conditions, the better19:25
bswartzagreed19:25
*** absubram has joined #openstack-manila19:25
gansobswartz: I haven't tested it myself, I trust the judgement of the other cores working on it :)19:25
*** furlongm_ has quit IRC19:26
gansobswartz: about the ordering change19:27
gansobswartz: are we going to fix that in a separate patch?19:27
*** timcl has joined #openstack-manila19:27
bswartzfor share instances?19:27
gansobswartz: ya19:27
bswartzyes don't worry about fixing that in your patch19:27
bswartzwe should bug it and deal with it after FF19:27
gansobswartz: I think it is better, it has been broken for a while, it will be confusing for a migration patch to change this behavior, this is a line the migration patch is not currently changing19:27
gansobswartz: ok19:28
bswartzfor your patch I'd like to see the snapshot instance returned always be the migration source when a migration is in progress19:28
bswartzif that's not too much effort to implement19:28
*** furlongm_ has joined #openstack-manila19:29
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Tooz integration  https://review.openstack.org/31833619:29
gansobswartz: so that's what we were discussing before you head out for lunch19:30
gansobswartz: gouthamr advised to put the migrating status before available, but still after deleting and error19:30
bswartzI was saying that method should be blind to the snapshot status and look only at the share status19:32
*** openstackgerrit has quit IRC19:33
gansobswartz: that's a change to the view, not migration at all19:33
bswartzit's a change to the model object, which is used all over the place unless I'm mistaken19:34
gansobswartz: I believe it does not look at the share at all currently19:34
bswartzganso: https://github.com/openstack/manila/blob/master/manila/db/sqlalchemy/models.py#L62619:35
bswartzthe existing code uses that filter function on the share instances19:36
gansobswartz: only for replicas19:36
bswartzI'm imagining a similar approach for migration instances19:37
gansobswartz: status == constants.MIGRATING19:37
*** david-lyle has joined #openstack-manila19:48
gansobswartz: the migration code guarantees that the share instance with migrating status has its respective snapshot instance set to migrating as well19:54
gansobswartz: I am coding the logic to check for the share instance status but it seems unnecessary19:54
*** xyang_ has joined #openstack-manila19:57
*** xyang_ has quit IRC19:58
*** absubram has quit IRC20:06
*** jprovazn has quit IRC20:09
bswartzganso: since this model sits underneath everything I think it's important to establish good patterns20:10
bswartzI'm not sure what the next usage of multiple instances will be20:10
*** openstackgerrit has joined #openstack-manila20:11
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Refactor Access Rules APIs  https://review.openstack.org/36966820:11
*** darrenc_ is now known as darrenc20:15
*** xyang_ has joined #openstack-manila20:16
*** zengyingzhe__ has joined #openstack-manila20:25
*** zengyingzhe_ has quit IRC20:25
openstackgerritYogesh proposed openstack/manila: Improve test coverage for share migration  https://review.openstack.org/41855920:28
*** lpetrut has joined #openstack-manila20:29
gansobswartz: ok I added the code, will submit in a few minutes20:30
*** xyang_ has quit IRC20:33
*** xyang_ has joined #openstack-manila20:46
*** pablo|500| has quit IRC20:57
*** kaisers has joined #openstack-manila20:58
*** timcl has quit IRC20:59
openstackgerritRodrigo Barbieri proposed openstack/manila: Share Migration Ocata Improvements  https://review.openstack.org/40630521:07
*** kaisers has quit IRC21:07
*** nkrinner_afk has quit IRC21:15
*** absubram has joined #openstack-manila21:15
*** nkrinner_afk has joined #openstack-manila21:16
*** zhonghua has joined #openstack-manila21:23
gouthamrxyang_ bswartz: The tooz patch is ready for your re-review https://review.openstack.org/#/c/318336/21:25
*** zhonghua2 has quit IRC21:26
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552621:26
*** eharney has quit IRC21:28
openstackgerritYogesh proposed openstack/manila: Improve test coverage for share migration  https://review.openstack.org/41855921:32
*** david-lyle has quit IRC21:35
xyang_gouthamr: ok21:36
* gouthamr xyang_: thank you. 21:37
openstackgerritRodrigo Barbieri proposed openstack/manila: Add cast_rules_to_readonly to share instances  https://review.openstack.org/41916321:37
*** david-lyle has joined #openstack-manila21:39
*** david-lyle has quit IRC21:39
*** david-lyle has joined #openstack-manila21:39
*** Yogi1 has quit IRC21:40
*** xyang_ has quit IRC21:47
*** xyang_ has joined #openstack-manila21:48
*** xyang_ has quit IRC21:49
*** xyang_ has joined #openstack-manila21:49
*** cknight has quit IRC21:51
*** cknight has joined #openstack-manila21:51
*** dustins has quit IRC21:52
*** cdelatte has quit IRC21:58
*** lpetrut has quit IRC22:03
*** JoseMello has quit IRC22:05
*** alyson_ has quit IRC22:07
openstackgerritRodrigo Barbieri proposed openstack/manila: Share Migration Ocata Improvements  https://review.openstack.org/40630522:09
*** adrianofr_ has quit IRC22:29
openstackgerritHelen Walsh proposed openstack/manila: VMAX manila plugin - Support for VMAX in Manila  https://review.openstack.org/40485922:30
openstackgerritGoutham Pacha Ravi proposed openstack/manila: Improve test coverage for share migration  https://review.openstack.org/41855922:45
*** gouthamr has quit IRC22:45
*** ianychoi has joined #openstack-manila22:49
openstackgerritRodrigo Barbieri proposed openstack/manila: Add mountable snapshots support  https://review.openstack.org/34552622:50
*** gouthamr has joined #openstack-manila23:02
*** david-lyle has quit IRC23:03
*** tommylikehu_ has joined #openstack-manila23:03
*** xyang_ has quit IRC23:04
*** tommylikehu_ has quit IRC23:05
*** absubram has quit IRC23:05
*** absubram has joined #openstack-manila23:09
*** makowals has quit IRC23:17
*** absubram has quit IRC23:23
*** cknight has quit IRC23:24
*** makowals has joined #openstack-manila23:55

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!