Skip to content

Commit

Permalink
Merge pull request #2108 from alexcreasy/issue-2088
Browse files Browse the repository at this point in the history
Adds 'Area' support for TrustyAI and Performance Metric feature sets
  • Loading branch information
openshift-merge-bot[bot] authored Nov 9, 2023
2 parents 453f440 + 27f5d5c commit e77d2c1
Show file tree
Hide file tree
Showing 67 changed files with 1,125 additions and 489 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
name: Pull request
on:
pull_request:
branches: [ main, f/** ]

jobs:
Tests:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/vuln_scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Run Trivy vulnerability scanner for filesystem
uses: aquasecurity/trivy-action@0.11.2
uses: aquasecurity/trivy-action@0.13.1
with:
scan-type: 'fs'
scan-ref: '.'
Expand Down
15 changes: 14 additions & 1 deletion docs/process-definition/branches.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ There are really two types of branches.
- Core branches (like `main` and `incubation`)
- [Bot branches](#bot-branches)

Every _new_ commit needs to come from a fork through a PR. We don't allow for pushing new content directly through our flows. New docs file, new code change, and even fixing a typo needs a PR from your fork to get into our repository.
Every _new_ commit needs to come from a fork through a PR. We don't allow for pushing new content directly through our flows. New docs file, new code change, and even fixing a typo needs a PR from your fork to get into our repository. This ensures automated tests pass before the change is merged.

With that said, there are really 3 types of flows that utilize both fork branches and Upstream branches.

Expand All @@ -33,6 +33,19 @@ There is only ever 1 `main` and 1 `incubation` branch. Feature branches start wi

Read more on git tags & releases in our [release documentation].

## Merging upstream branches

Understanding the commit history on an upstream branch is important. Therefore when merging an upstream branch into another, your PR branch must follow the pattern `merge-<source branch nam>`.

For example when merging `f/some-feature` into `incubation`, name your branch `merge-f/some-feature`. This will result in a commit message on the `incubation` branch of `Merge pull request # from <username>/merge-f/some-feature` when the PR is merged.

Use the following steps to create a PR when merging upstream branches:

- `git checkout -b merge-<source-branch> <target-branch>`
- `git pull --no-rebase upstream <source-branch>`
- Resolve all conflicts then post a PR.
- If the target branch is `main`, wait for the PR to be approved. For all other target branches, apply the `approved` and `lgtm` labels to the PR once all checks pass.

## Main

> aka "The Stable Branch"
Expand Down
4 changes: 4 additions & 0 deletions frontend/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@
"group": ["~/components/table/**", "!~/components/table/useTableColumnSort"],
"message": "Read from '~/components/table' instead."
},
{
"group": ["~/concepts/area/**"],
"message": "Read from '~/concepts/area' instead."
},
{
"group": ["~/components/table/useTableColumnSort"],
"message": "The data will be sorted in the table, don't use this hook outside of '~/components/table' repo. For more information, please check the props of the Table component."
Expand Down
9 changes: 5 additions & 4 deletions frontend/src/__mocks__/mockDashboardConfig.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { DashboardConfig } from '~/types';
import { KnownLabels } from '~/k8sTypes';
import { DashboardConfigKind, KnownLabels } from '~/k8sTypes';

type MockDashboardConfigType = {
disableInfo?: boolean;
Expand All @@ -11,6 +10,7 @@ type MockDashboardConfigType = {
disableAppLauncher?: boolean;
disableUserManagement?: boolean;
disableProjects?: boolean;
disablePipelines?: boolean;
disableModelServing?: boolean;
disableCustomServingRuntimes?: boolean;
};
Expand All @@ -27,7 +27,8 @@ export const mockDashboardConfig = ({
disableProjects = false,
disableModelServing = false,
disableCustomServingRuntimes = false,
}: MockDashboardConfigType): DashboardConfig => ({
disablePipelines = false,
}: MockDashboardConfigType): DashboardConfigKind => ({
apiVersion: 'opendatahub.io/v1alpha',
kind: 'OdhDashboardConfig',
metadata: {
Expand All @@ -51,7 +52,7 @@ export const mockDashboardConfig = ({
disableProjects,
disableModelServing,
disableCustomServingRuntimes,
disablePipelines: false,
disablePipelines,
disableProjectSharing: false,
disableBiasMetrics: false,
disablePerformanceMetrics: false,
Expand Down
20 changes: 20 additions & 0 deletions frontend/src/__mocks__/mockDscStatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { DataScienceClusterKindStatus } from '~/k8sTypes';
import { StackComponent } from '~/concepts/areas/types';

type MockDscStatus = {
installedComponents?: DataScienceClusterKindStatus['installedComponents'];
};

export const mockDscStatus = ({
installedComponents,
}: MockDscStatus): DataScienceClusterKindStatus => ({
conditions: [],
installedComponents: Object.values(StackComponent).reduce(
(acc, component) => ({
...acc,
[component]: installedComponents?.[component] ?? false,
}),
{},
),
phase: 'Ready',
});
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,19 @@ test('Legacy Serving Runtime', async ({ page }) => {
await expect(firstRow).toHaveClass('pf-m-expanded');
await expect(secondRow).not.toHaveClass('pf-m-expanded');
});

test('Add model server', async ({ page }) => {
await page.goto(navigateToStory('pages-modelserving-servingruntimelist', 'add-server'));

// wait for page to load
await page.waitForSelector('text=Add server');

// test that you can not submit on empty
await expect(await page.getByRole('button', { name: 'Add', exact: true })).toBeDisabled();

// test filling in minimum required fields
await page.getByLabel('Model server name *').fill('Test Server Name');
await page.locator('#serving-runtime-template-selection').click();
await page.getByText('New OVMS Server').click();
await expect(await page.getByRole('button', { name: 'Add', exact: true })).toBeEnabled();
});
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,14 @@ export const DeployModel: StoryObj = {
await userEvent.click(canvas.getAllByText('Deploy model', { selector: 'button' })[0]);
},
};

export const AddServer: StoryObj = {
render: Template,

play: async ({ canvasElement }) => {
// load page and wait until settled
const canvas = within(canvasElement);
await canvas.findByText('ovms', undefined, { timeout: 5000 });
await userEvent.click(canvas.getByText('Add server', { selector: 'button' }));
},
};
25 changes: 13 additions & 12 deletions frontend/src/api/prometheus/serving.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ import {
RefreshIntervalTitle,
TimeframeTitle,
} from '~/pages/modelServing/screens/types';
import useBiasMetricsEnabled from '~/concepts/explainability/useBiasMetricsEnabled';
import { ResponsePredicate } from '~/api/prometheus/usePrometheusQueryRange';
import useRefreshInterval from '~/utilities/useRefreshInterval';
import { QueryTimeframeStep, RefreshIntervalValue } from '~/pages/modelServing/screens/const';
import usePerformanceMetricsEnabled from '~/pages/modelServing/screens/metrics/usePerformanceMetricsEnabled';
import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas';
import useQueryRangeResourceData from './useQueryRangeResourceData';

export const useModelServingMetrics = (
Expand All @@ -36,8 +35,10 @@ export const useModelServingMetrics = (
refresh: () => void;
} => {
const [end, setEnd] = React.useState(lastUpdateTime);
const [biasMetricsEnabled] = useBiasMetricsEnabled();
const [performanceMetricsEnabled] = usePerformanceMetricsEnabled();
const biasMetricsAreaAvailable = useIsAreaAvailable(SupportedArea.BIAS_METRICS).status;
const performanceMetricsAreaAvailable = useIsAreaAvailable(
SupportedArea.PERFORMANCE_METRICS,
).status;

const defaultResponsePredicate = React.useCallback<ResponsePredicate>(
(data) => data.result?.[0]?.values || [],
Expand All @@ -49,7 +50,7 @@ export const useModelServingMetrics = (
>((data) => data.result || [], []);

const serverRequestCount = useQueryRangeResourceData(
performanceMetricsEnabled && type === PerformanceMetricType.SERVER,
performanceMetricsAreaAvailable && type === PerformanceMetricType.SERVER,
(queries as { [key in ServerMetricType]: string })[ServerMetricType.REQUEST_COUNT],
end,
timeframe,
Expand All @@ -60,7 +61,7 @@ export const useModelServingMetrics = (

const serverAverageResponseTime =
useQueryRangeResourceData<PrometheusQueryRangeResponseDataResult>(
performanceMetricsEnabled && type === PerformanceMetricType.SERVER,
performanceMetricsAreaAvailable && type === PerformanceMetricType.SERVER,
(queries as { [key in ServerMetricType]: string })[ServerMetricType.AVG_RESPONSE_TIME],
end,
timeframe,
Expand All @@ -70,7 +71,7 @@ export const useModelServingMetrics = (
);

const serverCPUUtilization = useQueryRangeResourceData(
performanceMetricsEnabled && type === PerformanceMetricType.SERVER,
performanceMetricsAreaAvailable && type === PerformanceMetricType.SERVER,
(queries as { [key in ServerMetricType]: string })[ServerMetricType.CPU_UTILIZATION],
end,
timeframe,
Expand All @@ -80,7 +81,7 @@ export const useModelServingMetrics = (
);

const serverMemoryUtilization = useQueryRangeResourceData(
performanceMetricsEnabled && type === PerformanceMetricType.SERVER,
performanceMetricsAreaAvailable && type === PerformanceMetricType.SERVER,
(queries as { [key in ServerMetricType]: string })[ServerMetricType.MEMORY_UTILIZATION],
end,
timeframe,
Expand All @@ -90,7 +91,7 @@ export const useModelServingMetrics = (
);

const modelRequestSuccessCount = useQueryRangeResourceData(
performanceMetricsEnabled && type === PerformanceMetricType.MODEL,
performanceMetricsAreaAvailable && type === PerformanceMetricType.MODEL,
(queries as { [key in ModelMetricType]: string })[ModelMetricType.REQUEST_COUNT_SUCCESS],
end,
timeframe,
Expand All @@ -100,7 +101,7 @@ export const useModelServingMetrics = (
);

const modelRequestFailedCount = useQueryRangeResourceData(
performanceMetricsEnabled && type === PerformanceMetricType.MODEL,
performanceMetricsAreaAvailable && type === PerformanceMetricType.MODEL,
(queries as { [key in ModelMetricType]: string })[ModelMetricType.REQUEST_COUNT_FAILED],
end,
timeframe,
Expand All @@ -110,7 +111,7 @@ export const useModelServingMetrics = (
);

const modelTrustyAISPD = useQueryRangeResourceData(
biasMetricsEnabled && type === PerformanceMetricType.MODEL,
biasMetricsAreaAvailable && type === PerformanceMetricType.MODEL,
(queries as { [key in ModelMetricType]: string })[ModelMetricType.TRUSTY_AI_SPD],
end,
timeframe,
Expand All @@ -121,7 +122,7 @@ export const useModelServingMetrics = (
);

const modelTrustyAIDIR = useQueryRangeResourceData(
biasMetricsEnabled && type === PerformanceMetricType.MODEL,
biasMetricsAreaAvailable && type === PerformanceMetricType.MODEL,
(queries as { [key in ModelMetricType]: string })[ModelMetricType.TRUSTY_AI_DIR],
end,
timeframe,
Expand Down
41 changes: 22 additions & 19 deletions frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useUser } from '~/redux/selectors';
import { DASHBOARD_MAIN_CONTAINER_ID } from '~/utilities/const';
import useDetectUser from '~/utilities/useDetectUser';
import ProjectsContextProvider from '~/concepts/projects/ProjectsContext';
import AreaContextProvider from '~/concepts/areas/AreaContext';
import Header from './Header';
import AppRoutes from './AppRoutes';
import NavSidebar from './NavSidebar';
Expand Down Expand Up @@ -89,25 +90,27 @@ const App: React.FC = () => {
dashboardConfig,
}}
>
<Page
className="odh-dashboard"
isManagedSidebar
header={<Header onNotificationsClick={() => setNotificationsOpen(!notificationsOpen)} />}
sidebar={isAllowed ? <NavSidebar /> : undefined}
notificationDrawer={<AppNotificationDrawer onClose={() => setNotificationsOpen(false)} />}
isNotificationDrawerExpanded={notificationsOpen}
mainContainerId={DASHBOARD_MAIN_CONTAINER_ID}
>
<ErrorBoundary>
<ProjectsContextProvider>
<QuickStarts>
<AppRoutes />
</QuickStarts>
</ProjectsContextProvider>
<ToastNotifications />
<TelemetrySetup />
</ErrorBoundary>
</Page>
<AreaContextProvider>
<Page
className="odh-dashboard"
isManagedSidebar
header={<Header onNotificationsClick={() => setNotificationsOpen(!notificationsOpen)} />}
sidebar={isAllowed ? <NavSidebar /> : undefined}
notificationDrawer={<AppNotificationDrawer onClose={() => setNotificationsOpen(false)} />}
isNotificationDrawerExpanded={notificationsOpen}
mainContainerId={DASHBOARD_MAIN_CONTAINER_ID}
>
<ErrorBoundary>
<ProjectsContextProvider>
<QuickStarts>
<AppRoutes />
</QuickStarts>
</ProjectsContextProvider>
<ToastNotifications />
<TelemetrySetup />
</ErrorBoundary>
</Page>
</AreaContextProvider>
</AppContext.Provider>
);
};
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/app/AppContext.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import * as React from 'react';
import { BuildStatus, DashboardConfig } from '~/types';
import { DashboardConfigKind } from '~/k8sTypes';
import { BuildStatus } from '~/types';

type AppContextProps = {
buildStatuses: BuildStatus[];
dashboardConfig: DashboardConfig;
dashboardConfig: DashboardConfigKind;
};

const defaultAppContext: AppContextProps = {
buildStatuses: [],
// At runtime dashboardConfig is never null -- DO NOT DO THIS usually
dashboardConfig: null as unknown as DashboardConfig,
dashboardConfig: null as unknown as DashboardConfigKind,
};

export const AppContext = React.createContext(defaultAppContext);
Expand Down
39 changes: 20 additions & 19 deletions frontend/src/app/NavSidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import React from 'react';
import { Link, useLocation } from 'react-router-dom';
import { Nav, NavExpandable, NavItem, NavList, PageSidebar } from '@patternfly/react-core';
import { getNavBarData, isNavDataGroup, NavDataGroup, NavDataHref } from '~/utilities/NavData';
import { useUser } from '~/redux/selectors';
import { useAppContext } from './AppContext';
import { isNavDataGroup, NavDataGroup, NavDataHref, useBuildNavData } from '~/utilities/NavData';

const checkLinkActiveStatus = (pathname: string, href: string) =>
href.split('/')[1] === pathname.split('/')[1];
Expand Down Expand Up @@ -45,24 +43,27 @@ const NavGroup: React.FC<{ item: NavDataGroup; pathname: string }> = ({ item, pa
};

const NavSidebar: React.FC = () => {
const { dashboardConfig } = useAppContext();
const routerLocation = useLocation();
const { isAdmin } = useUser();
const userNavData = getNavBarData(isAdmin, dashboardConfig);
const nav = (
<Nav theme="dark" aria-label="Nav">
<NavList>
{userNavData.map((item) =>
isNavDataGroup(item) ? (
<NavGroup key={item.id} item={item} pathname={routerLocation.pathname} />
) : (
<NavHref key={item.id} item={item} pathname={routerLocation.pathname} />
),
)}
</NavList>
</Nav>
const userNavData = useBuildNavData();

return (
<PageSidebar
nav={
<Nav theme="dark" aria-label="Nav">
<NavList>
{userNavData.map((item) =>
isNavDataGroup(item) ? (
<NavGroup key={item.id} item={item} pathname={routerLocation.pathname} />
) : (
<NavHref key={item.id} item={item} pathname={routerLocation.pathname} />
),
)}
</NavList>
</Nav>
}
theme="dark"
/>
);
return <PageSidebar nav={nav} theme="dark" />;
};

export default NavSidebar;
Loading

0 comments on commit e77d2c1

Please sign in to comment.