[GitHub] [submarine] cdmikechen commented on a diff in pull request #999: SUBMARINE-1296. Move seldon-core install to helm

[GitBox <gi...@apache.org>]

cdmikechen commented on code in PR #999:
URL: https://github.com/apache/submarine/pull/999#discussion_r982950114


##########
submarine-serve/installation/install.sh:
##########
@@ -33,13 +33,3 @@ if [[ ! -f ${CURRENT_PATH}/istioctl ]]; then
 fi
 
 ${CURRENT_PATH}/istioctl install --skip-confirmation
-
-kubectl create ns seldon-system
-kubectl apply -f ${CURRENT_PATH}/seldon-secret.yaml
-helm install seldon-core seldon-core-operator \
-    --repo https://storage.googleapis.com/seldon-charts \
-    --namespace seldon-system \
-    --version 1.10.0 \
-    --set istio.enabled=true \

Review Comment:
   We also need to synchronise the variables(`istio.enabled=true` and `executor.defaultEnvSecretRefName=submarine-serve-secret 2> /dev/null`) declared here with `values.yaml` to ensure consistency of configuration.



##########
helm-charts/submarine/templates/seldon-secret.yaml:
##########
@@ -1,3 +1,4 @@
+

Review Comment:
   I've checked this file and found that we need to replace namespace:
   ```yaml
   namespace: {{ .Release.Namespace }}
   ```
   And please ensure that the license information starts on the first line.



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