Make workspace routes more user-friendly #1672

dkwon17 · 2023-05-03T21:08:17Z

Fixes eclipse-che/che#20598

What does this PR do?

Workspace URL

Before this PR, the workspace url would be something like:

https://che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com/workspace13c0f27b694b4943/universal-developer-image/3100/

After this PR, the workspace URL becomes:

https://che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com/<username>/<workspace name>/3100/

Public endpoints defined in devfile

For public endpoints defined in the devfile (example)

Before this PR:

http://workspace294df8373ccb4bda-4.apps.che-dev.x6e0.p1.openshiftapps.com/food

After this PR:

http://<username>.<workspace name>.<endpoint name>.apps.che-dev.x6e0.p1.openshiftapps.com/food

Screenshot/screencast of this PR

Workspace URL:

Route spec.host:

What issues does this PR fix or reference?

eclipse-che/che#20598

How to test this PR?

chectl server:deploy \
     --installer operator \
     --platform openshift \
     --che-operator-image quay.io/dkwon17/che-operator:friendlypath

~~1. After deploying Che, update the che-server image:~~ (CheCluster patch not needed anymore)

spec:
  components:
    cheServer:
      deployment:
        containers:
          - image: 'quay.io/dkwon17/che-server:user-profile-che-operator'
            name: che

Create and start a workspace with this project: https://github.com/che-incubator/quarkus-api-example
After workspace start, verify that the workspace IDE url is: {CHE-HOST}/{USERNAME}/{DW-NAME}/3100/
Confirm that there is an OpenShift route named {DW-ID}-tools-8080-list-all-food, where the spec.host is {USERNAME}.{DW-NAME}.list-all-food.{INGRESS-DOMAIN}

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

openshift-ci · 2023-05-03T21:08:23Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

controllers/devworkspace/solver/che_routing.go

codecov · 2023-05-04T20:16:25Z

Codecov Report

Merging #1672 (2d0d278) into main (a716caf) will increase coverage by 0.25%.
The diff coverage is 85.00%.

@@            Coverage Diff             @@
##             main    #1672      +/-   ##
==========================================
+ Coverage   59.93%   60.18%   +0.25%     
==========================================
  Files          70       71       +1     
  Lines        8332     8430      +98     
==========================================
+ Hits         4994     5074      +80     
- Misses       2992     3008      +16     
- Partials      346      348       +2

Impacted Files	Coverage Δ
controllers/devworkspace/solver/che_routing.go	`78.40% <79.78%> (-0.64%)`	⬇️
...ntrollers/devworkspace/solver/endpoint_strategy.go	`95.23% <95.23%> (ø)`
...ontrollers/devworkspace/solver/endpoint_exposer.go	`93.06% <100.00%> (-0.08%)`	⬇️

dkwon17 · 2023-05-04T21:54:04Z

controllers/devworkspace/solver/che_routing.go

+	return normalize(routing.ObjectMeta.OwnerReferences[0].Name), nil
+}
+
+func normalize(username string) string {


This function is based off of https://github.com/eclipse-che/che-server/blob/75979a6d6a914e80c873d91778e20a93f920e8a4/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java#L636

dkwon17 · 2023-05-04T22:00:59Z

controllers/devworkspace/solver/che_routing.go

+		return "", err
+	}
+	username := string(secret.Data["name"])
+	return normalize(username), nil


The username is being read from the user-profile secret created by che-server, which is why this PR depends on eclipse-che/che-server#508

Another way to read the username could be to read the che.eclipse.org/username annotation from the user namespace

BTW, che.eclipse.org/username annotation might not exist

Do we have a backup plan instead of throwing an error if Secret doesn't exist ?

I'm afraid there is no backup plan right now if that secret does not exist, are there other ways to detect the username?

What about using the legacy route name if username is unknown?

whereas a backup plan could be nice to have, using user-profile section should be robust in general e.g. if it is deleted from the namespace it would be re-created next time user access the dashboard

After more investigation, I think the minikube tests are failing because the user-profile secret is not created when the user namespace and workspace is created by applying templates (ie, without using the dashboard)

As a result, workspaces cannot start

@dkwon17 thanks for the investigation, so in this case the flow when devworkspace is created using oc apply command is going to also fail if there is no user-profile secret in the namespace, right?

@ibuziuk yes, that's right

I am currently working on the legacy routes in this case, as suggested by @tolusha

tolusha · 2023-05-05T15:29:06Z

/retest

ibuziuk · 2023-05-08T13:30:39Z

/retest

dkwon17 · 2023-05-08T21:28:58Z

~~I'm quite confident the test failure is because this PR depends on eclipse-che/che-server#508~~ See #1672 (comment)

Without the che-server side PR, the user's username would not be found and workspaces would not start.

