Initial support for deploying Che to Kubernetes using Helm charts #8715

perspectivus1 · 2018-02-12T08:52:29Z

What does this PR do?

Enable deploying Che to Kubernetes using Helm charts.

What issues does this PR fix or reference?

One of the leftover tasks for SPI Kubernetes infrastructure issue

Release Notes

Docs PR

Signed-off-by: Eyal Barlev <perspectivus@gmail.com>

codenvy-ci · 2018-02-12T08:54:01Z

Can one of the admins verify this patch?

codenvy-ci · 2018-02-12T08:54:04Z

Can one of the admins verify this patch?

codenvy-ci · 2018-02-12T08:56:02Z

Can one of the admins verify this patch?

skabashnyuk · 2018-02-12T08:56:35Z

@eivantsov do you think we need some docs update with this pr?

sleshchenko · 2018-02-12T12:19:32Z

dockerfiles/init/modules/che-kubernetes-helm/templates/configmap.yaml

+  CHE_INFRA_KUBERNETES_NAMESPACE: ""
+  CHE_INFRA_KUBERNETES_TRUST__CERTS: "false"
+  CHE_INFRA_KUBERNETES_PVC_STRATEGY: "common"
+  CHE_INFRA_KUBERNETES_PVC_PRECREATE__SUBPATHS: "true"


Please change it to false according to #8726

sleshchenko · 2018-02-12T13:22:25Z

dockerfiles/init/modules/che-kubernetes-helm/templates/ingress.yaml

+    kubernetes.io/tls-acme: "true"
+{{ else }}
+    nginx.ingress.kubernetes.io/ssl-redirect: "false"
+    ingress.kubernetes.io/proxy-read-timeout: "3600"


Are you sure that there is no need to update read and connect timeout when TLS is disabled?

sleshchenko · 2018-02-12T13:24:06Z

dockerfiles/init/modules/che-kubernetes-helm/templates/configmap.yaml

+  CHE_PREDEFINED_STACKS_RELOAD__ON__START: "false"
+  JAVA_OPTS: "-XX:MaxRAMFraction=2 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10 -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:+UnlockExperimentalVMOptions -XX:+UseCGroupMemoryLimitForHeap -Dsun.zip.disableMemoryMapping=true -Xms20m "
+  CHE_WORKSPACE_AUTO_START: "false"
+  CHE_INFRA_KUBERNETES_INGRESS_ANNOTATIONS__JSON: '{"nginx.ingress.kubernetes.io/rewrite-target": "/","nginx.ingress.kubernetes.io/ssl-redirect": "false","nginx.ingress.kubernetes.io/proxy-connect-timeout": "3600","nginx.ingress.kubernetes.io/proxy-read-timeout": "3600"}'


Should not this value be updated according to the value of .Values.tlsEnabled configuration property?

It should, but we will tackle TLS once we fully support host-based routing for workspace ingresses.

sleshchenko · 2018-02-12T13:54:06Z

I tried to upgrade Che with Help when tlsEnabled is true and I got the following error

Can you point me what I did wrong? I use minikube v0.25.0.

garagatyi · 2018-02-13T09:52:28Z

dockerfiles/init/modules/che-kubernetes-helm/templates/ingress.yaml

+    kubernetes.io/tls-acme: "true"
+{{ else }}
+    nginx.ingress.kubernetes.io/ssl-redirect: "false"
+    ingress.kubernetes.io/proxy-read-timeout: "3600"


Shouldn't ingress.kubernetes.io/proxy-read-timeout and connect timeout be prefixed with nginx as other nginx specific annotations?

garagatyi

Looks good but I haven't checked that it runs myself

l0rd

@perspectivus1 thanks for this PR! Why have you created a new folder che-kubernetes-helm? Why not using kubernetes folder itself. Helm should be the default way to deploy Che on kube anyway.

perspectivus1 · 2018-02-13T14:10:53Z

TLS is not yet working for workspace ingresses. We should first support host-based routing. Hope to get to it in a few days.

The issue @sleshchenko encountered when trying to upgrade is a bug in helm that was fixed a couple of days ago in release 2.8.1.

After downloading the new helm executable, run this command to update tiller:

helm init --upgrade --service-account tiller

@l0rd, I agree that helm should be the default way to deploy Che to Kubernetes. First, the name of the directory must match the name of the chart in Chart.yaml and 'kubernetes' is not descriptive enough if we ever intend to upload this chart to a Helm repository. I guess that once this PR is merged we can remove the plain kubectl deployment.

…ress annotations.

l0rd · 2018-02-13T21:18:06Z

dockerfiles/init/modules/che-kubernetes-helm/readme.md

+  - Otherwise, you may have to modify the KUBECONFIG environment variable and then type `kubectl config use-context <my-context>`
+- Install tiller on your cluster:
+  - Create a tiller serviceAccount and bind it to the almighty cluster-admin role: `kubectl apply -f ./tiller-rbac.yaml`
+  - Install tiller itself: `helm init --service-acount tiller`


s/acount/account/

l0rd · 2018-02-13T21:45:15Z

dockerfiles/init/modules/che-kubernetes-helm/readme.md

+- Ensure that you have an NGINX-based ingress controller. Note: This is the default ingress controller on Minikube. You can start it with `minikube addons enable ingress`
+- DNS discovery should be enabled. Note: It is enabled by default in minikube.
+### Deployment Process
+- Obtain the address of your Kubernetes cluser:


I think you should say here that the address is needed to set the value of cheDomain in values.yaml

l0rd · 2018-02-13T21:47:22Z

dockerfiles/init/modules/che-kubernetes-helm/readme.md

+- Override default values by changing the values.yaml file and then typing:
+
+  ```bash
+  helm upgrade --install <my-che-installation> ./


It may be interesting to add the minikube specific command:

helm upgrade --install <my-che-installation> --set ingress.cheDomain=$(minikube ip).nip.io ./

In my opinion, the beauty of helm is that it works seamlessly for any Kubernetes cluster (minikube or other). We could add a script file that automates the deployment specifically for minikube. What do you think?

l0rd · 2018-02-13T21:54:51Z

dockerfiles/init/modules/che-kubernetes-helm/readme.md

+
+  ```bash
+  helm upgrade --install <my-che-installation> --namespace <my-che-namespace> --set ingress.cheDomain=<my-hostname> --set cheImage=<my-image> ./
+  ```


It may be interesting to add the command to uninstall Che:

helm delete <my-che-installation>

l0rd · 2018-02-13T21:59:35Z

dockerfiles/init/modules/che-kubernetes-helm/templates/ingress.yaml

+          serviceName: che-host
+          servicePort: 8080
+        path: /
+{{- if .Values.isHostBased }}


I think that this if condition should be removed: it's already done in cheHost template and it's not consistent with how CHE_HOST and CHE_APIare defined.

We're working on improving host-based routing support in Kubernetes. We'll cleanup the ingress in the process.

l0rd · 2018-02-13T22:04:12Z

dockerfiles/init/modules/che-kubernetes-helm/tiller-rbac.yaml

+  name: tiller-role-binding
+roleRef:
+  kind: ClusterRole
+  name: cluster-admin


Why do we need to add the service account tiller? And why should it have cluster-admin role?

Basically, it's because tiller needs to create Kubernetes resources in namespace other than the namespace in which tiller is installed. Here are more details.

sleshchenko · 2018-02-15T06:20:02Z

dockerfiles/init/modules/che-kubernetes-helm/templates/deployment.yaml

+    spec:
+      containers:
+      - env:
+        - name: CHE_DOMAIN


Is it used by the Che Server?

It is in prep for host-based routing in workspaces

sleshchenko

LGTM

l0rd

@perspectivus1 thanks for the updates and for your answers. This PR is an important step for k8s support. Are you ok if we merge this PR now?

perspectivus1 · 2018-02-16T12:01:29Z

Yes. Please go ahead with the merge.

Initial support for deploying Che to Kubernetes using Helm charts

4e18e05

Signed-off-by: Eyal Barlev <perspectivus@gmail.com>

perspectivus1 requested review from benoitf and riuvshin as code owners February 12, 2018 08:52

skabashnyuk requested review from garagatyi, sleshchenko and akorneta February 12, 2018 08:55

sleshchenko reviewed Feb 12, 2018

View reviewed changes

garagatyi reviewed Feb 13, 2018

View reviewed changes

garagatyi approved these changes Feb 13, 2018

View reviewed changes

l0rd reviewed Feb 13, 2018

View reviewed changes

Merge branch 'master' of https://github.com/eclipse/che

c29000d

perspectivus1 added 2 commits February 13, 2018 16:16

Initial support for workspace host-based routing. Some changes to ing…

e4a228c

…ress annotations.

Merge branch 'master' of https://github.com/eclipse/che

07c0247

l0rd reviewed Feb 13, 2018

View reviewed changes

Merge branch 'master' of https://github.com/eclipse/che

363e0d9

perspectivus1 added 4 commits February 14, 2018 10:21

Update readme

4f2db7f

Update readme

0519ef5

Merge branch 'master' of https://github.com/eclipse/che

306398d

Fix typo in configmap

bc5c69a

sleshchenko reviewed Feb 15, 2018

View reviewed changes

sleshchenko approved these changes Feb 15, 2018

View reviewed changes

riuvshin approved these changes Feb 15, 2018

View reviewed changes

akorneta approved these changes Feb 15, 2018

View reviewed changes

l0rd approved these changes Feb 16, 2018

View reviewed changes

l0rd merged commit 177e7a0 into eclipse-che:master Feb 16, 2018

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 23, 2018

benoitf added this to the 6.2.0 milestone Feb 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support for deploying Che to Kubernetes using Helm charts #8715

Initial support for deploying Che to Kubernetes using Helm charts #8715

perspectivus1 commented Feb 12, 2018

codenvy-ci commented Feb 12, 2018

codenvy-ci commented Feb 12, 2018

codenvy-ci commented Feb 12, 2018

skabashnyuk commented Feb 12, 2018

sleshchenko Feb 12, 2018

sleshchenko Feb 12, 2018

sleshchenko Feb 12, 2018

perspectivus1 Feb 13, 2018

sleshchenko Feb 13, 2018

sleshchenko commented Feb 12, 2018

garagatyi Feb 13, 2018 •

edited

Loading

garagatyi left a comment

l0rd left a comment

perspectivus1 commented Feb 13, 2018

l0rd Feb 13, 2018

l0rd Feb 13, 2018

l0rd Feb 13, 2018

perspectivus1 Feb 14, 2018

l0rd Feb 13, 2018

l0rd Feb 13, 2018

perspectivus1 Feb 14, 2018

l0rd Feb 13, 2018

perspectivus1 Feb 14, 2018

sleshchenko Feb 15, 2018

perspectivus1 Feb 15, 2018

sleshchenko left a comment

l0rd left a comment

perspectivus1 commented Feb 16, 2018

Initial support for deploying Che to Kubernetes using Helm charts #8715

Initial support for deploying Che to Kubernetes using Helm charts #8715

Conversation

perspectivus1 commented Feb 12, 2018

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

codenvy-ci commented Feb 12, 2018

codenvy-ci commented Feb 12, 2018

codenvy-ci commented Feb 12, 2018

skabashnyuk commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sleshchenko commented Feb 12, 2018

garagatyi Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

garagatyi left a comment

Choose a reason for hiding this comment

l0rd left a comment

Choose a reason for hiding this comment

perspectivus1 commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sleshchenko left a comment

Choose a reason for hiding this comment

l0rd left a comment

Choose a reason for hiding this comment

perspectivus1 commented Feb 16, 2018

garagatyi Feb 13, 2018 •

edited

Loading