From 47f520e43dbea0fe5c5ef6e9963e55850121b509 Mon Sep 17 00:00:00 2001 From: olavtar <94576904+olavtar@users.noreply.github.com> Date: Mon, 16 Dec 2024 11:28:20 -0800 Subject: [PATCH] feat: fetching resources names from the Account CR (#3535) * feat: fetching resources names from the Account CR Signed-off-by: Olga Lavtar * feat: fixed unit tests Signed-off-by: Olga Lavtar * feat: fixed cypress tests Signed-off-by: Olga Lavtar * feat: fixed unit test after code changes. Signed-off-by: Olga Lavtar * feat: applied changes based on the comments. Signed-off-by: Olga Lavtar * feat: rename accounts.ts to nimAccounts.ts Signed-off-by: Olga Lavtar * fix: pass resource key instead of the resource name. Signed-off-by: Olga Lavtar * fix: pass resource key instead of the resource name. Signed-off-by: Olga Lavtar * fix: remove NIMAccountConstants Signed-off-by: Olga Lavtar * fix: unit test fix Signed-off-by: Olga Lavtar * fix: using lodash to get the resourceName Signed-off-by: Olga Lavtar * fix: rename listAccounts to listNIMAccounts Signed-off-by: Olga Lavtar * fix: remove dashboardNamespace Signed-off-by: Olga Lavtar * fix: remove namespace when calling getNIMAccount, rename createNIMSecret to manageNIMSecret Signed-off-by: Olga Lavtar --------- Signed-off-by: Olga Lavtar --- .../src/routes/api/integrations/nim/index.ts | 4 +- .../routes/api/integrations/nim/nimUtils.ts | 2 +- backend/src/routes/api/nim-serving/index.ts | 57 ++++++---- frontend/src/__mocks__/mockNimAccount.ts | 43 +++++++ frontend/src/__mocks__/mockNimResource.ts | 8 +- .../cypress/cypress/support/commands/odh.ts | 8 +- .../modelRegistry/modelVersionDeploy.cy.ts | 4 + .../mocked/projects/projectDetails.cy.ts | 10 +- .../cypress/cypress/utils/nimUtils.ts | 13 ++- frontend/src/api/index.ts | 1 + frontend/src/api/k8s/nimAccounts.ts | 11 ++ frontend/src/api/models/odh.ts | 7 ++ frontend/src/k8sTypes.ts | 24 ++++ .../NIMServiceModal/ManageNIMServingModal.tsx | 27 +++-- .../screens/projects/__tests__/utils.spec.ts | 107 ++++++++++++------ .../modelServing/screens/projects/nimUtils.ts | 62 +++++++--- .../screens/projects/useNIMTemplateName.ts | 19 ++++ .../modelServing/screens/projects/utils.ts | 21 ++-- .../base/fetch-nim-account.rbac.yaml | 26 +++++ manifests/core-bases/base/kustomization.yaml | 1 + 20 files changed, 341 insertions(+), 114 deletions(-) create mode 100644 frontend/src/__mocks__/mockNimAccount.ts create mode 100644 frontend/src/api/k8s/nimAccounts.ts create mode 100644 frontend/src/pages/modelServing/screens/projects/useNIMTemplateName.ts create mode 100644 manifests/core-bases/base/fetch-nim-account.rbac.yaml diff --git a/backend/src/routes/api/integrations/nim/index.ts b/backend/src/routes/api/integrations/nim/index.ts index 9646f0c75e..25237c60d7 100644 --- a/backend/src/routes/api/integrations/nim/index.ts +++ b/backend/src/routes/api/integrations/nim/index.ts @@ -2,7 +2,7 @@ import { FastifyReply, FastifyRequest } from 'fastify'; import { secureAdminRoute } from '../../../../utils/route-security'; import { KubeFastifyInstance } from '../../../../types'; import { isString } from 'lodash'; -import { createNIMAccount, createNIMSecret, getNIMAccount, isAppEnabled } from './nimUtils'; +import { createNIMAccount, getNIMAccount, isAppEnabled, manageNIMSecret } from './nimUtils'; module.exports = async (fastify: KubeFastifyInstance) => { const PAGE_NOT_FOUND_MESSAGE = '404 page not found'; @@ -62,7 +62,7 @@ module.exports = async (fastify: KubeFastifyInstance) => { const enableValues = request.body; try { - await createNIMSecret(fastify, enableValues); + await manageNIMSecret(fastify, enableValues); // Ensure the account exists try { const account = await getNIMAccount(fastify); diff --git a/backend/src/routes/api/integrations/nim/nimUtils.ts b/backend/src/routes/api/integrations/nim/nimUtils.ts index 297e703e39..84b1045da9 100644 --- a/backend/src/routes/api/integrations/nim/nimUtils.ts +++ b/backend/src/routes/api/integrations/nim/nimUtils.ts @@ -62,7 +62,7 @@ export const createNIMAccount = async (fastify: KubeFastifyInstance): Promise => { diff --git a/backend/src/routes/api/nim-serving/index.ts b/backend/src/routes/api/nim-serving/index.ts index 770209277d..0adff6b6bf 100644 --- a/backend/src/routes/api/nim-serving/index.ts +++ b/backend/src/routes/api/nim-serving/index.ts @@ -1,40 +1,53 @@ import { KubeFastifyInstance, OauthFastifyRequest } from '../../../types'; import { createCustomError } from '../../../utils/requestUtils'; import { logRequestDetails } from '../../../utils/fileUtils'; - -const secretNames = ['nvidia-nim-access', 'nvidia-nim-image-pull']; -const configMapName = 'nvidia-nim-images-data'; +import { getNIMAccount } from '../integrations/nim/nimUtils'; +import { get } from 'lodash'; export default async (fastify: KubeFastifyInstance): Promise => { + const resourceMap: Record = { + apiKeySecret: { type: 'Secret', path: ['spec', 'apiKeySecret', 'name'] }, + nimPullSecret: { type: 'Secret', path: ['status', 'nimPullSecret', 'name'] }, + nimConfig: { type: 'ConfigMap', path: ['status', 'nimConfig', 'name'] }, + }; + fastify.get( '/:nimResource', - async ( - request: OauthFastifyRequest<{ - Params: { nimResource: string }; - }>, - ) => { + async (request: OauthFastifyRequest<{ Params: { nimResource: string } }>) => { logRequestDetails(fastify, request); const { nimResource } = request.params; const { coreV1Api, namespace } = fastify.kube; - if (secretNames.includes(nimResource)) { - try { - return await coreV1Api.readNamespacedSecret(nimResource, namespace); - } catch (e) { - fastify.log.error(`Failed to fetch secret ${nimResource}: ${e.message}`); - throw createCustomError('Not found', 'Secret not found', 404); - } + // Fetch the Account CR to determine the actual resource name dynamically + const account = await getNIMAccount(fastify); + if (!account) { + throw createCustomError('Not found', 'NIM account not found', 404); + } + + const resourceInfo = resourceMap[nimResource]; + if (!resourceInfo) { + throw createCustomError('Not found', `Invalid resource type: ${nimResource}`, 404); + } + + const resourceName = get(account, resourceInfo.path); + if (!resourceName) { + fastify.log.error(`Resource name for '${nimResource}' not found in account CR.`); + throw createCustomError('Not found', `${nimResource} name not found in account`, 404); } - if (nimResource === configMapName) { - try { - return await coreV1Api.readNamespacedConfigMap(configMapName, namespace); - } catch (e) { - fastify.log.error(`Failed to fetch configMap ${nimResource}: ${e.message}`); - throw createCustomError('Not found', 'ConfigMap not found', 404); + try { + // Fetch the resource from Kubernetes using the dynamically retrieved name + if (resourceInfo.type === 'Secret') { + return await coreV1Api.readNamespacedSecret(resourceName, namespace); + } else { + return await coreV1Api.readNamespacedConfigMap(resourceName, namespace); } + } catch (e: any) { + fastify.log.error( + `Failed to fetch ${resourceInfo.type.toLowerCase()} ${resourceName}: ${e.message}`, + ); + throw createCustomError('Not found', `${resourceInfo.type} not found`, 404); } - throw createCustomError('Not found', 'Resource not found', 404); }, ); }; diff --git a/frontend/src/__mocks__/mockNimAccount.ts b/frontend/src/__mocks__/mockNimAccount.ts new file mode 100644 index 0000000000..3567d5b21d --- /dev/null +++ b/frontend/src/__mocks__/mockNimAccount.ts @@ -0,0 +1,43 @@ +import { K8sCondition, NIMAccountKind } from '~/k8sTypes'; + +type MockResourceConfigType = { + name?: string; + namespace?: string; + uid?: string; + apiKeySecretName?: string; + nimConfigName?: string; + runtimeTemplateName?: string; + nimPullSecretName?: string; + conditions?: K8sCondition[]; +}; + +export const mockNimAccount = ({ + name = 'odh-nim-account', + namespace = 'opendatahub', + uid = 'test-uid', + apiKeySecretName = 'mock-nvidia-nim-access', + nimConfigName = 'mock-nvidia-nim-images-data', + runtimeTemplateName = 'mock-nvidia-nim-serving-template', + nimPullSecretName = 'mock-nvidia-nim-image-pull', + conditions = [], +}: MockResourceConfigType): NIMAccountKind => ({ + apiVersion: 'nim.opendatahub.io/v1', + kind: 'Account', + metadata: { + name, + namespace, + uid, + creationTimestamp: new Date().toISOString(), + }, + spec: { + apiKeySecret: { + name: apiKeySecretName, + }, + }, + status: { + nimConfig: nimConfigName ? { name: nimConfigName } : undefined, + runtimeTemplate: runtimeTemplateName ? { name: runtimeTemplateName } : undefined, + nimPullSecret: nimPullSecretName ? { name: nimPullSecretName } : undefined, + conditions, + }, +}); diff --git a/frontend/src/__mocks__/mockNimResource.ts b/frontend/src/__mocks__/mockNimResource.ts index 39b6684141..9d2ea1d1f5 100644 --- a/frontend/src/__mocks__/mockNimResource.ts +++ b/frontend/src/__mocks__/mockNimResource.ts @@ -19,7 +19,7 @@ import { mockPVCK8sResource } from './mockPVCK8sResource'; export const mockNimImages = (): ConfigMapKind => mockConfigMap({ - name: 'nvidia-nim-images-data', + name: 'mock-nvidia-nim-images-data', namespace: 'opendatahub', data: { alphafold2: JSON.stringify({ @@ -86,7 +86,7 @@ export const mockNimServingRuntime = (): ServingRuntimeKind => { export const mockNimServingRuntimeTemplate = (): TemplateKind => { const templateMock = mockServingRuntimeTemplateK8sResource({ - name: 'nvidia-nim-serving-template', + name: 'mock-nvidia-nim-serving-template', displayName: 'NVIDIA NIM', platforms: [ServingRuntimePlatform.SINGLE], apiProtocol: ServingRuntimeAPIProtocol.REST, @@ -101,7 +101,7 @@ export const mockNimServingRuntimeTemplate = (): TemplateKind => { export const mockNvidiaNimAccessSecret = (): SecretKind => { const secret = mockSecretK8sResource({ - name: 'nvidia-nim-access', + name: 'mock-nvidia-nim-access', }); delete secret.data; secret.data = {}; @@ -113,7 +113,7 @@ export const mockNvidiaNimAccessSecret = (): SecretKind => { export const mockNvidiaNimImagePullSecret = (): SecretKind => { const secret = mockSecretK8sResource({ - name: 'nvidia-nim-image-pull', + name: 'mock-nvidia-nim-image-pull', }); delete secret.data; secret.data = {}; diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts index 2e5d2c4662..28ee725b29 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts @@ -10,16 +10,16 @@ import type { RegisteredModelList, } from '~/concepts/modelRegistry/types'; import type { + ConsoleLinkKind, DashboardConfigKind, DataScienceClusterInitializationKindStatus, DataScienceClusterKindStatus, + ModelRegistryKind, + NotebookKind, OdhQuickStart, RoleBindingKind, ServingRuntimeKind, TemplateKind, - NotebookKind, - ModelRegistryKind, - ConsoleLinkKind, } from '~/k8sTypes'; import type { StartNotebookData } from '~/pages/projects/types'; @@ -675,7 +675,7 @@ declare global { type: 'GET /api/nim-serving/:resource', options: { path: { - resource: 'nvidia-nim-images-data' | 'nvidia-nim-access' | 'nvidia-nim-image-pull'; + resource: string; }; }, response: OdhResponse, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts index a89fe0e42b..846ff45995 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts @@ -8,6 +8,7 @@ import { import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; import { + NIMAccountModel, ProjectModel, SecretModel, ServiceModel, @@ -30,6 +31,7 @@ import { import { ServingRuntimePlatform } from '~/types'; import { kserveModal } from '~/__tests__/cypress/cypress/pages/modelServing'; import { mockModelArtifact } from '~/__mocks__/mockModelArtifact'; +import { mockNimAccount } from '~/__mocks__/mockNimAccount'; const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; @@ -177,6 +179,8 @@ const initIntercepts = ({ { namespace: 'opendatahub' }, ), ); + + cy.interceptK8sList(NIMAccountModel, mockK8sResourceList([mockNimAccount({})])); }; describe('Deploy model version', () => { diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/projectDetails.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/projectDetails.cy.ts index 469814f2dd..3bdb8d3766 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/projectDetails.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/projectDetails.cy.ts @@ -22,22 +22,24 @@ import { ServingRuntimePlatform } from '~/types'; import { DataSciencePipelineApplicationModel, ImageStreamModel, + InferenceServiceModel, + NIMAccountModel, NotebookModel, - PVCModel, PodModel, ProjectModel, + PVCModel, RouteModel, SecretModel, ServiceAccountModel, - TemplateModel, - InferenceServiceModel, ServingRuntimeModel, + TemplateModel, } from '~/__tests__/cypress/cypress/utils/models'; import { mockServingRuntimeK8sResource } from '~/__mocks__/mockServingRuntimeK8sResource'; import { mockInferenceServiceK8sResource } from '~/__mocks__/mockInferenceServiceK8sResource'; import { asProjectAdminUser } from '~/__tests__/cypress/cypress/utils/mockUsers'; import { NamespaceApplicationCase } from '~/pages/projects/types'; import { mockNimServingRuntimeTemplate } from '~/__mocks__/mockNimResource'; +import { mockNimAccount } from '~/__mocks__/mockNimAccount'; type HandlersProps = { isEmpty?: boolean; @@ -271,6 +273,8 @@ const initIntercepts = ({ }, buildMockPipelines(isEmpty ? [] : [mockPipelineKF({})]), ); + + cy.interceptK8sList(NIMAccountModel, mockK8sResourceList([mockNimAccount({})])); }; describe('Project Details', () => { diff --git a/frontend/src/__tests__/cypress/cypress/utils/nimUtils.ts b/frontend/src/__tests__/cypress/cypress/utils/nimUtils.ts index e11cb1d7cb..7bf22a7c6c 100644 --- a/frontend/src/__tests__/cypress/cypress/utils/nimUtils.ts +++ b/frontend/src/__tests__/cypress/cypress/utils/nimUtils.ts @@ -11,6 +11,7 @@ import { AcceleratorProfileModel, ConfigMapModel, InferenceServiceModel, + NIMAccountModel, ProjectModel, PVCModel, SecretModel, @@ -30,6 +31,7 @@ import { } from '~/__mocks__/mockNimResource'; import { mockAcceleratorProfile } from '~/__mocks__/mockAcceleratorProfile'; import type { InferenceServiceKind } from '~/k8sTypes'; +import { mockNimAccount } from '~/__mocks__/mockNimAccount'; /* ################################################### ###### Interception Initialization Utilities ###### @@ -77,6 +79,8 @@ export const initInterceptsToEnableNim = ({ hasAllModels = false }: EnableNimCon total: { 'nvidia.com/gpu': 1 }, allocated: { 'nvidia.com/gpu': 1 }, }); + + cy.interceptK8sList(NIMAccountModel, mockK8sResourceList([mockNimAccount({})])); }; // intercept all APIs required for deploying new NIM models in existing projects @@ -89,23 +93,24 @@ export const initInterceptsToDeployModel = (nimInferenceService: InferenceServic cy.interceptOdh( `GET /api/nim-serving/:resource`, - { path: { resource: 'nvidia-nim-images-data' } }, + { path: { resource: 'nimConfig' } }, mockNimServingResource(mockNimImages()), ); cy.interceptOdh( `GET /api/nim-serving/:resource`, - { path: { resource: 'nvidia-nim-access' } }, + { path: { resource: 'apiKeySecret' } }, mockNimServingResource(mockNvidiaNimAccessSecret()), ); cy.interceptOdh( `GET /api/nim-serving/:resource`, - { path: { resource: 'nvidia-nim-image-pull' } }, + { path: { resource: 'nimPullSecret' } }, mockNimServingResource(mockNvidiaNimImagePullSecret()), ); cy.interceptK8s('POST', PVCModel, mockNimModelPVC()); + cy.interceptK8s('GET', NIMAccountModel, mockNimAccount({})); }; // intercept all APIs required for deleting an existing NIM models @@ -154,4 +159,6 @@ export const initInterceptorsValidatingNimEnablement = ( ProjectModel, mockK8sResourceList([mockProjectK8sResource({ hasAnnotations: true })]), ); + + cy.interceptK8sList(NIMAccountModel, mockK8sResourceList([mockNimAccount({})])); }; diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index ceee524c0f..40033d7a77 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -1,4 +1,5 @@ // Normal SDK/pass-through network API calls +export * from './k8s/nimAccounts'; export * from './k8s/builds'; export * from './k8s/configMaps'; export * from './k8s/events'; diff --git a/frontend/src/api/k8s/nimAccounts.ts b/frontend/src/api/k8s/nimAccounts.ts new file mode 100644 index 0000000000..f7446e5865 --- /dev/null +++ b/frontend/src/api/k8s/nimAccounts.ts @@ -0,0 +1,11 @@ +import { k8sListResource } from '@openshift/dynamic-plugin-sdk-utils'; +import { NIMAccountModel } from '~/api/models'; +import { NIMAccountKind } from '~/k8sTypes'; + +export const listNIMAccounts = async (namespace: string): Promise => + k8sListResource({ + model: NIMAccountModel, + queryOptions: { + ns: namespace, + }, + }).then((listResource) => listResource.items); diff --git a/frontend/src/api/models/odh.ts b/frontend/src/api/models/odh.ts index daf18aef99..7a596f7991 100644 --- a/frontend/src/api/models/odh.ts +++ b/frontend/src/api/models/odh.ts @@ -41,3 +41,10 @@ export const InferenceServiceModel: K8sModelCommon = { kind: 'InferenceService', plural: 'inferenceservices', }; + +export const NIMAccountModel: K8sModelCommon = { + apiVersion: 'v1', + apiGroup: 'nim.opendatahub.io', + kind: 'Account', + plural: 'accounts', +}; diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index e6e4cd51a8..a06d5d778b 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -1338,3 +1338,27 @@ export type ModelRegistryKind = K8sResourceCommon & { conditions?: K8sCondition[]; }; }; + +export type NIMAccountKind = K8sResourceCommon & { + metadata: { + name: string; + namespace: string; + }; + spec: { + apiKeySecret: { + name: string; + }; + }; + status?: { + nimConfig?: { + name: string; + }; + runtimeTemplate?: { + name: string; + }; + nimPullSecret?: { + name: string; + }; + conditions?: K8sCondition[]; + }; +}; diff --git a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx index 3de12e9e32..1e7724c97f 100644 --- a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx @@ -20,6 +20,7 @@ import { import { AccessReviewResourceAttributes, InferenceServiceKind, + PersistentVolumeClaimKind, ProjectKind, SecretKind, ServingRuntimeKind, @@ -51,6 +52,7 @@ import { useDashboardNamespace } from '~/redux/selectors'; import { getServingRuntimeFromTemplate } from '~/pages/modelServing/customServingRuntimes/utils'; import { useNIMPVC } from '~/pages/modelServing/screens/projects/NIMServiceModal/useNIMPVC'; import AuthServingRuntimeSection from '~/pages/modelServing/screens/projects/ServingRuntimeModal/AuthServingRuntimeSection'; +import { useNIMTemplateName } from '~/pages/modelServing/screens/projects/useNIMTemplateName'; const NIM_SECRET_NAME = 'nvidia-nim-secrets'; const NIM_NGC_SECRET_NAME = 'ngc-secret'; @@ -151,19 +153,22 @@ const ManageNIMServingModal: React.FC = ({ !baseInputValueValid; const { dashboardNamespace } = useDashboardNamespace(); + const templateName = useNIMTemplateName(); React.useEffect(() => { if (editInfo?.servingRuntimeEditInfo?.servingRuntime) { setServingRuntimeSelected(editInfo.servingRuntimeEditInfo.servingRuntime); } else { const fetchNIMServingRuntimeTemplate = async () => { - const nimTemplate = await getNIMServingRuntimeTemplate(dashboardNamespace); - setServingRuntimeSelected(getServingRuntimeFromTemplate(nimTemplate)); + if (templateName) { + const nimTemplate = await getNIMServingRuntimeTemplate(dashboardNamespace, templateName); + setServingRuntimeSelected(getServingRuntimeFromTemplate(nimTemplate)); + } }; fetchNIMServingRuntimeTemplate(); } - }, [dashboardNamespace, editInfo]); + }, [templateName, dashboardNamespace, editInfo]); const isSecretNeeded = async (ns: string, secretName: string): Promise => { try { @@ -242,23 +247,19 @@ const ManageNIMServingModal: React.FC = ({ submitInferenceServiceResource({ dryRun: true }), ]) .then(async () => { - const promises: Promise[] = [ + const promises: Promise[] = [ submitServingRuntimeResources({ dryRun: false }).then(() => undefined), submitInferenceServiceResource({ dryRun: false }).then(() => undefined), ]; if (!editInfo) { if (await isSecretNeeded(namespace, NIM_SECRET_NAME)) { - promises.push( - createNIMSecret(namespace, NIM_SECRET_NAME, false, false).then(() => undefined), - ); + promises.push(createNIMSecret(namespace, 'apiKeySecret', false, false)); } if (await isSecretNeeded(namespace, NIM_NGC_SECRET_NAME)) { - promises.push( - createNIMSecret(namespace, NIM_NGC_SECRET_NAME, true, false).then(() => undefined), - ); + promises.push(createNIMSecret(namespace, 'nimPullSecret', true, false)); } - promises.push(createNIMPVC(namespace, nimPVCName, pvcSize, false).then(() => undefined)); + promises.push(createNIMPVC(namespace, nimPVCName, pvcSize, false)); } else if (pvc && pvc.spec.resources.requests.storage !== pvcSize) { const updatePvcData = { size: pvcSize, // New size @@ -266,9 +267,7 @@ const ManageNIMServingModal: React.FC = ({ description: pvc.metadata.annotations?.description || '', storageClassName: pvc.spec.storageClassName, }; - promises.push( - updatePvc(updatePvcData, pvc, namespace, { dryRun: false }).then(() => undefined), - ); + promises.push(updatePvc(updatePvcData, pvc, namespace, { dryRun: false })); } return Promise.all(promises); }) diff --git a/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts b/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts index 536376cd74..9bc30322f5 100644 --- a/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts +++ b/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts @@ -12,26 +12,27 @@ import { import { LabeledDataConnection, ServingPlatformStatuses } from '~/pages/modelServing/screens/types'; import { ServingRuntimePlatform } from '~/types'; import { mockInferenceServiceK8sResource } from '~/__mocks__/mockInferenceServiceK8sResource'; -import { createPvc, createSecret } from '~/api'; +import { createPvc, createSecret, listNIMAccounts } from '~/api'; import { PersistentVolumeClaimKind, ServingRuntimeKind } from '~/k8sTypes'; import { - getNGCSecretType, + fetchNIMAccountTemplateName, getNIMData, getNIMResource, updateServingRuntimeTemplate, } from '~/pages/modelServing/screens/projects/nimUtils'; +import { mockNimAccount } from '~/__mocks__/mockNimAccount'; jest.mock('~/api', () => ({ getSecret: jest.fn(), createSecret: jest.fn(), createPvc: jest.fn(), getInferenceServiceContext: jest.fn(), + listNIMAccounts: jest.fn(), })); jest.mock('~/pages/modelServing/screens/projects/nimUtils', () => ({ ...jest.requireActual('~/pages/modelServing/screens/projects/nimUtils'), getNIMData: jest.fn(), - getNGCSecretType: jest.fn(), getNIMResource: jest.fn(), })); @@ -237,14 +238,13 @@ describe('getCreateInferenceServiceLabels', () => { describe('createNIMSecret', () => { const projectName = 'test-project'; - const secretName = 'test-secret'; const dryRun = false; const nimSecretMock = { apiVersion: 'v1', kind: 'Secret', metadata: { - name: secretName, + name: 'ngc-secret', namespace: projectName, }, data: {}, @@ -260,30 +260,24 @@ describe('createNIMSecret', () => { beforeEach(() => { jest.clearAllMocks(); - (getNGCSecretType as jest.Mock).mockImplementation((isNGC: boolean) => - isNGC ? 'kubernetes.io/dockerconfigjson' : 'Opaque', - ); }); it('should create NGC secret when isNGC is true', async () => { (getNIMData as jest.Mock).mockResolvedValueOnce(nimSecretDataNGC); (createSecret as jest.Mock).mockResolvedValueOnce(nimSecretMock); - const result = await createNIMSecret(projectName, secretName, true, dryRun); + const result = await createNIMSecret(projectName, 'ngc-secret', true, dryRun); - expect(getNIMData).toHaveBeenCalledWith(true); - expect(getNGCSecretType).toHaveBeenCalledWith(true); + expect(getNIMData).toHaveBeenCalledWith('ngc-secret', true); expect(createSecret).toHaveBeenCalledWith( { apiVersion: 'v1', kind: 'Secret', metadata: { - name: secretName, + name: 'ngc-secret', namespace: projectName, }, - data: { - '.dockerconfigjson': 'mocked-dockerconfig-json', - }, + data: nimSecretDataNGC, type: 'kubernetes.io/dockerconfigjson', }, { dryRun }, @@ -295,21 +289,18 @@ describe('createNIMSecret', () => { (getNIMData as jest.Mock).mockResolvedValueOnce(nimSecretDataNonNGC); (createSecret as jest.Mock).mockResolvedValueOnce(nimSecretMock); - const result = await createNIMSecret(projectName, secretName, false, dryRun); + const result = await createNIMSecret(projectName, 'nvidia-nim-secrets', false, dryRun); - expect(getNIMData).toHaveBeenCalledWith(false); - expect(getNGCSecretType).toHaveBeenCalledWith(false); + expect(getNIMData).toHaveBeenCalledWith('nvidia-nim-secrets', false); expect(createSecret).toHaveBeenCalledWith( { apiVersion: 'v1', kind: 'Secret', metadata: { - name: secretName, + name: 'nvidia-nim-secrets', namespace: projectName, }, - data: { - NGC_API_KEY: 'mocked-api-key', - }, + data: nimSecretDataNonNGC, type: 'Opaque', }, { dryRun }, @@ -320,14 +311,25 @@ describe('createNIMSecret', () => { it('should reject if getNIMData throws an error', async () => { (getNIMData as jest.Mock).mockRejectedValueOnce(new Error('Error retrieving secret data')); - await expect(createNIMSecret(projectName, secretName, true, dryRun)).rejects.toThrow( - 'Error creating NIM NGC secret', + await expect(createNIMSecret(projectName, 'ngc-secret', true, dryRun)).rejects.toThrow( + 'Error creating NGC secret', + ); + }); + + it('should reject if createSecret throws an error', async () => { + (getNIMData as jest.Mock).mockResolvedValueOnce(nimSecretDataNonNGC); + (createSecret as jest.Mock).mockRejectedValueOnce(new Error('Error creating secret')); + + await expect(createNIMSecret(projectName, 'nvidia-nim-secrets', false, dryRun)).rejects.toThrow( + 'Error creating NIM secret', ); }); }); -describe('fetchNIMModelNames', () => { - const NIM_CONFIGMAP_NAME = 'nvidia-nim-images-data'; +describe('fetchNIMModelNames', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); const configMapMock = { data: { model1: JSON.stringify({ @@ -349,16 +351,12 @@ describe('fetchNIMModelNames', () => { }, }; - beforeEach(() => { - jest.clearAllMocks(); - }); - it('should return model infos when configMap has data', async () => { (getNIMResource as jest.Mock).mockResolvedValueOnce(configMapMock); const result = await fetchNIMModelNames(); - expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME); + expect(getNIMResource).toHaveBeenCalledWith('nimConfig'); expect(result).toEqual([ { name: 'model1', @@ -386,7 +384,7 @@ describe('fetchNIMModelNames', () => { const result = await fetchNIMModelNames(); - expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME); + expect(getNIMResource).toHaveBeenCalledWith('nimConfig'); expect(result).toBeUndefined(); }); @@ -395,7 +393,7 @@ describe('fetchNIMModelNames', () => { const result = await fetchNIMModelNames(); - expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME); + expect(getNIMResource).toHaveBeenCalledWith('nimConfig'); expect(result).toBeUndefined(); }); }); @@ -540,3 +538,46 @@ describe('updateServingRuntimeTemplate', () => { expect(result.spec.containers[0].volumeMounts).toBeUndefined(); }); }); + +describe('fetchNIMAccountTemplateName', () => { + const dashboardNamespace = 'test-namespace'; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return the template name when available', async () => { + const mockAccount = mockNimAccount({ runtimeTemplateName: 'test-template' }); + (listNIMAccounts as jest.Mock).mockResolvedValueOnce([mockAccount]); + + const result = await fetchNIMAccountTemplateName(dashboardNamespace); + + expect(result).toBe('test-template'); + expect(listNIMAccounts).toHaveBeenCalledWith(dashboardNamespace); + }); + + it('should return undefined when no accounts exist', async () => { + (listNIMAccounts as jest.Mock).mockResolvedValueOnce([]); + + const result = await fetchNIMAccountTemplateName(dashboardNamespace); + + expect(result).toBeUndefined(); + expect(listNIMAccounts).toHaveBeenCalledWith(dashboardNamespace); + }); + + it('should handle errors from listNIMAccounts', async () => { + const mockError = new Error('Server Error'); + (listNIMAccounts as jest.Mock).mockRejectedValue(mockError); + + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + const result = await fetchNIMAccountTemplateName(dashboardNamespace); + + expect(result).toBeUndefined(); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `Error fetching NIM account template name: ${mockError.message}`, + ); + + consoleErrorSpy.mockRestore(); + }); +}); diff --git a/frontend/src/pages/modelServing/screens/projects/nimUtils.ts b/frontend/src/pages/modelServing/screens/projects/nimUtils.ts index 72027577fc..f1c1f742fd 100644 --- a/frontend/src/pages/modelServing/screens/projects/nimUtils.ts +++ b/frontend/src/pages/modelServing/screens/projects/nimUtils.ts @@ -2,21 +2,17 @@ import { K8sResourceCommon } from '@openshift/dynamic-plugin-sdk-utils'; import { ProjectKind, SecretKind, ServingRuntimeKind, TemplateKind } from '~/k8sTypes'; -import { deletePvc, deleteSecret, getTemplate } from '~/api'; +import { deletePvc, deleteSecret, getTemplate, listNIMAccounts } from '~/api'; import { fetchInferenceServiceCount } from '~/pages/modelServing/screens/projects/utils'; -const NIM_SECRET_NAME = 'nvidia-nim-access'; -const NIM_NGC_SECRET_NAME = 'nvidia-nim-image-pull'; -const TEMPLATE_NAME = 'nvidia-nim-serving-template'; - export const getNGCSecretType = (isNGC: boolean): string => isNGC ? 'kubernetes.io/dockerconfigjson' : 'Opaque'; export const getNIMResource = async ( - resourceName: string, + resourceRef: string, ): Promise => { try { - const response = await fetch(`/api/nim-serving/${resourceName}`, { + const response = await fetch(`/api/nim-serving/${resourceRef}`, { method: 'GET', headers: { 'Content-Type': 'application/json', @@ -29,16 +25,18 @@ export const getNIMResource = async ( const resourceData = await response.json(); return resourceData.body; } catch (error) { - throw new Error(`Failed to fetch the resource: ${resourceName}.`); + throw new Error(`Failed to fetch the resource: ${resourceRef}.`); } }; -export const getNIMData = async (isNGC: boolean): Promise | undefined> => { - const nimSecretData: SecretKind = isNGC - ? await getNIMResource(NIM_NGC_SECRET_NAME) - : await getNIMResource(NIM_SECRET_NAME); + +export const getNIMData = async ( + secretKey: string, + isNGC: boolean, +): Promise | undefined> => { + const nimSecretData = await getNIMResource(secretKey); if (!nimSecretData.data) { - throw new Error(`Error retrieving NIM ${isNGC ? 'NGC' : ''} secret data`); + throw new Error(`Error retrieving ${isNGC ? 'NGC' : 'NIM'} secret data`); } const data: Record = {}; @@ -62,7 +60,13 @@ export const isNIMServingRuntimeTemplateAvailable = async ( dashboardNamespace: string, ): Promise => { try { - await getTemplate(TEMPLATE_NAME, dashboardNamespace); + const templateName = await fetchNIMAccountTemplateName(dashboardNamespace); + if (!templateName) { + // eslint-disable-next-line no-console + console.error('No NIM account template available.'); + return false; + } + await getTemplate(templateName, dashboardNamespace); return true; } catch (error) { return false; @@ -71,9 +75,10 @@ export const isNIMServingRuntimeTemplateAvailable = async ( export const getNIMServingRuntimeTemplate = async ( dashboardNamespace: string, + templateName: string, ): Promise => { try { - const template = await getTemplate(TEMPLATE_NAME, dashboardNamespace); + const template = await getTemplate(templateName, dashboardNamespace); return template; } catch (error) { return undefined; @@ -183,3 +188,30 @@ export const getNIMResourcesToDelete = async ( return resourcesToDelete; }; + +export const fetchNIMAccountTemplateName = async ( + dashboardNamespace: string, +): Promise => { + try { + const accounts = await listNIMAccounts(dashboardNamespace); + if (accounts.length === 0) { + throw new Error('NIM account does not exist.'); + } + + const nimAccount = accounts[0]; + if (!nimAccount.status?.runtimeTemplate?.name) { + throw new Error('Failed to retrieve the NIM account template name.'); + } + + return nimAccount.status.runtimeTemplate.name; + } catch (e) { + if (e instanceof Error) { + // eslint-disable-next-line no-console + console.error(`Error fetching NIM account template name: ${e.message}`); + } else { + // eslint-disable-next-line no-console + console.error(`Error fetching NIM account template name: ${String(e)}`); + } + return undefined; + } +}; diff --git a/frontend/src/pages/modelServing/screens/projects/useNIMTemplateName.ts b/frontend/src/pages/modelServing/screens/projects/useNIMTemplateName.ts new file mode 100644 index 0000000000..36009dbcaf --- /dev/null +++ b/frontend/src/pages/modelServing/screens/projects/useNIMTemplateName.ts @@ -0,0 +1,19 @@ +import * as React from 'react'; +import { fetchNIMAccountTemplateName } from '~/pages/modelServing/screens/projects/nimUtils'; +import { useDashboardNamespace } from '~/redux/selectors'; + +export const useNIMTemplateName = (): string | undefined => { + const { dashboardNamespace } = useDashboardNamespace(); + const [templateName, setTemplateName] = React.useState(); + + React.useEffect(() => { + const fetchTemplateName = async () => { + const template = await fetchNIMAccountTemplateName(dashboardNamespace); + setTemplateName(template); + }; + + fetchTemplateName(); + }, [dashboardNamespace]); + + return templateName; +}; diff --git a/frontend/src/pages/modelServing/screens/projects/utils.ts b/frontend/src/pages/modelServing/screens/projects/utils.ts index b97db12951..899430288b 100644 --- a/frontend/src/pages/modelServing/screens/projects/utils.ts +++ b/frontend/src/pages/modelServing/screens/projects/utils.ts @@ -48,16 +48,10 @@ import { import { isDataConnectionAWS } from '~/pages/projects/screens/detail/data-connections/utils'; import { containsOnlySlashes, isS3PathValid, removeLeadingSlash } from '~/utilities/string'; import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; -import { - getNGCSecretType, - getNIMData, - getNIMResource, -} from '~/pages/modelServing/screens/projects/nimUtils'; +import { getNIMData, getNIMResource } from '~/pages/modelServing/screens/projects/nimUtils'; import { AcceleratorProfileFormData } from '~/utilities/useAcceleratorProfileFormState'; import { Connection } from '~/concepts/connectionTypes/types'; -const NIM_CONFIGMAP_NAME = 'nvidia-nim-images-data'; - export const getServingRuntimeSizes = (config: DashboardConfigKind): ModelServingSize[] => { let sizes = config.spec.modelServerSizes || []; if (sizes.length === 0) { @@ -641,7 +635,7 @@ export interface ModelInfo { } export const fetchNIMModelNames = async (): Promise => { - const configMap = await getNIMResource(NIM_CONFIGMAP_NAME); + const configMap = await getNIMResource('nimConfig'); if (configMap.data && Object.keys(configMap.data).length > 0) { const modelInfos: ModelInfo[] = []; for (const [key, value] of Object.entries(configMap.data)) { @@ -668,26 +662,27 @@ export const fetchNIMModelNames = async (): Promise => export const createNIMSecret = async ( projectName: string, - secretName: string, + secretKey: string, isNGC: boolean, dryRun: boolean, ): Promise => { try { - const data = await getNIMData(isNGC); + const data = await getNIMData(secretKey, isNGC); const newSecret = { apiVersion: 'v1', kind: 'Secret', metadata: { - name: secretName, + name: isNGC ? 'ngc-secret' : 'nvidia-nim-secrets', namespace: projectName, }, data, - type: getNGCSecretType(isNGC), + type: isNGC ? 'kubernetes.io/dockerconfigjson' : 'Opaque', }; + return await createSecret(newSecret, { dryRun }); } catch (e) { - return Promise.reject(new Error(`Error creating NIM ${isNGC ? 'NGC' : ''} secret`)); + return Promise.reject(new Error(`Error creating ${isNGC ? 'NGC' : 'NIM'} secret`)); } }; diff --git a/manifests/core-bases/base/fetch-nim-account.rbac.yaml b/manifests/core-bases/base/fetch-nim-account.rbac.yaml new file mode 100644 index 0000000000..86670d2a3d --- /dev/null +++ b/manifests/core-bases/base/fetch-nim-account.rbac.yaml @@ -0,0 +1,26 @@ +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: nim-account-access +rules: + - apiGroups: + - nim.opendatahub.io + verbs: + - get + - list + - watch + resources: + - accounts +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: nim-account-access-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: nim-account-access +subjects: + - apiGroup: rbac.authorization.k8s.io + kind: Group + name: system:authenticated diff --git a/manifests/core-bases/base/kustomization.yaml b/manifests/core-bases/base/kustomization.yaml index 259790261b..a7e4e449fd 100644 --- a/manifests/core-bases/base/kustomization.yaml +++ b/manifests/core-bases/base/kustomization.yaml @@ -18,6 +18,7 @@ resources: - model-serving-role-binding.yaml - fetch-accelerators.rbac.yaml - fetch-hardwares.rbac.yaml + - fetch-nim-account.rbac.yaml images: - name: oauth-proxy newName: registry.redhat.io/openshift4/ose-oauth-proxy