Skip to content

K8s multiuser helm#8973

Merged
l0rd merged 5 commits intoeclipse-che:masterfrom
guydc:k8s-multiuser-helm
Mar 8, 2018
Merged

K8s multiuser helm#8973
l0rd merged 5 commits intoeclipse-che:masterfrom
guydc:k8s-multiuser-helm

Conversation

@guydc
Copy link
Copy Markdown
Contributor

@guydc guydc commented Mar 1, 2018

What does this PR do?

Adds helm charts for deployment of Multi-User Che on Kubernetes (minikube).

Reuses images provided with #8819 (thanks @eivantsov !).

Multi/Single user is configurable with helm values (global.multiuser).

Orchestration of deployment is performed with init-containers.
Post-Installation configuration is performed with Jobs.

  • Che & KeyCloak servers wait for postgres to be available (based on readiness/availability probes)
  • KeyCloak configuration is performed after KeyCloak server is available, using a Job

KeyCloak can be served from a dedicated host, or a host shared with master (/auth).

Test PR
Follow instructions to set up minikube, helm, tiller & deploy Che.

What issues does this PR fix or reference?

This PR is part of kubernetes infrastructure epic #5908.

Release Notes

Docs PR

readme

guydc added 3 commits March 1, 2018 16:23
Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
@guydc guydc requested review from benoitf and riuvshin as code owners March 1, 2018 14:52
@codenvy-ci
Copy link
Copy Markdown

Can one of the admins verify this patch?

2 similar comments
@codenvy-ci
Copy link
Copy Markdown

Can one of the admins verify this patch?

@codenvy-ci
Copy link
Copy Markdown

Can one of the admins verify this patch?

@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/enhancement A feature request - must adhere to the feature request template. kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed. labels Mar 1, 2018
Copy link
Copy Markdown

@garagatyi garagatyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stop putting stuff not related to dockerfiles into dockerfiles folder. We also discussed moving openshift stuff out of this folder.
@eivantsov @benoitf @riuvshin @l0rd WDYT?

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Mar 3, 2018

@garagatyi I agree with you in general but keeping these files in dockerfiles folder might have a sense if it will be integrated with CLI.

@garagatyi
Copy link
Copy Markdown

@riuvshin I don't think it will be integrated with the CLI. The same for OS. Admins would expect to deploy Che with k8s/OS dependent flows, not with a CLI that runs inside of docker.

@l0rd
Copy link
Copy Markdown
Contributor

l0rd commented Mar 5, 2018

@garagatyi agree but I don't think that we should block this PR for this. We can discuss on the proper issue where we should move the files because I don't think we have an agreement yet.

And yes CLI extension to support OpenShift and k8s has been delayed/cancelled in favor of minishift addon and helm.

@garagatyi
Copy link
Copy Markdown

@l0rd I didn't mean that files placement topic is blocking this PR. Just started this conversation since it is tightly related to the PR. Sure we can merge files in the current place and then move it if needed.

@garagatyi
Copy link
Copy Markdown

garagatyi commented Mar 6, 2018

@benoitf Can you review this PR?
@guydaichs @riuvshin is on vacation, so he probably won't be able to review your PR till the next week.
AFAIK initial contribution of helm support was done by @perspectivus1 so it would be great if you can review this PR too.
@l0rd you were reviewing the previous PR about helm, can you take a look on this one too?

@perspectivus1
Copy link
Copy Markdown
Contributor

I'm been following @guydaichs' work on this PR. I actually saw the multiuser deployment in action. Looks good to me 👍

@skabashnyuk
Copy link
Copy Markdown
Contributor

@guydaichs @perspectivus1 @garagatyi @l0rd here #8982 we are doing some adjustments in K8s files can you take a look and let us know if you see any issues?

@l0rd
Copy link
Copy Markdown
Contributor

l0rd commented Mar 7, 2018

FYI I am testing that locally right now. I will comment later today.

fix instructions
Copy link
Copy Markdown
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested and worked just fine. Great job @guydaichs! We can merge it tomorrow.

In the meantime can you please add license headers to the files that don't have it and add a newline at the end of a couple of files that don't have it (keycloak-configure-job.yaml and requirements.yaml).

@skabashnyuk
Copy link
Copy Markdown
Contributor

@guydaichs do you want something to add to this PR or you want to merge it? Are you going to squash commits or commiters can do that for you?

Signed-off-by: Guy Daich <guy.daich@sap.com>
@guydc
Copy link
Copy Markdown
Contributor Author

guydc commented Mar 8, 2018

@l0rd - I added license headers.
@skabashnyuk - please squash and merge.

@guydc guydc closed this Mar 8, 2018
@guydc guydc reopened this Mar 8, 2018
@l0rd l0rd merged commit 8c7ebc6 into eclipse-che:master Mar 8, 2018
@l0rd
Copy link
Copy Markdown
Contributor

l0rd commented Mar 8, 2018

@guydaichs thanks

hkolvenbach pushed a commit to hkolvenbach/che that referenced this pull request Mar 12, 2018
Signed-off-by: Guy Daich <guy.daich@sap.com>
@benoitf benoitf added this to the 6.3.0 milestone Mar 16, 2018
@benoitf 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 Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement A feature request - must adhere to the feature request template. kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants