Add file share design #27

PravinRanjan10 · 2019-03-27T15:27:40Z

This pr is design about file share that will let user:

create and allow multiple access of a share using only one protocol(eg. NFS/CIFS)
create and allow multiple access of a snapshot

xing-yang · 2019-03-28T13:21:46Z

Please work with Shruthi to add profiles design into this doc.

specs/capri/FileShare_Design.md

Edited Goals Non Goals added Edited Rest API impact Added Data model impact, Rest API impact, Security impact, Other end user impact Added Implementation of file share APIs for Profiles consolidated with block storage

specs/capri/FileShare_Design.md

xing-yang · 2019-04-04T01:21:52Z

specs/capri/FileShare_Design.md

+4. Internally, OpenSDS File Share API talk to controllers.
+5. Controllers will also maintain the File Share metadata in database.
+6. Controller communicates to Dock, specific driver to create a share.
+


Scheduler should be mentioned in the steps.

Other than NBP, user can also use CLI or dashboard.

Added Scheduler tasks

specs/capri/FileShare_Design.md

xing-yang · 2019-04-04T01:26:09Z

specs/capri/FileShare_Design.md

+
+### Data model impact
+
+No


New file share resources, etc. will be stored in the database so there will be data model impact.

Yes. I have updated.

specs/capri/FileShare_Design.md

leonwanghui · 2019-04-09T03:09:16Z

This design would depend on #29, please take a look thanks!

hirokikmr

Hi PravinRanjan10, I added some comments. Please take a look.

specs/capri/FileShare_Design.md

hirokikmr · 2019-04-08T09:44:49Z

specs/capri/FileShare_Design.md

+ ##### Response parameters
+```json
+ “snapshotId”: “string”
+


“shareId” is needed for the response.

Suggested change

“shareId” : “string”

specs/capri/FileShare_Design.md

hirokikmr · 2019-04-08T10:31:04Z

specs/capri/FileShare_Design.md

+                    "isSpaceEfficient"        : "bool"
+                },
+                "ioconnectivity" : {
+                  "accessProtocol" : "string",


Could you tell me what "accessProtocol" is?
If it means NFS or CIFS, I think it is not needed for file share because it's same as "protocols" parameter in '3. POST /v1beta/{tenantId}/file/shares'

Yes it means NFS/CIFS. Based on the create file share request parameter, there will be profiles selection. In controller profile selects dock that contacts the particular driver based on supported accessprocols. So profile should have accessprotocol information.

Thank you for your reply.
I understand that profile should have accessprotocol information.

I have an another question.
The create file share request parameter also has protocols parameter.
Could you tell me relationship between protocols parameter and accessprotocol parameter of profile?
What happens if both protocols parameter and accessprotocol parameter of profile are specified at the same time, which one has priority?

@hirokikmr Thanks for your review! That's a very good observation. I don't think we need to have a separate "protocols" parameter in Create file share request since it is already in Profiles.

@Shruthi-1MN and @PravinRanjan10, I think the Protocols in Create File share can be removed. We can just use the accessProtocol in Profiles instead. Let's make this "accessProtocol" an array ([]string).

@xing-yang Thank you for your reply. I agree with your suggestion.

@xing-yang @hirokikmr, i have removed the "protocols" parameter from POST API of fileshare. And also corrected "accessProtocol" an array ([]string).

AccessProtocol is a string, they should choose only one accessprotocol not multiple.

jimccfun · 2019-04-13T07:15:12Z

specs/capri/FileShare_Design.md

+
+  “profileId”: “string”
+
+  “snapshotId” : “string”


When the file share is creating, the snapshot has not been created and ''snapshotId'' doesn't exist yet.

Yeah right. In this case it will be simply blank. Do you think, we should not return the snapshotId in response, while creation of file share?

I think the snapshotId can be returned in response, While creation of file share, but it should not be a parameter in the request.

"snapshotId" is optional. It is only specified when creating a file share from a snapshot.

@PravinRanjan10 can you specify what field is required and what is optional? That will be more clear.

jimccfun · 2019-04-13T07:30:57Z

specs/capri/FileShare_Design.md

+##### 3.   POST /v1beta/{tenantId}/file/shares
+* Creates a share.
+##### Request
+```json


Whether Add Access is supported when creating file share.

No, Add access will only be supported once the file is created.

specs/capri/FileShare_Design.md

himanshuvar · 2019-04-14T12:38:02Z

specs/capri/FileShare_Design.md

+
+##### Request
+```json
+body*


Please correct the format for highlighting the newly added parameters text.

himanshuvar · 2019-04-15T06:58:36Z

specs/capri/FileShare_Design.md

+```json
+  “id” : “string”
+
+  “createdAt” : “2019-03-20T12:49:00.497Z”


Please make API specification dataType format consistent.

Please add sample API request/response as example.

I think it's ok for design doc to have json format timing detail. It's more clear way to represent.

specs/capri/FileShare_Design.md

himanshuvar · 2019-04-15T09:01:59Z

specs/capri/FileShare_Design.md

+```json
+  “id” : “sting”
+```
+##### 11.	POST /v1beta/{tenantId}/file/accesses/


API endpoint is confusing, acls are defined for file or fileshare. It can be POST /v1beta/{tenantId}/file/shares/{shareId}/acls/

Is it necessary that we should define API for defining acls, Can't we use the PUT fileshare API to provide acls?

PUT /v1beta/{tenantId}/file/shares/{shareId}
"accesscontrol": {
“accessCapability” : read/write/execute
other parameters etc.
}

I think endpoint is clear. we have to give shareId as mandatory parameter to provide acls on fileshare.

It is recommended that acl's should not set while creating the file share(It should be synchronous). And that is the reason we have separate API for that.

himanshuvar · 2019-04-15T09:24:07Z

specs/capri/FileShare_Design.md

+                "datastore""  : {
+
+                   ""characterCodeSet"        : "string",
+                    "maxFileNameLengthBytes"  : "int64",


Could it be possible to group the file share related parameters to File object?

It will impact the block profile APIs, so right now we are not doing it.

himanshuvar · 2019-04-15T13:17:07Z

specs/capri/FileShare_Design.md

+  tenantId* : "string"  //The project UUID in a multi-tenancy environment
+```
+
+##### Response


Could you please add sample request/response as an example.

himanshuvar · 2019-04-15T13:47:24Z

specs/capri/FileShare_Design.md

+  "message": "string"
+}
+```
+##### 2.	GET /v1beta/{tenantId}/profiles


Could you please sample Profile information related to Fileshare.

himanshuvar · 2019-04-15T13:48:20Z

specs/capri/FileShare_Design.md

+  "message": "string"
+}
+```
+##### 6.	POST /v1beta/{tenantId}/profiles/{profileId}/customeProperties


Could you please give example custom properties for FileShare

BaiHuoYu · 2019-04-23T09:58:08Z

specs/capri/FileShare_Design.md

+
+  “size” : “int64”
+
+  “userId” : “string“


How to use "userId"? The actual test found that "userId" in the response is an empty string.

@BaiHuoYu How did you test?. Have you provided dummy userId?
Can you also share your slack id, i can add you in fileshare group for more detail discussion

What is the difference between tenantID and userID? Which should be used when there are multi-tenant scenarios？

BaiHuoYu · 2019-04-26T06:58:37Z

specs/capri/FileShare_Design.md

+
+  “snapshotId” : “string”
+  ```
+##### Response parameters


Protocols is in FileShareSpec, why not use it here?

Do you mean, in the request body?
Since accessProtocols is already there in profile, we don't need here.

How to use Protocols in the profile? Why is there no Protoco in CreateFileShareOpts(model.pb.go)?

leonwanghui · 2019-06-20T07:49:16Z

specs/capri/FileShare_Design.md

+* Facilitating File Share Service by providing Standard API to manage multiple vendors, simplify File Share API definition  
+* File share across the users based on access capability
+* File share facilitates with profiling
+* Scope for now is to support only NFS, SMB file share protocols


What's the plan for CIFS which you mentioned above?

leonwanghui · 2019-06-20T07:53:06Z

specs/capri/FileShare_Design.md

+### REST API impact
+
+YES
+Fileshare API's needs to write and Existing Profile API's needs to be changed.


Please notice it has some grammar errors.

leonwanghui · 2019-06-20T07:53:26Z

specs/capri/FileShare_Design.md

+
+### Data model impact
+
+YES


Please elaborate the data model change here.

leonwanghui · 2019-06-20T07:55:36Z

specs/capri/FileShare_Design.md

+
+### Other end user impact
+
+No


I came up with an impact that with file share feature enabled, users (precisely by admin) need to create fileshare-type profile now.

leonwanghui · 2019-06-20T08:01:28Z

specs/capri/FileShare_Design.md

+     }
+   }
+]
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:01:36Z

specs/capri/FileShare_Design.md

+      }
+    }
+]
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:01:49Z

specs/capri/FileShare_Design.md

+    }
+ }
+]
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:01:59Z

