Skip to content
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

[Dashboard] Add Replicas to Serve Dashboard UI #33503

Merged
merged 10 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
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
4 changes: 4 additions & 0 deletions 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
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