Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

[Rest Server] Refactor storage by leveraging persistent volumes #4157

Merged
merged 13 commits into from
Mar 8, 2020
Merged

[Rest Server] Refactor storage by leveraging persistent volumes #4157

merged 13 commits into from
Mar 8, 2020

Conversation

abuccts
Copy link
Member

@abuccts abuccts commented Jan 20, 2020 •
edited
Loading

Refactor storage by leveraging kubernetes pv and pvc.

TODO:

  • backward compatibility
  • samba and azure storage tests
  • documents

@abuccts
Refactor storage by leveraging persistent volumes
ecc4aaa 
Refactor storage by leveraging kubernetes pv and pvc.
@abuccts
Fix lint
e2dbb74 
Fix lint.
@abuccts abuccts force-pushed the xiongyf/refactor-storage branch from ddb903a to e2dbb74 Compare January 20, 2020 09:51
@abuccts
Update
9bbc952 
* Add secret data into storage
* Backward compatibility for
  * GET /storage/config
  * GET /storage/server
* Update api docs
@abuccts abuccts force-pushed the xiongyf/refactor-storage branch from eddbd11 to 9bbc952 Compare February 21, 2020 12:04
@abuccts abuccts marked this pull request as ready for review February 21, 2020 12:45
@hzy46 hzy46 mentioned this pull request Feb 24, 2020
Closed
74 tasks
Binyang2014
Binyang2014 reviewed Feb 24, 2020
View reviewed changes
Copy link
Contributor

@Binyang2014 Binyang2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change only keep get and list API.
Do we need add some doc about how to create or delete storage?

And previous tools such as storagectl will not work.
Maybe we need to fix this tool to let admin add storage for certain group. Otherwise, we need to modify configMap manually.

src/rest-server/docs/swagger.yaml
@@ -1317,165 +1317,31 @@ paths:
$ref: '#/components/responses/NoJobConfigError/content/application~1json/examples/NoJobConfigError'
500:
$ref: '#/components/responses/UnknownError'
/api/v2/storage/server/{storage}:
Copy link
Contributor

@Binyang2014 Binyang2014 Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I don't know when to use v1 API and when to use v2 API...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know neither, previous storage uses v2 and just kept the same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK... Hope we can provide a process about how to promote a API from beta to release. And when to deprecated old API....

src/rest-server/docs/swagger.yaml Show resolved Hide resolved
src/rest-server/src/models/v2/group.js Outdated Show resolved Hide resolved
src/rest-server/src/models/v2/job/k8s.js Show resolved Hide resolved
src/rest-server/src/models/v2/job/k8s.js Show resolved Hide resolved
@abuccts
Update
ab6c336 
* fix default config issue
* fix multiple configs in url query
* resolve comments
@abuccts abuccts force-pushed the xiongyf/refactor-storage branch from 6c95e66 to ab6c336 Compare February 26, 2020 09:54
@abuccts
Update api docs
c5722cc 
Update swagger api docs.
@abuccts
Update
9a933a4 
Fix params and query difference in backward compatibility.
@fanyangCS fanyangCS mentioned this pull request Feb 27, 2020
Closed
@abuccts
Add default storages and check deprecated config
21448f9 
Add default storages and check deprecated config.
@abuccts
Update mount path
fbdd6e9 
Update mount path.
@abuccts abuccts force-pushed the xiongyf/refactor-storage branch from a7461eb to fbdd6e9 Compare February 28, 2020 08:20
@abuccts
Update
31cdae5 
* support storage config in NNI
* add accountsastoken for blobfuse flexvolume
@mydmdm
Copy link
Contributor

mydmdm commented Feb 29, 2020 •
edited
Loading

Could we mark to-be-deprecated to the storageConfig, storageServer API and the teamwise_storage plugin, considering

  • they are only released internally and for a short while,
  • we actually want users to use the new API and k8s pv/pvc to mount

@abuccts
Copy link
Member Author

abuccts commented Mar 2, 2020

Could we mark to-be-deprecated to the storageConfig, storageServer API and the teamwise_storage plugin, considering

  • they are only released internally and for a short while,
  • we actually want users to use the new API and k8s pv/pvc to mount

yes, those apis have been removed from api docs in this PR, @Binyang2014 will delete runtime plugin after this PR merged.

user will only see docs on new api and use new, existing code uses old api won't be effected.

abuccts added 2 commits March 4, 2020 14:14
@abuccts
Remove unused code
91020a8 
Remove unused code.
@abuccts
Add default option in storage query
ab8d474 
Add default option in storage query.
@abuccts
Add document on setting up storage
c6c30ff 
Add document on setting up storage.
@abuccts
Copy link
Member Author

abuccts commented Mar 4, 2020

This change only keep get and list API.
Do we need add some doc about how to create or delete storage?

pls refer to this document

And previous tools such as storagectl will not work.

yes, I have added a deprecated notice for it

Maybe we need to fix this tool to let admin add storage for certain group. Otherwise, we need to modify configMap manually.

admin could use paictl or group api to create/update/delete storage for a given group, as described in this document

Binyang2014
Binyang2014 approved these changes Mar 5, 2020
View reviewed changes
Copy link
Contributor

@Binyang2014 Binyang2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abuccts abuccts merged commit f9bf182 into master Mar 8, 2020
@abuccts abuccts deleted the xiongyf/refactor-storage branch March 9, 2020 12:40
@abuccts
Copy link
Member Author

abuccts commented Apr 22, 2020

Test case:

  1. follow docs to setup nfs/samba/azurefile/azureblob persistent volume separately
  2. check whether they're able to be mounted in PAI.

@hzy46 hzy46 mentioned this pull request Apr 22, 2020
Closed
36 tasks
@debuggy debuggy mentioned this pull request Apr 23, 2020
Closed
11 tasks
@hzy46 hzy46 mentioned this pull request Apr 27, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mydmdm mydmdm mydmdm left review comments

@Binyang2014 Binyang2014 Binyang2014 approved these changes

@sunqinzheng sunqinzheng Awaiting requested review from sunqinzheng

@yiyione yiyione Awaiting requested review from yiyione

Assignees
No one assigned
Labels
PureK8S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abuccts @mydmdm @Binyang2014