feat: manage default uri plugins for DevWorkspaces #425

dkwon17 · 2021-12-13T22:09:41Z

What does this PR do?

This PR provides support for adding/deleting default plugins for DevWorkspaces. This PR only supports managing plugins specified by url to a v2 devfile / DW / DWT.

Example plugin urls:

Default plugin urls are to be added to the Che CR's server.workspaceDefaultPlugins field, which is introduced in this PR: eclipse-che/che-operator#1248.

Example:

  server:
    workspacesDefaultPlugins:
      - editor: 'eclipse/che-theia/latest'
        plugins: ['https://eclipse-che.github.io/che-plugin-registry/main/v3/plugins/containers/buildah/latest/devfile.yaml']

What issues does this PR fix or reference?

This PR brings support for specifying default plugins by url in the CR for: eclipse-che/che#20090

Is it tested? How?

Deploy Che using the chectl command from this PR: eclipse-che/che-operator#1248, with this patch:

spec:
 devWorkspace:
   enabled: true
 server:
    dashboardImagePullPolicy: Always
    dashboardImage: 'quay.io/dkwon17/che-dashboard:che-20090'

Add this field to the existing CheCluster CR:

  server:
    workspacesDefaultPlugins:
      - editor: 'eclipse/che-theia/next'
        plugins: ['https://eclipse-che.github.io/che-plugin-registry/main/v3/plugins/containers/buildah/latest/devfile.yaml']

Start a devworkspace from the che-dashboard. For the screenshot below, I created a Go devworkspace.
Verify that the devworkspace has a plugin component for the default plugin
Remove server.workspacesDefaultPlugins from the CheCluster CR.
Start the same devworkspace from step 4, and verify that the devworkspace does not have the default plugin anymore

Release Notes

Docs PR

TODO

che-bot · 2021-12-13T22:09:43Z

Can one of the admins verify this patch?

dkwon17 · 2021-12-13T22:48:15Z

...ard-frontend/src/services/workspace-client/devworkspace/DevWorkspaceDefaultPluginsHandler.ts

