Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize the ingress configuration to support single-host in single-user mode on kubernetes #14134

Merged
merged 15 commits into from
Aug 26, 2019

Conversation

metlos
Copy link
Contributor

@metlos metlos commented Aug 5, 2019

What does this PR do?

This adds support for better control over the ingress path for workspaces running using single-host server strategy on k8s.

This PR tries to address the following scenarios:

What issues does this PR fix or reference?

#14002

deployments better.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot che-bot added kind/task Internal things, technical debt, and to-do tasks to be performed. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Aug 5, 2019
@musienko-maxim
Copy link
Contributor

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@dmytro-ndp
Copy link
Contributor

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 6, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14134
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@che-bot
Copy link
Contributor

che-bot commented Aug 6, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 6, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 6, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@metlos
Copy link
Contributor Author

metlos commented Aug 6, 2019

@dmytro-ndp, would you know what version of nginx-ingress-controller is running in the k8s cluster that the happy path tests are using?

@metlos
Copy link
Contributor Author

metlos commented Aug 6, 2019

if you have access, find the pod of the controller in the kube-system namespace and tell me the name of the image that the pod's container is running.

@metlos
Copy link
Contributor Author

metlos commented Aug 6, 2019

ah, I see the happy path is running on minikube 1.1.1 which would explain the failures. Currently, this PR doesn't work with that yet as explained in the description.

@eclipse-che eclipse-che deleted a comment from che-bot Aug 6, 2019
…properties

to make more sense.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 7, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: docker.io/maxura/che-server:14134

@metlos metlos marked this pull request as ready for review August 9, 2019 10:35
@metlos metlos changed the title Generalize the ingress configuration to support single-host on kubernetes Generalize the ingress configuration to support single-host in single-user mode on kubernetes Aug 9, 2019
@metlos
Copy link
Contributor Author

metlos commented Aug 9, 2019

So what is this CRW Jenkins job? I get a 404 when trying to access the details..

@metlos
Copy link
Contributor Author

metlos commented Aug 9, 2019

@eclipse/eclipse-che-qa ^

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Aug 9, 2019

Aha, I see. You mentioned CRW Jenkins in review section.
Please, ignore that - it was a job for verifying new pipeline job for PR check.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@metlos
Copy link
Contributor Author

metlos commented Aug 23, 2019

crw-ci-test

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@metlos
Copy link
Contributor Author

metlos commented Aug 23, 2019

@dmytro-ndp I don't see how the happy path failures are related to the changes in this PR. Could you please confirm that they're failing for other reasons?

@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

I've not tested your changes and reviewed quite deeply.

@amisevsk Maybe you're able to do another round of review as well

Good job! LGTM

@metlos
Copy link
Contributor Author

metlos commented Aug 23, 2019

ci-build

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@dmytro-ndp
Copy link
Contributor

@metlos:

@dmytro-ndp I don't see how the happy path failures are related to the changes in this PR. Could you please confirm that they're failing for other reasons?

I confirm that regression #14134 (comment) is not related to your PR directly. That is because of outdated Jenkinsfile which doesn't contain workaround to known issue #14202.
I would recommend you to update your PR with Jenkinsfile changes from master, where we turned off unit tests in time of build of Che.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14134

@che-bot
Copy link
Contributor

che-bot commented Aug 23, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

I'm okay with merge but would suggest tests for the PathDemangler -- currently only the trivial case is handled.

I'm unable to test this PR manually, so take my approval with a pinch of salt.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 26, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14134

@che-bot
Copy link
Contributor

che-bot commented Aug 26, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @metlos

@metlos metlos merged commit af26d29 into eclipse-che:master Aug 26, 2019
@che-bot che-bot added this to the 7.1.0 milestone Aug 26, 2019
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants