Skip to content

[ide] use code flag to install extensions #8313

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

Merged
merged 2 commits into from
Mar 1, 2022
Merged

[ide] use code flag to install extensions #8313

merged 2 commits into from
Mar 1, 2022

Conversation

mustard-mh
Copy link
Contributor

@mustard-mh mustard-mh commented Feb 18, 2022

Description

Track readiness for content ide ide-desktop ready

Upstream vscode has provided extension install with flag --install-extension and there are some bugs with our installation code
🐛 Remove extensionIdRegex for extensions that install via file path
🐛 Support extensions that their id starts with http or https

It's a Golang copy of these codes

And part of extension host timeout #7863 fixes, more detail see internal slack

Related Issue(s)

Relates #7863, fixes #7409

How to test

For test, we use this version of vscode which remove our custom extensions install only gitpod-io/openvscode-server@6d0747d

Just remove support of install with file path

  1. Make sure to use insider vscode https://hw-code-flag.staging.gitpod-dev.com/preferences
  2. Open a workspace, you can use this repo https://hw-code-flag.staging.gitpod-dev.com/#github.com/mustard-mh/test/tree/vs/exthost-no-file
    image
  3. Click Extensions button of vscode check if all your extensions installed
  4. Run ps -efj fww should have a process like
sh /ide/bin/gitpod-code --start-server --install-builtin-extension github.vscode-pull-request-github --install-extension bierner.color-info --install-extension [email protected] --install-extension magicstack.magicpython --install-extension formulahendry.code-runner --port 23000 --host 0.0.0.0 --without-connection-token --server-data-dir /workspace/.vscode-remote
  1. Go to segment.com search with supervisor_readiness to see event tracking

Release Notes

NONE

Documentation

/werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #8313 (8d31c75) into main (42d529c) will increase coverage by 18.73%.
The diff coverage is 0.00%.

❗ Current head 8d31c75 differs from pull request most recent head c8ec22e. Consider uploading reports for the commit c8ec22e to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8313       +/-   ##
===========================================
+ Coverage   12.31%   31.05%   +18.73%     
===========================================
  Files          20       37       +17     
  Lines        1161     5842     +4681     
===========================================
+ Hits          143     1814     +1671     
- Misses       1014     3890     +2876     
- Partials        4      138      +134     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-supervisor-app 35.12% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/supervisor/pkg/supervisor/supervisor.go 5.95% <0.00%> (ø)
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
components/supervisor/pkg/ports/served-ports.go 80.76% <0.00%> (ø)
components/supervisor/pkg/terminal/ring-buffer.go 45.65% <0.00%> (ø)
components/supervisor/pkg/ports/ports-config.go 80.00% <0.00%> (ø)
components/supervisor/pkg/supervisor/tasks.go 58.56% <0.00%> (ø)
...mponents/supervisor/pkg/supervisor/notification.go 83.95% <0.00%> (ø)
components/supervisor/pkg/supervisor/services.go 21.52% <0.00%> (ø)
components/supervisor/pkg/supervisor/ssh.go 0.00% <0.00%> (ø)
... and 12 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 42d529c...c8ec22e. Read the comment docs.

@akosyakov akosyakov requested a review from jeanp413 February 18, 2022 14:59
@jeanp413
Copy link
Member

@akosyakov we should keep supporting GITPOD_EXTERNAL_EXTENSIONS variable right? As typefox folks are using it context
cc @mustard-mh

@akosyakov
Copy link
Member

@jeanp413 for now we should preserve the same semantic like in in customisations to VS Code Server. For cleaning up let's file a separate issue and it will need some research and planning.

@mustard-mh
Copy link
Contributor Author

mustard-mh commented Feb 18, 2022

Updated, need re-review 😄

@akosyakov
Copy link
Member

@jeanp413 @mustard-mh I found #4630 which I filed last year. I think this PR goes in right direction if it does not rely on env vars anymore. I think it is fine to remove both GITPOD_EXTERNAL_EXTENSIONS and GITPOD_RESOLVED_EXTENSIONS. They were never intended to be used for other purposes. External extensions means VSIX files from .gitpod.yml. We already cover them with this PR.

We should do #4630 as a follow-up after all workspaces are running this PR. It is important since it will improve startup of workspace on the server.

@akosyakov
Copy link
Member

akosyakov commented Feb 19, 2022

@mustard-mh @jeanp413 the trouble with this PR that it will harm perceived performance, before a user could access VS Code UI before actually content was loaded or extensions are installed, now everything will be delayed. Is not there a way to start a server without extensions and then install them in parallel?

