Skip to content

Commit

Permalink
[Dashboard] Add Replicas to Serve Dashboard UI (#33503)
Browse files Browse the repository at this point in the history
adds replica rows to the Deployments table in Serve Applications details page
Updates the deployments table to support two row types. (Column names adjusted).
Adds a replica detail page
Adds serve button to the Main nav bar at the top
Updates the frontend models for /serve_head/applications after some backwards-incompatible changes.
Wrote tests
  • Loading branch information
alanwguo authored Mar 22, 2023
1 parent f8f7374 commit 2713311
Show file tree
Hide file tree
Showing 23 changed files with 800 additions and 98 deletions.
18 changes: 14 additions & 4 deletions dashboard/client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ import { ClusterDetailInfoPage } from "./pages/node/ClusterDetailInfoPage";
import { ClusterLayout } from "./pages/node/ClusterLayout";
import NodeDetailPage from "./pages/node/NodeDetail";
import { OverviewPage } from "./pages/overview/OverviewPage";
import { ServeApplicationDetailPage } from "./pages/serve/ServeApplicationDetailPage";
import {
ServeApplicationDetailLayout,
ServeApplicationDetailPage,
} from "./pages/serve/ServeApplicationDetailPage";
import { ServeApplicationsListPage } from "./pages/serve/ServeApplicationsListPage";
import { ServeLayout } from "./pages/serve/ServeLayout";
import { ServeReplicaDetailPage } from "./pages/serve/ServeReplicaDetailPage";
import { getNodeList } from "./service/node";
import { lightTheme } from "./theme";

Expand Down Expand Up @@ -191,9 +195,15 @@ const App = () => {
<Route element={<ServeLayout />} path="serve">
<Route element={<ServeApplicationsListPage />} path="" />
<Route
element={<ServeApplicationDetailPage />}
path="applications/:name"
/>
element={<ServeApplicationDetailLayout />}
path="applications/:applicationName"
>
<Route element={<ServeApplicationDetailPage />} path="" />
<Route
element={<ServeReplicaDetailPage />}
path=":deploymentName/:replicaId"
/>
</Route>
</Route>
<Route element={<LogsLayout />} path="logs">
{/* TODO(aguo): Refactor Logs component to use optional query
Expand Down
68 changes: 68 additions & 0 deletions dashboard/client/src/common/links.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { Link } from "@material-ui/core";
import React, { PropsWithChildren } from "react";
import { Link as RouterLink } from "react-router-dom";
import { ClassNameProps } from "./props";

type ActorLinkProps = PropsWithChildren<
{
actorId: string;
/**
* This can be provided to override where we link to.
*/
to?: string;
} & ClassNameProps
>;

export const generateActorLink = (actorId: string) => `/actors/${actorId}`;

/**
* A link to the top-level actors detail page.
*/
export const ActorLink = ({
actorId,
to,
children,
className,
}: ActorLinkProps) => {
return (
<Link
className={className}
component={RouterLink}
to={to ?? generateActorLink(actorId)}
>
{children ?? actorId}
</Link>
);
};

type NodeLinkProps = PropsWithChildren<
{
nodeId: string;
/**
* This can be provided to override where we link to.
*/
to?: string;
} & ClassNameProps
>;

export const generateNodeLink = (nodeId: string) => `/cluster/nodes/${nodeId}`;

/**
* A link to the top-level Cluster node detail page.
*/
export const NodeLink = ({
nodeId,
to,
children,
className,
}: NodeLinkProps) => {
return (
<Link
className={className}
component={RouterLink}
to={to ?? generateNodeLink(nodeId)}
>
{children ?? nodeId}
</Link>
);
};
20 changes: 11 additions & 9 deletions dashboard/client/src/components/ActorTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import React, { useContext, useState } from "react";
import { Link } from "react-router-dom";
import { GlobalContext } from "../App";
import { DurationText } from "../common/DurationText";
import { ActorLink } from "../common/links";
import { CpuProfilingLink, CpuStackTraceLink } from "../common/ProfilingLink";
import rowStyles from "../common/RowStyles";
import { Actor } from "../type/actor";
Expand Down Expand Up @@ -346,15 +347,16 @@ const ActorTable = ({
arrow
interactive
>
<Link
to={
detailPathPrefix
? `${detailPathPrefix}/${actorId}`
: actorId
}
>
{actorId}
</Link>
<div>
<ActorLink
actorId={actorId}
to={
detailPathPrefix
? `${detailPathPrefix}/${actorId}`
: actorId
}
/>
</div>
</Tooltip>
</TableCell>
<TableCell align="center">{actorClass}</TableCell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,68 @@ const COPY_BUTTON_LABEL = "copy";

describe("MetadataContentField", () => {
it("renders the content string", () => {
expect.assertions(3);
expect.assertions(4);

render(<MetadataContentField content={{ value: CONTENT_VALUE }} />);
render(
<MetadataContentField
content={{ value: CONTENT_VALUE }}
label="test-label"
/>,
);

expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument();
expect(screen.getByText(CONTENT_VALUE)).not.toHaveAttribute("href");
expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument();
expect(
screen.getByTestId("metadata-content-for-test-label"),
).toBeInTheDocument();
});

it("renders the content string with label", () => {
expect.assertions(3);
expect.assertions(4);

render(
<MetadataContentField
content={{ value: CONTENT_VALUE, link: LINK_VALUE }}
label="test-label"
/>,
);

expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument();
expect(screen.getByText(CONTENT_VALUE)).toHaveAttribute("href", LINK_VALUE);
expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument();
expect(
screen.getByTestId("metadata-content-for-test-label"),
).toBeInTheDocument();
});

it("renders the content string with copyable value", () => {
expect.assertions(3);
expect.assertions(4);
render(
<MetadataContentField
content={{ value: CONTENT_VALUE, copyableValue: COPYABLE_VALUE }}
label="test-label"
/>,
);
expect(screen.getByText(CONTENT_VALUE)).toBeInTheDocument();
expect(screen.getByText(CONTENT_VALUE)).not.toHaveAttribute("href");
expect(screen.getByLabelText(COPY_BUTTON_LABEL)).toBeInTheDocument();
expect(
screen.getByTestId("metadata-content-for-test-label"),
).toBeInTheDocument();
});

it("renders the content string with a JSX element", () => {
expect.assertions(2);
expect.assertions(3);

const CUSTOM_TEST_ID = "custom-test-id";
const customElement = <p data-testid={CUSTOM_TEST_ID}>Test</p>;
render(<MetadataContentField content={customElement} />);
render(<MetadataContentField content={customElement} label="test-label" />);

expect(screen.queryByLabelText(COPY_BUTTON_LABEL)).not.toBeInTheDocument();
expect(screen.getByTestId(CUSTOM_TEST_ID)).toBeInTheDocument();
expect(
screen.getByTestId("metadata-content-for-test-label"),
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ const useStyles = makeStyles((theme) =>
*/
export const MetadataContentField: React.FC<{
content: Metadata["content"];
}> = ({ content }) => {
label: string;
}> = ({ content, label }) => {
const classes = useStyles();
const [copyIconClicked, setCopyIconClicked] = useState<boolean>(false);

Expand All @@ -99,6 +100,7 @@ export const MetadataContentField: React.FC<{
className={classes.content}
variant="body2"
title={content?.value}
data-testid={`metadata-content-for-${label}`}
>
{content?.value ?? "-"}
</Typography>
Expand Down Expand Up @@ -127,20 +129,25 @@ export const MetadataContentField: React.FC<{
)}
</div>
) : content.link.startsWith("http") ? (
<Link className={classes.content} href={content.link}>
<Link
className={classes.content}
href={content.link}
data-testid={`metadata-content-for-${label}`}
>
{content.value}
</Link>
) : (
<Link
className={classes.content}
component={RouterLink}
to={content.link}
data-testid={`metadata-content-for-${label}`}
>
{content.value}
</Link>
);
}
return content;
return <div data-testid={`metadata-content-for-${label}`}>{content}</div>;
};

/**
Expand Down Expand Up @@ -168,7 +175,7 @@ const MetadataList: React.FC<{
</HelpInfo>
)}
</Box>
<MetadataContentField content={content} />
<MetadataContentField content={content} label={label} />
</Box>
))}
</Box>
Expand Down
14 changes: 13 additions & 1 deletion dashboard/client/src/components/StatusChip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ import {
green,
grey,
lightBlue,
orange,
red,
yellow,
} from "@material-ui/core/colors";
import { CSSProperties } from "@material-ui/core/styles/withStyles";
import React, { ReactNode } from "react";
import { ActorEnum } from "../type/actor";
import { PlacementGroupState } from "../type/placementGroup";
import { ServeApplicationStatus, ServeDeploymentStatus } from "../type/serve";
import {
ServeApplicationStatus,
ServeDeploymentStatus,
ServeReplicaState,
} from "../type/serve";
import { TypeTaskStatus } from "../type/task";

const colorMap = {
Expand Down Expand Up @@ -70,6 +75,13 @@ const colorMap = {
[ServeDeploymentStatus.HEALTHY]: green,
[ServeDeploymentStatus.UNHEALTHY]: red,
},
serveReplica: {
[ServeReplicaState.STARTING]: yellow,
[ServeReplicaState.UPDATING]: yellow,
[ServeReplicaState.RECOVERING]: orange,
[ServeReplicaState.RUNNING]: green,
[ServeReplicaState.STOPPING]: red,
},
} as {
[key: string]: {
[key: string]: Color | string;
Expand Down
9 changes: 7 additions & 2 deletions dashboard/client/src/components/TaskTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Link } from "react-router-dom";
import { GlobalContext } from "../App";
import DialogWithTitle from "../common/DialogWithTitle";
import { DurationText } from "../common/DurationText";
import { ActorLink, NodeLink } from "../common/links";
import rowStyles from "../common/RowStyles";
import { Task } from "../type/task";
import { useFilter } from "../util/hook";
Expand Down Expand Up @@ -259,7 +260,7 @@ const TaskTable = ({
arrow
interactive
>
<div>{node_id ? node_id : "-"}</div>
{node_id ? <NodeLink nodeId={node_id} /> : <div>-</div>}
</Tooltip>
</TableCell>
<TableCell align="center">
Expand All @@ -269,7 +270,11 @@ const TaskTable = ({
arrow
interactive
>
<div>{actor_id ? actor_id : "-"}</div>
{actor_id ? (
<ActorLink actorId={actor_id} />
) : (
<div>-</div>
)}
</Tooltip>
</TableCell>
<TableCell align="center">
Expand Down
6 changes: 5 additions & 1 deletion dashboard/client/src/pages/actor/ActorDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Link } from "react-router-dom";
import { GlobalContext } from "../../App";
import { DurationText } from "../../common/DurationText";
import { formatDateFromTimeMs } from "../../common/formatUtils";
import { generateNodeLink } from "../../common/links";
import {
CpuProfilingLink,
CpuStackTraceLink,
Expand Down Expand Up @@ -107,6 +108,9 @@ const ActorDetailPage = () => {
? {
value: actorDetail.address?.rayletId,
copyableValue: actorDetail.address?.rayletId,
link: actorDetail.address.rayletId
? generateNodeLink(actorDetail.address.rayletId)
: undefined,
}
: { value: "-" },
},
Expand Down Expand Up @@ -164,7 +168,7 @@ const ActorDetailPage = () => {
<div>
<Link
target="_blank"
to={`/log/${encodeURIComponent(
to={`/logs/${encodeURIComponent(
ipLogMap[actorDetail.address?.ipAddress],
)}?fileName=${actorDetail.jobId}-${actorDetail.pid}`}
>
Expand Down
12 changes: 6 additions & 6 deletions dashboard/client/src/pages/layout/MainNavLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,11 @@ const NAV_ITEMS = [
path: "/jobs",
id: "jobs",
},
// TODO(aguo): Add this nav button in once the page is fully implemented.
// {
// title: "Serve",
// path: "/serve",
// id: "serve",
// },
{
title: "Serve",
path: "/serve",
id: "serve",
},
{
title: "Cluster",
path: "/cluster",
Expand Down Expand Up @@ -246,6 +245,7 @@ const useMainNavBreadcrumbsStyles = makeStyles((theme) =>
},
breadcrumbItem: {
fontWeight: 500,
color: "#8C9196",
"&:not(:first-child)": {
marginLeft: theme.spacing(1),
},
Expand Down
Loading

0 comments on commit 2713311

Please sign in to comment.