You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2021/08/05 04:29:23 UTC

[GitHub] [submarine] Kenchu123 opened a new pull request #697: SUBMARINE-953. Submarine-server shouldn't create persistentVolume manually for notebook

Kenchu123 opened a new pull request #697:
URL: https://github.com/apache/submarine/pull/697


   ### What is this PR for?
   <!-- A few sentences describing the overall goals of the pull request's commits.
   First time? Check out the contributing guide - https://submarine.apache.org/contribution/contributions.html
   -->
   
   Source: https://github.com/apache/submarine/blob/72be805fc4672f7f29c96cca95e16d59358d4dd2/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sSubmitter.java#L379
   
   We can find out that when we create a notebook custom resource, the server will manually create the persistent volume for it.
   
   However, this made submarine-server has to grant access to persistent volume resources, which lead to setting cluster roles for the server. (https://github.com/apache/submarine/blob/72be805fc4/helm-charts/submarine/templates/rbac.yaml#L61)
   
   Since the server is in namespace scope, giving it cluster roles is inappropriate. Besides, this makes multi-tenancy more difficult.
   
   To fix the bug, we can create a storage class to dynamically provision persistent volumes, and submarine-server will only need to deal with persistent volume claim.
   
   Ref:
   - storage class: https://kubernetes.io/docs/concepts/storage/storage-classes/#local
   - notebook-cr spec: https://www.kubeflow.org/docs/reference/notebook/v1/ 
   
   ### What type of PR is it?
   [Bug Fix]
   
   ### Todos
   
   * [x] - add storageClass
   * [x] - add persistentVolumeClaim spec to submarine-server
   * [x] - remove submarine-server createPersistentVolume
   * [x] - replace clutserrolebinding to rolebinding in helm-charts
   * [x] - replace clusterrole to role in helm-charts
   * [x] - modify submarine operator 
   
   ### What is the Jira issue?
   <!-- * Open an issue on Jira https://issues.apache.org/jira/browse/SUBMARINE/
   * Put link here, and add [SUBMARINE-*Jira number*] in PR title, eg. `SUBMARINE-23. PR title`
   -->
   https://issues.apache.org/jira/browse/SUBMARINE-953
   
   ### How should this be tested?
   <!--
   * First time? Setup Travis CI as described on https://submarine.apache.org/contribution/contributions.html#continuous-integration
   * Strongly recommended: add automated unit tests for any new or changed behavior
   * Outline any manual steps to test the PR here.
   -->
   
   Can be tested either using `helm install` or `submarine-operator`.
   
   ### Screenshots (if appropriate)
   
   
   https://user-images.githubusercontent.com/17617373/128290829-ff38828c-3555-4c0c-b37d-6b147b762dde.mov
   
   
   
   ### Questions:
   * Do the license files need updating? No
   * Are there breaking changes for older versions? No
   * Does this need new documentation? No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] asfgit closed pull request #697: SUBMARINE-953. Submarine-server shouldn't create persistentVolume manually for notebook

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #697:
URL: https://github.com/apache/submarine/pull/697


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org