Skip to content

Commit

Permalink
Replace model registry hard-coded namespace with namespace from DSC
Browse files Browse the repository at this point in the history
  • Loading branch information
ppadti committed Sep 24, 2024
1 parent 5640b39 commit 0122d25
Show file tree
Hide file tree
Showing 25 changed files with 206 additions and 63 deletions.
1 change: 1 addition & 0 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ TRUSTYAI_TAIS_SERVICE_PORT=9443
MODEL_REGISTRY_NAME=modelregistry-sample
MODEL_REGISTRY_SERVICE_HOST=localhost
MODEL_REGISTRY_SERVICE_PORT=8085
MODEL_REGISTRY_NAMESPACE=odh-model-registries
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ ifdef NAMESPACE
'oc port-forward -n ${NAMESPACE} svc/ds-pipeline-md-${DSPA_NAME} ${METADATA_ENVOY_SERVICE_PORT}:8443' \
'oc port-forward -n ${NAMESPACE} svc/ds-pipeline-${DSPA_NAME} ${DS_PIPELINE_DSPA_SERVICE_PORT}:8443' \
'oc port-forward -n ${NAMESPACE} svc/${TRUSTYAI_NAME}-tls ${TRUSTYAI_TAIS_SERVICE_PORT}:443' \
'oc port-forward -n odh-model-registries svc/${MODEL_REGISTRY_NAME} ${MODEL_REGISTRY_SERVICE_PORT}:8080'
'oc port-forward -n ${MODEL_REGISTRY_NAMESPACE} svc/${MODEL_REGISTRY_NAME} ${MODEL_REGISTRY_SERVICE_PORT}:8080'
else
$(error Missing NAMESPACE variable)
endif
Expand Down
33 changes: 28 additions & 5 deletions backend/src/routes/api/modelRegistries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
deleteModelRegistryAndSecret,
getDatabasePassword,
getModelRegistry,
getModelRegistryNamespace,
listModelRegistries,
patchModelRegistryAndUpdatePassword,
} from './modelRegistryUtils';
Expand All @@ -17,6 +18,8 @@ type ModelRegistryAndDBPassword = {

// Lists ModelRegistries directly (does not look up passwords from associated Secrets, you must make a direct request to '/:modelRegistryName' for that)
export default async (fastify: KubeFastifyInstance): Promise<void> => {
const modelRegistryNamespace = await getModelRegistryNamespace(fastify);

fastify.get(
'/',
secureAdminRoute(fastify)(
Expand All @@ -26,7 +29,7 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
) => {
const { labelSelector } = request.query;
try {
return listModelRegistries(fastify, labelSelector);
return listModelRegistries(fastify, modelRegistryNamespace, labelSelector);
} catch (e) {
fastify.log.error(
`ModelRegistries could not be listed, ${e.response?.body?.message || e.message}`,
Expand All @@ -52,7 +55,13 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
const { dryRun } = request.query;
const { modelRegistry, databasePassword } = request.body;
try {
return createModelRegistryAndSecret(fastify, modelRegistry, databasePassword, !!dryRun);
return createModelRegistryAndSecret(
fastify,
modelRegistry,
modelRegistryNamespace,
databasePassword,
!!dryRun,
);
} catch (e) {
fastify.log.error(
`ModelRegistry ${modelRegistry.metadata.name} could not be created, ${
Expand All @@ -75,8 +84,16 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
) => {
const { modelRegistryName } = request.params;
try {
const modelRegistry = await getModelRegistry(fastify, modelRegistryName);
const databasePassword = await getDatabasePassword(fastify, modelRegistry);
const modelRegistry = await getModelRegistry(
fastify,
modelRegistryName,
modelRegistryNamespace,
);
const databasePassword = await getDatabasePassword(
fastify,
modelRegistry,
modelRegistryNamespace,
);
return { modelRegistry, databasePassword } satisfies ModelRegistryAndDBPassword;
} catch (e) {
fastify.log.error(
Expand Down Expand Up @@ -110,6 +127,7 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
const modelRegistry = await patchModelRegistryAndUpdatePassword(
fastify,
modelRegistryName,
modelRegistryNamespace,
patchBody,
databasePassword,
!!dryRun,
Expand Down Expand Up @@ -141,7 +159,12 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
const { dryRun } = request.query;
const { modelRegistryName } = request.params;
try {
deleteModelRegistryAndSecret(fastify, modelRegistryName, !!dryRun);
deleteModelRegistryAndSecret(
fastify,
modelRegistryName,
modelRegistryNamespace,
!!dryRun,
);
} catch (e) {
fastify.log.error(
`ModelRegistry ${modelRegistryName} could not be deleted, ${
Expand Down
87 changes: 66 additions & 21 deletions backend/src/routes/api/modelRegistries/modelRegistryUtils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { MODEL_REGISTRY_NAMESPACE } from '../../../utils/constants';
import { KubeFastifyInstance, ModelRegistryKind, RecursivePartial } from '../../../types';
import { PatchUtils, V1Secret, V1Status } from '@kubernetes/client-node';
import { getClusterStatus } from '../../../utils/dsc';

const MODEL_REGISTRY_API_GROUP = 'modelregistry.opendatahub.io';
const MODEL_REGISTRY_API_VERSION = 'v1alpha1';
const MODEL_REGISTRY_PLURAL = 'modelregistries';

export const getModelRegistryNamespace = async (fastify: KubeFastifyInstance): Promise<string> => {
const modelRegistryNamespace = await getClusterStatus(fastify);
return modelRegistryNamespace.components.modelregistry.registriesNamespace;
};

const base64encode = (value?: string): string => {
// This usage of toString is fine for encoding
// eslint-disable-next-line no-restricted-properties
Expand All @@ -28,12 +33,13 @@ const getDatabaseSpec = (

export const listModelRegistries = async (
fastify: KubeFastifyInstance,
modelRegistryNamespace: string,
labelSelector?: string,
): Promise<{ items: ModelRegistryKind[] }> => {
const response = await (fastify.kube.customObjectsApi.listNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
MODEL_REGISTRY_PLURAL,
undefined,
undefined,
Expand All @@ -47,6 +53,7 @@ export const listModelRegistries = async (
const createDatabasePasswordSecret = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
databasePassword?: string,
dryRun = false,
): Promise<V1Secret | null> => {
Expand All @@ -59,7 +66,7 @@ const createDatabasePasswordSecret = async (
apiVersion: 'v1',
metadata: {
generateName: `${modelRegistry.metadata.name}-db-`,
namespace: MODEL_REGISTRY_NAMESPACE,
namespace: modelRegistryNamespace,
annotations: {
'template.openshift.io/expose-database_name': "{.data['database-name']}",
'template.openshift.io/expose-username': "{.data['database-user']}",
Expand All @@ -74,7 +81,7 @@ const createDatabasePasswordSecret = async (
type: 'Opaque',
};
const response = await fastify.kube.coreV1Api.createNamespacedSecret(
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
secret,
undefined,
dryRun ? 'All' : undefined,
Expand All @@ -85,6 +92,7 @@ const createDatabasePasswordSecret = async (
const createModelRegistry = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
secret?: V1Secret,
dryRun = false,
): Promise<ModelRegistryKind> => {
Expand All @@ -108,7 +116,7 @@ const createModelRegistry = async (
const response = await (fastify.kube.customObjectsApi.createNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
MODEL_REGISTRY_PLURAL,
modelRegistryWithSecretRef,
undefined,
Expand All @@ -121,16 +129,23 @@ const createModelRegistry = async (
export const createModelRegistryAndSecret = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
databasePassword?: string,
dryRunOnly = false,
): Promise<ModelRegistryKind> => {
const createBoth = async (dryRun = false) => {
const dbSpec = getDatabaseSpec(modelRegistry);
const newSecret =
databasePassword && dbSpec
? await createDatabasePasswordSecret(fastify, modelRegistry, databasePassword, dryRun)
? await createDatabasePasswordSecret(
fastify,
modelRegistry,
modelRegistryNamespace,
databasePassword,
dryRun,
)
: undefined;
return createModelRegistry(fastify, modelRegistry, newSecret, dryRun);
return createModelRegistry(fastify, modelRegistry, modelRegistryNamespace, newSecret, dryRun);
};
// Dry run both MR and Secret creation first so there are no changes if either would fail
const dryRunResult = await createBoth(true);
Expand All @@ -143,11 +158,12 @@ export const createModelRegistryAndSecret = async (
export const getModelRegistry = async (
fastify: KubeFastifyInstance,
modelRegistryName: string,
modelRegistryNamespace: string,
): Promise<ModelRegistryKind> => {
const response = await (fastify.kube.customObjectsApi.getNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
MODEL_REGISTRY_PLURAL,
modelRegistryName,
// getNamespacedCustomObject doesn't support TS generics and returns body as `object`, so we assert its real type
Expand All @@ -158,42 +174,51 @@ export const getModelRegistry = async (
const getDatabasePasswordSecret = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
): Promise<{ secret?: V1Secret; passwordDataKey?: string }> => {
const secretRef = getDatabaseSpec(modelRegistry)?.passwordSecret;
if (!secretRef) {
return {};
}
const response = await fastify.kube.coreV1Api.readNamespacedSecret(
secretRef.name,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
);
return { secret: response.body, passwordDataKey: secretRef.key };
};

export const getDatabasePassword = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
): Promise<string | undefined> => {
const { secret, passwordDataKey } = await getDatabasePasswordSecret(fastify, modelRegistry);
const { secret, passwordDataKey } = await getDatabasePasswordSecret(
fastify,
modelRegistry,
modelRegistryNamespace,
);
return base64decode(secret.data[passwordDataKey]);
};

const deleteDatabasePasswordSecret = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
dryRun = false,
): Promise<V1Status | undefined> => {
let existingSecret: V1Secret | undefined;
try {
existingSecret = (await getDatabasePasswordSecret(fastify, modelRegistry)).secret;
existingSecret = (
await getDatabasePasswordSecret(fastify, modelRegistry, modelRegistryNamespace)
).secret;
} catch (e) {
// If the secret doesn't exist, don't try to delete and cause a 404 error, just do nothing.
// The user may have deleted their own secret and we don't want to block deleting the model registry.
return;
}
const response = await fastify.kube.coreV1Api.deleteNamespacedSecret(
existingSecret.metadata.name,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
undefined,
dryRun ? 'All' : undefined,
);
Expand All @@ -203,13 +228,14 @@ const deleteDatabasePasswordSecret = async (
const patchModelRegistry = async (
fastify: KubeFastifyInstance,
modelRegistryName: string,
modelRegistryNamespace: string,
patchBody: RecursivePartial<ModelRegistryKind>,
dryRun = false,
): Promise<ModelRegistryKind> => {
const response = await (fastify.kube.customObjectsApi.patchNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
MODEL_REGISTRY_PLURAL,
modelRegistryName,
patchBody,
Expand All @@ -225,17 +251,22 @@ const patchModelRegistry = async (
const updateDatabasePassword = async (
fastify: KubeFastifyInstance,
modelRegistry: ModelRegistryKind,
modelRegistryNamespace: string,
databasePassword?: string,
dryRun = false,
): Promise<void> => {
const { secret, passwordDataKey } = await getDatabasePasswordSecret(fastify, modelRegistry);
const { secret, passwordDataKey } = await getDatabasePasswordSecret(
fastify,
modelRegistry,
modelRegistryNamespace,
);
if (!secret) {
return;
}
if (databasePassword) {
await fastify.kube.coreV1Api.patchNamespacedSecret(
secret.metadata.name,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
{
data: {
...secret.data,
Expand All @@ -246,20 +277,33 @@ const updateDatabasePassword = async (
dryRun ? 'All' : undefined,
);
} else {
await deleteDatabasePasswordSecret(fastify, modelRegistry);
await deleteDatabasePasswordSecret(fastify, modelRegistry, modelRegistryNamespace);
}
};

export const patchModelRegistryAndUpdatePassword = async (
fastify: KubeFastifyInstance,
modelRegistryName: string,
modelRegistryNamespace: string,
patchBody: RecursivePartial<ModelRegistryKind>,
databasePassword?: string,
dryRunOnly = false,
): Promise<ModelRegistryKind> => {
const patchBoth = async (dryRun = false) => {
const modelRegistry = await patchModelRegistry(fastify, modelRegistryName, patchBody, dryRun);
await updateDatabasePassword(fastify, modelRegistry, databasePassword, dryRun);
const modelRegistry = await patchModelRegistry(
fastify,
modelRegistryName,
modelRegistryNamespace,
patchBody,
dryRun,
);
await updateDatabasePassword(
fastify,
modelRegistry,
modelRegistryNamespace,
databasePassword,
dryRun,
);
return modelRegistry;
};
// Dry run both patches first so there are no changes if either would fail
Expand All @@ -273,22 +317,23 @@ export const patchModelRegistryAndUpdatePassword = async (
export const deleteModelRegistryAndSecret = async (
fastify: KubeFastifyInstance,
modelRegistryName: string,
modelRegistryNamespace: string,
dryRunOnly = false,
): Promise<V1Status> => {
const modelRegistry = await getModelRegistry(fastify, modelRegistryName);
const modelRegistry = await getModelRegistry(fastify, modelRegistryName, modelRegistryNamespace);
const deleteBoth = async (dryRun = false) => {
const response = await fastify.kube.customObjectsApi.deleteNamespacedCustomObject(
MODEL_REGISTRY_API_GROUP,
MODEL_REGISTRY_API_VERSION,
MODEL_REGISTRY_NAMESPACE,
modelRegistryNamespace,
MODEL_REGISTRY_PLURAL,
modelRegistryName,
undefined,
undefined,
undefined,
dryRun ? 'All' : undefined,
);
await deleteDatabasePasswordSecret(fastify, modelRegistry);
await deleteDatabasePasswordSecret(fastify, modelRegistry, modelRegistryNamespace);
return response.body;
};
// Dry run both deletes first so there are no changes if either would fail
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/service/modelregistry/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { getModelRegistryNamespace } from '../../../api/modelRegistries/modelRegistryUtils';
import { ServiceAddressAnnotation } from '../../../../types';
import { MODEL_REGISTRY_NAMESPACE } from '../../../../utils/constants';
import { proxyService } from '../../../../utils/proxy';

export default proxyService(
null,
{
addressAnnotation: ServiceAddressAnnotation.EXTERNAL_REST,
internalPort: 8080,
namespace: MODEL_REGISTRY_NAMESPACE,
namespace: getModelRegistryNamespace,
},
{
// Use port forwarding for local development:
Expand Down
Loading

0 comments on commit 0122d25

Please sign in to comment.