-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[server] Allow re-triggering failed Prebuilds #5836
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
Changes from all commits
7c71f3b
f9826c7
7dbf882
2f0eea5
4d954d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,15 +7,17 @@ | |
import moment from "moment"; | ||
import { PrebuildWithStatus, WorkspaceInstance } from "@gitpod/gitpod-protocol"; | ||
import { useContext, useEffect, useState } from "react"; | ||
import { useLocation, useRouteMatch } from "react-router"; | ||
import { useHistory, useLocation, useRouteMatch } from "react-router"; | ||
import Header from "../components/Header"; | ||
import PrebuildLogs from "../components/PrebuildLogs"; | ||
import Spinner from "../icons/Spinner.svg"; | ||
import { getGitpodService, gitpodHostUrl } from "../service/service"; | ||
import { TeamsContext, getCurrentTeam } from "../teams/teams-context"; | ||
import { PrebuildInstanceStatus } from "./Prebuilds"; | ||
import { shortCommitMessage } from "./render-utils"; | ||
|
||
export default function () { | ||
const history = useHistory(); | ||
const location = useLocation(); | ||
|
||
const { teams } = useContext(TeamsContext); | ||
|
@@ -27,6 +29,7 @@ export default function () { | |
|
||
const [ prebuild, setPrebuild ] = useState<PrebuildWithStatus | undefined>(); | ||
const [ prebuildInstance, setPrebuildInstance ] = useState<WorkspaceInstance | undefined>(); | ||
const [ isRerunningPrebuild, setIsRerunningPrebuild ] = useState<boolean>(false); | ||
|
||
useEffect(() => { | ||
if (!teams || !projectName || !prebuildId) { | ||
|
@@ -76,6 +79,22 @@ export default function () { | |
setPrebuildInstance(instance); | ||
} | ||
|
||
const rerunPrebuild = async () => { | ||
if (!prebuild) { | ||
return; | ||
} | ||
setIsRerunningPrebuild(true); | ||
try { | ||
await getGitpodService().server.triggerPrebuild(prebuild.info.projectId, prebuild.info.branch); | ||
// TODO: Open a Prebuilds page that's specific to `prebuild.info.branch`? | ||
history.push(`/${!!team ? 't/'+team.slug : 'projects'}/${projectName}/prebuilds`); | ||
} catch (error) { | ||
console.error('Could not rerun prebuild', error); | ||
} finally { | ||
setIsRerunningPrebuild(false); | ||
} | ||
} | ||
|
||
useEffect(() => { document.title = 'Prebuild — Gitpod' }, []); | ||
|
||
return <> | ||
|
@@ -88,9 +107,14 @@ export default function () { | |
<div className="h-20 px-6 bg-gray-50 dark:bg-gray-800 border-t border-gray-200 dark:border-gray-600 flex space-x-2"> | ||
{prebuildInstance && <PrebuildInstanceStatus prebuildInstance={prebuildInstance} />} | ||
<div className="flex-grow" /> | ||
{prebuildInstance?.status.phase === "stopped" | ||
? <a className="my-auto" href={gitpodHostUrl.withContext(`${prebuild?.info.changeUrl}`).toString()}><button>New Workspace</button></a> | ||
: <button disabled={true}>New Workspace</button>} | ||
{(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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. ❓ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point, thanks! Would it be cool to:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: This fails to trigger any prebuild when there's a configuration error:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh 😳 I guess you're seeing this error in the console? Could you please provide more details on how you achieved this situation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jankeromnes yes, this is in console. Steps to reproduce:
|
||
</button> | ||
: (prebuild?.status === 'available' | ||
? <a className="my-auto" href={gitpodHostUrl.withContext(`${prebuild?.info.changeUrl}`).toString()}><button>New Workspace ({prebuild?.info.branch})</button></a> | ||
: <button disabled={true}>New Workspace ({prebuild?.info.branch})</button>)} | ||
</div> | ||
</div> | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -5,9 +5,9 @@ | |||
*/ | ||||
|
||||
import moment from "moment"; | ||||
import { PrebuildInfo, PrebuildWithStatus, PrebuiltWorkspaceState, Project, WorkspaceInstance } from "@gitpod/gitpod-protocol"; | ||||
import { PrebuildWithStatus, PrebuiltWorkspaceState, Project, WorkspaceInstance } from "@gitpod/gitpod-protocol"; | ||||
import { useContext, useEffect, useState } from "react"; | ||||
import { useHistory, useLocation, useRouteMatch } from "react-router"; | ||||
import { useLocation, useRouteMatch } from "react-router"; | ||||
import Header from "../components/Header"; | ||||
import DropDown, { DropDownEntry } from "../components/DropDown"; | ||||
import { ItemsList, Item, ItemField, ItemFieldContextMenu } from "../components/ItemsList"; | ||||
|
@@ -22,7 +22,6 @@ import { ContextMenuEntry } from "../components/ContextMenu"; | |||
import { shortCommitMessage } from "./render-utils"; | ||||
|
||||
export default function () { | ||||
const history = useHistory(); | ||||
const location = useLocation(); | ||||
|
||||
const { teams } = useContext(TeamsContext); | ||||
|
@@ -45,7 +44,7 @@ export default function () { | |||
const registration = getGitpodService().registerClient({ | ||||
onPrebuildUpdate: (update: PrebuildWithStatus) => { | ||||
if (update.info.projectId === project.id) { | ||||
setPrebuilds(prev => [update, ...prev.filter(p => p.info.id !== update.info.id)]) | ||||
setPrebuilds(prev => [update, ...prev.filter(p => p.info.id !== update.info.id)]); | ||||
} | ||||
} | ||||
}); | ||||
|
@@ -77,18 +76,17 @@ export default function () { | |||
}, [teams]); | ||||
|
||||
const prebuildContextMenu = (p: PrebuildWithStatus) => { | ||||
const running = p.status === "building"; | ||||
const isFailed = p.status === "aborted" || p.status === "timeout" || !!p.error; | ||||
const isRunning = p.status === "building"; | ||||
const entries: ContextMenuEntry[] = []; | ||||
entries.push({ | ||||
title: "View Prebuild", | ||||
onClick: () => openPrebuild(p.info) | ||||
}); | ||||
entries.push({ | ||||
title: "Trigger Prebuild", | ||||
onClick: () => triggerPrebuild(p.info.branch), | ||||
separator: running | ||||
}); | ||||
if (running) { | ||||
if (isFailed) { | ||||
entries.push({ | ||||
title: `Rerun Prebuild (${p.info.branch})`, | ||||
onClick: () => triggerPrebuild(p.info.branch), | ||||
separator: isRunning | ||||
}); | ||||
} | ||||
if (isRunning) { | ||||
entries.push({ | ||||
title: "Cancel Prebuild", | ||||
customFontStyle: 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300', | ||||
|
@@ -131,10 +129,6 @@ export default function () { | |||
return -1; | ||||
} | ||||
|
||||
const openPrebuild = (pb: PrebuildInfo) => { | ||||
history.push(`/${!!team ? 't/'+team.slug : 'projects'}/${projectName}/${pb.id}`); | ||||
} | ||||
|
||||
const triggerPrebuild = (branchName: string | null) => { | ||||
if (project) { | ||||
getGitpodService().server.triggerPrebuild(project.id, branchName); | ||||
|
@@ -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>} | ||||
Comment on lines
+156
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "disappearing button" could be improved as an "empty state" for the Prebuilds page (but maybe after we rework it to be per-branch). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, an empty state for the Prebuilds page (with an illustration, and maybe this infamous This would also solve the problem that it is impossible to Currently, it's a problem to show I see the following solutions:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The appearing and disappearing button feels a bit flaky, no? Flaky appearing button ⬇️ Untitled2.movFlaky disappearing button ⬇️ Untitled3.movThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is flaky. 😞 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😞 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current state of the
Are you proposing to always allow triggering new Prebuilds? I.e.
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) |
||||
</div> | ||||
<ItemsList className="mt-2"> | ||||
<Item header={true} className="grid grid-cols-3"> | ||||
|
@@ -171,18 +166,17 @@ export default function () { | |||
</ItemField> | ||||
<ItemField> | ||||
<span>Branch</span> | ||||
<ItemFieldContextMenu /> | ||||
</ItemField> | ||||
</Item> | ||||
{prebuilds.filter(filter).sort(prebuildSorter).map((p, index) => <Item key={`prebuild-${p.info.id}`} className="grid grid-cols-3"> | ||||
<ItemField className="flex items-center"> | ||||
<div className="cursor-pointer" onClick={() => openPrebuild(p.info)}> | ||||
<a href={`/${!!team ? 't/'+team.slug : 'projects'}/${projectName}/${p.info.id}`} className="cursor-pointer"> | ||||
<div className="text-base text-gray-900 dark:text-gray-50 font-medium uppercase mb-1"> | ||||
<div className="inline-block align-text-bottom mr-2 w-4 h-4">{prebuildStatusIcon(p.status)}</div> | ||||
{prebuildStatusLabel(p.status)} | ||||
<div className="inline-block align-text-bottom mr-2 w-4 h-4">{prebuildStatusIcon(p)}</div> | ||||
{prebuildStatusLabel(p)} | ||||
</div> | ||||
<p>{p.info.startedByAvatar && <img className="rounded-full w-4 h-4 inline-block align-text-bottom mr-2" src={p.info.startedByAvatar || ''} alt={p.info.startedBy} />}Triggered {formatDate(p.info.startedAt)}</p> | ||||
</div> | ||||
</a> | ||||
</ItemField> | ||||
<ItemField className="flex items-center"> | ||||
<div> | ||||
|
@@ -204,41 +198,39 @@ export default function () { | |||
</>; | ||||
} | ||||
|
||||
export function prebuildStatusLabel(status: PrebuiltWorkspaceState | undefined) { | ||||
switch (status) { | ||||
case "aborted": | ||||
export function prebuildStatusLabel(prebuild?: PrebuildWithStatus) { | ||||
if (prebuild?.error) { | ||||
return (<span className="font-medium text-red-500 uppercase">failed</span>); | ||||
} | ||||
switch (prebuild?.status) { | ||||
case undefined: // Fall through | ||||
case "queued": | ||||
return (<span className="font-medium text-orange-500 uppercase">pending</span>); | ||||
case "building": | ||||
return (<span className="font-medium text-blue-500 uppercase">running</span>); | ||||
case "aborted": // Fall through | ||||
case "timeout": | ||||
return (<span className="font-medium text-red-500 uppercase">failed</span>); | ||||
case "available": | ||||
return (<span className="font-medium text-green-500 uppercase">ready</span>); | ||||
case "building": | ||||
return (<span className="font-medium text-blue-500 uppercase">running</span>); | ||||
case "queued": | ||||
return (<span className="font-medium text-orange-500 uppercase">pending</span>); | ||||
default: | ||||
break; | ||||
} | ||||
} | ||||
|
||||
export function prebuildStatusIcon(status: PrebuiltWorkspaceState | undefined) { | ||||
switch (status) { | ||||
case "aborted": | ||||
return (<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||||
<path fill-rule="evenodd" clip-rule="evenodd" d="M8 16C12.4183 16 16 12.4183 16 8C16 3.58172 12.4183 0 8 0C3.58172 0 0 3.58172 0 8C0 12.4183 3.58172 16 8 16ZM6.70711 5.29289C6.31658 4.90237 5.68342 4.90237 5.29289 5.29289C4.90237 5.68342 4.90237 6.31658 5.29289 6.70711L6.58579 8L5.29289 9.29289C4.90237 9.68342 4.90237 10.3166 5.29289 10.7071C5.68342 11.0976 6.31658 11.0976 6.70711 10.7071L8 9.41421L9.29289 10.7071C9.68342 11.0976 10.3166 11.0976 10.7071 10.7071C11.0976 10.3166 11.0976 9.68342 10.7071 9.29289L9.41421 8L10.7071 6.70711C11.0976 6.31658 11.0976 5.68342 10.7071 5.29289C10.3166 4.90237 9.68342 4.90237 9.29289 5.29289L8 6.58579L6.70711 5.29289Z" fill="#F87171" /> | ||||
</svg>) | ||||
case "available": | ||||
return (<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||||
<path fill-rule="evenodd" clip-rule="evenodd" d="M8 16C12.4183 16 16 12.4183 16 8C16 3.58172 12.4183 0 8 0C3.58172 0 0 3.58172 0 8C0 12.4183 3.58172 16 8 16ZM11.7071 6.70711C12.0976 6.31658 12.0976 5.68342 11.7071 5.29289C11.3166 4.90237 10.6834 4.90237 10.2929 5.29289L7 8.58578L5.7071 7.29289C5.31658 6.90237 4.68342 6.90237 4.29289 7.29289C3.90237 7.68342 3.90237 8.31658 4.29289 8.70711L6.29289 10.7071C6.68342 11.0976 7.31658 11.0976 7.7071 10.7071L11.7071 6.70711Z" fill="#84CC16" /> | ||||
</svg>); | ||||
case "building": | ||||
return (<svg width="17" height="16" viewBox="0 0 17 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||||
<path fill-rule="evenodd" clip-rule="evenodd" d="M8.99609 16C13.4144 16 16.9961 12.4183 16.9961 8C16.9961 3.58172 13.4144 0 8.99609 0C4.57781 0 0.996094 3.58172 0.996094 8C0.996094 12.4183 4.57781 16 8.99609 16ZM9.99609 4C9.99609 3.44772 9.54837 3 8.99609 3C8.4438 3 7.99609 3.44772 7.99609 4V8C7.99609 8.26522 8.10144 8.51957 8.28898 8.70711L11.1174 11.5355C11.5079 11.9261 12.1411 11.9261 12.5316 11.5355C12.9221 11.145 12.9221 10.5118 12.5316 10.1213L9.99609 7.58579V4Z" fill="#60A5FA" /> | ||||
</svg>); | ||||
export function prebuildStatusIcon(prebuild?: PrebuildWithStatus) { | ||||
if (prebuild?.error) { | ||||
return <img className="h-4 w-4" src={StatusFailed} />; | ||||
} | ||||
switch (prebuild?.status) { | ||||
case undefined: // Fall through | ||||
case "queued": | ||||
return (<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||||
<path fill-rule="evenodd" clip-rule="evenodd" d="M16 8C16 12.4183 12.4183 16 8 16C3.58172 16 0 12.4183 0 8C0 3.58172 3.58172 0 8 0C12.4183 0 16 3.58172 16 8ZM5 6C5 5.44772 5.44772 5 6 5C6.55228 5 7 5.44772 7 6V10C7 10.5523 6.55228 11 6 11C5.44772 11 5 10.5523 5 10V6ZM10 5C9.44771 5 9 5.44772 9 6V10C9 10.5523 9.44771 11 10 11C10.5523 11 11 10.5523 11 10V6C11 5.44772 10.5523 5 10 5Z" fill="#FBBF24" /> | ||||
</svg>); | ||||
default: | ||||
break; | ||||
return <img className="h-4 w-4" src={StatusPaused} />; | ||||
case "building": | ||||
return <img className="h-4 w-4" src={StatusRunning} />; | ||||
case "aborted": // Fall through | ||||
case "timeout": | ||||
return <img className="h-4 w-4" src={StatusFailed} />; | ||||
case "available": | ||||
return <img className="h-4 w-4" src={StatusDone} />; | ||||
} | ||||
} | ||||
|
||||
|
@@ -262,7 +254,8 @@ export function PrebuildInstanceStatus(props: { prebuildInstance?: WorkspaceInst | |||
case 'creating': // Fall through | ||||
case 'initializing': // Fall through | ||||
case 'running': // Fall through | ||||
case 'interrupted': | ||||
case 'interrupted': // Fall through | ||||
case 'stopping': | ||||
status = <div className="flex space-x-1 items-center text-blue-600"> | ||||
<img className="h-4 w-4" src={StatusRunning} /> | ||||
<span>RUNNING</span> | ||||
|
@@ -272,7 +265,6 @@ export function PrebuildInstanceStatus(props: { prebuildInstance?: WorkspaceInst | |||
<span>Prebuild in progress ...</span> | ||||
</div>; | ||||
break; | ||||
case 'stopping': // Fall through | ||||
case 'stopped': | ||||
status = <div className="flex space-x-1 items-center text-green-600"> | ||||
<img className="h-4 w-4" src={StatusDone} /> | ||||
|
@@ -286,7 +278,7 @@ export function PrebuildInstanceStatus(props: { prebuildInstance?: WorkspaceInst | |||
</div>; | ||||
break; | ||||
} | ||||
if (props.prebuildInstance?.status.conditions.failed) { | ||||
if (props.prebuildInstance?.status.conditions.failed || props.prebuildInstance?.status.conditions.headlessTaskFailed) { | ||||
status = <div className="flex space-x-1 items-center text-gitpod-red"> | ||||
<img className="h-4 w-4" src={StatusFailed} /> | ||||
<span>FAILED</span> | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even with the current solution this seems to to work good enough UX-wise