+
+    workspace.spec.template.components.push({
+      name: pluginName,
+      attributes: { [DEFAULT_PLUGIN_ATTRIBUTE]: true },


An attribute is being set here (default-plugin), in order to distinguish between non-default and default plugins in spec.template.components:

ibuziuk · 2021-12-14T13:35:24Z

...ard-frontend/src/services/workspace-client/devworkspace/DevWorkspaceDefaultPluginsHandler.ts

+    });
+  }
+
+  private isUri(str: string) {


my understanding is that if the definition of the plugin is not valid (not URI), there will be no error message, and the plugins will not be processed at all. Probably a good idea, but can we possibly at least log it somehow

Yes, that's how it works, and sounds good, I've updated the PR with logging:

che-dashboard/packages/dashboard-frontend/src/services/workspace-client/devworkspace/DevWorkspaceDefaultPluginsHandler.ts

Lines 36 to 42 in 6aeacf6

const defaultUriPlugins = new Set(defaultPlugins.filter(plugin => {

if (this.isUri(plugin)) {

return true;

}

console.log(`Default plugin ${plugin} is not a uri. Ignoring.`);

return false;

}));

ibuziuk · 2021-12-14T13:36:34Z

...ard-frontend/src/services/workspace-client/devworkspace/DevWorkspaceDefaultPluginsHandler.ts

+import { V1alpha2DevWorkspaceSpecTemplateComponents } from '@devfile/api';
+import * as DwApi from '../../dashboard-backend-client/devWorkspaceApi';
+
+const DEFAULT_PLUGIN_ATTRIBUTE = 'default-plugin';


@amisevsk wdyt? is it ok to add extra attribute in the plugin definition - https://github.com/eclipse-che/che-dashboard/pull/425/files#r768182143

Yeah, it's not a problem. Attributes are intended to be implementation-dependant additional info, so this field is intended for use by other tools (DWO does this too). The only recommendation I'd have is to use a prefix to scope the attribute to Che itself, e.g. che.eclipse.org/default-plugin

Thanks, updated to use che.eclipse.org/default-plugin

codecov · 2021-12-14T16:26:23Z

Codecov Report

Merging #425 (0903fac) into main (6dcde16) will increase coverage by 0.20%.
The diff coverage is 69.81%.

❗ Current head 0903fac differs from pull request most recent head 6aeacf6. Consider uploading reports for the commit 6aeacf6 to get more accurate results

@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
+ Coverage   49.61%   49.81%   +0.20%     
==========================================
  Files         211      213       +2     
  Lines        7242     7265      +23     
  Branches     1199     1201       +2     
==========================================
+ Hits         3593     3619      +26     
+ Misses       3308     3300       -8     
- Partials      341      346       +5

Impacted Files	Coverage Δ
...ontend/src/store/Workspaces/devWorkspaces/index.ts	`10.19% <0.00%> (-0.70%)`	⬇️
...rvices/dashboard-backend-client/serverConfigApi.ts	`36.36% <36.36%> (ø)`
.../devworkspace/DevWorkspaceDefaultPluginsHandler.ts	`78.94% <78.94%> (ø)`
...orkspace-client/devworkspace/devWorkspaceClient.ts	`45.88% <100.00%> (+0.71%)`	⬆️
...ashboard-frontend/src/components/LogsTab/index.tsx	`76.59% <0.00%> (-4.66%)`	⬇️
...frontend/src/containers/WorkspaceActions/index.tsx	`77.38% <0.00%> (-0.78%)`	⬇️
...s/dashboard-frontend/src/store/Workspaces/index.ts	`7.25% <0.00%> (ø)`
...rd-frontend/src/containers/FactoryLoader/index.tsx	`73.59% <0.00%> (ø)`
...es/dashboard-frontend/src/containers/IdeLoader.tsx	`50.96% <0.00%> (+0.01%)`	⬆️
... and 4 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 6dcde16...6aeacf6. Read the comment docs.

openshift-ci · 2021-12-15T15:48:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amisevsk, dkwon17, ibuziuk

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

Needs approval from an approver in each of these files:

OWNERS

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

dkwon17 · 2021-12-15T16:35:11Z

I have created a new PR on the original repository: #426

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

openshift-ci · 2021-12-16T16:55:35Z

New changes are detected. LGTM label has been removed.

dmytro-ndp · 2021-12-16T17:42:49Z

[dashboard-ci-test]

che-bot · 2021-12-16T18:00:20Z

✅ E2E dashboard tests succeed 🎉

See Details

Jenkins job
test report
logs and configs
Che dashboard image:quay.io/crw_pr/che-dashboard:425
Che server image: quay.io/crw_pr/che-server:425

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

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

openshift-ci · 2021-12-20T10:36:26Z

@dkwon17: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/v8-dashboard-happy-path	`a313f38`	link	true	`/test v8-dashboard-happy-path`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

dkwon17 · 2021-12-21T01:49:23Z

Closing in favour of #425

dkwon17 requested review from akurinnoy and olexii4 as code owners December 13, 2021 22:09

dkwon17 commented Dec 13, 2021

View reviewed changes

ibuziuk reviewed Dec 14, 2021

View reviewed changes

ibuziuk approved these changes Dec 15, 2021

View reviewed changes

openshift-ci bot assigned ibuziuk Dec 15, 2021

openshift-ci bot added the lgtm label Dec 15, 2021

dkwon17 force-pushed the che-20090 branch from 0903fac to 35ea6de Compare December 15, 2021 14:04

openshift-ci bot removed the lgtm label Dec 15, 2021

dkwon17 force-pushed the che-20090 branch from 35ea6de to 6aeacf6 Compare December 15, 2021 14:05

amisevsk approved these changes Dec 15, 2021

View reviewed changes

openshift-ci bot assigned amisevsk Dec 15, 2021

openshift-ci bot added the lgtm label Dec 15, 2021

dkwon17 mentioned this pull request Dec 15, 2021

feat: manage default uri plugins for DevWorkspaces #426

Merged

feat: manage default uri plugins for DevWorkspaces

a313f38

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

dkwon17 force-pushed the che-20090 branch from 6aeacf6 to a313f38 Compare December 16, 2021 16:55

openshift-ci bot removed the lgtm label Dec 16, 2021

dkwon17 closed this Dec 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: manage default uri plugins for DevWorkspaces #425

feat: manage default uri plugins for DevWorkspaces #425

dkwon17 commented Dec 13, 2021

che-bot commented Dec 13, 2021

dkwon17 Dec 13, 2021

ibuziuk Dec 14, 2021

dkwon17 Dec 15, 2021 •

edited

Loading

ibuziuk Dec 14, 2021

amisevsk Dec 14, 2021

dkwon17 Dec 15, 2021

codecov bot commented Dec 14, 2021 •

edited

Loading

openshift-ci bot commented Dec 15, 2021

dkwon17 commented Dec 15, 2021

openshift-ci bot commented Dec 16, 2021

dmytro-ndp commented Dec 16, 2021

che-bot commented Dec 16, 2021

openshift-ci bot commented Dec 20, 2021

dkwon17 commented Dec 21, 2021

	const defaultUriPlugins = new Set(defaultPlugins.filter(plugin => {
	if (this.isUri(plugin)) {
	return true;
	}
	console.log(`Default plugin ${plugin} is not a uri. Ignoring.`);
	return false;
	}));

feat: manage default uri plugins for DevWorkspaces #425

feat: manage default uri plugins for DevWorkspaces #425

Conversation

dkwon17 commented Dec 13, 2021

What does this PR do?

What issues does this PR fix or reference?

Is it tested? How?

Release Notes

Docs PR

che-bot commented Dec 13, 2021

dkwon17 Dec 13, 2021

Choose a reason for hiding this comment

ibuziuk Dec 14, 2021

Choose a reason for hiding this comment

dkwon17 Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

ibuziuk Dec 14, 2021

Choose a reason for hiding this comment

amisevsk Dec 14, 2021

Choose a reason for hiding this comment

dkwon17 Dec 15, 2021

Choose a reason for hiding this comment

codecov bot commented Dec 14, 2021 • edited Loading

Codecov Report

openshift-ci bot commented Dec 15, 2021

dkwon17 commented Dec 15, 2021

openshift-ci bot commented Dec 16, 2021

dmytro-ndp commented Dec 16, 2021

che-bot commented Dec 16, 2021

openshift-ci bot commented Dec 20, 2021

dkwon17 commented Dec 21, 2021

dkwon17 Dec 15, 2021 •

edited

Loading

codecov bot commented Dec 14, 2021 •

edited

Loading