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

feat: Modify NIM enablement process #3455

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
869facf
Modify NIM enablement process
yzhao583 Nov 8, 2024
09a9b3e
Clean up code and remove unnecessary manifests
yzhao583 Nov 8, 2024
0eb2a2e
Add logic to check the conditions of the odh-nim-account CR
yzhao583 Nov 9, 2024
33656d4
Added more check for enabled integration apps
yzhao583 Nov 11, 2024
0f42fe5
If failed to fetch integration app status, should show error on the r…
yzhao583 Nov 11, 2024
f9ef8cb
check integration app status in explore application page
yzhao583 Nov 11, 2024
259f82e
Fix lint issue
yzhao583 Nov 11, 2024
0149969
Fix lint issue
yzhao583 Nov 11, 2024
a270f11
add annotations to the secret and the account CR
yzhao583 Nov 12, 2024
05fe3ae
Update backend/src/routes/api/integrations/nim/index.ts
yzhao583 Nov 12, 2024
592c0e8
Update backend/src/routes/api/components/list.ts
yzhao583 Nov 12, 2024
d2fbe24
clean up
yzhao583 Nov 12, 2024
ac93af3
feat: listing all NIM accounts in the namespace, returning the first …
olavtar Nov 12, 2024
31e45b4
Avoid mutating object in useWatchIntegrationComponents
yzhao583 Nov 12, 2024
ae817f1
Clean up
yzhao583 Nov 13, 2024
3c770de
Reslove conflict
yzhao583 Nov 13, 2024
203ce77
feat: added logic for displaying the tile correctly
olavtar Nov 14, 2024
634fa5d
feat: changes for enabling
olavtar Nov 14, 2024
2e2b4e2
feat: updated mock component with the new properties.
olavtar Nov 14, 2024
0a6bc2c
feat: addressed PR comments with updates and improvements
olavtar Nov 14, 2024
e476c16
feat: backend bug fix
olavtar Nov 14, 2024
3e24dce
feat: Missed change from previous commit
olavtar Nov 14, 2024
830602b
feat: fix for the enabled page and enabled.cy.ts
olavtar Nov 15, 2024
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 backend/src/routes/api/components/list.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { FastifyRequest } from 'fastify';
import { KubeFastifyInstance, OdhApplication } from '../../../types';
import { getApplications, updateApplications } from '../../../utils/resourceUtils';
import {
getApplications,
updateApplications,
isIntegrationApp,
} from '../../../utils/resourceUtils';
import { checkJupyterEnabled, getRouteForApplication } from '../../../utils/componentUtils';

