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

Update samples #1025

Merged
merged 2 commits into from
Feb 9, 2023
Merged

Update samples #1025

merged 2 commits into from
Feb 9, 2023

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Jan 27, 2023

What does this PR do?

  • Use https://eclipse-che.github.io/che-plugin-registry/ for plugin devfiles instead of https://che-plugin-registry-main.surge.sh
  • Use che-code as editor plugin for all samples except explicit Theia sample
  • Fix issues with commands (wrong path -- use $PROJECT_SOURCE instead of $PROJECTS_ROOT)
  • Add dev containers to main samples (code-latest.yaml and theia-latest.yaml) so that container contributions are used (editor should merge component into dev component)
  • Use contributions for all plugins (rather than plugin components)

What issues does this PR fix or reference?

N/A

Is it tested? How?

Test that samples generally work as expected. If testing on minikube, it may be necessary to pre-pull images, as the UDI image is very large (3.7GB):

minikube ssh docker pull quay.io/devfile/universal-developer-image:latest
# Or, depending on the sample used
minikube ssh docker pull quay.io/devfile/universal-developer-image@sha256:d1709bbdfa076474f58f796026a2ed2dd3b24fea7e51ce2cc984e729663ff62c

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

* Update samples to use eclipse-che.github.io for plugin samples

Update samples to use eclipse-che.github.io/plugin-registry for Theia
instead of https://che-plugin-registry-main.surge.sh

* Update all samples to use contributions instead of plugin components

* Use che-code for all samples instead of Theia

(Keep old Theia sample though)

* Fix samples to use $PROJECT_SOURCE instead of $PROJECT_ROOT

$PROJECTS_ROOT is not normally used anymore, since $PROJECTS_SOURCE
points to the full path of the repository on disk inside the workspace
(when there is only one project).

Also fixes an issue where the previous path used for commands was
invalid.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
This represents a more "normal" DevWorkspace sample, as it contains a
'dev' component, using UDI latest, that we expect DWO to merge
contributions into automatically.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Base: 50.27% // Head: 50.27% // No change to project coverage 👍

Coverage data is based on head (5bef0c8) compared to base (6088128).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1025   +/-   ##
=======================================
  Coverage   50.27%   50.27%           
=======================================
  Files          70       70           
  Lines        6021     6021           
=======================================
  Hits         3027     3027           
  Misses       2765     2765           
  Partials      229      229           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@AObuchow AObuchow left a comment

Choose a reason for hiding this comment

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

Tested on OpenShift 4.12.

  • pod-overrides sample failed since the kata RuntimeClass was not found (to be expected)
  • theia-latest sample works as expected
  • Unfortunately, for all the samples using Che Code, I got the following error about the application not being served at the endpoint:

image

@amisevsk
Copy link
Collaborator Author

Unfortunately, for all the samples using Che Code, I got the following error about the application not being served at the endpoint

Hm, I'll have to take a look

@l0rd
Copy link
Collaborator

l0rd commented Jan 31, 2023

I have tested on a fresh OCP 4.12 cluster (I have installed DWO with make install) and k apply -f code-latest.yaml failed with "Init Container project-clone has state CrashLoopBackOff" and from the logs:

2023/01/31 11:44:54 Failed to get temporary directory for setting up projects: mkdir /projects/project-clone-1091890368: permission denied

@l0rd
Copy link
Collaborator

l0rd commented Jan 31, 2023

Otherwise I have the same result as @AObuchow but removing path: '?tkn=eclipse-che' in vscode definition (or opening the main URL without ?tkn=eclipse-che) fixes the problem. But that's not related to this PR so I have opened a separate one.

@nickboldt
Copy link
Collaborator

related to eclipse-che/che#21973

Note that the ?tkn=eclipse-che fix has also been done downstream for 3.5/3.x: redhat-developer/devspaces#880

@AObuchow
Copy link
Collaborator

Note that the ?tkn=eclipse-che fix has also been done downstream for 3.5/3.x: redhat-developer/devspaces#880

Will do a re-test, thanks Nick 👍 😎

@AObuchow
Copy link
Collaborator

I've opened the following 2 issues which are related to this PR. They could be solved in separate commits in this PR, or as separate PRs:

#1028
#1027

@AObuchow
Copy link
Collaborator

I seem to be encountering some CheCode (?) bugs:

When trying out the git-clone sample, code latest and the ephemeral storage sample, I got the following errors on Firefox specifically:
image
image

Refresh the page would cause either of these errors would pop up.

On the per-workspace sample, the editor initially started up, but clicking on the web-nodejs-sample project in the file explorer (which was written in yellow text) gave the following error:

image

This issue occured on Chrome less often, but still happened sometimes (The workbench failed to connect to the server (Error: WebSocket close with status code 10006)).

Another oddity: Though the git-clone sample workspace opened in Chrome, there's no devworkspace-operator project in the file explorer? And the devfile in the workspace is incorrect?

image

exec'ing into the workspace pod (or opening up a terminal in the editor) showed that the devworkspace-operator project was there. I thought at first this issue occured cause I had multiple workspaces started at the same time, but this still happened after deleting all workspaces and starting the git-clone sample again.

I'm assuming these are all Che Code bugs but not sure yet. Will file a bug report about these issues.
Even then, it might be best for these issues to be resolved before we merge this PR so that the samples all work as expected.

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 1, 2023

Retested samples on OpenShift 4.12, using Chromium 109.0.5414.74 and Firefox 109.0:

  • Che-code samples start now, after Mario's fix. This led me to discover a bug in DWO's 'basic' routing reconciler: DevWorkspaceRouting reconciler breaks endpoints when they specify a path #1031. I'll fix this one asap.
  • I cannot reproduce issues around lost connection, etc., and workspaces work as expected (to my experience). I'm able to open a terminal session, files, etc.
  • The issue for the git clone sample is expected behavior; when Code launches in the workspace, it opens the first project rather than the projects folder. You can open a workspace with all your projects manually.
    • This shouldn't be a problem, as devfiles are currently intended to only have one project

@amisevsk
Copy link
Collaborator Author

amisevsk commented Feb 7, 2023

@AObuchow This PR is still waiting on approve/request changes

@AObuchow
Copy link
Collaborator

AObuchow commented Feb 8, 2023

@AObuchow This PR is still waiting on approve/request changes

Thanks for the ping @amisevsk, I lost track of this - will finalize my review tomorrow and so we can get this merged

Copy link
Collaborator

@AObuchow AObuchow left a comment

Choose a reason for hiding this comment

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

Tried all the updated samples and everything works as expected 🙏

@openshift-ci
Copy link

openshift-ci bot commented Feb 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, AObuchow

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@amisevsk amisevsk merged commit 93627ae into devfile:main Feb 9, 2023
@amisevsk amisevsk deleted the update-samples branch February 9, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants