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

fix: Plugins without sidecars were not injected #1248

Merged
merged 2 commits into from
Nov 5, 2021

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Nov 4, 2021

What does this PR do?

At the beginning, theia component was part of the devWorkspace and then we
had access to the atttributes from the DevWorkspace object
Now, it's part of the devWorkspaceTemplate object

Switch to the flattened devfile instead so nothing to query externally
Also, in order to reduce sed usage, use jq/yq instead as there was also a bug
on empty list

Added some unit tests as well

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#20718

How to test this PR?

Start the workspace from the issue
Eslint should not be there (check output widget inside theia or command palette)
if you check the logs of the init container you might as well see tons of errors

Open a terminal and drop anything located inside /plugins folder

Edit the DevWorkspaceTemplate object

Search for the remote-runtime component

app.kubernetes.io/component: remote-runtime-injector
app.kubernetes.io/part-of: che-theia.eclipse.org

and replace the default image by quay.io/fbenoit/che-theia-endpoint-runtime-binary:20211104c

restart the workspace
eslint should be there

Also you might check the log trace of the init-container and see something like

downloading https://download.jboss.org/jbosstools/vscode/3rdparty/vscode-eslint/vscode-eslint-2.1.1-1e15d3.vsix to /plugins for component theia

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

Change-Id: Ic8b417484596f8440a157e0d24cd7bbf4da9c40c
Signed-off-by: Florent Benoit fbenoit@redhat.com

@codecov
Copy link

codecov bot commented Nov 4, 2021

Codecov Report

Merging #1248 (8a87c0c) into main (c299f59) will decrease coverage by 0.10%.
The diff coverage is 25.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1248      +/-   ##
==========================================
- Coverage   32.78%   32.67%   -0.11%     
==========================================
  Files         290      296       +6     
  Lines        9885     9872      -13     
  Branches     1457     1327     -130     
==========================================
- Hits         3241     3226      -15     
- Misses       6641     6642       +1     
- Partials        3        4       +1     
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%> (ø)
...rowser/src/browser/che-mini-browser-environment.ts 0.00% <0.00%> (ø)
...ia-plugin-remote/src/node/hosted-plugin-service.ts 0.00% <0.00%> (ø)
...in-remote/src/node/plugin-remote-backend-module.ts 0.00% <0.00%> (ø)
...theia-plugin-remote/src/node/plugin-remote-init.ts 0.00% <0.00%> (ø)
... and 213 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 123ca86...8a87c0c. Read the comment docs.

Copy link
Contributor

@svor svor left a comment

Choose a reason for hiding this comment

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

Works with the sample described in the issue:

screenshot-che-eclipse-che apps cluster-pcccs pcccs sandbox279 opentlc com-2021 11 04-13_01_18

Also tried with scala sample: https://github.com/svor/scala-sbt/tree/devfile2 (contains 2 extensions, one of them without sidecar):
screenshot-che-eclipse-che apps cluster-pcccs pcccs sandbox279 opentlc com-2021 11 04-13_22_55

Works as expected!

@che-bot
Copy link
Contributor

che-bot commented Nov 4, 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:1248
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1248

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

@benoitf

This comment has been minimized.

At the beginning, theia component was part of the devWorkspace and then we
had access to the atttributes from the DevWorkspace object
Now, it's part of the devWorkspaceTemplate object

Switch to the flattened devfile instead so nothing to query externally
Also, in order to reduce sed usage, use jq/yq instead as there was also a bug
on empty list

Added some unit tests as well

Change-Id: Ic8b417484596f8440a157e0d24cd7bbf4da9c40c
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
…ng, theia component was part of the devWorkspace and then we had access to the atttributes from the DevWorkspace object Now, it's part of the devWorkspaceTemplate object

Change-Id: I75d55afc97f4888d4c77a307aa25b7776b308f73
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 5, 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:1248
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1248

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

@benoitf benoitf merged commit 11c0813 into eclipse-che:main Nov 5, 2021
@benoitf benoitf deleted the CHE-20718 branch November 5, 2021 14:23
@che-bot che-bot added this to the 7.39 milestone Nov 5, 2021
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.

4 participants