feat: added ability to deploy more than one NIM model. (#3453)

* feat: added ability to deploy more than one NIM model. Signed-off-by: Olga Lavtar <[email protected]> * feat: refactored NIM related logic to nimUtils Signed-off-by: Olga Lavtar <[email protected]> * feat: changes to the error handling Signed-off-by: Olga Lavtar <[email protected]> * feat: changes to the error handling Signed-off-by: Olga Lavtar <[email protected]> * feat: changes .some for .forEach Signed-off-by: Olga Lavtar <[email protected]> * feat: deleting secrets fix Signed-off-by: Olga Lavtar <[email protected]> * feat: deploying the same model pvc fix Signed-off-by: Olga Lavtar <[email protected]> * feat: added a unit test for getNIMResourcesToDelete Signed-off-by: Olga Lavtar <[email protected]> * feat: will add a unit test for getNIMResourcesToDelete later Signed-off-by: Olga Lavtar <[email protected]> --------- Signed-off-by: Olga Lavtar <[email protected]>
opendatahub-io · Nov 13, 2024 · 01165de · 01165de
1 parent 6b06634
commit 01165de
Show file tree

Hide file tree

Showing 7 changed files with 189 additions and 83 deletions.
diff --git a/frontend/src/api/k8s/__tests__/pvcs.spec.ts b/frontend/src/api/k8s/__tests__/pvcs.spec.ts
@@ -1,13 +1,21 @@
 import {
-  K8sStatus,
   k8sCreateResource,
   k8sDeleteResource,
+  k8sGetResource,
   k8sListResourceItems,
+  K8sStatus,
   k8sUpdateResource,
 } from '@openshift/dynamic-plugin-sdk-utils';
 import { mock200Status, mock404Error } from '~/__mocks__/mockK8sStatus';
 import { mockPVCK8sResource } from '~/__mocks__/mockPVCK8sResource';
-import { assemblePvc, createPvc, deletePvc, getDashboardPvcs, updatePvc } from '~/api/k8s/pvcs';
+import {
+  assemblePvc,
+  createPvc,
+  deletePvc,
+  getDashboardPvcs,
+  getPvc,
+  updatePvc,
+} from '~/api/k8s/pvcs';
 import { PVCModel } from '~/api/models/k8s';
 import { PersistentVolumeClaimKind } from '~/k8sTypes';
 import { CreatingStorageObject } from '~/pages/projects/types';
@@ -25,6 +33,7 @@ const k8sListResourceItemsMock = jest.mocked(k8sListResourceItems<PersistentVolu
 const k8sCreateResourceMock = jest.mocked(k8sCreateResource<PersistentVolumeClaimKind>);
 const k8sUpdateResourceMock = jest.mocked(k8sUpdateResource<PersistentVolumeClaimKind>);
 const k8sDeleteResourceMock = jest.mocked(k8sDeleteResource<PersistentVolumeClaimKind, K8sStatus>);
+const k8sGetResourceMock = jest.mocked(k8sGetResource<PersistentVolumeClaimKind>);
 
 const data: CreatingStorageObject = {
   nameDesc: {
@@ -193,3 +202,30 @@ describe('deletePvc', () => {
     });
   });
 });
+
+describe('getPvc', () => {
+  it('should fetch and return PVC', async () => {
+    k8sGetResourceMock.mockResolvedValue(pvcMock);
+    const result = await getPvc('projectName', 'pvcName');
+
+    expect(k8sGetResourceMock).toHaveBeenCalledWith({
+      fetchOptions: { requestInit: {} },
+      model: PVCModel,
+      queryOptions: { name: 'pvcName', ns: 'projectName', queryParams: {} },
+    });
+    expect(k8sGetResourceMock).toHaveBeenCalledTimes(1);
+    expect(result).toStrictEqual(pvcMock);
+  });
+
+  it('should handle errors and rethrow', async () => {
+    k8sGetResourceMock.mockRejectedValue(new Error('error1'));
+
+    await expect(getPvc('projectName', 'pvcName')).rejects.toThrow('error1');
+    expect(k8sGetResourceMock).toHaveBeenCalledTimes(1);
+    expect(k8sGetResourceMock).toHaveBeenCalledWith({
+      fetchOptions: { requestInit: {} },
+      model: PVCModel,
+      queryOptions: { name: 'pvcName', ns: 'projectName', queryParams: {} },
+    });
+  });
+});
diff --git a/frontend/src/pages/modelServing/screens/global/DeleteInferenceServiceModal.tsx b/frontend/src/pages/modelServing/screens/global/DeleteInferenceServiceModal.tsx
@@ -1,10 +1,13 @@
 import * as React from 'react';
 import DeleteModal from '~/pages/projects/components/DeleteModal';
 import { InferenceServiceKind, ServingRuntimeKind } from '~/k8sTypes';
-import { deleteInferenceService, deletePvc, deleteSecret, deleteServingRuntime } from '~/api';
+import { deleteInferenceService, deleteServingRuntime } from '~/api';
 import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
 import { byName, ProjectsContext } from '~/concepts/projects/ProjectsContext';
-import { isProjectNIMSupported } from '~/pages/modelServing/screens/projects/nimUtils';
+import {
+  getNIMResourcesToDelete,
+  isProjectNIMSupported,
+} from '~/pages/modelServing/screens/projects/nimUtils';
 
 type DeleteInferenceServiceModalProps = {
   inferenceService?: InferenceServiceKind;
@@ -34,62 +37,43 @@ const DeleteInferenceServiceModal: React.FC<DeleteInferenceServiceModalProps> =
     ? getDisplayNameFromK8sResource(inferenceService)
     : 'this deployed model';
 
+  const onDelete = async () => {
+    if (!inferenceService) {
+      return;
+    }
+
+    setIsDeleting(true);
+    try {
+      const nimResourcesToDelete =
+        isKServeNIMEnabled && project && servingRuntime
+          ? await getNIMResourcesToDelete(project.metadata.name, servingRuntime)
+          : [];
+
+      await Promise.all([
+        deleteInferenceService(inferenceService.metadata.name, inferenceService.metadata.namespace),
+        ...(servingRuntime
+          ? [deleteServingRuntime(servingRuntime.metadata.name, servingRuntime.metadata.namespace)]
+          : []),
+        ...nimResourcesToDelete,
+      ]);
+
+      onBeforeClose(true);
+    } catch (e: unknown) {
+      if (e instanceof Error) {
+        setError(e);
+      } else {
+        setError(new Error('An unknown error occurred'));
+      }
+      setIsDeleting(false);
+    }
+  };
+
   return (
     <DeleteModal
       title="Delete deployed model?"
       onClose={() => onBeforeClose(false)}
       submitButtonLabel="Delete deployed model"
-      onDelete={() => {
-        if (inferenceService) {
-          setIsDeleting(true);
-          const pvcName = servingRuntime?.spec.volumes?.find(
-            (vol) => vol.persistentVolumeClaim?.claimName,
-          )?.persistentVolumeClaim?.claimName;
-          const containerWithEnv = servingRuntime?.spec.containers.find(
-            (container) =>
-              container.env && container.env.some((env) => env.valueFrom?.secretKeyRef?.name),
-          );
-          const nimSecretName = containerWithEnv?.env?.find(
-            (env) => env.valueFrom?.secretKeyRef?.name,
-          )?.valueFrom?.secretKeyRef?.name;
-          const imagePullSecretName = servingRuntime?.spec.imagePullSecrets?.[0]?.name ?? '';
-          Promise.all([
-            deleteInferenceService(
-              inferenceService.metadata.name,
-              inferenceService.metadata.namespace,
-            ),
-            ...(servingRuntime
-              ? [
-                  deleteServingRuntime(
-                    servingRuntime.metadata.name,
-                    servingRuntime.metadata.namespace,
-                  ),
-                ]
-              : []),
-            ...(isKServeNIMEnabled && pvcName
-              ? [deletePvc(pvcName, inferenceService.metadata.namespace)]
-              : []),
-            ...(isKServeNIMEnabled &&
-            project &&
-            nimSecretName &&
-            nimSecretName.length > 0 &&
-            imagePullSecretName.length > 0
-              ? [
-                  deleteSecret(project.metadata.name, nimSecretName),
-                  deleteSecret(project.metadata.name, imagePullSecretName),
-                ]
-              : []),
-          ])
-
-            .then(() => {
-              onBeforeClose(true);
-            })
-            .catch((e) => {
-              setError(e);
-              setIsDeleting(false);
-            });
-        }
-      }}
+      onDelete={onDelete}
       deleting={isDeleting}
       error={error}
       deleteName={displayName}
@@ -98,5 +82,4 @@ const DeleteInferenceServiceModal: React.FC<DeleteInferenceServiceModalProps> =
     </DeleteModal>
   );
 };
-
 export default DeleteInferenceServiceModal;
diff --git a/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx b/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx
@@ -222,23 +222,19 @@ const ModelServingPlatform: React.FC = () => {
           shouldShowPlatformSelection || platformError || emptyModelServer
             ? undefined
             : [
-                ...(!isKServeNIMEnabled
-                  ? [
-                      <ModelServingPlatformButtonAction
-                        isProjectModelMesh={isProjectModelMesh}
-                        testId={`${isProjectModelMesh ? 'add-server' : 'deploy'}-button`}
-                        emptyTemplates={emptyTemplates}
-                        onClick={() => {
-                          setPlatformSelected(
-                            isProjectModelMesh
-                              ? ServingRuntimePlatform.MULTI
-                              : ServingRuntimePlatform.SINGLE,
-                          );
-                        }}
-                        key="serving-runtime-actions"
-                      />,
-                    ]
-                  : []),
+                <ModelServingPlatformButtonAction
+                  isProjectModelMesh={isProjectModelMesh}
+                  testId={`${isProjectModelMesh ? 'add-server' : 'deploy'}-button`}
+                  emptyTemplates={emptyTemplates}
+                  onClick={() => {
+                    setPlatformSelected(
+                      isProjectModelMesh
+                        ? ServingRuntimePlatform.MULTI
+                        : ServingRuntimePlatform.SINGLE,
+                    );
+                  }}
+                  key="serving-runtime-actions"
+                />,
               ]
         }
         description={

diff --git a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx
@@ -43,7 +43,7 @@ import {
   translateDisplayNameForK8s,
   translateDisplayNameForK8sAndReport,
 } from '~/concepts/k8s/utils';
-import { updatePvc, useAccessReview } from '~/api';
+import { getSecret, updatePvc, useAccessReview } from '~/api';
 import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas';
 import KServeAutoscalerReplicaSection from '~/pages/modelServing/screens/projects/kServeModal/KServeAutoscalerReplicaSection';
 import NIMPVCSizeSection from '~/pages/modelServing/screens/projects/NIMServiceModal/NIMPVCSizeSection';
@@ -169,6 +169,15 @@ const ManageNIMServingModal: React.FC<ManageNIMServingModalProps> = ({
     }
   }, [dashboardNamespace, editInfo]);
 