export const listComponents = async (
Expand All @@ -17,6 +21,10 @@ export const listComponents = async (
return Promise.resolve(applications);
}
for (const app of applications) {
if (isIntegrationApp(app)) {
// Include all integration apps -- Client can check if it's enabled
installedComponents.push(app);
}
if (app.spec.shownOnEnabledPage) {
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved
const newApp = {
...app,
Expand Down
84 changes: 84 additions & 0 deletions backend/src/routes/api/integrations/nim/index.ts
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { FastifyReply, FastifyRequest } from 'fastify';
import { secureAdminRoute } from '../../../../utils/route-security';
import { KubeFastifyInstance } from '../../../../types';
import { isString } from 'lodash';
import { isAppEnabled, getNIMAccount, createNIMAccount, createNIMSecret } from './nimUtils';

module.exports = async (fastify: KubeFastifyInstance) => {
const { namespace } = fastify.kube;
const PAGE_NOT_FOUND_MESSAGE = '404 page not found';

fastify.get(
'/',
secureAdminRoute(fastify)(async (request: FastifyRequest, reply: FastifyReply) => {
await getNIMAccount(fastify, namespace)
.then((response) => {
if (isAppEnabled(response)) {
reply.send({ isAppEnabled: true, canEnable: false, error: '' });
} else {
reply.send({ isAppEnabled: false, canEnable: true, error: '' });
}
})
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
.catch((e) => {
if (e.response?.statusCode === 404) {
// 404 error means the Account CRD does not exist, so cannot create CR based on it.
if (
isString(e.response.body) &&
e.response.body.trim() === PAGE_NOT_FOUND_MESSAGE.trim()
) {
fastify.log.error(`NIM not installed, ${e.response?.body}`);
reply
.status(404)
.send({ isAppEnabled: false, canEnable: false, error: 'NIM not installed' });
} else {
fastify.log.error(`NIM account does not exist, ${e.response.body.message}`);
reply.send({ isAppEnabled: false, canEnable: true, error: '' });
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
}
}
});
}),
);

fastify.post(
'/',
secureAdminRoute(fastify)(
async (
request: FastifyRequest<{
Body: { [key: string]: string };
}>,
reply: FastifyReply,
) => {
const enableValues = request.body;

await createNIMSecret(fastify, namespace, enableValues)
.then(async () => {
await createNIMAccount(fastify, namespace)
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved
.then((response) => {
if (isAppEnabled(response)) {
reply.send({ isAppEnabled: true, canEnable: false, error: '' });
} else {
reply.send({ isAppEnabled: false, canEnable: true, error: '' });
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
}
})
.catch((e) => {
fastify.log.error(`Failed to create NIM account.`);
reply
.status(e.response.statusCode)
.send(new Error(`Failed to create NIM account, ${e.response?.body?.message}`));
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
});
})
.catch((e) => {
if (e.response?.statusCode === 409) {
fastify.log.error(`NIM secret already exists, skipping creation.`);
reply.status(409).send(new Error(`NIM secret already exists, skipping creation.`));
} else {
fastify.log.error(`Failed to create NIM secret.`);
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
reply
.status(e.response.statusCode)
.send(new Error(`Failed to create NIM secret, ${e.response?.body?.message}`));
}
});
},
),
);
};
88 changes: 88 additions & 0 deletions backend/src/routes/api/integrations/nim/nimUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { KubeFastifyInstance, NIMAccountKind } from '../../../../types';

const NIM_SECRET_NAME = 'nvidia-nim-access';
const NIM_ACCOUNT_NAME = 'odh-nim-account';

export const isAppEnabled = (app: NIMAccountKind): boolean => {
const conditions = app?.status?.conditions || [];
return (
conditions.find(
(condition) => condition.type === 'AccountStatus' && condition.status === 'True',
) !== undefined
);
};

export const getNIMAccount = async (
fastify: KubeFastifyInstance,
namespace: string,
): Promise<NIMAccountKind> => {
const { customObjectsApi } = fastify.kube;
try {
const response = await customObjectsApi.getNamespacedCustomObject(
'nim.opendatahub.io',
'v1',
namespace,
'accounts',
NIM_ACCOUNT_NAME,
);
return Promise.resolve(response.body as NIMAccountKind);
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
return Promise.reject(e);
}
};

export const createNIMAccount = async (
fastify: KubeFastifyInstance,
namespace: string,
): Promise<NIMAccountKind> => {
const { customObjectsApi } = fastify.kube;
const account = {
apiVersion: 'nim.opendatahub.io/v1',
kind: 'Account',
metadata: {
name: NIM_ACCOUNT_NAME,
namespace,
},
spec: {
apiKeySecret: {
name: NIM_SECRET_NAME,
},
},
};
try {
const response = await customObjectsApi.createNamespacedCustomObject(
'nim.opendatahub.io',
'v1',
namespace,
'accounts',
account,
);
return Promise.resolve(response.body as NIMAccountKind);
} catch (e) {
return Promise.reject(e);
}
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved
};

export const createNIMSecret = async (
fastify: KubeFastifyInstance,
namespace: string,
enableValues: { [key: string]: string },
): Promise<void> => {
const { coreV1Api } = fastify.kube;
const nimSecret = {
apiVersion: 'v1',
kind: 'Secret',
metadata: {
name: NIM_SECRET_NAME,
namespace,
},
type: 'Opaque',
stringData: enableValues,
};
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved

try {
await coreV1Api.createNamespacedSecret(namespace, nimSecret);
} catch (e) {
return Promise.reject(e);
}
yzhao583 marked this conversation as resolved.
Show resolved Hide resolved
};
16 changes: 16 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ export type OdhApplication = {
displayName: string;
docsLink: string;
hidden?: boolean | null;
internalRoute?: string;
enable?: {
actionLabel: string;
description?: string;
Expand Down Expand Up @@ -1228,3 +1229,18 @@ export enum ServiceAddressAnnotation {
EXTERNAL_REST = 'routing.opendatahub.io/external-address-rest',
EXTERNAL_GRPC = 'routing.opendatahub.io/external-address-grpc',
}

export type NIMAccountKind = K8sResourceCommon & {
metadata: {
name: string;
namespace: string;
};
spec: {
apiKeySecret: {
name: string;
};
};
status?: {
conditions?: K8sCondition[];
};
};
andrewballantyne marked this conversation as resolved.
Show resolved Hide resolved
55 changes: 31 additions & 24 deletions backend/src/utils/resourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,33 +332,38 @@ export const fetchApplications = async (
.then((result) => result.body)
.catch(() => null);
for (const appDef of applicationDefs) {
appDef.spec.shownOnEnabledPage = enabledAppsCM?.data?.[appDef.metadata.name] === 'true';
appDef.spec.isEnabled = await getIsAppEnabled(fastify, appDef).catch((e) => {
fastify.log.warn(
`"${
appDef.metadata.name
}" OdhApplication is being disabled due to an error determining if it's enabled. ${
e.response?.body?.message || e.message
}`,
);
if (isIntegrationApp(appDef)) {
// Ignore logic for apps that use internal routes for status information
applications.push(appDef);
} else {
appDef.spec.shownOnEnabledPage = enabledAppsCM?.data?.[appDef.metadata.name] === 'true';
appDef.spec.isEnabled = await getIsAppEnabled(fastify, appDef).catch((e) => {
fastify.log.warn(
`"${
appDef.metadata.name
}" OdhApplication is being disabled due to an error determining if it's enabled. ${
e.response?.body?.message || e.message
}`,
);

return false;
});
if (appDef.spec.isEnabled) {
if (!appDef.spec.shownOnEnabledPage) {
changed = true;
enabledAppsCMData[appDef.metadata.name] = 'true';
appDef.spec.shownOnEnabledPage = true;
return false;
});
if (appDef.spec.isEnabled) {
if (!appDef.spec.shownOnEnabledPage) {
changed = true;
enabledAppsCMData[appDef.metadata.name] = 'true';
appDef.spec.shownOnEnabledPage = true;
}
}
applications.push({
...appDef,
spec: {
...appDef.spec,
getStartedLink: getRouteForClusterId(fastify, appDef.spec.getStartedLink),
link: appDef.spec.isEnabled ? await getRouteForApplication(fastify, appDef) : undefined,
},
});
}
applications.push({
...appDef,
spec: {
...appDef.spec,
getStartedLink: getRouteForClusterId(fastify, appDef.spec.getStartedLink),
link: appDef.spec.isEnabled ? await getRouteForApplication(fastify, appDef) : undefined,
},
});
}
if (changed) {
// write enabled apps configmap
Expand Down Expand Up @@ -1037,3 +1042,5 @@ export const translateDisplayNameForK8s = (name: string): string =>
.toLowerCase()
.replace(/\s/g, '-')
.replace(/[^A-Za-z0-9-]/g, '');
export const isIntegrationApp = (app: OdhApplication): boolean =>
app.spec.internalRoute?.startsWith('/api/');
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { OdhApplication } from '~/types';
import ApplicationsPage from '~/pages/ApplicationsPage';
import OdhAppCard from '~/components/OdhAppCard';
import { fireMiscTrackingEvent } from '~/concepts/analyticsTracking/segmentIOUtils';
import { useWatchIntegrationComponents } from '~/utilities/useWatchIntegrationComponents';

const description = `Launch your enabled applications, view documentation, or get started with quick start instructions and tasks.`;

Expand All @@ -21,18 +22,20 @@ let enabledComponents: OdhApplication[] = [];
export const EnabledApplicationsInner: React.FC<EnabledApplicationsInnerProps> = React.memo(
({ loaded, loadError, components }) => {
const isEmpty = components.length === 0;
const { checkedComponents, isIntegrationComponentsChecked } =
useWatchIntegrationComponents(components);

return (
<ApplicationsPage
title="Enabled"
description={description}
loaded={loaded}
loaded={loaded && isIntegrationComponentsChecked}
empty={isEmpty}
loadError={loadError}
>
<PageSection isFilled data-testid="enabled-application">
<Gallery maxWidths={{ default: '330px' }} role="list" hasGutter>
{components.map((c) => (
{checkedComponents.map((c) => (
<OdhAppCard key={c.metadata.name} odhApp={c} />
))}
</Gallery>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/exploreApplication/EnableModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const EnableModal: React.FC<EnableModalProps> = ({ selectedApp, shown, onClose }
selectedApp.metadata.name,
selectedApp.spec.displayName,
enableValues,
selectedApp.spec.internalRoute,
);
const focusRef = (element: HTMLElement | null) => {
if (element) {
Expand Down
17 changes: 10 additions & 7 deletions frontend/src/pages/exploreApplication/ExploreApplications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { removeQueryArgument, setQueryArgument } from '~/utilities/router';
import { ODH_PRODUCT_NAME } from '~/utilities/const';
import { useAppContext } from '~/app/AppContext';
import { fireMiscTrackingEvent } from '~/concepts/analyticsTracking/segmentIOUtils';
import { useWatchIntegrationComponents } from '~/utilities/useWatchIntegrationComponents';
import GetStartedPanel from './GetStartedPanel';

import './ExploreApplications.scss';
Expand Down Expand Up @@ -98,10 +99,12 @@ const ExploreApplications: React.FC = () => {
const selectedId = queryParams.get('selectId');
const [selectedComponent, setSelectedComponent] = React.useState<OdhApplication>();
const isEmpty = components.length === 0;
const { checkedComponents, isIntegrationComponentsChecked } =
useWatchIntegrationComponents(components);

const updateSelection = React.useCallback(
(currentSelectedId?: string | null): void => {
const selection = components.find(
const selection = checkedComponents.find(
(c) => c.metadata.name && c.metadata.name === currentSelectedId,
);
if (currentSelectedId && selection) {
Expand All @@ -114,26 +117,26 @@ const ExploreApplications: React.FC = () => {
removeQueryArgument(navigate, 'selectId');
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[components],
[checkedComponents],
);

const exploreComponents = React.useMemo<OdhApplication[]>(
() =>
_.cloneDeep(components)
_.cloneDeep(checkedComponents)
.filter((component) => !component.spec.hidden)
.toSorted((a, b) => a.spec.displayName.localeCompare(b.spec.displayName)),
[components],
[checkedComponents],
);

React.useEffect(() => {
if (components.length > 0) {
if (checkedComponents.length > 0) {
updateSelection(selectedId);
}
}, [updateSelection, selectedId, components]);
}, [updateSelection, selectedId, checkedComponents]);

return (
<ExploreApplicationsInner
loaded={loaded}
loaded={loaded && isIntegrationComponentsChecked}
isEmpty={isEmpty}
loadError={loadError}
exploreComponents={exploreComponents}
Expand Down
Loading
Loading