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/04/26 02:55:19 UTC

[GitHub] [submarine] MortalHappiness opened a new pull request #566: SUBMARINE-806. Create a submarine-database for single user

MortalHappiness opened a new pull request #566:
URL: https://github.com/apache/submarine/pull/566


   ### What is this PR for?
   
   Create a submarine database for a single user via the submarine operator.
   
   Reference: https://github.com/apache/submarine/blob/master/helm-charts/submarine/templates/submarine-database.yaml
   
   ### What type of PR is it?
   [Feature]
   
   ### Todos
   
   ### What is the Jira issue?
   
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-806
   
   ### How should this be tested?
   
   ```
   # initialization
   # Step1: Register Custom Resource Definition
   kubectl apply -f artifacts/examples/crd.yaml
   
   # Step2: Create a Custom Resource
   kubectl apply -f artifacts/examples/example-submarine.yaml
   
   
   # helm
   cd submarine
   helm install submarine ./helm-charts/submarine
   
   # in-cluster
   cd submarine/submarine-cloud-v2
   eval $(minikube docker-env)
   make image
   kubectl apply -f artifacts/examples/submarine-operator-service-account.yaml
   kubectl apply -f artifacts/examples/submarine-operator.yaml
   
   # out-of-cluster
   go build -o submarine-operator
   ./submarine-operator
   ```
   
   ### Screenshots (if appropriate)
   
   + PersistentVolume (helm)
   
   ![helm-pv](https://user-images.githubusercontent.com/47914085/116022379-aa678a80-a67c-11eb-854b-0bbfcf347b01.png)
   
   + PersistentVolume (operator)
   
   **Note**: PersistentVolumes are not namespaced resources, so we add the namespace as a suffix to distinguish them
   
   ![operator-pv](https://user-images.githubusercontent.com/47914085/116022417-bd7a5a80-a67c-11eb-831a-6eb57cbeae16.png)
   
   + PersistentVolumeClaim (helm)
   
   ![helm-pvc](https://user-images.githubusercontent.com/47914085/116022424-c5d29580-a67c-11eb-91a0-5c947dce1434.png)
   
   + PersistentVolumeClaim (operator)
   
   ![operator-pvc](https://user-images.githubusercontent.com/47914085/116022534-f31f4380-a67c-11eb-9635-4ee5f04df312.png)
   
   + Deployment (helm)
   
   ![helm-deployment](https://user-images.githubusercontent.com/47914085/116022555-ff0b0580-a67c-11eb-8ae2-4f77d692acf9.png)
   
   + Deployment (operator)
   
   ![operator-deployment](https://user-images.githubusercontent.com/47914085/116022581-06caaa00-a67d-11eb-94ba-5d5b2bae4a30.png)
   
   + Service (helm)
   
   ![helm-service](https://user-images.githubusercontent.com/47914085/116022599-0e8a4e80-a67d-11eb-8a17-39e44ed028f0.png)
   
   + Service (opertator)
   
   ![operator-service](https://user-images.githubusercontent.com/47914085/116022642-2235b500-a67d-11eb-8ae3-615ee794619e.png)
   
   ### 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.

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

[GitHub] [submarine] MortalHappiness edited a comment on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness edited a comment on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826478362


   @kevin85421 Yes. I've tested it in both in-cluster and out-cluster. The result is the same.


-- 
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.

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

[GitHub] [submarine] MortalHappiness commented on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness commented on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826468114


   @kevin85421 I've finished creating submarine-database for single user, thanks.


-- 
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.

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

[GitHub] [submarine] kevin85421 commented on a change in pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
kevin85421 commented on a change in pull request #566:
URL: https://github.com/apache/submarine/pull/566#discussion_r620828762



##########
File path: submarine-cloud-v2/controller.go
##########
@@ -475,6 +483,230 @@ func (c *Controller) newSubmarineServerRBAC(serviceaccount_namespace string) err
 	return nil
 }
 
+// newSubmarineDatabase is a function to create submarine-database.
+// Reference: https://github.com/apache/submarine/blob/master/helm-charts/submarine/templates/submarine-database.yaml
+func (c *Controller) newSubmarineDatabase(namespace string, spec *v1alpha1.SubmarineSpec) error {
+	klog.Info("[newSubmarineDatabase]")
+	databaseName := "submarine-database"
+
+	databaseImage := spec.Database.Image
+	if databaseImage == "" {
+		databaseImage = "apache/submarine:database-" + spec.Version
+	}
+
+	// Step1: Create PersistentVolume
+	// PersistentVolumes are not namespaced resources, so we add the namespace
+	// as a suffix to distinguish them
+	pvName := databaseName + "-pv--" + namespace
+	pv, pv_err := c.persistentvolumeLister.Get(pvName)
+	// If the resource doesn't exist, we'll create it
+	if errors.IsNotFound(pv_err) {
+		var persistentVolumeSource corev1.PersistentVolumeSource
+		switch spec.Storage.StorageType {
+		case "nfs":

Review comment:
       Does uppercase or lowercase matter in values.yaml?




-- 
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.

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

[GitHub] [submarine] MortalHappiness commented on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness commented on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826478362


   @kevin85421 Yes. I've tested it in both in-cluster ans out-cluster. The result is the same.


-- 
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.

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

[GitHub] [submarine] kevin85421 merged pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
kevin85421 merged pull request #566:
URL: https://github.com/apache/submarine/pull/566


   


-- 
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.

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

[GitHub] [submarine] MortalHappiness commented on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness commented on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826468114






-- 
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.

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

[GitHub] [submarine] MortalHappiness edited a comment on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness edited a comment on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826478362


   @kevin85421 Yes. I've tested it in both in-cluster and out-cluster. The result is the same.


-- 
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.

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

[GitHub] [submarine] kevin85421 commented on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
kevin85421 commented on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826477738


   @MortalHappiness Thank you for your contribution! Have you tested this PR with the in-cluster submarine operator?


-- 
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.

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

[GitHub] [submarine] kevin85421 commented on pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
kevin85421 commented on pull request #566:
URL: https://github.com/apache/submarine/pull/566#issuecomment-826477738


   @MortalHappiness Thank you for your contribution! Have you tested this PR with the in-cluster submarine operator?


-- 
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.

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

[GitHub] [submarine] MortalHappiness commented on a change in pull request #566: SUBMARINE-806. Create a submarine-database for single user

Posted by GitBox <gi...@apache.org>.
MortalHappiness commented on a change in pull request #566:
URL: https://github.com/apache/submarine/pull/566#discussion_r620847180



##########
File path: submarine-cloud-v2/controller.go
##########
@@ -475,6 +483,230 @@ func (c *Controller) newSubmarineServerRBAC(serviceaccount_namespace string) err
 	return nil
 }
 
+// newSubmarineDatabase is a function to create submarine-database.
+// Reference: https://github.com/apache/submarine/blob/master/helm-charts/submarine/templates/submarine-database.yaml
+func (c *Controller) newSubmarineDatabase(namespace string, spec *v1alpha1.SubmarineSpec) error {
+	klog.Info("[newSubmarineDatabase]")
+	databaseName := "submarine-database"
+
+	databaseImage := spec.Database.Image
+	if databaseImage == "" {
+		databaseImage = "apache/submarine:database-" + spec.Version
+	}
+
+	// Step1: Create PersistentVolume
+	// PersistentVolumes are not namespaced resources, so we add the namespace
+	// as a suffix to distinguish them
+	pvName := databaseName + "-pv--" + namespace
+	pv, pv_err := c.persistentvolumeLister.Get(pvName)
+	// If the resource doesn't exist, we'll create it
+	if errors.IsNotFound(pv_err) {
+		var persistentVolumeSource corev1.PersistentVolumeSource
+		switch spec.Storage.StorageType {
+		case "nfs":

Review comment:
       Although in `submarine-database.yaml` there is a line `{{- if eq (.type | lower) "nfs" }}` so we can use "nfs" case-insensitively in our `values.yaml`, for example
   ```
   storage:
       type: Nfs # "host" or "nfs"
       nfs:
         ip: 10.96.0.2
         path: "/"
   ```
   as shown above, the value of *type* can be case-insensitive. However, the *nfs* variable below *type* still need to be case-sensitive. Therefore, I think it is unnecessary to support the case-insensitivity in only *type* parameter. I also write a regex validation in our `crd.yaml` to let the user know that the only valid values are "host" or "nfs".
   
   https://github.com/apache/submarine/blob/2b3903e84468d28256cfa86979dfaff1aaebb32e/submarine-cloud-v2/artifacts/examples/crd.yaml#L76-L81




-- 
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.

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