specs/capri/FileShare_Design.md

+    }
+}
+
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:02:04Z

specs/capri/FileShare_Design.md

+##### Response
+```json
+200   OK
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:02:10Z

specs/capri/FileShare_Design.md

+    "key3": "value3"
+}
+
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:02:16Z

specs/capri/FileShare_Design.md

+    "key3": "value3"
+}
+
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

leonwanghui · 2019-06-20T08:02:24Z

specs/capri/FileShare_Design.md

+##### Response
+```json
+200   OK
+400   Unauthorized


400 means Bad Request, Unauthorized is 401.

xing-yang

lgtm

root1 and others added 3 commits March 26, 2019 19:46

PR for fileshare

d90ea03

Merge branch 'master' of https://github.com/opensds/design-specs

60e53bb

Add file share design

3ef7c22

PravinRanjan10 added 3 commits April 1, 2019 11:41

Update FileShare_Design.md

a05e59b

Update FileShare_Design.md

5577c58

Update FileShare_Design.md

ad8cbcd

leonwanghui reviewed Apr 1, 2019

View reviewed changes

specs/capri/FileShare_Design.md Show resolved Hide resolved

specs/capri/FileShare_Design.md Show resolved Hide resolved

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

leonwanghui assigned PravinRanjan10 Apr 1, 2019

PravinRanjan10 added 2 commits April 1, 2019 14:49

Update FileShare_Design.md

a504bae

Update FileShare_Design.md

8deef0a

anvithks reviewed Apr 2, 2019

View reviewed changes

specs/capri/FileShare_Design.md Show resolved Hide resolved

PravinRanjan10 and others added 2 commits April 2, 2019 10:26

Update FileShare_Design.md

68fe306

Added Profile for file share by consolidated block storage profile

36f65f5

Edited Goals Non Goals added Edited Rest API impact Added Data model impact, Rest API impact, Security impact, Other end user impact Added Implementation of file share APIs for Profiles consolidated with block storage

xing-yang reviewed Apr 4, 2019

View reviewed changes

Update FileShare_Design.md

fa59140

hirokikmr reviewed Apr 9, 2019

View reviewed changes

PravinRanjan10 added 3 commits April 9, 2019 11:50

Update FileShare_Design.md

18b3c28

Update FileShare_Design.md

06f6f13

Update FileShare_Design.md

1b1a5ef

jimccfun reviewed Apr 13, 2019

View reviewed changes

specs/capri/FileShare_Design.md Show resolved Hide resolved

himanshuvar reviewed Apr 14, 2019

View reviewed changes

himanshuvar reviewed Apr 15, 2019

View reviewed changes

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

himanshuvar reviewed Apr 15, 2019

View reviewed changes

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

himanshuvar reviewed Apr 15, 2019

View reviewed changes

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

himanshuvar reviewed Apr 15, 2019

View reviewed changes

specs/capri/FileShare_Design.md Outdated Show resolved Hide resolved

himanshuvar reviewed Apr 15, 2019

View reviewed changes

PravinRanjan10 and others added 4 commits April 19, 2019 16:26

Update FileShare_Design.md

cb7de76

Update FileShare_Design.md

cf059a0

Update FileShare_Design.md

b942260

Update FileShare_Design.md

34b2abc

BaiHuoYu reviewed Apr 23, 2019

View reviewed changes

BaiHuoYu reviewed Apr 26, 2019

View reviewed changes

PravinRanjan10 and others added 5 commits April 26, 2019 14:05

Update FileShare_Design.md

c80a259

Update FileShare_Design.md

180d377

Update FileShare_Design.md

789dbc9

Update FileShare_Design.md

1dece1e

Update FileShare_Design.md

d7521bc

leonwanghui reviewed Jun 20, 2019

View reviewed changes

Update FileShare_Design.md

0791461

xing-yang approved these changes Jun 28, 2019

View reviewed changes

xing-yang merged commit cf5188b into sodafoundation:master Jun 28, 2019


		“profileId”: “string”

		“snapshotId” : “string”

Add file share design #27

Add file share design #27

Conversation

PravinRanjan10 commented Mar 27, 2019

xing-yang commented Mar 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonwanghui commented Apr 9, 2019

hirokikmr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shruthi-1MN Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himanshuvar Apr 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himanshuvar Apr 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xing-yang left a comment

Choose a reason for hiding this comment

Shruthi-1MN Apr 10, 2019 •

edited

Loading

himanshuvar Apr 14, 2019 •

edited

Loading

himanshuvar Apr 15, 2019 •

edited

Loading