Skip to content

Commit

Permalink
fix: retrieve ConfigMap using service account and added ${uid} to PVC…
Browse files Browse the repository at this point in the history 
… name to avoid waiting for resource termination after deletion

Signed-off-by: Olga Lavtar <olavtar@redhat.com>
  • Loading branch information
@olavtar
olavtar committed Oct 1, 2024
1 parent fac3687 commit bc3c9ac
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 14 deletions.
45 changes: 43 additions & 2 deletions frontend/src/api/k8s/servingRuntimes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const assembleServingRuntime = (
initialAcceleratorProfile?: AcceleratorProfileState,
selectedAcceleratorProfile?: AcceleratorProfileSelectFieldState,
isModelMesh?: boolean,
nimPVCName?: string,
): ServingRuntimeKind => {
const {
name: displayName,
Expand Down Expand Up @@ -133,6 +134,15 @@ export const assembleServingRuntime = (
if (!volumeMounts.find((volumeMount) => volumeMount.mountPath === '/dev/shm')) {
volumeMounts.push(getshmVolumeMount());
}
const updatedVolumeMounts = volumeMounts.map((volumeMount) => {
if (volumeMount.name === 'nim-pvc' && nimPVCName) {
return {
...volumeMount,
name: nimPVCName,
};
}
return volumeMount;
});

const updatedContainer = {
...container,
Expand All @@ -145,7 +155,7 @@ export const assembleServingRuntime = (
...containerWithoutResources,
...(isModelMesh ? { resources } : {}),
affinity,
volumeMounts,
volumeMounts: updatedVolumeMounts,
};
},
);
Expand All @@ -171,8 +181,33 @@ export const assembleServingRuntime = (
volumes.push(getshmVolume('2Gi'));
}

updatedServingRuntime.spec.volumes = volumes;
if (nimPVCName) {
const updatedVolumes = volumes.map((volume) => {
if (volume.name === 'nim-pvc') {
return {

Check warning on line 187 in frontend/src/api/k8s/servingRuntimes.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/api/k8s/servingRuntimes.ts#L186-L187 

Added lines #L186 - L187 were not covered by tests
...volume,
name: nimPVCName,
persistentVolumeClaim: {
claimName: nimPVCName,
},
};
}
return volume;

Check warning on line 195 in frontend/src/api/k8s/servingRuntimes.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/api/k8s/servingRuntimes.ts#L195 

Added line #L195 was not covered by tests
});

if (!updatedVolumes.find((volume) => volume.name === nimPVCName)) {
updatedVolumes.push({

Check warning on line 199 in frontend/src/api/k8s/servingRuntimes.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/api/k8s/servingRuntimes.ts#L199 

Added line #L199 was not covered by tests
name: nimPVCName,
persistentVolumeClaim: {
claimName: nimPVCName,
},
});
}

updatedServingRuntime.spec.volumes = updatedVolumes;

Check warning on line 207 in frontend/src/api/k8s/servingRuntimes.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/api/k8s/servingRuntimes.ts#L207 

Added line #L207 was not covered by tests
} else {
updatedServingRuntime.spec.volumes = volumes;
}
return updatedServingRuntime;
};

Expand Down Expand Up @@ -242,6 +277,7 @@ export const updateServingRuntime = (options: {
initialAcceleratorProfile?: AcceleratorProfileState;
selectedAcceleratorProfile?: AcceleratorProfileSelectFieldState;
isModelMesh?: boolean;
nimPVCName?: string;
}): Promise<ServingRuntimeKind> => {
const {
data,
Expand All @@ -251,6 +287,7 @@ export const updateServingRuntime = (options: {
initialAcceleratorProfile,
selectedAcceleratorProfile,
isModelMesh,
nimPVCName,
} = options;

const updatedServingRuntime = assembleServingRuntime(
Expand All @@ -262,6 +299,7 @@ export const updateServingRuntime = (options: {
initialAcceleratorProfile,
selectedAcceleratorProfile,
isModelMesh,
nimPVCName,
);

return k8sUpdateResource<ServingRuntimeKind>(
Expand All @@ -284,6 +322,7 @@ export const createServingRuntime = (options: {
initialAcceleratorProfile?: AcceleratorProfileState;
selectedAcceleratorProfile?: AcceleratorProfileSelectFieldState;
isModelMesh?: boolean;
nimPVCName?: string;
}): Promise<ServingRuntimeKind> => {
const {
data,
Expand All @@ -294,6 +333,7 @@ export const createServingRuntime = (options: {
initialAcceleratorProfile,
selectedAcceleratorProfile,
isModelMesh,
nimPVCName,
} = options;
const assembledServingRuntime = assembleServingRuntime(
data,
Expand All @@ -304,6 +344,7 @@ export const createServingRuntime = (options: {
initialAcceleratorProfile,
selectedAcceleratorProfile,
isModelMesh,
nimPVCName,
);

return k8sCreateResource<ServingRuntimeKind>(
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/pages/modelServing/screens/projects/NIMServiceModal/DeployNIMServiceModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Alert,
AlertActionCloseButton,
Form,
getUniqueId,
Modal,
Stack,
StackItem,
Expand Down Expand Up @@ -50,7 +51,6 @@ import { getServingRuntimeFromTemplate } from '~/pages/modelServing/customServin

const NIM_SECRET_NAME = 'nvidia-nim-secrets';
const NIM_NGC_SECRET_NAME = 'ngc-secret';
const NIM_PVC_NAME = 'nim-pvc';

const accessReviewResource: AccessReviewResourceAttributes = {
group: 'rbac.authorization.k8s.io',
Expand Down Expand Up @@ -95,6 +95,7 @@ const DeployNIMServiceModal: React.FC<DeployNIMServiceModalProps> = ({
const isAuthorinoEnabled = useIsAreaAvailable(SupportedArea.K_SERVE_AUTH).status;
const currentProjectName = projectContext?.currentProject.metadata.name;
const namespace = currentProjectName || createDataInferenceService.project;
const nimPVCName = getUniqueId('nim-pvc');

Check warning on line 98 in frontend/src/pages/modelServing/screens/projects/NIMServiceModal/DeployNIMServiceModal.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/modelServing/screens/projects/NIMServiceModal/DeployNIMServiceModal.tsx#L98 

Added line #L98 was not covered by tests

const [translatedName] = translateDisplayNameForK8sAndReport(createDataInferenceService.name, {
maxLength: 253,
Expand Down Expand Up @@ -202,6 +203,7 @@ const DeployNIMServiceModal: React.FC<DeployNIMServiceModalProps> = ({
projectContext?.currentProject,
servingRuntimeName,
true,
nimPVCName,
);

const submitInferenceServiceResource = getSubmitInferenceServiceResourceFn(
Expand All @@ -226,7 +228,7 @@ const DeployNIMServiceModal: React.FC<DeployNIMServiceModalProps> = ({
submitInferenceServiceResource({ dryRun: false }),
createNIMSecret(namespace, NIM_SECRET_NAME, false, false),
createNIMSecret(namespace, NIM_NGC_SECRET_NAME, true, false),
createNIMPVC(namespace, NIM_PVC_NAME, pvcSize, false),
createNIMPVC(namespace, nimPVCName, pvcSize, false),
]),
)
.then(() => onSuccess())
Expand Down
23 changes: 13 additions & 10 deletions frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,24 @@ import {
import { LabeledDataConnection, ServingPlatformStatuses } from '~/pages/modelServing/screens/types';
import { ServingRuntimePlatform } from '~/types';
import { mockInferenceServiceK8sResource } from '~/__mocks__/mockInferenceServiceK8sResource';
import { createPvc, createSecret, getConfigMap } from '~/api';
import { createPvc, createSecret } from '~/api';
import { PersistentVolumeClaimKind } from '~/k8sTypes';
import { getNGCSecretType, getNIMData } from '~/pages/modelServing/screens/projects/nimUtils';
import {
getNGCSecretType,
getNIMData,
getNIMResource,
} from '~/pages/modelServing/screens/projects/nimUtils';

jest.mock('~/api', () => ({
getSecret: jest.fn(),
createSecret: jest.fn(),
getConfigMap: jest.fn(),
createPvc: jest.fn(),
}));

jest.mock('~/pages/modelServing/screens/projects/nimUtils', () => ({
getNIMData: jest.fn(),
getNGCSecretType: jest.fn(),
getNIMResource: jest.fn(),
}));

describe('filterOutConnectionsWithoutBucket', () => {
Expand Down Expand Up @@ -312,7 +316,6 @@ describe('createNIMSecret', () => {
});
});
describe('fetchNIMModelNames', () => {
const dashboardNamespace = 'test-namespace';
const NIM_CONFIGMAP_NAME = 'nvidia-nim-images-data';

const configMapMock = {
Expand Down Expand Up @@ -341,11 +344,11 @@ describe('fetchNIMModelNames', () => {
});

it('should return model infos when configMap has data', async () => {
(getConfigMap as jest.Mock).mockResolvedValueOnce(configMapMock);
(getNIMResource as jest.Mock).mockResolvedValueOnce(configMapMock);

const result = await fetchNIMModelNames();

expect(getConfigMap).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(result).toEqual([
{
name: 'model1',
Expand All @@ -369,20 +372,20 @@ describe('fetchNIMModelNames', () => {
});

it('should return undefined if configMap has no data', async () => {
(getConfigMap as jest.Mock).mockResolvedValueOnce({ data: {} });
(getNIMResource as jest.Mock).mockResolvedValueOnce({ data: {} });

const result = await fetchNIMModelNames();

expect(getConfigMap).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(result).toBeUndefined();
});

it('should return undefined if configMap.data is not defined', async () => {
(getConfigMap as jest.Mock).mockResolvedValueOnce({ data: undefined });
(getNIMResource as jest.Mock).mockResolvedValueOnce({ data: undefined });

const result = await fetchNIMModelNames();

expect(getConfigMap).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(getNIMResource).toHaveBeenCalledWith(NIM_CONFIGMAP_NAME);
expect(result).toBeUndefined();
});
});
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/pages/modelServing/screens/projects/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ export const getSubmitServingRuntimeResourcesFn = (
currentProject?: ProjectKind,
name?: string,
isModelMesh?: boolean,
nimPVCName?: string,
): ((opts: { dryRun?: boolean }) => Promise<void | (string | void | ServingRuntimeKind)[]>) => {
if (!servingRuntimeSelected) {
return () =>
Expand Down Expand Up @@ -501,6 +502,7 @@ export const getSubmitServingRuntimeResourcesFn = (
selectedAcceleratorProfile: controlledState,
initialAcceleratorProfile,
isModelMesh,
nimPVCName,
}),
setUpTokenAuth(
servingRuntimeData,
Expand All @@ -527,6 +529,7 @@ export const getSubmitServingRuntimeResourcesFn = (
selectedAcceleratorProfile: controlledState,
initialAcceleratorProfile,
isModelMesh,
nimPVCName,
}).then((servingRuntime) =>
setUpTokenAuth(
servingRuntimeData,
Expand Down

0 comments on commit bc3c9ac

Please sign in to comment.