I've also added a new commit to deal with providing the Che-operator ClusterRole permissions to update ingresses, which is needed for this PR for allowing starting older workspaces to start minikube. This is needed because, this PR will update older workspaces' ingresses to have an updated spec.host.

The ClusterRole update likely fixes eclipse-che/che#22166, although I haven't confirmed that

ibuziuk · 2023-05-10T11:54:48Z

/retest

ibuziuk · 2023-05-17T09:08:10Z

@dkwon17 important note, that we should cherry-pick the PR to 7.67.x for 3.7 once merged

dkwon17 · 2023-05-18T20:34:44Z

I've updated the PR to use legacy routing if the username and devworkspace name cannot be detected

To test the legacy routing:

Create a namespace where the username is not specified as an annotation:

kind: Namespace
apiVersion: v1
metadata:
  name: my-namespace
  labels:
    app.kubernetes.io/part-of: che.eclipse.org
    app.kubernetes.io/component: workspaces-namespace

Create a devworkspace within the new namespace

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: my-devworkspace
  namespace: my-namespace
spec:
  routingClass: che
  started: false
  contributions:
    - name: ide
      uri: http://plugin-registry.eclipse-che.svc:8080/v3/plugins/che-incubator/che-code/insiders/devfile.yaml
  template:
    components:
      - name: tooling-container
        container:
          image: quay.io/devfile/universal-developer-image:ubi8-latest
          cpuLimit: 100m

When the workspace starts, verify that the status.mainUrl of the devworkspace follows this format (the old format):

<CHE-HOST>/workspace13c0f27b694b4943/tooling-container/3100/

dkwon17 · 2023-05-18T20:36:13Z

controllers/devworkspace/solver/che_routing.go

 			configs = append(configs, *workspaceConfig)
 		}
 	}

 	return configs, nil
 }

-func (c *CheRoutingSolver) getInfraSpecificExposer(cheCluster *chev2.CheCluster, routing *dwo.DevWorkspaceRouting, objs *solvers.RoutingObjects) (func(info *EndpointInfo), error) {
+func getNormalizedUsername(c client.Client, namespace string) (string, error) {
+	username, err := getUsernameFromNamespace(c, namespace)


The username is also being read from the namespace as well, in the case that the admin has pre-provisioned the namespaces following the doc, since in this case, the user-profile secret would not be created

dkwon17 · 2023-05-29T16:10:27Z

Hi @dmytro-ndp for your comment #1672 (comment), I noticed that the devfile registry pod scheduling happens when the quay.io/eclipse/che-operator:next che-operator pod terminates and is replaced by the che-operator pod with quay.io/dkwon17/che-operator:friendlypath image.

I wonder if replacing the che-operator pod with the custom container image affects the scheduling for the devfile registry pod, potentially causing the devfile registry pod scheduling to take longer

Hi @tolusha , could this be the reason for the chectl failure occuring in [1]?

[1] https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Che/job/ocp/job/basic/job/install-che-to-ocp/15/consoleFull

dkwon17 · 2023-05-30T23:52:51Z

IMHO, it's somehow related to quay.io/dkwon17/che-operator:friendlypath, because Eclipse Che Next has installed successfully using chectl with quay.io/eclipse/che-operator:next operator on the same cluster [3].

@dmytro-ndp I cannot reproduce the issue with clusterbot, but by any chance, do you know if the same error will happen if you use a different che-operator image, like quay.io/eclipse/che-operator:7.67.0?

tolusha · 2023-05-31T07:20:30Z

Hi @tolusha , could this be the reason for the chectl failure occuring in [1]?

No, that can't be a reason.

I can't reproduce the issue either.
https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Che/job/ocp/job/basic/job/install-che-to-ocp/23/console

tolusha · 2023-05-31T08:18:12Z

We are ok to merge as long as all checks passed.

Signed-off-by: David Kwon <dakwon@redhat.com>

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha · 2023-05-31T08:32:29Z

Conflicts resolved.

dmytro-ndp · 2023-05-31T09:22:43Z

@tolusha , @dkwon17 : is there fresh che-operator image with recent changes available for testing?

tolusha · 2023-05-31T09:37:47Z

Please use this one quay.io/abazko/operator:friendlyroute

devstudio-release · 2023-05-31T14:49:01Z

Build 3.8 :: operator-bundle_3.x/1437: Console, Changes, Git Data

dmytro-ndp · 2023-05-31T17:22:30Z

"chectl server:deploy" command had failed when deploy to test OCP cluster with even though there was --k8spoderrorrechecktimeout=600000 parameter added https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Che/job/ocp/job/basic/job/install-che-to-ocp/25/console :

chectl server:deploy --platform=openshift --che-operator-cr-patch-yaml=custom-resource-patch.yaml --che-operator-image=quay.io/abazko/operator:friendlyroute --k8spoderrorrechecktimeout=600000

...

 [07:31:00] Scheduling [started]
 [07:32:03] Scheduling [failed]
 [07:32:03] → Timeout: there are no pods in the namespace: eclipse-che, selector: app.kubernetes.io/name=che,app.kubernetes.io/component=devfile-registry. Check Eclipse Che logs for details. Consider increasing error recheck timeout with --k8spoderrorrechecktimeout flag.
 [07:32:03] Devfile Registry pod bootstrap [failed]
 [07:32:03] → Timeout: there are no pods in the namespace: eclipse-che, selector: app.kubernetes.io/name=che,app.kubernetes.io/component=devfile-registry. Check Eclipse Che logs for details. Consider increasing error recheck timeout with --k8spoderrorrechecktimeout flag.
 [07:32:03] Wait for Eclipse Che ready [failed]
 [07:32:03] → Timeout: there are no pods in the namespace: eclipse-che, selector: app.kubernetes.io/name=che,app.kubernetes.io/component=devfile-registry. Check Eclipse Che logs for details. Consider increasing error recheck timeout with --k8spoderrorrechecktimeout flag.
     Error: Command server:deploy failed with the error: Timeout: there are no 
     pods in the namespace: eclipse-che, selector: 
     app.kubernetes.io/name=che,app.kubernetes.io/component=devfile-registry. 
     Check Eclipse Che logs for details. Consider increasing error recheck 
     timeout with --k8spoderrorrechecktimeout flag. See details: 
     /home/hudson/.cache/chectl/error.log. Eclipse Che logs: 
     /tmp/chectl-logs/1685532564341.

At the same time Eclipse Che has deployed successful after doesn't matter chectl command had failed.
"Apache Camel K" workspace has started successfully as well, with correct workspace URL: https://eclipse-che.apps.ocp410-dnochev.crw-qe.com/**admin**/**apache-camel-k**/3100/
and "EmptyWorkspace": https://eclipse-che.apps.ocp410-dnochev.crw-qe.com/admin/empty-8gqu/3100/
and "Java Spring Boot": https://eclipse-che.apps.ocp410-dnochev.crw-qe.com/admin/spring-petclinic/3100/

I have also checked new endpoint URL additionally, using "spring-petclinic" sample > "devfile: run-with-mysql" task - it worked as expected:

endpoint "http://admin.spring-petclinic.8080-tcp.apps.ocp410-dnochev.crw-qe.com/"

screencast-eclipse-che.apps.ocp410-dnochev.crw-qe.com-2023.05.31-20_09_26.webm

I have also checked the case when there are several workspaces using the same endpoint.
URLs http://<username>.<workspace name>.<endpoint name>.<ocp host> looked unique to particular workspace:

Conclusion: PR looked good to merge.

devstudio-release · 2023-05-31T18:48:46Z

Build 3.8 :: operator-bundle_3.x/1438: Console, Changes, Git Data

devstudio-release · 2023-05-31T22:48:46Z

Build 3.8 :: operator-bundle_3.x/1439: Console, Changes, Git Data

devstudio-release · 2023-06-01T02:48:46Z

Build 3.8 :: operator-bundle_3.x/1440: Console, Changes, Git Data

devstudio-release · 2023-06-01T06:49:00Z

Build 3.8 :: operator-bundle_3.x/1441: Console, Changes, Git Data

l0rd · 2023-06-02T18:21:57Z

@dkwon17 my understanding is that for the editor endpoint we use <endpoint port> rather than <endpoint name> and this is not consistent with other workspaces endpoints for which we use the <endpoint name>. Is it for a technical reason?

dkwon17 · 2023-06-02T19:37:44Z

@l0rd I didn't have a technical reason, I used <endpoint port> to follow the suggested url in the original issue: eclipse-che/che#20598

brandonp42 · 2023-06-06T04:48:43Z

@dkwon17 is there a strong reason you decided that external ingress host names should have a period in them causing them to be treated as subdomains? I'm struggling with this choice to make it work properly with real SSL certificates. Please see the eclipse/che issue I tagged this on above and if you have any thoughts on workarounds I'd appreciate it.

openshift-ci bot added the do-not-merge/work-in-progress label May 3, 2023

dkwon17 mentioned this pull request May 3, 2023

feat: allow che-operator to detect user-profile secret eclipse-che/che-server#508

Merged

9 tasks

dkwon17 commented May 3, 2023

View reviewed changes

controllers/devworkspace/solver/che_routing.go Outdated Show resolved Hide resolved

dkwon17 force-pushed the friendlyroute branch from 753b93a to f22fb5d Compare May 4, 2023 20:11

dkwon17 closed this May 4, 2023

dkwon17 reopened this May 4, 2023

dkwon17 marked this pull request as ready for review May 4, 2023 21:46

dkwon17 requested review from nickboldt, tolusha, AndrienkoAleksandr and flacatus as code owners May 4, 2023 21:46

openshift-ci bot removed the do-not-merge/work-in-progress label May 4, 2023

dkwon17 commented May 4, 2023

View reviewed changes

dkwon17 force-pushed the friendlyroute branch from f22fb5d to ed9ce34 Compare May 4, 2023 22:10

dkwon17 force-pushed the friendlyroute branch from ed9ce34 to c25db9c Compare May 8, 2023 16:16

dkwon17 mentioned this pull request May 8, 2023

[DO-NOT-MERGE] Make workspace routes more user-friendly #1676

Closed

10 tasks

openshift-merge-robot added the needs-rebase label May 17, 2023

dkwon17 force-pushed the friendlyroute branch 2 times, most recently from efd218b to 00b627d Compare May 18, 2023 19:02

openshift-merge-robot removed the needs-rebase label May 18, 2023

dkwon17 commented May 18, 2023

View reviewed changes

ibuziuk mentioned this pull request May 29, 2023

fix: Add permissions to update ingress k8s resources #1692

Merged

10 tasks

dkwon17 and others added 8 commits May 31, 2023 11:23

Make workspace routes more user-friendly

e83daf2

Signed-off-by: David Kwon <dakwon@redhat.com>

Add update verb for ingress in che-operator CR

878a08a

Signed-off-by: David Kwon <dakwon@redhat.com>

Legacy support

4074828

Signed-off-by: David Kwon <dakwon@redhat.com>

Remove componentName field from from legacy strategy

4213983

Signed-off-by: David Kwon <dakwon@redhat.com>

Determine path strategy in different function

cf3e310

Signed-off-by: David Kwon <dakwon@redhat.com>

Enforce lowercase for username and dw name

b259760

Signed-off-by: David Kwon <dakwon@redhat.com>

Run make update-dev-resources'

7dfb2fe

Signed-off-by: David Kwon <dakwon@redhat.com>

Update dev resources

2d0d278

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha force-pushed the friendlyroute branch from 20ccd00 to 2d0d278 Compare May 31, 2023 08:32

tolusha merged commit 34a1085 into eclipse-che:main May 31, 2023

dkwon17 mentioned this pull request May 31, 2023

Cherry-pick PR-1672 to 7.67.x branch #1694

Merged

10 tasks

brandonp42 mentioned this pull request Jun 6, 2023

How to get port redirect ingress hostnames to use dashes instead of periods eclipse-che/che#22267

Closed

tolusha mentioned this pull request Aug 28, 2023

Accessing public endpoints of workspace pod eclipse-che/che#22461

Closed

Make workspace routes more user-friendly #1672

Make workspace routes more user-friendly #1672

Conversation

dkwon17 commented May 3, 2023 • edited by tolusha Loading

What does this PR do?

Workspace URL

Public endpoints defined in devfile

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

openshift-ci bot commented May 3, 2023

codecov bot commented May 4, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

dkwon17 May 4, 2023 • edited Loading

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

dkwon17 May 11, 2023 • edited Loading

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

tolusha commented May 5, 2023

ibuziuk commented May 8, 2023

dkwon17 commented May 8, 2023 • edited Loading

ibuziuk commented May 10, 2023

ibuziuk commented May 17, 2023

dkwon17 commented May 18, 2023

dkwon17 May 18, 2023 • edited Loading

Choose a reason for hiding this comment

dkwon17 commented May 29, 2023 • edited Loading

dkwon17 commented May 30, 2023

tolusha commented May 31, 2023

tolusha commented May 31, 2023

tolusha commented May 31, 2023

dmytro-ndp commented May 31, 2023

tolusha commented May 31, 2023

devstudio-release commented May 31, 2023

dmytro-ndp commented May 31, 2023 • edited Loading

devstudio-release commented May 31, 2023

devstudio-release commented May 31, 2023

devstudio-release commented Jun 1, 2023

devstudio-release commented Jun 1, 2023

l0rd commented Jun 2, 2023 • edited Loading

dkwon17 commented Jun 2, 2023

brandonp42 commented Jun 6, 2023

dkwon17 commented May 3, 2023 •

edited by tolusha

Loading

codecov bot commented May 4, 2023 •

edited

Loading

dkwon17 May 4, 2023 •

edited

Loading

dkwon17 May 11, 2023 •

edited

Loading

dkwon17 commented May 8, 2023 •

edited

Loading

dkwon17 May 18, 2023 •

edited

Loading

dkwon17 commented May 29, 2023 •

edited

Loading

dmytro-ndp commented May 31, 2023 •

edited

Loading

l0rd commented Jun 2, 2023 •

edited

Loading