It is in regards to .gitpod.yml parsing and downloading vsix files (since it requires content ready), for other cases like adding built-in extensions it is alright. Could you check whether WorkspaceContext already has a info about initially parser gitpod.yml maybe we are better to fetch it instead of parsing .gitpod.yml from the disk?

@mustard-mh
Copy link
Contributor Author

We can start code first then command extentions management to install extensions

@mustard-mh
Copy link
Contributor Author

But we should wait until code exthost ready, if cli command didn't await
cc @akosyakov @jeanp413

@mustard-mh
Copy link
Contributor Author

mustard-mh commented Feb 19, 2022

Extensions that supervisor provided, only serve once and it's reading .gitpod.yml file with github API, and save it to DB.

If we use it, user start and stop worksapces to trigger extensions intall not work

So we should

🅰️

  1. Start code first
  2. Read .gitpod.yml from disk after content init ready
  3. And after exthost ready, install extensions. Wait by watch vscode.lock file? Or @jeanp413 have other ideas?

🅱️

  1. Save extensions again in DB before workspace stopped, (need discuss with WebApp team why it designed like this, and if changes break somthing)
  2. Install extensions
    (a) Start code with install extensions
    (b) Same with 🅰️ step3

💡 Other ideas...

cc @akosyakov

@akosyakov
Copy link
Member

akosyakov commented Feb 19, 2022

I think .gitpod.yml already parsed and stored in db as a part of Workspace. We could fetch it from the server with getWorkspace or better server could install an env var like GITPOD_VSCODE_CONFIG. So we can access it faster. It will remove requirements to wait for content load and we could immediately start server. We will still need to wait for vsix files to download, but it should be rather a corner case.

@jeanp413
Copy link
Member

I was thinking about this yesterday too, I'll write some questions I did myself:

  • What does contentAvailable mean?, does it mean the whole /workspace folder is mounted/restored? does it mean only /workspace/repo folder is available after cloning? does it mean both?
  • If it means whole /workspace is restored then we really want to wait for it as .vscode-remote is inside /workspace folder
  • If not, then I think we should not wait for it (I think we already do this in vscode workbench.ts right?) nor reading .gitpod.yml from filesystem as it will delay server startup, I see the server already knows .gitpod.yml contents by using github/gitlab/etc filesystem API, so I would vote for keep using env variables or calling server api to get it's contents (I think we do this to create task terminals?)
  • Regarding installing extensions from URL, we first need to download it so we can pass a path to the vsix file to the server cli, but the downloading will delay server startup too, so I think we should look into other options:
    • maybe create an internal task that uses gp await 23000 to ensure server started, download them and install them using server cli
    • install them using gitpod extension once extension host started

@akosyakov
Copy link
Member

akosyakov commented Feb 19, 2022

If it means whole /workspace is restored then we really want to wait for it as .vscode-remote is inside /workspace folder

It is a good point, contentAvailable means that everything under /workspace was restored from backup for instance. 😬

Maybe we could move it somehow in server? i.e. to delay file system access internally?

If not, then I think we should not wait for it (I think we already do this in vscode workbench.ts right?) nor reading .gitpod.yml from filesystem as it will delay server startup, I see the server already knows .gitpod.yml contents by using github/gitlab/etc filesystem API, so I would vote for keep using env variables or calling server api to get it's contents (I think we do this to create task terminals?)

I think we still should do it in regardless of contentAvailable via env var is fastest.

Regarding installing extensions from URL, we first need to download it so we can pass a path to the vsix file to the server cli, but the downloading will delay server startup too, so I think we should look into other options:

I think we should ignore it for now and delay. I don't think that it is very popular feature. Maybe we should add some analytics for server startup time and usage of vsix files in .gitpod.yml?

@jeanp413
Copy link
Member

jeanp413 commented Feb 19, 2022

Maybe we could move it somehow in server? i.e. to delay file system access internally?

You mean /ide folder? does it persist data? if not it would be the same as moving it back to home folder, nice thing about having it in workspace folder is that we can look into logs of previous run and extensions already installed on workspace restart

I think we still should do it in regardless of contentAvailable via env var is fastest.

Agree 👍

I think we should ignore it for now and delay. I don't think that it is very popular feature. Maybe we should add some analytics for server startup time and usage of vsix files in .gitpod.yml?

👍 to adding some analytics, also small thing about options I listed is we don't need to worry about race condition in await extensionsReady in server on startup

@akosyakov
Copy link
Member

You mean /ide folder? does it persist data? if not it would be the same as moving it back to home folder, nice thing about having it in workspace folder is that we can look into logs of previous run and extensions already installed on workspace restart

