[server] Allow re-triggering failed Prebuilds #5836

jankeromnes · 2021-09-23T15:04:05Z

Description

Make it possible to re-trigger failed or timed out Prebuilds (for the same commit)
Only show the option to re-trigger Prebuilds in the UI when it's actually possible, and provide immediate visual feedback upon (re-)triggering
Drive-by fix: Show "stopping" prebuild instances as still "running" and not yet "ready" (see community)

Related Issue(s)

How to test

Release Notes

Make it possible to re-trigger failed or timed out Prebuilds

/uncc

jankeromnes · 2021-09-27T09:29:01Z

🎰

/werft run

👍 started the job as gitpod-build-jx-allow-retriggering-prebuilds.3

jankeromnes · 2021-09-27T12:28:40Z

Note to self:

Don't render ItemList ContextMenus when there are no options (looks weird with 0 options, should be omitted instead)

jankeromnes · 2021-09-28T15:54:22Z

Getting there! 🎉

Final TODOs:

Add a Rerun Prebuild button to a failed Prebuild logs page
Remove Trigger Prebuild button when it makes no sense (or when the list of Prebuilds isn't empty)

AlexTugarev · 2021-09-29T11:17:47Z

components/server/src/projects/projects-service.ts

+                const p = prebuilds.find(p => p.id === info.id)!;
+                const r: PrebuildWithStatus = { info, status: p.state };
+                if (p.error) {
+                    r.error = p.error;


AlexTugarev · 2021-09-30T11:01:46Z

@gtsiolis, this looks good already, I hope you find some time to check soon.

gtsiolis · 2021-09-30T11:46:06Z

gtsiolis · 2021-09-30T11:51:56Z

/werft run

👍 started the job as gitpod-build-jx-allow-retriggering-prebuilds.24

gtsiolis

Hey @jankeromnes! Took a quick look at the UX changes here. 👀

Left some comment below! 🐈

Curious if this PR really fixes #5771 and #5374. 💭

Approving to unblock merging but holding in case we need to make any further changes.

/hold

gtsiolis · 2021-09-30T13:12:02Z

components/dashboard/src/projects/Prebuild.tsx

+                    {(prebuild?.status === 'aborted' || prebuild?.status === 'timeout' || !!prebuild?.error)
+                        ? <button className="flex items-center space-x-2" disabled={isRerunningPrebuild} onClick={rerunPrebuild}>
+                            {isRerunningPrebuild && <img className="h-4 w-4 animate-spin filter brightness-150" src={Spinner} />}
+                            <span>Rerun Prebuild ({prebuild.info.branch})</span>


issue: Including the branch name in the dropdown action could be confusing as most of the times this will overflow and a truncated text will be presented.

Also, since the branch name is the key value in this page (first column field), if we need to repeat this value in the dropdown we may have to reconsider the design of this list layout. ❌

suggestion: What do you think of removing the branch name reference from this dropdown action. We could keep the title attribute if you think it helps. ❓

Ah, good point, thanks! Would it be cool to:

Make the dropdown items larger?

Add a title attribute or tooltip with the full text?

gtsiolis · 2021-09-30T13:14:13Z

components/dashboard/src/projects/Prebuilds.tsx

+                {(!!project && prebuilds.length === 0) &&
+                    <button onClick={() => triggerPrebuild(null)} className="ml-2">Run Prebuild</button>}


thought: There's already an issue for adding a minimal empty state for this page in #5022. Ideally, in future iteration a better empty state with an illustration, title, and description would be great.

gtsiolis · 2021-09-30T13:19:51Z

components/dashboard/src/projects/Prebuild.tsx

+                    {(prebuild?.status === 'aborted' || prebuild?.status === 'timeout' || !!prebuild?.error)
+                        ? <button className="flex items-center space-x-2" disabled={isRerunningPrebuild} onClick={rerunPrebuild}>
+                            {isRerunningPrebuild && <img className="h-4 w-4 animate-spin filter brightness-150" src={Spinner} />}
+                            <span>Rerun Prebuild ({prebuild.info.branch})</span>


issue: This fails to trigger any prebuild when there's a configuration error:

Error: Request triggerPrebuild failed with message: Invalid gitpod.yml: should be object

question: Ideally, this could trigger a new failed prebuild, right?

thought: Once we add the toast component (#3530) we could surface such errors with a toast notification.

Huh 😳 I guess you're seeing this error in the console? Could you please provide more details on how you achieved this situation?

@jankeromnes yes, this is in console.

Steps to reproduce:

Add a project with an invalid .gitpod.yml like https://github.com/issue-reproduce/configuration-error.

Trigger a prebuild for any branch of that project

gtsiolis · 2021-09-30T13:28:24Z

components/dashboard/src/projects/Prebuilds.tsx

@@ -159,7 +153,8 @@ export default function () {
                <div className="py-3 pl-3">
                    <DropDown prefix="Prebuild Status: " contextMenuWidth="w-32" entries={statusFilterEntries()} />
                </div>
-                <button disabled={!project} onClick={() => triggerPrebuild(null)} className="ml-2">Trigger Prebuild</button>
+                {(!!project && prebuilds.length === 0) &&
+                    <button onClick={() => triggerPrebuild(null)} className="ml-2">Run Prebuild</button>}


issue: The appearing and disappearing button feels a bit flaky, no?

Flaky appearing button ⬇️

Untitled2.mov

Flaky disappearing button ⬇️

Untitled3.mov

Yes, it is flaky. 😞

gtsiolis · 2021-09-30T15:12:07Z

components/dashboard/src/projects/Prebuilds.tsx

@@ -159,7 +153,8 @@ export default function () {
                <div className="py-3 pl-3">
                    <DropDown prefix="Prebuild Status: " contextMenuWidth="w-32" entries={statusFilterEntries()} />
                </div>
-                <button disabled={!project} onClick={() => triggerPrebuild(null)} className="ml-2">Trigger Prebuild</button>
+                {(!!project && prebuilds.length === 0) &&
+                    <button onClick={() => triggerPrebuild(null)} className="ml-2">Run Prebuild</button>}


suggestion: Also, although I think the run prebuild being always visible is the best approach long term (see #5374 (comment)), instead of hiding this button and causing any confusion should we aim for addign a boring (simple) empty state like the one we have in workspaces for teams with no projects?

Workspaces list for teams with no projects

Current state of the Run Prebuild button:

It requests a new Prebuild for a specific branch (by default, the default branch, but we could make the button branch-specific, or ask with a modal)

If there is already a successful Prebuild for the requested branch, the backend returns "a successful Prebuild already exists for this branch!" (but we don't show anything in the UI for that case)

If there is a Prebuild for the requested branch, but it has failed, then the backend will actually trigger a new Prebuild to "try again" (this is new with this PR -- previously it would just say "there is already a (failed) Prebuild for this branch, doing nothing!")

Are you proposing to always allow triggering new Prebuilds? I.e.

Even if there is a successful Prebuild for a branch (i.e. all the prebuilding work for the latest commit has already been done, and the result is ready to go) we force the creation of a new Prebuild for the same commit?

I don't see the value of this latter change (apart from making our button always enabled) even though it adds a bit more complexity to the backend (i.e. allow forcing the creation of new Prebuilds even if there is already a successful one)

roboquat · 2021-09-30T15:13:28Z

LGTM label has been added.

Git tree hash: 2c29e244a1683b632b6dc82c9f58d626f326a0e9

roboquat · 2021-09-30T15:13:30Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: gtsiolis

Associated issue: #5771

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:

~~components/dashboard/OWNERS~~ [gtsiolis]
components/ee/db-sync/OWNERS
components/gitpod-db/OWNERS
components/gitpod-protocol/OWNERS
components/server/OWNERS

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

jankeromnes · 2021-09-30T15:39:55Z

Many thanks for the review @gtsiolis! 💯

I think a lot of the confusion we have in many Teams & Projects pages stems from the fact that we never actually re-trigger a Prebuild. Instead, we trigger a new Prebuild for a branch.

This means that the Rerun Prebuild action on a specific Prebuild makes no sense. As a first improvement, I've tried to add the explicit branch name in every re-trigger action/button, in order to teach users that they actually trigger a new Prebuild for a specific branch (independently of any specific Prebuild -- you cannot re-trigger one Prebuild in particular).

Eventually, I think this option should only exist for Branches (e.g. Project > Branch > Run Prebuild). That's why I'm really looking forward to #5791 removing the current (all) Prebuilds page with its confusing Run Prebuild button, and replace it with a Prebuilds of branch 'X' page with a precise Run Prebuild button that makes sense.

I'll now fix the merge conflict and address all feedback, in order to get this in rather soon and then iterate on follow-ups. 🙂

…t 'Ready' yet

… with a start date' Fixes: 1) Basic unidirectional replication Should replicate with a start date: Error: Timeout of 2000ms exceeded.

…uttons

roboquat · 2021-09-30T15:41:20Z

New changes are detected. LGTM label has been removed.

gtsiolis · 2021-09-30T15:51:25Z

I think a lot of the confusion we have in many Teams & Projects pages stems from the fact that we never actually re-trigger a Prebuild. Instead, we trigger a new Prebuild for a branch.

Agree! This is still confusing for me sometimes.

That's why I'm really looking forward to #5791 ...

Agree! 💯

I'll now fix the merge conflict and address all feedback, in order to get this in rather soon and then iterate on follow-ups.

@jankeromnes DEAL 🤝

gtsiolis · 2021-09-30T15:51:55Z

/meow

roboquat · 2021-09-30T15:51:57Z

@gtsiolis:

In response to this:

/meow

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.

AlexTugarev · 2021-10-01T07:02:00Z

@jankeromnes can this be merged?

We've seen other issues related to preparing/building status of prebuilds/pods, so I'd like to have this on master before going deeper and checking on staging.

jankeromnes · 2021-10-01T07:09:46Z

@AlexTugarev Sure! We can merge. Most things can be done as follow-ups.

I'm just very unclear on what to do with the infamous Run Prebuild button in the Prebuilds page (every time I try to fix it, I feel like giving up and closing the PR again 🙄).

I'm not sure the current "flakily disappearing button" is actually better than the previous "mostly not-working button", but maybe this can also be improved as a follow-up? 🤷‍♂️

jankeromnes · 2021-10-01T07:11:49Z

Carrying over the LGTM, since I've just rebased the PR.

/hold cancel

jankeromnes added the approved label Sep 23, 2021

roboquat added do-not-merge/work-in-progress release-note size/XS labels Sep 23, 2021

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch from 4493484 to 4663d1a Compare September 27, 2021 08:52

roboquat added size/M and removed size/XS labels Sep 27, 2021

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch from 4663d1a to 8dc1dd4 Compare September 27, 2021 09:12

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch 3 times, most recently from 35d0061 to 4ade98c Compare September 27, 2021 12:00

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch from 4ade98c to 8a97fd8 Compare September 27, 2021 13:39

roboquat added size/L and removed size/M labels Sep 27, 2021

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch 11 times, most recently from 8b5709e to 00086db Compare September 28, 2021 15:47

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch 2 times, most recently from 8454797 to 95220d4 Compare September 29, 2021 07:35

AlexTugarev reviewed Sep 29, 2021

View reviewed changes

gtsiolis approved these changes Sep 30, 2021

View reviewed changes

roboquat added the do-not-merge/hold label Sep 30, 2021

roboquat assigned gtsiolis Sep 30, 2021

roboquat added the lgtm label Sep 30, 2021

jankeromnes added 5 commits September 30, 2021 15:41

[dashboard] Show stopping Prebuilds as still 'Running', as they're no…

7c71f3b

…t 'Ready' yet

[dashboard] Properly render empty context menus

f9826c7

[db-sync] Increase test timeout to fix intermittent 'Should replicate…

7dbf882

… with a start date' Fixes: 1) Basic unidirectional replication Should replicate with a start date: Error: Timeout of 2000ms exceeded.

[server] Allow re-triggering failed Prebuilds

2f0eea5

[dashboard] Better surface Prebuild errors & fix all 'Run Prebuild' b…

4d954d3

…uttons

jankeromnes force-pushed the jx/allow-retriggering-prebuilds branch from fcfbe64 to 4d954d3 Compare September 30, 2021 15:41

roboquat removed the lgtm label Sep 30, 2021

roboquat removed the do-not-merge/hold label Oct 1, 2021

jankeromnes added the lgtm label Oct 1, 2021

roboquat merged commit 5503c9b into main Oct 1, 2021

roboquat deleted the jx/allow-retriggering-prebuilds branch October 1, 2021 07:12

gtsiolis mentioned this pull request Oct 20, 2021

Do now allow prebuild re-triggering on successful prebuilds #5784

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server] Allow re-triggering failed Prebuilds #5836

[server] Allow re-triggering failed Prebuilds #5836

jankeromnes commented Sep 23, 2021 •

edited

Loading

jankeromnes commented Sep 27, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Sep 27, 2021 •

edited

Loading

jankeromnes commented Sep 28, 2021 •

edited

Loading

AlexTugarev Sep 29, 2021

AlexTugarev commented Sep 30, 2021

gtsiolis commented Sep 30, 2021

gtsiolis commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Sep 30, 2021

jankeromnes Sep 30, 2021

gtsiolis Sep 30, 2021

gtsiolis Sep 30, 2021

jankeromnes Sep 30, 2021

gtsiolis Oct 1, 2021

gtsiolis Sep 30, 2021

jankeromnes Sep 30, 2021

gtsiolis Oct 1, 2021

gtsiolis Sep 30, 2021

jankeromnes Sep 30, 2021 •

edited

Loading

roboquat commented Sep 30, 2021

roboquat commented Sep 30, 2021

jankeromnes commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

roboquat commented Sep 30, 2021

gtsiolis commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Sep 30, 2021

roboquat commented Sep 30, 2021

AlexTugarev commented Oct 1, 2021

jankeromnes commented Oct 1, 2021 •

edited

Loading

jankeromnes commented Oct 1, 2021

		{(!!project && prebuilds.length === 0) &&
		<button onClick={() => triggerPrebuild(null)} className="ml-2">Run Prebuild</button>}

[server] Allow re-triggering failed Prebuilds #5836

[server] Allow re-triggering failed Prebuilds #5836

Conversation

jankeromnes commented Sep 23, 2021 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

jankeromnes commented Sep 27, 2021 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented Sep 27, 2021 • edited Loading

jankeromnes commented Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

AlexTugarev commented Sep 30, 2021

gtsiolis commented Sep 30, 2021

gtsiolis commented Sep 30, 2021 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes Sep 30, 2021 • edited Loading

Choose a reason for hiding this comment

roboquat commented Sep 30, 2021

roboquat commented Sep 30, 2021

jankeromnes commented Sep 30, 2021 • edited by werft-gitpod-dev-com bot Loading

roboquat commented Sep 30, 2021

gtsiolis commented Sep 30, 2021 • edited by werft-gitpod-dev-com bot Loading

gtsiolis commented Sep 30, 2021

roboquat commented Sep 30, 2021

AlexTugarev commented Oct 1, 2021

jankeromnes commented Oct 1, 2021 • edited Loading

jankeromnes commented Oct 1, 2021

jankeromnes commented Sep 23, 2021 •

edited

Loading

jankeromnes commented Sep 27, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Sep 27, 2021 •

edited

Loading

jankeromnes commented Sep 28, 2021 •

edited

Loading

gtsiolis commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes Sep 30, 2021 •

edited

Loading

jankeromnes commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Oct 1, 2021 •

edited

Loading