Skip to content
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

Implement project configurator #4631

Merged
merged 1 commit into from
Jul 22, 2021
Merged

Implement project configurator #4631

merged 1 commit into from
Jul 22, 2021

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Jun 28, 2021

Initial pass to fix #4648

Depends on & contains #4846

What it does:

  • Adds a config (text) field to d_b_project, so that Projects can have a custom .gitpod.yml content
  • Adds a Project Configurator view in the dashboard, as final step in the Add Project flow (also currently accessible via a Project's UI)
  • When determining the config of a new workspace, use this priority order:
    1. From repository
    2. (new) From Project DB
    3. From definitely-gp
    4. Derived (Go) or default
  • When starting from a custom .gitpod.yml (from the Project DB), the initializer also places the uncomitted .gitpod.yml into the workspace (to encourage repo-based configs)

How to test:

  • Create a new team
  • Add a new project (either by inserting into d_b_project, or we set up a GitHub App integration -- @AlexTugarev could you help with the latter?)
  • If the project already has a repo-based config, the configurator should indicate that and be disabled. If there is no repo config, the configurator should allow you to write a new .gitpod.yml (e.g. based on one of the presets) and then test it / iterate on it in a quick loop (with the build output on the right)

The UX of this is still rough, but the goal is to add this final step rather soon, and then iterate on the entire Add Project flow.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jun 28, 2021

/werft run

👍 started the job as gitpod-build-jx-configure-project.9

@jankeromnes jankeromnes force-pushed the jx/configure-project branch 4 times, most recently from 96138a6 to 9dc9489 Compare July 1, 2021 16:22
@jankeromnes jankeromnes force-pushed the jx/configure-project branch 11 times, most recently from 2c0fad9 to 6e3f86d Compare July 7, 2021 15:17
@jankeromnes jankeromnes force-pushed the jx/configure-project branch 7 times, most recently from 566b26b to d1d91c8 Compare July 13, 2021 09:57
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 13, 2021

/werft with-clean-slate-deployment

👎 unknown command: with-clean-slate-deployment
Use /werft help to list the available commands

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 13, 2021

/werft help

👍 You can interact with werft using: /werft command <args>.
Available commands are:

  • /werft run [annotation=value] which starts a new werft job from this context.
    You can optionally pass multiple whitespace-separated annotations.
  • /werft help displays this help

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 13, 2021

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-jx-configure-project.32

@jankeromnes jankeromnes force-pushed the jx/configure-project branch 2 times, most recently from 7952900 to 653d938 Compare July 15, 2021 07:42
@jankeromnes jankeromnes force-pushed the jx/configure-project branch 2 times, most recently from 8601021 to c669905 Compare July 21, 2021 08:45
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 21, 2021

/werft run from-scratch

👍 started the job as gitpod-build-jx-configure-project.51

@jankeromnes jankeromnes force-pushed the jx/configure-project branch 2 times, most recently from e12cd02 to 88c945f Compare July 21, 2021 10:14
Comment on lines +43 to +45
disposables.push(watchHeadlessLogs(service.server, info.latestInstance.id, chunk => {
logsEmitter.emit('logs', chunk);
}, async () => workspaceInstance?.status.phase === 'stopped'));
Copy link
Contributor Author

@jankeromnes jankeromnes Jul 21, 2021

Choose a reason for hiding this comment

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

@geropl Please double-check here: Is this really what the checkIfDone function is supposed to do?

(I'm not sure yet how watchHeadlessLogs works exactly.)

Copy link
Member

Choose a reason for hiding this comment

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

@jankeromnes Yes, if workspaceInstance is updated properly (I assume it is).

Ideally we'd have a API for "getPrebuildState" so we don't have to interpret this from the underlying instance, but the result should be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! (In the future, I think it would be cool if watchHeadlessLogs can figure out on its own whenever the job it watches is done or not 😇)

</>;
}

export function watchHeadlessLogs(server: GitpodServer, instanceId: string, onLog: (chunk: string) => void, checkIsDone: () => Promise<boolean>): DisposableCollection {
Copy link
Contributor Author

@jankeromnes jankeromnes Jul 21, 2021

Choose a reason for hiding this comment

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

Note: I'm moving this function out of WorkspaceLogs.tsx and into the new component PrebuildLogs.tsx, because it is imported eagerly in various places.

However, we don't want to import WorkspaceLogs.tsx eagerly, because that file imports all of Xterm.js (you'll notice that WorkspaceLogs itself is always imported lazily -- same for MonacoEditor now).

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 21, 2021

Alright, I think this could be merged as is.

@svenefftinge and @AlexTugarev could you please take another look?

How to test

  1. Join the "Gitpod" Team using this link (EDIT: Updated link 2021-07-22)
  2. In the team projects, open any of the 2 projects, then open the "Configure" tab:
    • For the "gitpod" project, a config is detected in the repo, so the editor should be read-only
    • For the "test" project (empty repository), you should be able to pick a configuration preset and then test it

Note: There is no GitHub App/Integration for this test environment. The Project Configurator is setup as the last step in the wizard, but I couldn't test the entire flow yet (I only tested the configurator specifically).

Follow-ups

A few review discussions are still open. I think it's best to address them in follow-up issues & Pull Requests, in order to get this first pass merged sooner and spend more time iterating on the "wizard" flow holistically.

Still, please do feel free to share thoughts & feedback, and anything bigger than a quick fix before merging will become a follow-up issue.

@svenefftinge
Copy link
Member

svenefftinge commented Jul 22, 2021

/werft run

👍 started the job as gitpod-build-jx-configure-project.56

Copy link
Member

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

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

There seems to be an issue with the prebuild logs. They logs stay empty for me and I could only see them after reloading.
Try with: https://jx-configure-project.staging.gitpod-dev.com/#prebuild/https://github.com/gitpod-io/spring-petclinic
Could it be because of the changes in the PR?

@@ -48,7 +48,7 @@ export default function Menu() {
})();

const userFullName = user?.fullName || user?.name || '...';
const showTeamsUI = user?.rolesOrPermissions?.includes('teams-and-projects') || window.location.hostname.endsWith('gitpod-dev.com') || window.location.hostname.endsWith('gitpod-io-dev.com');
const showTeamsUI = user?.rolesOrPermissions?.includes('teams-and-projects');
Copy link
Member

Choose a reason for hiding this comment

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

🙏

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jul 22, 2021

There seems to be an issue with the prebuild logs. They logs stay empty for me and I could only see them after reloading.
Try with: https://jx-configure-project.staging.gitpod-dev.com/#prebuild/https://github.com/gitpod-io/spring-petclinic
Could it be because of the changes in the PR?

Hmm, I also no longer see prebuild logs therer, in the "manual prefix" page or in the "Prebuild in Progress" page.

Also, I got this error twice in the console:

re-trying headless-logs because: error while listening to stream Error: Headless logs for f1ceab3a-3678-4607-ab74-9f070b39f406 not found

Then the prebuild was eventually successfully finished, and started in a new workspace, although I didn't get any logs.

I don't know if this can be related to this PR, or if this is a problem with dev-staging (I'm under the impression that headless logs often simply don't work in dev-staging, while they do work better in staging & production).

Are other dev-staging deployments affected as well? EDIT: Tested two other dev-staging deployments, where it seems to work better. Will double-check my changes here.

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2f6484b357b0c46d8754a426ba813d1eb6df81c5

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jankeromnes, svenefftinge

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@jankeromnes
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit 1ef7161 into main Jul 22, 2021
@roboquat roboquat deleted the jx/configure-project branch July 22, 2021 10:58
@gtsiolis
Copy link
Contributor

/woof

@roboquat
Copy link
Contributor

@gtsiolis: dog image

In response to this:

/woof

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Project Configuration
6 participants