As for now only /workspace folder is persisted. I meant in VS Code Server remote file system service, but it will require changing ms code.

Let's start with contentAvailable check but in parallel with resolving extensions? Plus adding analytics for startup times with info about extensions in .gitpod.yml. In one month we could make a better decision then.

@jeanp413
Copy link
Member

Let's start with contentAvailable check but in parallel with resolving extensions?

If we resolve extensions via env variable, contentAvailable check should be done in supervisor?

log.WithError(err).WithField("wsInfo", wsInfo).WithField("cstate", contentStatus).Error("resolve workspace info failed")
return
}
log.WithField("cost", time.Now().Local().Sub(startTime).Milliseconds()).Info("content available")
Copy link
Member

Choose a reason for hiding this comment

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

Where can I see this logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Open a workspace in this preview env, get URL like https://mustardmh-test-fibq5bqd3jm.ws-dev.hw-code-flag.staging.gitpod-dev.com/, copy prefix mustardmh-test-fibq5bqd3jm
  2. Make sure you setup your GCP auth, see notion
  3. Open a gitpod-io/gitpod workspace
  4. Run kubens staging-hw-code-flag to switch to the namespace
  5. Run kubectl get pods -l metaID=mustardmh-test-fibq5bqd3jm to get correct workspace
  6. Run kubectl logs <workspace_pod_name> -f to access and follow the logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or you can go to GCP. Click correct workspace pod to access logs

@jeanp413
Copy link
Member

Also need to add --do-not-sync to the server arguments in both arrays to avoid syncing this extensions

@mustard-mh
Copy link
Contributor Author

mustard-mh commented Feb 26, 2022

I can't find this flag --do-not-sync @jeanp413
image

@jeanp413
Copy link
Member

jeanp413 commented Feb 26, 2022

Not all options appear in help but it's there and it's used here

@jeanp413
Copy link
Member

LGTM, left a minor comment in the gp-code branch gitpod-io/openvscode-server@6d0747d#r67646084

@mustard-mh does this still needs to be done? if not could you squash so we can merge it

need to clean up custom insider for test and debug log for track

WORKSPACE.yaml Outdated
@@ -7,7 +7,7 @@ defaultArgs:
jbMarketplacePublishTrigger: "false"
publishToJBMarketplace: true
localAppVersion: unknown
codeCommit: 0175d2fd7b619fb1c34dcf2a0e17a909acd1be3e
codeCommit: 6d0747dbcd685cfe47251d56a452b619c9da444b
Copy link
Member

Choose a reason for hiding this comment

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

It does not make it stable, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for test only

@akosyakov
Copy link
Member

@mustard-mh @jeanp413 Please add a new event here: https://www.notion.so/gitpod/dd900d5177944fc38d22bb8c7d3d2cce?v=54cdae8ff0f54b4d8235429ab9893e49

Timestamp int64 `json:"timestamp,omitempty"`
}
trackFn := func(ctx context.Context, gitpodService *gitpod.APIoverJSONRPC, cfg *Config, kind string) {
gitpodService.TrackEvent(ctx, &gitpod.RemoteTrackMessage{
Copy link
Member

Choose a reason for hiding this comment

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

it does not return err or something like that to log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I print this error in a new commit, but when this reproduce, no error is returned and there is no useful information in the log, see mustardmh-test-aabar7hcoyk

@akosyakov
Copy link
Member

Does it really fix #7863 ? I think it is just a clean up we did not really identify a root cause?

@mustard-mh
Copy link
Contributor Author

mustard-mh commented Mar 1, 2022

@jeanp413 found out that there is some race when installing extensions internal chat

This is because our custom extension install code contains both id and URL of extension internal chat

This PR is related to #7863

Our plans to fix #7863 are:

  1. Using server arguments (This PR)
  2. Remove our custom extension install code in openvscode-server

cc @akosyakov

@akosyakov
Copy link
Member

Yeah, I seen internal chat, ok, let's close it for now and keep eyes on it.

Copy link
Member

@jeanp413 jeanp413 left a comment

Choose a reason for hiding this comment

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

LGTM

@jeanp413
Copy link
Member

jeanp413 commented Mar 1, 2022

/unhold

@mustard-mh
Copy link
Contributor Author

I print this error in a new commit, but when this reproduce, no error is returned and there is no useful information in the log, see mustardmh-test-aabar7hcoyk

File an issue about segment #8516

@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production release-note-none size/XXL team: IDE
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Clean up initial extension installation on vscode web
4 participants