-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Conversation
deployments better. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
ci-test |
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
@dmytro-ndp, would you know what version of nginx-ingress-controller is running in the k8s cluster that the happy path tests are using? |
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. |
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. |
…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>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
So what is this CRW Jenkins job? I get a 404 when trying to access the details.. |
@eclipse/eclipse-che-qa ^ |
Aha, I see. You mentioned |
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
crw-ci-test |
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
@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? |
...se/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java
Outdated
Show resolved
Hide resolved
...g/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java
Show resolved
Hide resolved
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
There was a problem hiding this 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
ci-build |
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
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. |
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
There was a problem hiding this 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.
...g/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java
Show resolved
Hide resolved
...se/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java
Outdated
Show resolved
Hide resolved
...se/che/workspace/infrastructure/kubernetes/server/external/IngressPathTransformInverter.java
Show resolved
Hide resolved
...org/eclipse/che/workspace/infrastructure/kubernetes/server/KubernetesServerResolverTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
E2E tests of Eclipse Che Multiuser on OCP has been successful:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @metlos
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:
<WORKSPACE_ENDPOINT>/services
websocket never replies with any response to the requests the UI side is sending)support single-host server strategy in the multi-user server (this most probably also involves fixing JWT proxy doesn't preserve request path when redirects to WS master for authentication #12988)This is now tracked in its own issue - Support single-host/default-host in multi-user server mode #14189What issues does this PR fix or reference?
#14002