Skip to content

feat(site): warn on provisioner health during builds #15589

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 18 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,12 +682,20 @@ class ApiMethods {

/**
* @param organization Can be the organization's ID or name
* @param tags to filter provisioner daemons by.
*/
getProvisionerDaemonsByOrganization = async (
organization: string,
tags?: Record<string, string>,
): Promise<TypesGen.ProvisionerDaemon[]> => {
const params = new URLSearchParams();

if (tags) {
params.append("tags", JSON.stringify(tags));
}

const response = await this.axios.get<TypesGen.ProvisionerDaemon[]>(
`/api/v2/organizations/${organization}/provisionerdaemons`,
`/api/v2/organizations/${organization}/provisionerdaemons?${params.toString()}`,
);
return response.data;
};
Expand Down
18 changes: 10 additions & 8 deletions site/src/api/queries/organizations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,18 @@ export const organizations = () => {
};
};

export const getProvisionerDaemonsKey = (organization: string) => [
"organization",
organization,
"provisionerDaemons",
];
export const getProvisionerDaemonsKey = (
organization: string,
tags?: Record<string, string>,
) => ["organization", organization, tags, "provisionerDaemons"];

export const provisionerDaemons = (organization: string) => {
export const provisionerDaemons = (
organization: string,
tags?: Record<string, string>,
) => {
return {
queryKey: getProvisionerDaemonsKey(organization),
queryFn: () => API.getProvisionerDaemonsByOrganization(organization),
queryKey: getProvisionerDaemonsKey(organization, tags),
queryFn: () => API.getProvisionerDaemonsByOrganization(organization, tags),
};
};

Expand Down
3 changes: 3 additions & 0 deletions site/src/components/Alert/Alert.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import MuiAlert, {
type AlertColor as MuiAlertColor,
type AlertProps as MuiAlertProps,
// biome-ignore lint/nursery/noRestrictedImports: Used as base component
} from "@mui/material/Alert";
Expand All @@ -11,6 +12,8 @@ import {
useState,
} from "react";

export type AlertColor = MuiAlertColor;

export type AlertProps = MuiAlertProps & {
actions?: ReactNode;
dismissible?: boolean;
Expand Down
55 changes: 55 additions & 0 deletions site/src/modules/provisioners/ProvisionerAlert.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import type { Meta, StoryObj } from "@storybook/react";
import { chromatic } from "testHelpers/chromatic";
import { MockTemplateVersion } from "testHelpers/entities";
import { ProvisionerAlert } from "./ProvisionerAlert";

const meta: Meta<typeof ProvisionerAlert> = {
title: "modules/provisioners/ProvisionerAlert",
parameters: {
chromatic,
layout: "centered",
},
component: ProvisionerAlert,
args: {
matchingProvisioners: 0,
availableProvisioners: 0,
tags: MockTemplateVersion.job.tags,
},
};

export default meta;
type Story = StoryObj<typeof ProvisionerAlert>;

export const HealthyProvisioners: Story = {
args: {
matchingProvisioners: 1,
availableProvisioners: 1,
},
};

export const UndefinedMatchingProvisioners: Story = {
args: {
matchingProvisioners: undefined,
availableProvisioners: undefined,
},
};

export const UndefinedAvailableProvisioners: Story = {
args: {
matchingProvisioners: 1,
availableProvisioners: undefined,
},
};

export const NoMatchingProvisioners: Story = {
args: {
matchingProvisioners: 0,
},
};

export const NoAvailableProvisioners: Story = {
args: {
matchingProvisioners: 1,
availableProvisioners: 0,
},
};
97 changes: 97 additions & 0 deletions site/src/modules/provisioners/ProvisionerAlert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import AlertTitle from "@mui/material/AlertTitle";
import { Alert, type AlertColor } from "components/Alert/Alert";
import { AlertDetail } from "components/Alert/Alert";
import { Stack } from "components/Stack/Stack";
import { ProvisionerTag } from "modules/provisioners/ProvisionerTag";
import type { FC } from "react";

interface ProvisionerAlertProps {
matchingProvisioners: number | undefined;
availableProvisioners: number | undefined;
tags: Record<string, string>;
}

export const ProvisionerAlert: FC<ProvisionerAlertProps> = ({
matchingProvisioners,
availableProvisioners,
tags,
}) => {
let title: string;
let detail: string;
switch (true) {
case matchingProvisioners === 0:
title = "Provisioning Cannot Proceed";
detail =
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available, provisioning will continue.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available, provisioning will continue.";
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available, your build will continue.";

Also, are we able to link to the docs on external provisioners?

If the user is an admin, it may be nice to remove "Contact your administrator." And also link to the provisioners page for the given organization.

Copy link
Member

Choose a reason for hiding this comment

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

OK to do this as a follow-up? I've seen plenty of systems where a "contact your administrator" is shown regardless of your user's capabilities.

break;
case availableProvisioners === 0:
title = "Provisioning Delayed";
detail =
"Provisioners that accept the required tags are currently anavailable. This may delay your build. Please contact your administrator if your build does not complete.";
Copy link
Member

Choose a reason for hiding this comment

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

Are we able to show a queue length, like we do in the workspaces page? If nontrivial, this copy LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

I think it might be non-trivial so let's leave it as a follow-up?

break;
default:
return null;
}

return (
<Alert
severity="warning"
css={(theme) => ({
borderRadius: 0,
border: 0,
borderBottom: `1px solid ${theme.palette.divider}`,
borderLeft: `2px solid ${theme.palette.error.main}`,
})}
>
<AlertTitle>{title}</AlertTitle>
<AlertDetail>
<div>{detail}</div>
<Stack direction="row" spacing={1} wrap="wrap">
{Object.entries(tags)
.filter(([key]) => key !== "owner")
.map(([key, value]) => (
<ProvisionerTag key={key} tagName={key} tagValue={value} />
))}
</Stack>
</AlertDetail>
</Alert>
);
};

interface ProvisionerJobAlertProps {
title: string;
detail: string;
severity: AlertColor;
tags: Record<string, string>;
}

export const ProvisionerJobAlert: FC<ProvisionerJobAlertProps> = ({
title,
detail,
severity,
tags,
}) => {
return (
<Alert
severity={severity}
css={(theme) => ({
borderRadius: 0,
border: 0,
borderBottom: `1px solid ${theme.palette.divider}`,
borderLeft: `2px solid ${theme.palette.error.main}`,
})}
>
<AlertTitle>{title}</AlertTitle>
<AlertDetail>
<div>{detail}</div>
<Stack direction="row" spacing={1} wrap="wrap">
{Object.entries(tags)
.filter(([key]) => key !== "owner")
.map(([key, value]) => (
<ProvisionerTag key={key} tagName={key} tagValue={value} />
))}
</Stack>
</AlertDetail>
</Alert>
);
};
37 changes: 37 additions & 0 deletions site/src/pages/CreateTemplatePage/BuildLogsDrawer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,43 @@ export const MissingVariables: Story = {
},
};

export const NoProvisioners: Story = {
args: {
templateVersion: {
...MockTemplateVersion,
matched_provisioners: {
count: 0,
available: 0,
},
},
},
};

export const ProvisionersUnhealthy: Story = {
args: {
templateVersion: {
...MockTemplateVersion,
matched_provisioners: {
count: 1,
available: 0,
},
},
},
};

export const ProvisionersHealthy: Story = {
args: {
templateVersion: {
...MockTemplateVersion,
organization_id: "org-id",
matched_provisioners: {
count: 1,
available: 1,
},
},
},
};

export const Logs: Story = {
args: {
templateVersion: {
Expand Down
13 changes: 13 additions & 0 deletions site/src/pages/CreateTemplatePage/BuildLogsDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { visuallyHidden } from "@mui/utils";
import { JobError } from "api/queries/templates";
import type { TemplateVersion } from "api/typesGenerated";
import { Loader } from "components/Loader/Loader";
import { ProvisionerAlert } from "modules/provisioners/ProvisionerAlert";
import { useWatchVersionLogs } from "modules/templates/useWatchVersionLogs";
import { WorkspaceBuildLogs } from "modules/workspaces/WorkspaceBuildLogs/WorkspaceBuildLogs";
import { type FC, useLayoutEffect, useRef } from "react";
Expand All @@ -27,6 +28,10 @@ export const BuildLogsDrawer: FC<BuildLogsDrawerProps> = ({
variablesSectionRef,
...drawerProps
}) => {
const matchingProvisioners = templateVersion?.matched_provisioners?.count;
const availableProvisioners =
templateVersion?.matched_provisioners?.available;

const logs = useWatchVersionLogs(templateVersion);
const logsContainer = useRef<HTMLDivElement>(null);

Expand Down Expand Up @@ -65,6 +70,14 @@ export const BuildLogsDrawer: FC<BuildLogsDrawerProps> = ({
</IconButton>
</header>

{!logs && (
<ProvisionerAlert
matchingProvisioners={matchingProvisioners}
availableProvisioners={availableProvisioners}
tags={templateVersion?.job.tags ?? {}}
/>
)}

{isMissingVariables ? (
<MissingVariablesBanner
onFillVariables={() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,73 @@ type Story = StoryObj<typeof TemplateVersionEditor>;

export const Example: Story = {};

export const UndefinedLogs: Story = {
Copy link
Member

Choose a reason for hiding this comment

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

👍

args: {
defaultTab: "logs",
buildLogs: undefined,
templateVersion: {
...MockTemplateVersion,
job: MockRunningProvisionerJob,
},
},
};

export const EmptyLogs: Story = {
args: {
defaultTab: "logs",
buildLogs: [],
templateVersion: {
...MockTemplateVersion,
job: MockRunningProvisionerJob,
},
},
};

export const NoProvisioners: Story = {
args: {
defaultTab: "logs",
buildLogs: [],
templateVersion: {
...MockTemplateVersion,
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 0,
available: 0,
},
},
},
};

export const UnavailableProvisioners: Story = {
args: {
defaultTab: "logs",
buildLogs: [],
templateVersion: {
...MockTemplateVersion,
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 1,
available: 0,
},
},
},
};

export const HealthyProvisioners: Story = {
args: {
defaultTab: "logs",
buildLogs: [],
templateVersion: {
...MockTemplateVersion,
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 1,
available: 1,
},
},
},
};

export const Logs: Story = {
args: {
defaultTab: "logs",
Expand Down
Loading
Loading