-
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
Add an ability to list available k8s namespaces #14541
Conversation
82ea302
to
ec0afac
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4fc4288
to
b830db7
Compare
This comment has been minimized.
This comment has been minimized.
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:
|
This comment has been minimized.
This comment has been minimized.
2df41cc
to
3b3eeb5
Compare
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
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.
Great work, just a few more minor comments.
...g/eclipse/che/workspace/infrastructure/kubernetes/api/server/KubernetesNamespaceService.java
Outdated
Show resolved
Hide resolved
...g/eclipse/che/workspace/infrastructure/kubernetes/api/server/KubernetesNamespaceService.java
Outdated
Show resolved
Hide resolved
...rg/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java
Show resolved
Hide resolved
...rg/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java
Show resolved
Hide resolved
...clipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactoryTest.java
Show resolved
Hide resolved
...java/org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactory.java
Outdated
Show resolved
Hide resolved
...rg/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java
Outdated
Show resolved
Hide resolved
.../org/eclipse/che/workspace/infrastructure/openshift/project/OpenShiftProjectFactoryTest.java
Show resolved
Hide resolved
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 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, well done.
crw-ci-test |
174fc73
to
b7de42c
Compare
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.
I've approved this. The only comment I have is to ever so slightly increase the test coverage, but I don't think that blocks merging this.
...g/eclipse/che/workspace/infrastructure/kubernetes/api/server/KubernetesNamespaceService.java
Show resolved
Hide resolved
b7de42c
to
e35533f
Compare
Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
Signed-off-by: Sergii Leshchenko <sleshche@redhat.com>
e35533f
to
feb18ac
Compare
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
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:
|
What does this PR do?
It does not change the current workspace creating flow, but used namespace still managed by
che.infra.kubernetes.namespace
. See more how workspace creation flow will be changed #14376 (comment).And this PR introduces API that will be used by clients to provide an ability for users to see target namespace info, and choose one if multiple are available.
So, there are two configuration properties that are used for configuring a list of available namespaces:
1.
Example of the implemented API response:
What can be improved later
kube-system
. @metlos propose to introduce a dedicate property to configure selector for available namespaces likeche.infra.kubernetes.namespace.user_available_selector=app=che
.But on some tests environments like minikube, minishift, crc a user is able to create new namespaces... So, it may be useful (may be not) to implement an ability to type free namespace name value and then Che would create this on the first workspace start.
It may be implemented in two ways: checks automatically if a user is able to create a new namespace OR let che admin to configure such an ability with a dedicated configuration property.
API could expose such an ability by returning a special namespace holder, like:
Is this PR well-tested?
Not yet. It will be tested and as proof will be here screenshots for different configurations and infrastructures.
Tested configurations
Kubernetes: Single-User
che.infra.kubernetes.namespace.default=<username>-che
che.infra.kubernetes.namespace.allow_user_defined=false
Since in single-user mode there is only one
che
user, default namespace ische-che
, which is not good but should not a big issue and may be handled by chectl - like override default namespace for single user to be the same as che namespace, it actully works in such way.So, che-che namespace does not exist:
che.infra.kubernetes.namespace.default=che
che.infra.kubernetes.namespace.allow_user_defined=false
che.infra.kubernetes.namespace.default=NULL
che.infra.kubernetes.namespace.allow_user_defined=true
OpenShift - Multiuser
che.infra.kubernetes.namespace.default=<username>-che
che.infra.kubernetes.namespace.allow_user_defined=false
Logged in as developer, developer-che exists:
che.infra.kubernetes.namespace.default=<username>-che
che.infra.kubernetes.namespace.allow_user_defined=true
Logged in as developer, the user has developer-che and myproject projects:
che.infra.kubernetes.namespace.default=<username>-che
che.infra.kubernetes.namespace.allow_user_defined=true
Logged in as developer, the user has myproject project only:
che.infra.kubernetes.namespace.default=NULL
che.infra.kubernetes.namespace.allow_user_defined=true
Logged in as developer, the user has myproject project only:
What issues does this PR fix or reference?
It resolves #14376
Release Notes
N/A
Docs PR
N/A
The workflow that user/admin would use is not complete yet, documentation will be provided in further PRs where workflow will be available.