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 1 commit
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
Prev Previous commit
Next Next commit
make fmt
  • Loading branch information
SasSwart committed Nov 27, 2024
commit bec2913df6be69005347db022d65b41764c74b0e
6 changes: 3 additions & 3 deletions site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -686,16 +686,16 @@ class ApiMethods {
*/
getProvisionerDaemonsByOrganization = async (
organization: string,
tags?: Record<string, string>
tags?: Record<string, string>,
): Promise<TypesGen.ProvisionerDaemon[]> => {
const params = new URLSearchParams();

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

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

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

export const provisionerDaemons = (organization: string, tags?: Record<string, string>) => {
export const provisionerDaemons = (
organization: string,
tags?: Record<string, string>,
) => {
return {
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
14 changes: 7 additions & 7 deletions site/src/modules/provisioners/ProvisionerAlert.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,32 @@ export const HealthyProvisioners: Story = {
args: {
matchingProvisioners: 1,
availableProvisioners: 1,
}
},
};

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

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

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

export const NoAvailableProvisioners: Story = {
args: {
matchingProvisioners: 1,
availableProvisioners: 0,
}
},
};
52 changes: 23 additions & 29 deletions site/src/modules/provisioners/ProvisionerAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,31 +1,33 @@
import Alert, { type AlertColor } from "@mui/material/Alert";
import AlertTitle from "@mui/material/AlertTitle";
import { Stack } from "components/Stack/Stack";
import { Alert, type AlertColor } from "components/Alert/Alert";
import { AlertDetail } from "components/Alert/Alert";
import type { FC } from "react";
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>
matchingProvisioners: number | undefined;
availableProvisioners: number | undefined;
tags: Record<string, string>;
}

export const ProvisionerAlert : FC<ProvisionerAlertProps> = ({
export const ProvisionerAlert: FC<ProvisionerAlertProps> = ({
matchingProvisioners,
availableProvisioners,
tags
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."
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.";
break;
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.

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."
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.";
break;
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?

default:
return null;
Expand All @@ -48,11 +50,7 @@ export const ProvisionerAlert : FC<ProvisionerAlertProps> = ({
{Object.entries(tags)
.filter(([key]) => key !== "owner")
.map(([key, value]) => (
<ProvisionerTag
key={key}
tagName={key}
tagValue={value}
/>
<ProvisionerTag key={key} tagName={key} tagValue={value} />
))}
</Stack>
</AlertDetail>
Expand All @@ -61,13 +59,13 @@ export const ProvisionerAlert : FC<ProvisionerAlertProps> = ({
};

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

export const ProvisionerJobAlert : FC<ProvisionerJobAlertProps> = ({
export const ProvisionerJobAlert: FC<ProvisionerJobAlertProps> = ({
title,
detail,
severity,
Expand All @@ -90,11 +88,7 @@ export const ProvisionerJobAlert : FC<ProvisionerJobAlertProps> = ({
{Object.entries(tags)
.filter(([key]) => key !== "owner")
.map(([key, value]) => (
<ProvisionerTag
key={key}
tagName={key}
tagValue={value}
/>
<ProvisionerTag key={key} tagName={key} tagValue={value} />
))}
</Stack>
</AlertDetail>
Expand Down
7 changes: 3 additions & 4 deletions site/src/pages/CreateTemplatePage/BuildLogsDrawer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const NoProvisioners: Story = {
matched_provisioners: {
count: 0,
available: 0,
}
},
},
},
};
Expand All @@ -53,7 +53,7 @@ export const ProvisionersUnhealthy: Story = {
matched_provisioners: {
count: 1,
available: 0,
}
},
},
},
};
Expand All @@ -66,12 +66,11 @@ export const ProvisionersHealthy: Story = {
matched_provisioners: {
count: 1,
available: 1,
}
},
},
},
};


export const Logs: Story = {
args: {
templateVersion: {
Expand Down
9 changes: 5 additions & 4 deletions site/src/pages/CreateTemplatePage/BuildLogsDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ 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";
import { navHeight } from "theme/constants";
import { ProvisionerAlert } from "modules/provisioners/ProvisionerAlert";

type BuildLogsDrawerProps = {
error: unknown;
Expand All @@ -28,8 +28,9 @@ export const BuildLogsDrawer: FC<BuildLogsDrawerProps> = ({
variablesSectionRef,
...drawerProps
}) => {
const matchingProvisioners = templateVersion?.matched_provisioners?.count
const availableProvisioners = templateVersion?.matched_provisioners?.available
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 @@ -69,7 +70,7 @@ export const BuildLogsDrawer: FC<BuildLogsDrawerProps> = ({
</IconButton>
</header>

{ !logs && (
{!logs && (
<ProvisionerAlert
matchingProvisioners={matchingProvisioners}
availableProvisioners={availableProvisioners}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export const NoProvisioners: Story = {
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 0,
available: 0
}
available: 0,
},
},
},
};
Expand All @@ -95,8 +95,8 @@ export const UnavailableProvisioners: Story = {
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 1,
available: 0
}
available: 0,
},
},
},
};
Expand All @@ -110,8 +110,8 @@ export const HealthyProvisioners: Story = {
job: MockRunningProvisionerJob,
matched_provisioners: {
count: 1,
available: 1
}
available: 1,
},
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import ArrowBackOutlined from "@mui/icons-material/ArrowBackOutlined";
import CloseOutlined from "@mui/icons-material/CloseOutlined";
import PlayArrowOutlined from "@mui/icons-material/PlayArrowOutlined";
import WarningOutlined from "@mui/icons-material/WarningOutlined";
import AlertTitle from "@mui/material/AlertTitle";
import Button from "@mui/material/Button";
import ButtonGroup from "@mui/material/ButtonGroup";
import IconButton from "@mui/material/IconButton";
Expand All @@ -17,7 +16,7 @@ import type {
VariableValue,
WorkspaceResource,
} from "api/typesGenerated";
import { Alert, AlertDetail } from "components/Alert/Alert";
import { Alert } from "components/Alert/Alert";
import { Sidebar } from "components/FullPageLayout/Sidebar";
import {
Topbar,
Expand All @@ -29,6 +28,10 @@ import {
} from "components/FullPageLayout/Topbar";
import { Loader } from "components/Loader/Loader";
import { linkToTemplate, useLinks } from "modules/navigation";
import {
ProvisionerAlert,
ProvisionerJobAlert,
} from "modules/provisioners/ProvisionerAlert";
import { TemplateFileTree } from "modules/templates/TemplateFiles/TemplateFileTree";
import { isBinaryData } from "modules/templates/TemplateFiles/isBinaryData";
import { TemplateResourcesTable } from "modules/templates/TemplateResourcesTable/TemplateResourcesTable";
Expand Down Expand Up @@ -60,7 +63,6 @@ import { MonacoEditor } from "./MonacoEditor";
import { ProvisionerTagsPopover } from "./ProvisionerTagsPopover";
import { PublishTemplateVersionDialog } from "./PublishTemplateVersionDialog";
import { TemplateVersionStatusBadge } from "./TemplateVersionStatusBadge";
import { ProvisionerAlert, ProvisionerJobAlert } from "modules/provisioners/ProvisionerAlert";

type Tab = "logs" | "resources" | undefined; // Undefined is to hide the tab

Expand Down Expand Up @@ -127,9 +129,8 @@ export const TemplateVersionEditor: FC<TemplateVersionEditorProps> = ({
const [deleteFileOpen, setDeleteFileOpen] = useState<string>();
const [renameFileOpen, setRenameFileOpen] = useState<string>();
const [dirty, setDirty] = useState(false);
const matchingProvisioners = templateVersion.matched_provisioners?.count
const availableProvisioners = templateVersion.matched_provisioners?.available

const matchingProvisioners = templateVersion.matched_provisioners?.count;
const availableProvisioners = templateVersion.matched_provisioners?.available;

const triggerPreview = useCallback(async () => {
await onPreview(fileTree);
Expand Down
19 changes: 12 additions & 7 deletions site/src/testHelpers/storybook.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { StoryContext } from "@storybook/react";
import { withDefaultFeatures } from "api/api";
import { getAuthorizationKey } from "api/queries/authCheck";
import { getProvisionerDaemonsKey } from "api/queries/organizations";
import { hasFirstUserKey, meKey } from "api/queries/users";
import type { Entitlements } from "api/typesGenerated";
import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
Expand All @@ -17,7 +18,6 @@ import {
MockDeploymentConfig,
MockEntitlements,
} from "./entities";
import { getProvisionerDaemonsKey } from "api/queries/organizations";

export const withDashboardProvider = (
Story: FC,
Expand Down Expand Up @@ -124,21 +124,26 @@ export const withAuthProvider = (Story: FC, { parameters }: StoryContext) => {

export const withProvisioners = (Story: FC, { parameters }: StoryContext) => {
if (!parameters.organization_id) {
throw new Error("You forgot to add `parameters.organization_id` to your story");
throw new Error(
"You forgot to add `parameters.organization_id` to your story",
);
}
if (!parameters.provisioners) {
throw new Error("You forgot to add `parameters.provisioners` to your story");
throw new Error(
"You forgot to add `parameters.provisioners` to your story",
);
}
if (!parameters.tags) {
throw new Error("You forgot to add `parameters.tags` to your story");
}

const queryClient = useQueryClient();
queryClient.setQueryData(getProvisionerDaemonsKey(parameters.organization_id, parameters.tags), parameters.provisioners);
queryClient.setQueryData(
getProvisionerDaemonsKey(parameters.organization_id, parameters.tags),
parameters.provisioners,
);

return (
<Story/>
)
return <Story />;
};

export const withGlobalSnackbar = (Story: FC) => (
Expand Down
Loading