Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Install plugins to devworkspaces #1267

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Conversation

vitaliy-guliy
Copy link
Contributor

@vitaliy-guliy vitaliy-guliy commented Nov 30, 2021

What does this PR do?

Allows to manage che-theia plugins for devworkspaces by Che-Theia Plugins view.

Screenshot/screencast of this PR

Screenshot from 2022-01-31 13-56-11

Screenshot from 2022-01-31 13-56-23

What issues does this PR fix or reference?

eclipse-che/che#20638

How to test this PR?

Create a workspace from this repository https://github.com/vitaliy-guliy/java-spring-petclinic/tree/devfilev2-plugin-installation

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.

Happy Path Channel

HAPPY_PATH_CHANNEL=next

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@benoitf
Copy link
Contributor

benoitf commented Dec 1, 2021

I would avoid to mix code refactoring and developing a new feature in the same PR

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #1267 (140bab7) into main (c299f59) will increase coverage by 4.04%.
The diff coverage is 39.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1267      +/-   ##
==========================================
+ Coverage   32.78%   36.83%   +4.04%     
==========================================
  Files         290      330      +40     
  Lines        9885    11305    +1420     
  Branches     1457     1557     +100     
==========================================
+ Hits         3241     4164     +923     
- Misses       6641     7136     +495     
- Partials        3        5       +2     
Impacted Files Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx 0.00% <0.00%> (ø)
...credentials/src/browser/che-credentials-service.ts 0.00% <0.00%> (ø)
...entials/src/browser/credentials-frontend-module.ts 0.00% <0.00%> (ø)
...eia-credentials/src/common/credentials-protocol.ts 0.00% <0.00%> (ø)
...eia-credentials/src/node/che-credentials-server.ts 0.00% <0.00%> (ø)
...s/src/node/che-theia-credentials-backend-module.ts 0.00% <0.00%> (ø)
...ashboard/src/browser/che-theia-dashboard-module.ts 0.00% <0.00%> (ø)
...ia-dashboard/src/browser/theia-dashboard-client.ts 0.00% <0.00%> (ø)
...rowser/src/browser/che-mini-browser-environment.ts 0.00% <0.00%> (ø)
...in-ext/src/browser/che-sidecar-file-system-main.ts 100.00% <ø> (ø)
... and 295 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c12893c...140bab7. Read the comment docs.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot
Copy link
Contributor

che-bot commented Dec 17, 2021

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1267
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1267

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

build.sh Outdated Show resolved Hide resolved
@vitaliy-guliy
Copy link
Contributor Author

@benoitf
Copy link
Contributor

benoitf commented Dec 22, 2021

Tried with che-server mode and it was ok
On DevWorkspace mode, I clicked on 'installing' Typescript Language Features.

Workspace is yellow, and workspace can't restart:

$ oc get dw --all-namespaces 
NAMESPACE                   NAME               DEVWORKSPACE ID             PHASE    INFO
che-kube-admin-che-ljwzmv   spring-petclinic   workspace3ea5705de711434f   Failed   duplicate init containers in the workspace definition: project-clone

So I'm wondering why it's updating the devWorkspace template when I just want to try it and then we can't restart the workspace so better to not do that for now

image

@vitaliy-guliy
Copy link
Contributor Author

vitaliy-guliy commented Dec 22, 2021

Tried with che-server mode and it was ok On DevWorkspace mode, I clicked on 'installing' Typescript Language Features.

Workspace is yellow, and workspace can't restart:

$ oc get dw --all-namespaces 
NAMESPACE                   NAME               DEVWORKSPACE ID             PHASE    INFO
che-kube-admin-che-ljwzmv   spring-petclinic   workspace3ea5705de711434f   Failed   duplicate init containers in the workspace definition: project-clone

So I'm wondering why it's updating the devWorkspace template when I just want to try it and then we can't restart the workspace so better to not do that for now

I've noticed earlier, we have several issues eclipse-che/che#20638 (comment)

  • Typescript Language Features specifies a container to run the extension. For devworkspaces we inject such extensions into tools container, not into the editor. Theia deploy plugin command deploys the plugin it into theia container, but it should be running in sidecar as remote. So, the common way (and the cheapest one) for both types of extensions (that are running inside theia and tools container) - is to restart the workspace.

  • why the workspace is autorestarted? Its because there is a trigger on the backend, which performs restart when we update devworkspace object (we discussed that earlier)

@che-bot
Copy link
Contributor

che-bot commented Dec 22, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1267
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1267

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

  • meta.yaml should not be used with DevWorkspace engine (the counter part file is che-theia-plugin.yaml)
  • We should not update the whole devfile but just patch devWorkspace object
  • It should prompt the user that workspace will be restarted before really installing the plug-in
  • the whole ensureComponentIsReadyToRunExtensions is boiler plate that should be removed (no dev container = error)
  • there is a library jsonc-parser to parse json-c files

@vitaliy-guliy vitaliy-guliy marked this pull request as ready for review January 31, 2022 16:26
@benoitf benoitf dismissed their stale review February 1, 2022 07:10

code has been changed, so dismissing my previous review / request for changes

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

I think it'll nice if we're able to merge just after 7.43 is released

@svor
Copy link
Contributor

svor commented Feb 1, 2022

I've tried to install/uninstall plugins and it looks good to me, good work @vitaliy-guliy 👍
screenshot-che-eclipse-che apps cluster-2glnx 2glnx sandbox1911 opentlc com-2022 02 01-16_57_20

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

Thanks for providing the test Devfile!
After testing, I can confirm the functionality works as expected. 👍

@benoitf
Copy link
Contributor

benoitf commented Feb 3, 2022

so let's merge ?

@vitaliy-guliy vitaliy-guliy merged commit 09906cd into main Feb 3, 2022
@vitaliy-guliy vitaliy-guliy deleted the devworkspace-plugins branch February 3, 2022 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants