diff --git a/.env.development b/.env.development index 60e65148e8..9f757bfc5c 100644 --- a/.env.development +++ b/.env.development @@ -14,3 +14,8 @@ DS_PIPELINE_DSPA_SERVICE_PORT=8443 TRUSTYAI_NAME=trustyai-service TRUSTYAI_TAIS_SERVICE_HOST=localhost TRUSTYAI_TAIS_SERVICE_PORT=9443 + + +MODEL_REGISTRY_NAME=modelregistry-sample +MODEL_REGISTRY_SERVICE_HOST=localhost +MODEL_REGISTRY_SERVICE_PORT=8085 diff --git a/Makefile b/Makefile index c0af316ae1..2207f12d15 100644 --- a/Makefile +++ b/Makefile @@ -91,6 +91,7 @@ ifdef NAMESPACE 'oc port-forward -n ${NAMESPACE} svc/ds-pipeline-metadata-envoy-${DSPA_NAME} ${METADATA_ENVOY_SERVICE_PORT}:9090' \ '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' else $(error Missing NAMESPACE variable) endif diff --git a/backend/src/routes/api/service/modelregistry/index.ts b/backend/src/routes/api/service/modelregistry/index.ts new file mode 100644 index 0000000000..f4cd428dad --- /dev/null +++ b/backend/src/routes/api/service/modelregistry/index.ts @@ -0,0 +1,34 @@ +import httpProxy from '@fastify/http-proxy'; +import { KubeFastifyInstance } from '../../../../types'; +import { DEV_MODE } from '../../../../utils/constants'; +import { getParam, setParam } from '../../../../utils/proxy'; + +export default async (fastify: KubeFastifyInstance): Promise => { + if (DEV_MODE) { + fastify.register(httpProxy, { + upstream: '', + prefix: '/:name', + rewritePrefix: '', + replyOptions: { + // preHandler must set the `upstream` param + getUpstream: (request) => getParam(request, 'upstream'), + }, + preHandler: (request, _, done) => { + const name = getParam(request, 'name'); + + const upstream = DEV_MODE + ? // Use port forwarding for local development: + // kubectl port-forward -n svc/ : + `http://${process.env.MODEL_REGISTRY_SERVICE_HOST}:${process.env.MODEL_REGISTRY_SERVICE_PORT}` + : // Construct service URL + `http://${name}.odh-model-registries.svc.cluster.local:8080`; + + // assign the `upstream` param so we can dynamically set the upstream URL for http-proxy + setParam(request, 'upstream', upstream); + + fastify.log.info(`Proxy ${request.method} request ${request.url} to ${upstream}`); + done(); + }, + }); + } +}; diff --git a/backend/src/types.ts b/backend/src/types.ts index 8308d931d6..54ed48d199 100644 --- a/backend/src/types.ts +++ b/backend/src/types.ts @@ -1021,3 +1021,9 @@ export type TrustyAIKind = K8sResourceCommon & { conditions?: K8sCondition[]; }; }; + +export type ModelRegistryKind = K8sResourceCommon & { + status?: { + conditions?: K8sCondition[]; + }; +}; diff --git a/backend/src/utils/proxy.ts b/backend/src/utils/proxy.ts index de729ce06a..61242a5956 100644 --- a/backend/src/utils/proxy.ts +++ b/backend/src/utils/proxy.ts @@ -6,10 +6,10 @@ import { DEV_MODE } from './constants'; import { createCustomError } from './requestUtils'; import { getAccessToken, getDirectCallOptions } from './directCallUtils'; -const getParam = >(req: F, name: string) => +export const getParam = >(req: F, name: string): string => (req.params as { [key: string]: string })[name]; -const setParam = (req: FastifyRequest, name: string, value: string) => +export const setParam = (req: FastifyRequest, name: string, value: string): string => ((req.params as { [key: string]: string })[name] = value); const notFoundError = (kind: string, name: string, e?: any, overrideMessage?: string) => { diff --git a/frontend/src/api/modelRegistry/__tests__/custom.spec.ts b/frontend/src/api/modelRegistry/__tests__/custom.spec.ts index 4232b2e043..d899326f6c 100644 --- a/frontend/src/api/modelRegistry/__tests__/custom.spec.ts +++ b/frontend/src/api/modelRegistry/__tests__/custom.spec.ts @@ -20,6 +20,7 @@ import { patchModelVersion, patchRegisteredModel, } from '~/api/modelRegistry/custom'; +import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const'; const mockProxyPromise = Promise.resolve(); @@ -56,7 +57,7 @@ describe('createRegisteredModel', () => { expect(proxyCREATEMock).toHaveBeenCalledTimes(1); expect(proxyCREATEMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/registered_models', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, { description: 'test', externalID: '1', @@ -88,7 +89,7 @@ describe('createModelVersion', () => { expect(proxyCREATEMock).toHaveBeenCalledTimes(1); expect(proxyCREATEMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_versions', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions`, { description: 'test', externalID: '1', @@ -126,7 +127,7 @@ describe('createModelArtifact', () => { expect(proxyCREATEMock).toHaveBeenCalledTimes(1); expect(proxyCREATEMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_artifacts', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts`, { description: 'test', externalID: 'test', @@ -154,7 +155,7 @@ describe('getRegisteredModel', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/registered_models/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1`, {}, K8sAPIOptionsMock, ); @@ -169,7 +170,7 @@ describe('getModelVersion', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_versions/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/1`, {}, K8sAPIOptionsMock, ); @@ -184,7 +185,7 @@ describe('getModelArtifact', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_artifacts/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts/1`, {}, K8sAPIOptionsMock, ); @@ -199,7 +200,7 @@ describe('getListRegisteredModels', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/registered_models', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, {}, K8sAPIOptionsMock, ); @@ -214,7 +215,7 @@ describe('getListModelArtifacts', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_artifacts', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts`, {}, K8sAPIOptionsMock, ); @@ -229,7 +230,7 @@ describe('getListModelVersions', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_versions', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions`, {}, K8sAPIOptionsMock, ); @@ -244,7 +245,7 @@ describe('getModelVersionsByRegisteredModel', () => { expect(proxyGETMock).toHaveBeenCalledTimes(1); expect(proxyGETMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/registered_models/1/versions', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1/versions`, {}, K8sAPIOptionsMock, ); @@ -261,7 +262,7 @@ describe('patchRegisteredModel', () => { expect(proxyPATCHMock).toHaveBeenCalledTimes(1); expect(proxyPATCHMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/registered_models/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1`, { description: 'new test' }, K8sAPIOptionsMock, ); @@ -278,7 +279,7 @@ describe('patchModelVersion', () => { expect(proxyPATCHMock).toHaveBeenCalledTimes(1); expect(proxyPATCHMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_versions/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/1`, { description: 'new test' }, K8sAPIOptionsMock, ); @@ -295,7 +296,7 @@ describe('patchModelArtifact', () => { expect(proxyPATCHMock).toHaveBeenCalledTimes(1); expect(proxyPATCHMock).toHaveBeenCalledWith( 'hostPath', - '/api/model_registry/v1alpha2/model_artifacts/1', + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts/1`, { description: 'new test' }, K8sAPIOptionsMock, ); diff --git a/frontend/src/api/modelRegistry/custom.ts b/frontend/src/api/modelRegistry/custom.ts index 504e3c5d83..272eebe953 100644 --- a/frontend/src/api/modelRegistry/custom.ts +++ b/frontend/src/api/modelRegistry/custom.ts @@ -11,27 +11,46 @@ import { } from '~/concepts/modelRegistry/types'; import { proxyCREATE, proxyGET, proxyPATCH } from '~/api/proxyUtils'; import { K8sAPIOptions } from '~/k8sTypes'; +import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const'; import { handleModelRegistryFailures } from './errorUtils'; export const createRegisteredModel = (hostpath: string) => (opts: K8sAPIOptions, data: CreateRegisteredModelData): Promise => handleModelRegistryFailures( - proxyCREATE(hostpath, '/api/model_registry/v1alpha2/registered_models', data, {}, opts), + proxyCREATE( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, + data, + {}, + opts, + ), ); export const createModelVersion = (hostpath: string) => (opts: K8sAPIOptions, data: CreateModelVersionData): Promise => handleModelRegistryFailures( - proxyCREATE(hostpath, '/api/model_registry/v1alpha2/model_versions', data, {}, opts), + proxyCREATE( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions`, + data, + {}, + opts, + ), ); export const createModelArtifact = (hostPath: string) => (opts: K8sAPIOptions, data: CreateModelArtifactData): Promise => handleModelRegistryFailures( - proxyCREATE(hostPath, '/api/model_registry/v1alpha2/model_artifacts', data, {}, opts), + proxyCREATE( + hostPath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts`, + data, + {}, + opts, + ), ); export const getRegisteredModel = @@ -40,7 +59,7 @@ export const getRegisteredModel = handleModelRegistryFailures( proxyGET( hostPath, - `/api/model_registry/v1alpha2/registered_models/${registeredModelID}`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelID}`, {}, opts, ), @@ -50,7 +69,12 @@ export const getModelVersion = (hostpath: string) => (opts: K8sAPIOptions, modelversionId: string): Promise => handleModelRegistryFailures( - proxyGET(hostpath, `/api/model_registry/v1alpha2/model_versions/${modelversionId}`, {}, opts), + proxyGET( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/${modelversionId}`, + {}, + opts, + ), ); export const getModelArtifact = @@ -59,7 +83,7 @@ export const getModelArtifact = handleModelRegistryFailures( proxyGET( hostPath, - `/api/model_registry/v1alpha2/model_artifacts/${modelArtifactId}`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts/${modelArtifactId}`, {}, opts, ), @@ -69,21 +93,36 @@ export const getListModelArtifacts = (hostpath: string) => (opts: K8sAPIOptions): Promise => handleModelRegistryFailures( - proxyGET(hostpath, `/api/model_registry/v1alpha2/model_artifacts`, {}, opts), + proxyGET( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts`, + {}, + opts, + ), ); export const getListModelVersions = (hostpath: string) => (opts: K8sAPIOptions): Promise => handleModelRegistryFailures( - proxyGET(hostpath, `/api/model_registry/v1alpha2/model_versions`, {}, opts), + proxyGET( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions`, + {}, + opts, + ), ); export const getListRegisteredModels = (hostpath: string) => (opts: K8sAPIOptions): Promise => handleModelRegistryFailures( - proxyGET(hostpath, `/api/model_registry/v1alpha2/registered_models`, {}, opts), + proxyGET( + hostpath, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, + {}, + opts, + ), ); export const getModelVersionsByRegisteredModel = @@ -92,7 +131,7 @@ export const getModelVersionsByRegisteredModel = handleModelRegistryFailures( proxyGET( hostpath, - `/api/model_registry/v1alpha2/registered_models/${registeredmodelId}/versions`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredmodelId}/versions`, {}, opts, ), @@ -108,7 +147,7 @@ export const patchRegisteredModel = handleModelRegistryFailures( proxyPATCH( hostPath, - `/api/model_registry/v1alpha2/registered_models/${registeredModelID}`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelID}`, data, opts, ), @@ -124,7 +163,7 @@ export const patchModelVersion = handleModelRegistryFailures( proxyPATCH( hostPath, - `/api/model_registry/v1alpha2/model_versions/${modelversionId}`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/${modelversionId}`, data, opts, ), @@ -140,7 +179,7 @@ export const patchModelArtifact = handleModelRegistryFailures( proxyPATCH( hostPath, - `/api/model_registry/v1alpha2/model_artifacts/${modelartifactId}`, + `/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_artifacts/${modelartifactId}`, data, opts, ), diff --git a/frontend/src/concepts/modelRegistry/const.ts b/frontend/src/concepts/modelRegistry/const.ts index c1a92f48d5..ced9db3475 100644 --- a/frontend/src/concepts/modelRegistry/const.ts +++ b/frontend/src/concepts/modelRegistry/const.ts @@ -1,2 +1,2 @@ -export const MODEL_REGISTRY_DEFINITION_NAME = 'modelregistry-sample'; // TODO: harcoded value, we need to dynamically fetch this -export const MODEL_REGISTRY_DEFAULT_NAMESPACE = 'shared'; // TODO: harcoded value, we need to check which will be the default namespace +export const MODEL_REGISTRY_DEFAULT_NAMESPACE = 'odh-model-registries'; // The default namespace for the model registry service, controlled by the operator. +export const MODEL_REGISTRY_API_VERSION = 'v1alpha3'; diff --git a/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx b/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx index 9d9575e8ca..b5ddda600d 100644 --- a/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx +++ b/frontend/src/concepts/modelRegistry/context/ModelRegistryContext.tsx @@ -1,11 +1,10 @@ import * as React from 'react'; import { Alert, Bullseye } from '@patternfly/react-core'; import { SupportedArea, conditionalArea } from '~/concepts/areas'; -import { MODEL_REGISTRY_DEFINITION_NAME } from '~/concepts/modelRegistry/const'; import { ModelRegistryKind } from '~/k8sTypes'; import useModelRegistries from '~/concepts/modelRegistry/apiHooks/useModelRegistries'; +import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; import useModelRegistryAPIState, { ModelRegistryAPIState } from './useModelRegistryAPIState'; -import useModelRegistryAPIRoute from './useModelRegistryAPIRoute'; import { hasServerTimedOut, isModelRegistryAvailable, @@ -27,7 +26,7 @@ export type ModelRegistryContextType = { type ModelRegistryContextProviderProps = { children: React.ReactNode; - namespace: string; + modelRegistryName: string; }; export const ModelRegistryContext = React.createContext({ @@ -46,12 +45,12 @@ export const ModelRegistryContext = React.createContext( SupportedArea.MODEL_REGISTRY, true, -)(({ children, namespace }) => { +)(({ children, modelRegistryName }) => { const [modelRegistries] = useModelRegistries(); const [preferredModelRegistry, setPreferredModelRegistry] = React.useState(undefined); - const crState = useModelRegistryNamespaceCR(namespace, MODEL_REGISTRY_DEFINITION_NAME); // TODO: dynamially change the model registry name + const crState = useModelRegistryNamespaceCR(MODEL_REGISTRY_DEFAULT_NAMESPACE, modelRegistryName); const [modelRegistryNamespaceCR, crLoaded, crLoadError, refreshCR] = crState; const isCRReady = isModelRegistryAvailable(crState); @@ -61,13 +60,7 @@ export const ModelRegistryContextProvider = conditionalArea Promise.all([refreshCR(), refreshRoute()]).then(() => undefined), - [refreshRoute, refreshCR], + () => Promise.all([refreshCR()]).then(() => undefined), + [refreshCR], ); const updatePreferredModelRegistry = React.useCallback< @@ -88,7 +81,7 @@ export const ModelRegistryContextProvider = conditionalArea diff --git a/frontend/src/concepts/modelRegistry/context/useModelRegistryAPIRoute.ts b/frontend/src/concepts/modelRegistry/context/useModelRegistryAPIRoute.ts deleted file mode 100644 index 5cead5f3ab..0000000000 --- a/frontend/src/concepts/modelRegistry/context/useModelRegistryAPIRoute.ts +++ /dev/null @@ -1,63 +0,0 @@ -import React from 'react'; -import useFetchState, { - FetchState, - FetchStateCallbackPromise, - NotReadyError, -} from '~/utilities/useFetchState'; -import { getModelRegistryAPIRoute } from '~/api/'; -import { RouteKind } from '~/k8sTypes'; -import { FAST_POLL_INTERVAL } from '~/utilities/const'; -import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; - -type State = string | null; - -// TODO: Ask the model registry team to provide a route rather than a service!! -const useModelRegistryAPIRoute = ( - hasCR: boolean, - modelRegistryName: string, - namespace: string, -): FetchState => { - const modelRegistryAreaAvailable = useIsAreaAvailable(SupportedArea.MODEL_REGISTRY).status; - const callback = React.useCallback>( - (opts) => { - if (!modelRegistryAreaAvailable) { - return Promise.reject(new NotReadyError('Model registry not enabled')); - } - - if (!hasCR) { - return Promise.reject(new NotReadyError('CR not created')); - } - - return getModelRegistryAPIRoute(namespace, modelRegistryName, opts) - .then((result: RouteKind) => `http://${result.spec.host}`) // TODO: check why it's not working with https!!! - .catch((e) => { - if (e.statusObject?.code === 404) { - // Not finding is okay, not an error - return null; - } - throw e; - }); - }, - [hasCR, modelRegistryName, namespace, modelRegistryAreaAvailable], - ); - - const state = useFetchState(callback, null, { - initialPromisePurity: true, - }); - - const [data, , , refresh] = state; - - const hasData = !!data; - React.useEffect(() => { - let interval: ReturnType; - if (!hasData) { - interval = setInterval(refresh, FAST_POLL_INTERVAL); - } - return () => { - clearInterval(interval); - }; - }, [hasData, refresh]); - return state; -}; - -export default useModelRegistryAPIRoute; diff --git a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx index 036a449f87..a5094c5c51 100644 --- a/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx +++ b/frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx @@ -1,10 +1,11 @@ import * as React from 'react'; import { Outlet } from 'react-router'; -import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const'; + import { ModelRegistryContextProvider } from '~/concepts/modelRegistry/context/ModelRegistryContext'; +// TODO: Parametrize this to make the route dynamic const ModelRegistryCoreLoader: React.FC = () => ( - + );