feat: Use post-start event to start VS Code #1566

RomanNikitenko · 2023-02-01T13:25:02Z

depends on: #1613

What does this PR do?

Use post-start event instead of command to start VS Code

What issues does this PR fix or reference?

eclipse-che/che#21879

How to test this PR?

We should check that VS Code starts successfully after resolving devfile/devworkspace-operator#1029

I tested it like:

go to the dashboard => Create Workspace page
put https://github.com/RomanNikitenko/devfile-stacks-nodejs-react/tree/post-start to the Git Repo URL
click Create & Open button

The sample contains the corresponding changes for the editor description.

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.

Signed-off-by: Roman Nikitenko rnikiten@redhat.com

openshift-ci · 2023-02-01T13:25:07Z

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

github-actions · 2023-02-01T13:25:15Z

Click here to review and test in web IDE:

github-actions · 2023-02-01T19:40:43Z

Click here to review and test in web IDE:

github-actions · 2023-02-16T13:33:17Z

Click here to review and test in web IDE:

codecov · 2023-02-16T13:35:08Z

Codecov Report

Merging #1566 (74b8114) into main (d874c08) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 74b8114 differs from pull request most recent head ebf56c6. Consider uploading reports for the commit ebf56c6 to get more accurate results

@@           Coverage Diff           @@
##             main    #1566   +/-   ##
=======================================
  Coverage   93.68%   93.68%           
=======================================
  Files          47       47           
  Lines        1457     1457           
  Branches      253      253           
=======================================
  Hits         1365     1365           
  Misses         92       92

Impacted Files	Coverage Δ
...ols/build/src/devfile/meta-yaml-to-devfile-yaml.ts	`100.00% <100.00%> (ø)`
...uild/src/editor/che-editors-meta-yaml-generator.ts	`100.00% <100.00%> (ø)`

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

RomanNikitenko · 2023-02-16T16:11:57Z

che-editors.yaml

    events:
      preStart:
        - init-container-command
+      postStart:
+        - init-che-code-command


The main idea is - initialization of the che-code on the post-start event vs execution entrypoint as a command to avoid overriding the UDI entrypoint.

I've tested creation of a workspace for the VS Code editor without Dashboard usage and I can confirm that the UDI entrypoint is not overridden anymore if I use these changes for the description of the editor.

At the same time there is still a problem when I create a workspace from the dashboard. So - I guess:

logic on the DWO side works correctly

we still need some changes on the dashboard side

I guess resolving this issue should fix the problem on the dashboard side.
So, let's keep the PR still as a draft.

I tested it using this sample, it contains the same changes for the editor description.
It works well after merging the corresponding changes on the dashboard side.

github-actions · 2023-03-03T12:57:57Z

Click here to review and test in web IDE:

github-actions · 2023-03-03T13:02:25Z

Click here to review and test in web IDE:

svor

Final editor's devfile.yaml doesn't contain postStart command: it could be checked in published PR's content in surge:

To fix it, need to change this logic: https://github.com/eclipse-che/che-plugin-registry/blob/main/tools/build/src/devfile/meta-yaml-to-devfile-yaml.ts#L170-L203

RomanNikitenko · 2023-03-06T08:35:42Z

Final editor's devfile.yaml doesn't contain postStart command: it could be checked in published PR's content in surge

To fix it, need to change this logic: https://github.com/eclipse-che/che-plugin-registry/blob/main/tools/build/src/devfile/meta-yaml-to-devfile-yaml.ts#L170-L203

thank you very much, Valerii, for the reviewing my PR!
I was not aware about that logic and just tested my changes as I described in #1566 (comment).

At the moment I have to switch to another blocker issue, I'm going to come back soon to investigate that logic and provide the corresponding changes...

github-actions · 2023-03-17T19:26:37Z

Click here to review and test in web IDE:

github-actions · 2023-03-18T12:42:56Z

Click here to review and test in web IDE:

github-actions · 2023-03-20T13:38:20Z

Click here to review and test in web IDE:

RomanNikitenko · 2023-03-20T13:41:03Z

@svor
I created a separate issue for the problem you mentioned in #1566 (review).

The issue: eclipse-che/che#22071
The PR with fix: #1613

svor

LGTM

@RomanNikitenko probably we need the same changes in downstream https://github.com/redhat-developer/devspaces/blob/devspaces-3-rhel-8/dependencies/che-plugin-registry/che-editors.yaml

svor · 2023-03-21T20:58:49Z

/retest

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

RomanNikitenko · 2023-03-22T05:54:33Z

eclipse-che/che#21879

thank you for the prompt!
Done: redhat-developer/devspaces#921

svor

A workspace has started with che-code editor and current changes:

tested with:
oc apply -f dw.yaml
where dw.yaml is:

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: git-clone-sample-devworkspace
spec:
  started: true
  template:
    projects:
      - name: web-nodejs-sample
        git:
          remotes:
            origin: "https://github.com/che-samples/web-nodejs-sample.git"
    commands:
      - id: say-hello
        exec:
          component: che-code-runtime-description
          commandLine: echo "Hello from $(pwd)"
          workingDir: ${PROJECT_SOURCE}/app
  contributions:
    - name: che-code
      uri: https://gist.githubusercontent.com/svor/272c56c4aab0468a6079ac932d562a75/raw/6fa7505409e93cabf11e73d2c70478db6d43c07e/che-code2.yaml
      components:
        - name: che-code-runtime-description
          container:
            env:
              - name: CODE_HOST
                value: 0.0.0.0

openshift-ci · 2023-03-22T08:22:18Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: RomanNikitenko, svor

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

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

RomanNikitenko force-pushed the post-start-event branch from d4ea77d to 74a3c60 Compare February 16, 2023 13:33

This comment was marked as duplicate.

Sign in to view

RomanNikitenko commented Feb 16, 2023

View reviewed changes

RomanNikitenko force-pushed the post-start-event branch from 74a3c60 to 5cc07f0 Compare March 3, 2023 12:57

RomanNikitenko force-pushed the post-start-event branch from 5cc07f0 to 9763cd7 Compare March 3, 2023 13:02

RomanNikitenko changed the title ~~Use post-start event to start VS Code~~ feat: Use post-start event to start VS Code Mar 3, 2023

RomanNikitenko marked this pull request as ready for review March 3, 2023 13:03

RomanNikitenko requested review from vinokurig, vitaliy-guliy and svor as code owners March 3, 2023 13:03

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

svor requested changes Mar 6, 2023

View reviewed changes

openshift-ci bot assigned svor Mar 6, 2023

RomanNikitenko mentioned this pull request Mar 8, 2023

To start VS Code, use post-start rather than overriding the entrypoint eclipse-che/che#21879

Closed

RomanNikitenko force-pushed the post-start-event branch from 9763cd7 to 19dad8a Compare March 17, 2023 19:26

This was referenced Mar 20, 2023

Events and commands defined in che-editors.yaml file are ignored eclipse-che/che#22071

Closed

fix: Do not ignore events and commands for an editor #1613

Merged

RomanNikitenko force-pushed the post-start-event branch from 74b8114 to 3c86e5c Compare March 20, 2023 13:38

svor approved these changes Mar 21, 2023

View reviewed changes

openshift-ci bot added the lgtm label Mar 21, 2023

feat: Use post-start event to start VS Code

ebf56c6

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

RomanNikitenko mentioned this pull request Mar 22, 2023

feat: Use post-start event to start VS Code redhat-developer/devspaces#921

Merged

RomanNikitenko force-pushed the post-start-event branch from 3c86e5c to ebf56c6 Compare March 22, 2023 07:07

openshift-ci bot removed the lgtm label Mar 22, 2023

svor approved these changes Mar 22, 2023

View reviewed changes

openshift-ci bot added the lgtm label Mar 22, 2023

RomanNikitenko merged commit e30461c into main Mar 22, 2023

RomanNikitenko deleted the post-start-event branch March 22, 2023 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use post-start event to start VS Code #1566

feat: Use post-start event to start VS Code #1566

RomanNikitenko commented Feb 1, 2023 •

edited

Loading

openshift-ci bot commented Feb 1, 2023

github-actions bot commented Feb 1, 2023

github-actions bot commented Feb 1, 2023

github-actions bot commented Feb 16, 2023

codecov bot commented Feb 16, 2023 •

edited

Loading

This comment was marked as duplicate.

RomanNikitenko Feb 16, 2023 •

edited

Loading

RomanNikitenko Mar 3, 2023 •

edited

Loading

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

svor left a comment

RomanNikitenko commented Mar 6, 2023

github-actions bot commented Mar 17, 2023

github-actions bot commented Mar 18, 2023

github-actions bot commented Mar 20, 2023

RomanNikitenko commented Mar 20, 2023 •

edited

Loading

svor left a comment •

edited

Loading

svor commented Mar 21, 2023

RomanNikitenko commented Mar 22, 2023

svor left a comment

openshift-ci bot commented Mar 22, 2023

feat: Use post-start event to start VS Code #1566

feat: Use post-start event to start VS Code #1566

Conversation

RomanNikitenko commented Feb 1, 2023 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

openshift-ci bot commented Feb 1, 2023

github-actions bot commented Feb 1, 2023

github-actions bot commented Feb 1, 2023

github-actions bot commented Feb 16, 2023

codecov bot commented Feb 16, 2023 • edited Loading

Codecov Report

This comment was marked as duplicate.

RomanNikitenko Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

RomanNikitenko Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

svor left a comment

Choose a reason for hiding this comment

RomanNikitenko commented Mar 6, 2023

github-actions bot commented Mar 17, 2023

github-actions bot commented Mar 18, 2023

github-actions bot commented Mar 20, 2023

RomanNikitenko commented Mar 20, 2023 • edited Loading

svor left a comment • edited Loading

Choose a reason for hiding this comment

svor commented Mar 21, 2023

RomanNikitenko commented Mar 22, 2023

svor left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Mar 22, 2023

RomanNikitenko commented Feb 1, 2023 •

edited

Loading

codecov bot commented Feb 16, 2023 •

edited

Loading

RomanNikitenko Feb 16, 2023 •

edited

Loading

RomanNikitenko Mar 3, 2023 •

edited

Loading

RomanNikitenko commented Mar 20, 2023 •

edited

Loading

svor left a comment •

edited

Loading