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

[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