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 2020/04/27 14:37:13 UTC
[GitHub] [submarine] JohnTing opened a new pull request #270: SUBMARIN-462. Submarine server should support helm charts installation
JohnTing opened a new pull request #270:
URL: https://github.com/apache/submarine/pull/270
### What is this PR for?
Currently, there are no tools to install submarine on k8s in one command.
Helm chart is a perfect tool we can choose. In order to do this:
Create helm charts supporting deployments of tensorflow operator, pytorch operator, and submarine server, MySQL.
Create Dockerfiles used to build submarine components' docker images and create tools to handle the required version push to docker hub.
Setup proxy using kubectl to access the submarine server locally
Verify that sample job spec can be submitted to the submarine server successfully.
Verify that workbench can be accessed in a local browser.
After this, if the user has a k8s cluster. The below command will get the installation done.
helm install <submarine dir>/charts
### What type of PR is it?
Feature
### Todos
* [ ] - Create helm charts supporting deployments of tensorflow operator, pytorch operator, and submarine server, MySQL.
* [ ] - Create Dockerfiles used to build submarine components' docker images and create tools to handle the required version push to docker hub.
* [ ] - Setup proxy using kubectl to access the submarine server locally
* [ ] - Verify that sample job spec can be submitted to the submarine server successfully.
* [ ] - Verify that workbench can be accessed in a local browser.
### What is the Jira issue?
https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-462
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs 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] JohnTing commented on a change in pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #270:
URL: https://github.com/apache/submarine/pull/270#discussion_r421912634
##########
File path: dev-support/docker-images/submarine/build.sh
##########
@@ -35,7 +35,7 @@ submarine_dist_exists=$(find -L "${SUBMARINE_HOME}/submarine-dist/target" -name
# Build source code if the package doesn't exist.
if [[ -z "${submarine_dist_exists}" ]]; then
cd "${SUBMARINE_HOME}"
- mvn clean package -DskipTests
+ mvn clean package -DskipTests -pl '!submarine-cloud'
Review comment:
Thanks for @jiwq comment
When I build submarine-cloud with normal user, there will be a /submarine-cloud/bin/ submarine-operator and requires root permission to delete
I have to manually delete with root permissions to proceed to the next build
So I usually skip submarine-cloud when build
----------------------------------------------------------------
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] jiwq commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
jiwq commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633074681
> tensorflow (tensorflow-operator)
> pytouch (pytouch-operator)
@JohnTing I think we should rename the `tensorflow` to `tfjob` and `pytorch` to `pytorchjob`, because we need to be consistent with with CRD's singular name.
----------------------------------------------------------------
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] tangzhankun commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-632476242
@JohnTing could you please update based on our offline discussion?
@liuxunorg , As we discussed offline, we'll first go with no-operator helm charts. Once the operator is ready. We can easily update to use operator to deploy all things. For 0.4 release, let's go with the simplified version. Does this make sense?
----------------------------------------------------------------
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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-632478897
@tangzhankun, Thanks for your comment
There are now 3 helm charts
submarine (server & database)
tensorflow (tensorflow-operator)
pytouch (pytouch -operator)
The current version is no submarine-operator
Am I missing something?
----------------------------------------------------------------
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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633860842
> @JohnTing , I've discussed with the intention of multiple helm charts with Wanqiang. Only one big concern is that it seems we'd have to release multiple helms chars if we would like "helm install stable/submarine"?
No, after I update dependencies , I can merge 3 helm charts into one now.
----------------------------------------------------------------
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] tangzhankun commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633823502
@JohnTing , I've discussed with the intention of multiple helm charts with Wanqiang. Only one big concern is that it seems we'd have to release multiple helms chars then?
----------------------------------------------------------------
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] tangzhankun commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-635211174
@JohnTing , Thanks for the clarification! LGTM. +1
----------------------------------------------------------------
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] tangzhankun commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633454218
@JohnTing I'm kind of confusing. Why are we making 3 helm charts? Any technical reason for that?
I was expecting a single command like "helm install submarine". Anything I missed?
----------------------------------------------------------------
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] jiwq edited a comment on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
jiwq edited a comment on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-634344776
> > @JohnTing , I've discussed with the intention of multiple helm charts with Wanqiang. Only one big concern is that it seems we'd have to release multiple helms chars if we would like "helm install stable/submarine"?
>
> No, after I update dependencies , I can merge 3 helm charts into one now.
@JohnTing Thanks for your patience and careful work. If we decide merge charts, maybe we should keep only the `submarine` charts under `helm-charts` folder. Due to charts that use `file://` are not allowed in the official Helm repository, so if we want to release it to official repo, you should merge to one chart rather use the dependencies feature.
By the way, the purpose of the split proposal is for easy maintenance and scale at later, also the submarine user can select which machine learning framework can be deployed within him/her cluster. And in the initial stage, I did not consider to release the charts to official repo.
I suggest use the split proposal, but if we want to release it to official repo, I think the single chart is the best practice.
cc: @tangzhankun @liuxunorg
----------------------------------------------------------------
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] JohnTing edited a comment on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-632478897
@tangzhankun, Thanks for your comment
There are now 3 helm charts
submarine (server & database)
tensorflow (tensorflow-operator)
pytouch (pytouch -operator)
The current version is no submarine-operator
Am I missing something?
Wait, I forgot to delete 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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633454781
@jiwq @liuxunorg I have updated this pr, please review it. 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] jiwq commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
jiwq commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-634344776
> > @JohnTing , I've discussed with the intention of multiple helm charts with Wanqiang. Only one big concern is that it seems we'd have to release multiple helms chars if we would like "helm install stable/submarine"?
>
> No, after I update dependencies , I can merge 3 helm charts into one now.
@JohnTing Thanks for your patience and careful work. If we decide merge charts, maybe we should keep only the `submarine` charts under `helm-charts` folder. Due to charts that use `file://` are not allowed in the official Helm repository, so if we want to release it to official repo, you should merge to one chart rather use the dependencies feature.
By the way, the purpose of the split proposal is for easy maintenance and scale at later, also the submarine user can select which machine learning framework can be deployed within him/her cluster. And in the initial stage, I did not consider to release the charts to official repo.
I suggest use the split proposal, but if we want to release it to official repo, I think the single chart is the best practice.
cc: @tangzhankun
----------------------------------------------------------------
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] liuxunorg commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-632479067
> @JohnTing could you please update based on our offline discussion?
> @liuxunorg , As we discussed offline, we'll first go with no-operator helm charts. Once the operator is ready. We can easily update to use operator to deploy all things. For 0.4 release, let's go with the simplified version. Does this make sense?
Good idea! i agree.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #270:
URL: https://github.com/apache/submarine/pull/270#discussion_r421871426
##########
File path: dev-support/docker-images/submarine/build.sh
##########
@@ -35,7 +35,7 @@ submarine_dist_exists=$(find -L "${SUBMARINE_HOME}/submarine-dist/target" -name
# Build source code if the package doesn't exist.
if [[ -z "${submarine_dist_exists}" ]]; then
cd "${SUBMARINE_HOME}"
- mvn clean package -DskipTests
+ mvn clean package -DskipTests -pl '!submarine-cloud'
Review comment:
Why should modifying it?
##########
File path: docs/submarine-server/setup-kubernetes.md
##########
@@ -98,6 +98,17 @@ cd <submarine_code_path_root>/dev-support/k8s/pytorchjob
```
+#### Use Helm Chart to deploy
+It will deploy tensorflow operator, pytorch operator, and submarine server, submarine database in kubernetes.
+If you want to use Helm to install submarine, you must uninstall the existing tensorflow operator, pytorch operator, etc. in k8s.
+
+
Review comment:
```suggestion
```
##########
File path: docs/submarine-server/setup-kubernetes.md
##########
@@ -98,6 +98,17 @@ cd <submarine_code_path_root>/dev-support/k8s/pytorchjob
```
+#### Use Helm Chart to deploy
+It will deploy tensorflow operator, pytorch operator, and submarine server, submarine database in kubernetes.
+If you want to use Helm to install submarine, you must uninstall the existing tensorflow operator, pytorch operator, etc. in k8s.
+
+
+##### Local
+```bash
+helm install submarine helm-charts/submarine
+```
+
+
Review comment:
```suggestion
```
##########
File path: helm-charts/submarine/templates/pytorchjob/rbac.yaml
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+ labels:
+ app: pytorch-operator
+ name: pytorch-operator
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: ClusterRole
+metadata:
+ labels:
+ app: pytorch-operator
+ name: pytorch-operator
+rules:
+- apiGroups:
+ - kubeflow.org
+ resources:
+ - pytorchjobs
+ - pytorchjobs/status
+ verbs:
+ - '*'
+- apiGroups:
+ - apiextensions.k8s.io
+ resources:
+ - customresourcedefinitions
+ verbs:
+ - '*'
+- apiGroups:
+ - ""
+ resources:
+ - pods
+ - services
+ - endpoints
+ - events
+ verbs:
+ - '*'
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: ClusterRoleBinding
+metadata:
+ labels:
+ app: pytorch-operator
+ name: pytorch-operator
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: pytorch-operator
+subjects:
+- kind: ServiceAccount
+ name: pytorch-operator
+ namespace: default
Review comment:
I suggest use the built-in objects, more info see https://helm.sh/docs/chart_template_guide/builtin_objects/
```suggestion
namespace: {{ .Release.Namespace }}
```
##########
File path: helm-charts/submarine/values.yaml
##########
@@ -0,0 +1,21 @@
+# Default values for submarine.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+submarine:
+ server:
+ replicas: 1
+ name: submarine-server
+ image: apache/submarine:server-0.4.0-SNAPSHOT
+ servicePort: 8080
+ database:
+ replicas: 1
+ name: submarine-database
+ image: apache/submarine:database-0.4.0-SNAPSHOT
+ servicePort: 3306
+ mysqlRootPassword: password
+
+customResourceDefinitions:
Review comment:
Can it be deleted?
##########
File path: helm-charts/submarine/values.yaml
##########
@@ -0,0 +1,21 @@
+# Default values for submarine.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+submarine:
+ server:
+ replicas: 1
+ name: submarine-server
+ image: apache/submarine:server-0.4.0-SNAPSHOT
+ servicePort: 8080
+ database:
+ replicas: 1
+ name: submarine-database
+ image: apache/submarine:database-0.4.0-SNAPSHOT
+ servicePort: 3306
+ mysqlRootPassword: password
Review comment:
```suggestion
mysqlRootPassword: password
```
The caller should modify too, if you accept.
----------------------------------------------------------------
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] tangzhankun commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-635152329
@JohnTing Thanks for the hard work! @jiwq @liuxunorg Thanks for the review. Let's first go with the one helm charts without dependencies. This PR looks good to me.
Only one question, how does this image come from? Do we already have a Dockerfile for this? If so, we need to document how to build it in the document.
"apache/submarine:server-0.4.0-SNAPSHOT"
----------------------------------------------------------------
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] JohnTing commented on a change in pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #270:
URL: https://github.com/apache/submarine/pull/270#discussion_r421912634
##########
File path: dev-support/docker-images/submarine/build.sh
##########
@@ -35,7 +35,7 @@ submarine_dist_exists=$(find -L "${SUBMARINE_HOME}/submarine-dist/target" -name
# Build source code if the package doesn't exist.
if [[ -z "${submarine_dist_exists}" ]]; then
cd "${SUBMARINE_HOME}"
- mvn clean package -DskipTests
+ mvn clean package -DskipTests -pl '!submarine-cloud'
Review comment:
Thanks for @jiwq comment
When I build submarine-cloud with normal user, there will be a /submarine-cloud/bin/ submarine-operator and requires root permission to delete
I have to manually delete with root permissions to proceed to the next build
----------------------------------------------------------------
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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-634459834
Thanks @jiwq for your suggestion.
I changed the folder structure.
Placing charts in the charts directory will also automatically deploy,
so there is no need to set the dependency.
This is the current folder structure.
----------------------------------------------------------------
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] tangzhankun edited a comment on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun edited a comment on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-635152329
@JohnTing Thanks for the hard work! @jiwq @liuxunorg Thanks for the review. Let's first go with the one helm charts without dependencies. This PR looks good to me.
Only one question, how does this image come from? Do we already have a Dockerfile for this? If so, we need to document how to build it.
_"apache/submarine:server-0.4.0-SNAPSHOT"_
----------------------------------------------------------------
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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-632705979
@liuxunorg
This pr is done.
There are three helm charts.
submarine (server & database)
tensorflow (tensorflow-operator)
pytouch (pytouch-operator)
Wrote the document in setup-kubernetes.md.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #270:
URL: https://github.com/apache/submarine/pull/270#discussion_r422497580
##########
File path: helm-charts/submarine/values.yaml
##########
@@ -0,0 +1,36 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# Default values for submarine.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+submarine:
+ server:
+ replicas: 1
+ name: submarine-server
+ image: apache/submarine:server-0.4.0-SNAPSHOT
+ servicePort: 8080
+ database:
+ replicas: 1
+ name: submarine-database
+ image: apache/submarine:database-0.4.0-SNAPSHOT
+ servicePort: 3306
+ mysqlRootPassword: password
+
+
+
Review comment:
Should remove the redundant blank lines.
```suggestion
```
##########
File path: helm-charts/submarine/templates/pytorchjob/deployment.yaml
##########
@@ -0,0 +1,50 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: extensions/v1beta1
+#apiVersion: apps/v1
Review comment:
```suggestion
apiVersion: apps/v1
```
##########
File path: helm-charts/submarine/templates/submarine-database.yaml
##########
@@ -0,0 +1,52 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: apps/v1beta1
+kind: Deployment
+metadata:
+ name: "{{ .Values.submarine.database.name }}"
+spec:
+ replicas: {{ .Values.submarine.database.replicas }}
+ selector:
+ matchLabels:
+ app: "{{ .Values.submarine.database.name }}"
+ template:
+ metadata:
+ labels:
+ app: "{{ .Values.submarine.database.name }}"
+ spec:
+ containers:
+ - name: "{{ .Values.submarine.database.name }}"
+ image: "{{ .Values.submarine.database.image }}"
+ ports:
+ - containerPort: 3306
+ env:
+ - name: MYSQL_ROOT_PASSWORD
+ value: "{{ .Values.submarine.database.mysqlRootPassword }}"
+
+---
+apiVersion: v1
+kind: Service
+metadata:
+ name: "{{ .Values.submarine.database.name }}"
+spec:
+ ports:
+ - name: "{{ .Values.submarine.database.name }}"
+ port: 3306
+ targetPort: {{ .Values.submarine.server.servicePort }}
Review comment:
```suggestion
targetPort: {{ .Values.submarine.database.servicePort }}
```
##########
File path: helm-charts/submarine/templates/submarine-server.yaml
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: v1
+kind: Service
+metadata:
+ name: "{{ .Values.submarine.server.name }}"
+ labels:
+ run: "{{ .Values.submarine.server.name }}"
+spec:
+ ports:
+ - port: 8080
+ targetPort: {{ .Values.submarine.server.containerPort }}
Review comment:
```suggestion
targetPort: {{ .Values.submarine.server.servicePort }}
```
----------------------------------------------------------------
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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-635190317
@tangzhankun
Yes, we already have several Dockerfiles to build server and database.
I have updated setup-kubernetes.md.
----------------------------------------------------------------
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] tangzhankun edited a comment on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
tangzhankun edited a comment on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633823502
----------------------------------------------------------------
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] liuxunorg commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-624965319
@jiwq @tangzhankun Please help review this PR, 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] liuxunorg commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-631520799
@JohnTing If you complete this PR, please notify me. 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] JohnTing commented on pull request #270: SUBMARINE-462. Submarine server should support helm charts installation
Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #270:
URL: https://github.com/apache/submarine/pull/270#issuecomment-633470682
> @JohnTing I'm kind of confusing. Why are we making 3 helm charts? Any technical reason for that?
> I was expecting a single command like "helm install submarine". Anything I missed?
A few days ago, jiwq suggested that I should divide charts into three parts https://github.com/apache/submarine/pull/270#pullrequestreview-407925752
----------------------------------------------------------------
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