+  const isSecretNeeded = async (ns: string, secretName: string): Promise<boolean> => {
+    try {
+      await getSecret(ns, secretName);
+      return false; // Secret exists, no need to create
+    } catch {
+      return true; // Secret does not exist, needs to be created
+    }
+  };
+
   const onBeforeClose = (submitted: boolean) => {
     onClose(submitted);
     setError(undefined);
@@ -239,14 +248,21 @@ const ManageNIMServingModal: React.FC<ManageNIMServingModalProps> = ({
           submitServingRuntimeResources({ dryRun: false }).then(() => undefined),
           submitInferenceServiceResource({ dryRun: false }).then(() => undefined),
         ];
+
         if (!editInfo) {
-          promises.push(
-            createNIMSecret(namespace, NIM_SECRET_NAME, false, false).then(() => undefined),
-            createNIMSecret(namespace, NIM_NGC_SECRET_NAME, true, false).then(() => undefined),
-            createNIMPVC(namespace, nimPVCName, pvcSize, false).then(() => undefined),
-          );
+          if (await isSecretNeeded(namespace, NIM_SECRET_NAME)) {
+            promises.push(
+              createNIMSecret(namespace, NIM_SECRET_NAME, false, false).then(() => undefined),
+            );
+          }
+          if (await isSecretNeeded(namespace, NIM_NGC_SECRET_NAME)) {
+            promises.push(
+              createNIMSecret(namespace, NIM_NGC_SECRET_NAME, true, false).then(() => undefined),
+            );
+          }
+          promises.push(createNIMPVC(namespace, nimPVCName, pvcSize, false).then(() => undefined));
         } else if (pvc && pvc.spec.resources.requests.storage !== pvcSize) {
-          const createData: CreatingStorageObject = {
+          const updatePvcData: CreatingStorageObject = {
             size: pvcSize, // New size
             nameDesc: {
               name: pvc.metadata.name,
@@ -255,7 +271,7 @@ const ManageNIMServingModal: React.FC<ManageNIMServingModalProps> = ({
             storageClassName: pvc.spec.storageClassName,
           };
           promises.push(
-            updatePvc(createData, pvc, namespace, { dryRun: false }).then(() => undefined),
+            updatePvc(updatePvcData, pvc, namespace, { dryRun: false }).then(() => undefined),
           );
         }
         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
@@ -25,6 +25,7 @@ jest.mock('~/api', () => ({
   getSecret: jest.fn(),
   createSecret: jest.fn(),
   createPvc: jest.fn(),
+  getInferenceServiceContext: jest.fn(),
 }));
 
 jest.mock('~/pages/modelServing/screens/projects/nimUtils', () => ({

diff --git a/frontend/src/pages/modelServing/screens/projects/nimUtils.ts b/frontend/src/pages/modelServing/screens/projects/nimUtils.ts
@@ -2,7 +2,8 @@
 
 import { K8sResourceCommon } from '@openshift/dynamic-plugin-sdk-utils';
 import { ProjectKind, SecretKind, ServingRuntimeKind, TemplateKind } from '~/k8sTypes';
-import { getTemplate } from '~/api';
+import { deletePvc, deleteSecret, getTemplate } 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';
@@ -107,7 +108,7 @@ export const updateServingRuntimeTemplate = (
 
   if (updatedServingRuntime.spec.volumes) {
     const updatedVolumes = updatedServingRuntime.spec.volumes.map((volume) => {
-      if (volume.name === 'nim-pvc') {
+      if (volume.name.startsWith('nim-pvc')) {
         return {
           ...volume,
           name: pvcName,
@@ -123,3 +124,62 @@ export const updateServingRuntimeTemplate = (
   }
   return updatedServingRuntime;
 };
+
+export const getNIMResourcesToDelete = async (
+  projectName: string,
+  servingRuntime: ServingRuntimeKind,
+): Promise<Promise<void>[]> => {
+  const resourcesToDelete: Promise<void>[] = [];
+
+  let inferenceCount = 0;
+
+  try {
+    inferenceCount = await fetchInferenceServiceCount(projectName);
+  } catch (error) {
+    if (error instanceof Error) {
+      // eslint-disable-next-line no-console
+      console.error(
+        `Failed to fetch inference service count for project "${projectName}": ${error.message}`,
+      );
+    } else {
+      // eslint-disable-next-line no-console
+      console.error(
+        `Failed to fetch inference service count for project "${projectName}": ${error}`,
+      );
+    }
+  }
+
+  const pvcName = servingRuntime.spec.volumes?.find((vol) =>
+    vol.persistentVolumeClaim?.claimName.startsWith('nim-pvc'),
+  )?.persistentVolumeClaim?.claimName;
+
+  if (pvcName) {
+    resourcesToDelete.push(deletePvc(pvcName, projectName).then(() => undefined));
+  }
+
+  let nimSecretName: string | undefined;
+  let imagePullSecretName: string | undefined;
+
+  const pullNGCSecret = servingRuntime.spec.imagePullSecrets?.[0]?.name ?? '';
+  if (pullNGCSecret === 'ngc-secret') {
+    imagePullSecretName = pullNGCSecret;
+  }
+
+  servingRuntime.spec.containers.forEach((container) => {
+    container.env?.forEach((env) => {
+      const secretName = env.valueFrom?.secretKeyRef?.name;
+      if (secretName === 'nvidia-nim-secrets') {
+        nimSecretName = secretName;
+      }
+    });
+  });
+
+  if (nimSecretName && imagePullSecretName && inferenceCount === 1) {
+    resourcesToDelete.push(
+      deleteSecret(projectName, nimSecretName).then(() => undefined),
+      deleteSecret(projectName, imagePullSecretName).then(() => undefined),
+    );
+  }
+
+  return resourcesToDelete;
+};
diff --git a/frontend/src/pages/modelServing/screens/projects/utils.ts b/frontend/src/pages/modelServing/screens/projects/utils.ts
@@ -41,6 +41,7 @@ import {
   createPvc,
   createSecret,
   createServingRuntime,
+  getInferenceServiceContext,
   updateInferenceService,
   updateServingRuntime,
 } from '~/api';
@@ -709,3 +710,16 @@ export const getCreateInferenceServiceLabels = (
 
 export const isConnectionPathValid = (path: string): boolean =>
   !(containsOnlySlashes(path) || !isS3PathValid(path) || path === '');
+
+export const fetchInferenceServiceCount = async (namespace: string): Promise<number> => {
+  try {
+    const inferenceServices = await getInferenceServiceContext(namespace);
+    return inferenceServices.length;
+  } catch (error) {
+    throw new Error(
+      `Failed to fetch inference services for namespace "${namespace}": ${
+        error instanceof Error ? error.message : error
+      }`,
+    );
+  }
+};