From bfa5075d76c52b4e2fc2749c2caf357d36fa64f9 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Tue, 4 Apr 2023 16:24:53 -0400 Subject: [PATCH 01/18] feat(ossm): adds OSSM annotations to the relevant cluster resources This change takes care of populating the following annotations to resources created through ODH Dashboard - `opendatahub.io/service-mesh` can be set to `true` or `false` and will be used to alternate ODH components and make them part of the Service Mesh. This is applied on both `Project` and `Notebook` resources - `opendatahub.io/hub-url` is added to the `Notebook` resources at this point and is inteded for use with Authorino's `AuthConfig` host - The dashboard will now link to notebooks through the mesh if feature flag `disableServiceMesh=false` In addition, the code hides service-mesh-specific changes to annotations behind `disableServiceMesh` flag which is added in https://github.com/opendatahub-io/opendatahub-operator/pull/217 We have tested this manually. If you can offer us some hints on how to test feature flags and resource creation through Jest tests we are happy to extend this PR with it. Until this [PR](https://github.com/opendatahub-io/opendatahub-operator/pull/217) is merged and a new bundle is released you can use my build of the ODH Operator that includes the `disableServiceMesh` flag as part of the `ODHDashboardConfig`. Do so by running `operator-sdk run bundle quay.io/cgarriso/opendatahub-operator-bundle:dev-0.0.1 --namespace $OPERATOR_NAMESPACE` (req operator-sdk v1.24.1 ) Either build and push this dashboard image or use `quay.io/maistra-dev/odh-dashboard:ossm_annotations` image. Co-authored-by: bartoszmajsak --- .../routes/api/namespaces/namespaceUtils.ts | 4 ++ backend/src/routes/api/notebooks/utils.ts | 21 ++++-- backend/src/types.ts | 5 ++ backend/src/utils/constants.ts | 1 + backend/src/utils/notebookUtils.ts | 65 ++++++++++++++++++- docs/dashboard_config.md | 1 + frontend/src/__mocks__/mockDashboardConfig.ts | 23 +++---- frontend/src/api/k8s/notebooks.ts | 20 ++++-- frontend/src/api/k8s/routes.ts | 21 +++++- frontend/src/k8sTypes.ts | 9 +++ .../projects/notebook/useRouteForNotebook.ts | 15 +++-- .../screens/spawner/SpawnerFooter.tsx | 7 +- frontend/src/types.ts | 1 + .../src/utilities/notebookControllerUtils.ts | 2 +- ...dhdashboardconfigs.opendatahub.io.crd.yaml | 2 + .../odh-dashboard-config.yaml | 1 + 16 files changed, 167 insertions(+), 31 deletions(-) diff --git a/backend/src/routes/api/namespaces/namespaceUtils.ts b/backend/src/routes/api/namespaces/namespaceUtils.ts index 05b1a7dc7f..1983bdbb95 100644 --- a/backend/src/routes/api/namespaces/namespaceUtils.ts +++ b/backend/src/routes/api/namespaces/namespaceUtils.ts @@ -2,6 +2,7 @@ import { PatchUtils, V1SelfSubjectAccessReview } from '@kubernetes/client-node'; import { NamespaceApplicationCase } from './const'; import { K8sStatus, KubeFastifyInstance, OauthFastifyRequest } from '../../../types'; import { createCustomError } from '../../../utils/requestUtils'; +import { getDashboardConfig } from '../../../utils/resourceUtils'; import { isK8sStatus, safeURLPassThrough } from '../k8s/pass-through'; const checkNamespacePermission = ( @@ -60,11 +61,14 @@ export const applyNamespaceChange = async ( throw createCustomError('Forbidden', "You don't have the access to update the namespace", 403); } + const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; + let labels = {}; switch (context) { case NamespaceApplicationCase.DSG_CREATION: labels = { 'opendatahub.io/dashboard': 'true', + 'opendatahub.io/service-mesh': String(!disableServiceMesh), 'modelmesh-enabled': 'true', }; break; diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts index c4de264e86..b7a70888da 100644 --- a/backend/src/routes/api/notebooks/utils.ts +++ b/backend/src/routes/api/notebooks/utils.ts @@ -3,12 +3,14 @@ import { PatchUtils, V1ContainerStatus, V1Pod, V1PodList } from '@kubernetes/cli import { createCustomError } from '../../../utils/requestUtils'; import { getUserName } from '../../../utils/userUtils'; import { RecursivePartial } from '../../../typeHelpers'; +import { getDashboardConfig } from '../../../utils/resourceUtils'; import { createNotebook, generateNotebookNameFromUsername, getNamespaces, getNotebook, getRoute, + getGatewayRoute, updateNotebook, } from '../../../utils/notebookUtils'; import { FastifyRequest } from 'fastify'; @@ -27,10 +29,19 @@ export const getNotebookStatus = async ( const notebookName = notebook?.metadata.name; let newNotebook: Notebook; if (isRunning && !notebook?.metadata.annotations?.['opendatahub.io/link']) { - const route = await getRoute(fastify, namespace, notebookName).catch((e) => { - fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); - return undefined; - }); + const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; + let route: Route; + if (!disableServiceMesh) { + route = await getGatewayRoute(fastify, 'istio-system', 'odh-gateway').catch((e) => { + fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); + return undefined; + }); + } else { + route = await getRoute(fastify, namespace, notebookName).catch((e) => { + fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); + return undefined; + }); + } if (route) { newNotebook = await patchNotebookRoute(fastify, route, namespace, notebookName).catch((e) => { fastify.log.warn(`Failed patching route to notebook ${notebookName}: ${e.message}`); @@ -78,7 +89,7 @@ export const patchNotebookRoute = async ( const patch: RecursivePartial = { metadata: { annotations: { - 'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}`, + 'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}/`, }, }, }; diff --git a/backend/src/types.ts b/backend/src/types.ts index 3c85ad8afa..a60397f87b 100644 --- a/backend/src/types.ts +++ b/backend/src/types.ts @@ -27,6 +27,7 @@ export type DashboardConfig = K8sResourceCommon & { disableModelServing: boolean; disableProjectSharing: boolean; disableCustomServingRuntimes: boolean; + disableServiceMesh: boolean; modelMetricsNamespace: string; disablePipelines: boolean; }; @@ -403,6 +404,10 @@ export type Notebook = K8sResourceCommon & { 'opendatahub.io/link': string; // redirect notebook url 'opendatahub.io/username': string; // the untranslated username behind the notebook + // Openshift Service Mesh specific annotations. They're needed to orchestrate additional resources for nb namespaces. + 'opendatahub.io/service-mesh': string; + 'opendatahub.io/hub-host': string; + // TODO: Can we get this from the data in the Notebook?? 'notebooks.opendatahub.io/last-image-selection': string; // the last image they selected 'notebooks.opendatahub.io/last-size-selection': string; // the last notebook size they selected diff --git a/backend/src/utils/constants.ts b/backend/src/utils/constants.ts index 2ac758bde0..45e976bd3c 100644 --- a/backend/src/utils/constants.ts +++ b/backend/src/utils/constants.ts @@ -51,6 +51,7 @@ export const blankDashboardCR: DashboardConfig = { disableModelServing: false, disableProjectSharing: false, disableCustomServingRuntimes: false, + disableServiceMesh: true, modelMetricsNamespace: '', disablePipelines: false, }, diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index 32512311bd..b720cc61cb 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -68,6 +68,54 @@ export const getRoute = async ( return kubeResponse.body as Route; }; +interface RouteListResponse { + apiVersion: string; + kind: string; + metadata: { + resourceVersion: string; + }; + items: Route[]; +} + +export const getGatewayRoute = async ( + fastify: KubeFastifyInstance, + namespace: string, + gatewayName: string, +): Promise => { + const selector = `maistra.io/gateway-name=${gatewayName}`; + const kubeResponse = await fastify.kube.customObjectsApi + .listNamespacedCustomObject( + 'route.openshift.io', + 'v1', + namespace, + 'routes', + undefined, + undefined, + undefined, + selector, + ) + .catch((res) => { + const e = res.response.body; + const error = createCustomError('Error getting Gateway Route', e.message, e.code); + fastify.log.error(error); + throw error; + }); + + const body = kubeResponse.body as unknown; + const typedResponse = body as RouteListResponse; + + if (typedResponse.items.length === 0) { + const error = createCustomError( + 'Route not found', + `Could not find Route with label: ${selector}`, + 404, + ); + fastify.log.error(error); + throw error; + } + return typedResponse.items[0]; +}; + export const createRBAC = async ( fastify: KubeFastifyInstance, namespace: string, @@ -250,6 +298,10 @@ export const assembleNotebook = async ( }, })); + const serviceMeshEnabled = String( + !getDashboardConfig().spec?.dashboardConfig?.disableServiceMesh, + ); + return { apiVersion: 'kubeflow.org/v1', kind: 'Notebook', @@ -265,6 +317,8 @@ export const assembleNotebook = async ( 'notebooks.opendatahub.io/last-size-selection': notebookSize.name, 'notebooks.opendatahub.io/last-image-selection': imageSelection, 'opendatahub.io/username': username, + 'opendatahub.io/service-mesh': serviceMeshEnabled, + 'opendatahub.io/hub-host': url, 'kubeflow-resource-stopped': null, }, name: name, @@ -413,6 +467,8 @@ export const createNotebook = async ( url: string, notebookData?: NotebookData, ): Promise => { + const config = getDashboardConfig(); + if (!notebookData) { const error = createCustomError( 'Missing Notebook', @@ -438,7 +494,14 @@ export const createNotebook = async ( notebookAssembled.metadata.annotations = {}; } - notebookAssembled.metadata.annotations['notebooks.opendatahub.io/inject-oauth'] = 'true'; + notebookAssembled.metadata.annotations['notebooks.opendatahub.io/inject-oauth'] = String( + config.spec.dashboardConfig.disableServiceMesh, + ); + notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String( + !config.spec.dashboardConfig.disableServiceMesh, + ); + notebookAssembled.metadata.annotations['opendatahub.io/hub-url'] = url; + const notebookContainers = notebookAssembled.spec.template.spec.containers; if (!notebookContainers[0]) { diff --git a/docs/dashboard_config.md b/docs/dashboard_config.md index 88f0a405b8..8c9c9b0784 100644 --- a/docs/dashboard_config.md +++ b/docs/dashboard_config.md @@ -23,6 +23,7 @@ The following are a list of features that are supported, along with there defaul | disableProjectSharing | false | Disables Project Sharing from Data Science Projects. | | disableCustomServingRuntimes | false | Disables Custom Serving Runtimes from the Admin Panel. | | modelMetricsNamespace | false | Enables the namespace in which the Model Serving Metrics' Prometheus Operator is installed. | +| disableServiceMesh | true | Disables use of service mesh for routing and authorization. | ## Defaults diff --git a/frontend/src/__mocks__/mockDashboardConfig.ts b/frontend/src/__mocks__/mockDashboardConfig.ts index e59cc5300a..488d2ec583 100644 --- a/frontend/src/__mocks__/mockDashboardConfig.ts +++ b/frontend/src/__mocks__/mockDashboardConfig.ts @@ -40,17 +40,18 @@ export const mockDashboardConfig = ({ spec: { dashboardConfig: { enablement: true, - disableInfo, - disableSupport, - disableClusterManager, - disableTracking, - disableBYONImageStream, - disableISVBadges, - disableAppLauncher, - disableUserManagement, - disableProjects, - disableModelServing, - disableCustomServingRuntimes, + disableInfo: false, + disableSupport: false, + disableClusterManager: false, + disableTracking: true, + disableBYONImageStream: false, + disableISVBadges: false, + disableAppLauncher: false, + disableUserManagement: false, + disableProjects: false, + disableModelServing: false, + disableCustomServingRuntimes: false, + disableServiceMesh: true, modelMetricsNamespace: 'test-project', disablePipelines: false, disableProjectSharing: false, diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 80a87d17fb..57059082f6 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -25,11 +25,13 @@ import { } from '~/concepts/pipelines/elyra/utils'; import { createRoleBinding } from '~/api'; import { Volume, VolumeMount } from '~/types'; +import { DashboardConfig } from '~/types'; import { assemblePodSpecOptions } from './utils'; const assembleNotebook = ( data: StartNotebookData, username: string, + dashboardConfig: DashboardConfig, canEnablePipelines?: boolean, ): NotebookKind => { const { @@ -86,7 +88,13 @@ const assembleNotebook = ( 'notebooks.opendatahub.io/oauth-logout-url': `${origin}/projects/${projectName}?notebookLogout=${notebookId}`, 'notebooks.opendatahub.io/last-size-selection': notebookSize.name, 'notebooks.opendatahub.io/last-image-selection': imageSelection, - 'notebooks.opendatahub.io/inject-oauth': 'true', + 'notebooks.opendatahub.io/inject-oauth': String( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ), + 'opendatahub.io/service-mesh': String( + !dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ), + 'opendatahub.io/hub-url': origin, 'opendatahub.io/username': username, }, name: notebookId, @@ -229,9 +237,10 @@ export const startNotebook = async ( export const createNotebook = ( data: StartNotebookData, username: string, + dashboardConfig: DashboardConfig, canEnablePipelines?: boolean, ): Promise => { - const notebook = assembleNotebook(data, username, canEnablePipelines); + const notebook = assembleNotebook(data, username, dashboardConfig, canEnablePipelines); const notebookPromise = k8sCreateResource({ model: NotebookModel, @@ -251,9 +260,10 @@ export const updateNotebook = ( existingNotebook: NotebookKind, data: StartNotebookData, username: string, + dashboardConfig: DashboardConfig, ): Promise => { data.notebookId = existingNotebook.metadata.name; - const notebook = assembleNotebook(data, username); + const notebook = assembleNotebook(data, username, dashboardConfig); const oldNotebook = structuredClone(existingNotebook); const container = oldNotebook.spec.template.spec.containers[0]; @@ -274,9 +284,10 @@ export const updateNotebook = ( export const createNotebookWithoutStarting = ( data: StartNotebookData, username: string, + dashboardConfig: DashboardConfig, ): Promise => new Promise((resolve, reject) => - createNotebook(data, username).then((notebook) => + createNotebook(data, username, dashboardConfig).then((notebook) => setTimeout( () => stopNotebook(notebook.metadata.name, notebook.metadata.namespace) @@ -286,7 +297,6 @@ export const createNotebookWithoutStarting = ( ), ), ); - export const deleteNotebook = (notebookName: string, namespace: string): Promise => k8sDeleteResource({ model: NotebookModel, diff --git a/frontend/src/api/k8s/routes.ts b/frontend/src/api/k8s/routes.ts index 74d815fc33..f19a3fa196 100644 --- a/frontend/src/api/k8s/routes.ts +++ b/frontend/src/api/k8s/routes.ts @@ -1,6 +1,6 @@ import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils'; import { RouteModel } from '~/api/models'; -import { K8sAPIOptions, RouteKind } from '~/k8sTypes'; +import { K8sAPIOptions, RouteKind, List } from '~/k8sTypes'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; export const getRoute = ( @@ -14,3 +14,22 @@ export const getRoute = ( queryOptions: { name, ns: namespace }, }), ); + +export const getGatewayRoute = ( + namespace: string, + gatewayName: string, +): Promise => { + const labelSelector = `maistra.io/gateway-name=${gatewayName}`; + const queryOptions = { + ns: namespace, + labelSelector, + }; + return k8sGetResource>({ model: RouteModel, queryOptions }) + .then((response) => { + const routes = response.items.filter( + (route) => route.metadata?.labels?.['maistra.io/gateway-name'] === gatewayName, + ); + return routes.length > 0 ? routes[0] : null; + }) + .catch(() => null); +}; diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index dc2b3f209b..f2976afcf8 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -67,6 +67,8 @@ export type NotebookAnnotations = Partial<{ 'notebooks.kubeflow.org/last-activity': string; // datestamp of last use 'opendatahub.io/link': string; // redirect notebook url 'opendatahub.io/username': string; // the untranslated username behind the notebook + 'opendatahub.io/service-mesh': string; // Openshift Service Mesh : determines if mesh configuration should be applied + 'opendatahub.io/hub-url': string; // Openshift Service Mesh : holds origination host for authorization rules 'notebooks.opendatahub.io/last-image-selection': string; // the last image they selected 'notebooks.opendatahub.io/last-size-selection': string; // the last notebook size they selected }>; @@ -407,6 +409,13 @@ export type RouteKind = K8sResourceCommon & { }; }; +export type List = { + apiVersion?: string; + kind?: string; + metadata: Record; + items: T[]; +} & K8sResourceCommon; + export type SecretKind = K8sResourceCommon & { metadata: { name: string; diff --git a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts index 24d6d5740b..07adc668c6 100644 --- a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts +++ b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts @@ -1,6 +1,7 @@ import * as React from 'react'; -import { getRoute } from '~/api'; +import { getGatewayRoute, getRoute } from '~/api'; import { FAST_POLL_INTERVAL } from '~/utilities/const'; +import { useAppContext } from '~/app/AppContext'; const useRouteForNotebook = ( notebookName?: string, @@ -10,6 +11,7 @@ const useRouteForNotebook = ( const [route, setRoute] = React.useState(null); const [loaded, setLoaded] = React.useState(false); const [loadError, setLoadError] = React.useState(null); + const { dashboardConfig } = useAppContext(); React.useEffect(() => { let watchHandle; @@ -19,12 +21,17 @@ const useRouteForNotebook = ( return; } if (notebookName && projectName) { - getRoute(notebookName, projectName) + // if not using service mesh fetch openshift route, otherwise get Istio Ingress Gateway route + const getRoutePromise = dashboardConfig.spec.dashboardConfig.disableServiceMesh + ? getRoute(notebookName, projectName) + : getGatewayRoute('istio-system', 'odh-gateway'); + + getRoutePromise .then((route) => { if (cancelled) { return; } - setRoute(`https://${route.spec.host}/notebook/${projectName}/${notebookName}`); + setRoute(`https://${route?.spec.host}/notebook/${projectName}/${notebookName}/`); setLoadError(null); setLoaded(true); }) @@ -46,7 +53,7 @@ const useRouteForNotebook = ( cancelled = true; clearTimeout(watchHandle); }; - }, [notebookName, projectName, isRunning]); + }, [notebookName, projectName, isRunning, dashboardConfig]); return [route, loaded, loadError]; }; diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index df82b5cb05..15d1424d0d 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -17,7 +17,7 @@ import { } from '~/pages/projects/types'; import { useUser } from '~/redux/selectors'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; -import { AppContext } from '~/app/AppContext'; +import { AppContext, useAppContext } from '~/app/AppContext'; import { fireTrackingEvent } from '~/utilities/segmentIOUtils'; import { createPvcDataForNotebook, @@ -76,6 +76,7 @@ const SpawnerFooter: React.FC = ({ editNotebook, existingDataConnections, ); + const { dashboardConfig } = useAppContext(); const afterStart = (name: string, type: 'created' | 'updated') => { const { gpus, notebookSize, image } = startNotebookData; @@ -149,7 +150,7 @@ const SpawnerFooter: React.FC = ({ envFrom, tolerationSettings, }; - updateNotebook(editNotebook, newStartNotebookData, username) + updateNotebook(editNotebook, newStartNotebookData, username, dashboardConfig) .then((notebook) => afterStart(notebook.metadata.name, 'updated')) .catch(handleError); } @@ -207,7 +208,7 @@ const SpawnerFooter: React.FC = ({ tolerationSettings, }; - createNotebook(newStartData, username, canEnablePipelines) + createNotebook(newStartData, username, dashboardConfig, canEnablePipelines) .then((notebook) => afterStart(notebook.metadata.name, 'created')) .catch(handleError); }; diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 4cfc66ad41..f64dfe710b 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -87,6 +87,7 @@ export type DashboardCommonConfig = { disableModelServing: boolean; disableProjectSharing: boolean; disableCustomServingRuntimes: boolean; + disableServiceMesh: boolean; modelMetricsNamespace: string; disablePipelines: boolean; }; diff --git a/frontend/src/utilities/notebookControllerUtils.ts b/frontend/src/utilities/notebookControllerUtils.ts index e643421dd0..2537b29e96 100644 --- a/frontend/src/utilities/notebookControllerUtils.ts +++ b/frontend/src/utilities/notebookControllerUtils.ts @@ -213,7 +213,7 @@ export const useNotebookRedirectLink = (): (() => Promise) => { const call = (resolve, reject) => { getRoute(notebookNamespace, routeName) .then((route) => { - resolve(`https://${route.spec.host}/notebook/${notebookNamespace}/${routeName}`); + resolve(`https://${route.spec.host}/notebook/${notebookNamespace}/${routeName}/`); }) .catch((e) => { if (backupRoute) { diff --git a/manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml b/manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml index d75f306ee6..58f759c346 100644 --- a/manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml +++ b/manifests/crd/odhdashboardconfigs.opendatahub.io.crd.yaml @@ -49,6 +49,8 @@ spec: type: boolean disableCustomServingRuntimes: type: boolean + disableServiceMesh: + type: boolean modelMetricsNamespace: type: string disablePipelines: diff --git a/manifests/overlays/odhdashboardconfig/odh-dashboard-config.yaml b/manifests/overlays/odhdashboardconfig/odh-dashboard-config.yaml index 159ab96837..616b1232bd 100644 --- a/manifests/overlays/odhdashboardconfig/odh-dashboard-config.yaml +++ b/manifests/overlays/odhdashboardconfig/odh-dashboard-config.yaml @@ -18,6 +18,7 @@ spec: disableModelServing: true disableProjectSharing: true disableCustomServingRuntimes: true + disableServiceMesh: true modelMetricsNamespace: '' notebookController: enabled: true From 1408c29db665cd881e2ef4c65d1a7208af9c70f9 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Tue, 18 Apr 2023 18:58:16 +0200 Subject: [PATCH 02/18] fix: adds mesh annotation to the ns --- backend/src/routes/api/namespaces/namespaceUtils.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/src/routes/api/namespaces/namespaceUtils.ts b/backend/src/routes/api/namespaces/namespaceUtils.ts index 1983bdbb95..c5f7627e33 100644 --- a/backend/src/routes/api/namespaces/namespaceUtils.ts +++ b/backend/src/routes/api/namespaces/namespaceUtils.ts @@ -64,13 +64,16 @@ export const applyNamespaceChange = async ( const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; let labels = {}; + let annotations = {} switch (context) { case NamespaceApplicationCase.DSG_CREATION: labels = { 'opendatahub.io/dashboard': 'true', - 'opendatahub.io/service-mesh': String(!disableServiceMesh), 'modelmesh-enabled': 'true', }; + annotations = { + 'opendatahub.io/service-mesh': String(!disableServiceMesh), + } break; case NamespaceApplicationCase.MODEL_SERVING_PROMOTION: labels = { @@ -82,7 +85,7 @@ export const applyNamespaceChange = async ( } return fastify.kube.coreV1Api - .patchNamespace(name, { metadata: { labels } }, undefined, undefined, undefined, undefined, { + .patchNamespace(name, { metadata: { labels, annotations } }, undefined, undefined, undefined, undefined, { headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH }, }) .then(() => ({ applied: true })) From 166e40950ca9f663db14b284b357dff7d3c52d56 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Tue, 23 May 2023 15:07:13 -0400 Subject: [PATCH 03/18] fix: use project controller created route for ds project routing (#4) * use project annotation for ds project nb routing * chore: passes host instead of the entire route and removes hub-url annotation as it is not needed * remove reference to hub-host --------- Co-authored-by: bartoszmajsak --- .../routes/api/namespaces/namespaceUtils.ts | 18 +++++++++++---- backend/src/routes/api/notebooks/utils.ts | 11 ++++++--- backend/src/types.ts | 1 - backend/src/utils/notebookUtils.ts | 2 -- frontend/src/api/k8s/notebooks.ts | 1 - frontend/src/api/k8s/routes.ts | 23 +++++-------------- frontend/src/k8sTypes.ts | 17 +++++++------- .../projects/notebook/useRouteForNotebook.ts | 10 ++++---- 8 files changed, 41 insertions(+), 42 deletions(-) diff --git a/backend/src/routes/api/namespaces/namespaceUtils.ts b/backend/src/routes/api/namespaces/namespaceUtils.ts index c5f7627e33..96543fe309 100644 --- a/backend/src/routes/api/namespaces/namespaceUtils.ts +++ b/backend/src/routes/api/namespaces/namespaceUtils.ts @@ -64,7 +64,7 @@ export const applyNamespaceChange = async ( const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; let labels = {}; - let annotations = {} + let annotations = {}; switch (context) { case NamespaceApplicationCase.DSG_CREATION: labels = { @@ -73,7 +73,7 @@ export const applyNamespaceChange = async ( }; annotations = { 'opendatahub.io/service-mesh': String(!disableServiceMesh), - } + }; break; case NamespaceApplicationCase.MODEL_SERVING_PROMOTION: labels = { @@ -85,9 +85,17 @@ export const applyNamespaceChange = async ( } return fastify.kube.coreV1Api - .patchNamespace(name, { metadata: { labels, annotations } }, undefined, undefined, undefined, undefined, { - headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH }, - }) + .patchNamespace( + name, + { metadata: { labels, annotations } }, + undefined, + undefined, + undefined, + undefined, + { + headers: { 'Content-type': PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH }, + }, + ) .then(() => ({ applied: true })) .catch((e) => { fastify.log.error( diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts index b7a70888da..4951c30894 100644 --- a/backend/src/routes/api/notebooks/utils.ts +++ b/backend/src/routes/api/notebooks/utils.ts @@ -43,7 +43,12 @@ export const getNotebookStatus = async ( }); } if (route) { - newNotebook = await patchNotebookRoute(fastify, route, namespace, notebookName).catch((e) => { + newNotebook = await patchNotebookRoute( + fastify, + route.spec.host, + namespace, + notebookName, + ).catch((e) => { fastify.log.warn(`Failed patching route to notebook ${notebookName}: ${e.message}`); return notebook; }); @@ -82,14 +87,14 @@ export const checkPodContainersReady = (pod: V1Pod): boolean => { export const patchNotebookRoute = async ( fastify: KubeFastifyInstance, - route: Route, + host: string, namespace: string, name: string, ): Promise => { const patch: RecursivePartial = { metadata: { annotations: { - 'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}/`, + 'opendatahub.io/link': `https://${host}/notebook/${namespace}/${name}/`, }, }, }; diff --git a/backend/src/types.ts b/backend/src/types.ts index a60397f87b..5773bcc69a 100644 --- a/backend/src/types.ts +++ b/backend/src/types.ts @@ -406,7 +406,6 @@ export type Notebook = K8sResourceCommon & { // Openshift Service Mesh specific annotations. They're needed to orchestrate additional resources for nb namespaces. 'opendatahub.io/service-mesh': string; - 'opendatahub.io/hub-host': string; // TODO: Can we get this from the data in the Notebook?? 'notebooks.opendatahub.io/last-image-selection': string; // the last image they selected diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index b720cc61cb..c1fe866181 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -318,7 +318,6 @@ export const assembleNotebook = async ( 'notebooks.opendatahub.io/last-image-selection': imageSelection, 'opendatahub.io/username': username, 'opendatahub.io/service-mesh': serviceMeshEnabled, - 'opendatahub.io/hub-host': url, 'kubeflow-resource-stopped': null, }, name: name, @@ -500,7 +499,6 @@ export const createNotebook = async ( notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String( !config.spec.dashboardConfig.disableServiceMesh, ); - notebookAssembled.metadata.annotations['opendatahub.io/hub-url'] = url; const notebookContainers = notebookAssembled.spec.template.spec.containers; diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 57059082f6..ffa5d20a55 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -94,7 +94,6 @@ const assembleNotebook = ( 'opendatahub.io/service-mesh': String( !dashboardConfig.spec.dashboardConfig.disableServiceMesh, ), - 'opendatahub.io/hub-url': origin, 'opendatahub.io/username': username, }, name: notebookId, diff --git a/frontend/src/api/k8s/routes.ts b/frontend/src/api/k8s/routes.ts index f19a3fa196..69dfc44a66 100644 --- a/frontend/src/api/k8s/routes.ts +++ b/frontend/src/api/k8s/routes.ts @@ -1,6 +1,6 @@ import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils'; -import { RouteModel } from '~/api/models'; -import { K8sAPIOptions, RouteKind, List } from '~/k8sTypes'; +import { RouteModel, NamespaceModel } from '~/api/models'; +import { K8sAPIOptions, RouteKind, NamespaceKind } from '~/k8sTypes'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; export const getRoute = ( @@ -15,21 +15,10 @@ export const getRoute = ( }), ); -export const getGatewayRoute = ( - namespace: string, - gatewayName: string, -): Promise => { - const labelSelector = `maistra.io/gateway-name=${gatewayName}`; +export const getServiceMeshGwHost = async (namespace: string): Promise => { const queryOptions = { - ns: namespace, - labelSelector, + name: namespace, }; - return k8sGetResource>({ model: RouteModel, queryOptions }) - .then((response) => { - const routes = response.items.filter( - (route) => route.metadata?.labels?.['maistra.io/gateway-name'] === gatewayName, - ); - return routes.length > 0 ? routes[0] : null; - }) - .catch(() => null); + const project = await k8sGetResource({ model: NamespaceModel, queryOptions }); + return project?.metadata?.annotations?.['opendatahub.io/service-mesh-gw-host'] || null; }; diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index f2976afcf8..bb22322c93 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -68,7 +68,6 @@ export type NotebookAnnotations = Partial<{ 'opendatahub.io/link': string; // redirect notebook url 'opendatahub.io/username': string; // the untranslated username behind the notebook 'opendatahub.io/service-mesh': string; // Openshift Service Mesh : determines if mesh configuration should be applied - 'opendatahub.io/hub-url': string; // Openshift Service Mesh : holds origination host for authorization rules 'notebooks.opendatahub.io/last-image-selection': string; // the last image they selected 'notebooks.opendatahub.io/last-size-selection': string; // the last notebook size they selected }>; @@ -288,6 +287,15 @@ export type ProjectKind = K8sResourceCommon & { }; }; +export type NamespaceKind = K8sResourceCommon & { + metadata: { + annotations?: DisplayNameAnnotations; + }; + status?: { + phase: 'Active' | 'Terminating'; + }; +}; + export type ServiceAccountKind = K8sResourceCommon & { metadata: { annotations?: DisplayNameAnnotations; @@ -409,13 +417,6 @@ export type RouteKind = K8sResourceCommon & { }; }; -export type List = { - apiVersion?: string; - kind?: string; - metadata: Record; - items: T[]; -} & K8sResourceCommon; - export type SecretKind = K8sResourceCommon & { metadata: { name: string; diff --git a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts index 07adc668c6..ed74373096 100644 --- a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts +++ b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import { getGatewayRoute, getRoute } from '~/api'; +import { getServiceMeshGwHost, getRoute } from '~/api'; import { FAST_POLL_INTERVAL } from '~/utilities/const'; import { useAppContext } from '~/app/AppContext'; @@ -23,15 +23,15 @@ const useRouteForNotebook = ( if (notebookName && projectName) { // if not using service mesh fetch openshift route, otherwise get Istio Ingress Gateway route const getRoutePromise = dashboardConfig.spec.dashboardConfig.disableServiceMesh - ? getRoute(notebookName, projectName) - : getGatewayRoute('istio-system', 'odh-gateway'); + ? getRoute(notebookName, projectName).then((route) => route?.spec.host) + : getServiceMeshGwHost(projectName); getRoutePromise - .then((route) => { + .then((host) => { if (cancelled) { return; } - setRoute(`https://${route?.spec.host}/notebook/${projectName}/${notebookName}/`); + setRoute(`https://${host}/notebook/${projectName}/${notebookName}/`); setLoadError(null); setLoaded(true); }) From 47e43acb17cc8bc5541847a8da7a42c9e128c83a Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Mon, 26 Jun 2023 11:35:16 -0400 Subject: [PATCH 04/18] feat: use annotation to fetch host for notebook routing (#5) * update existing annotation reference * use ns annotation in backend route fetching --- backend/src/routes/api/notebooks/utils.ts | 20 ++++----- backend/src/utils/notebookUtils.ts | 52 ++++++++--------------- frontend/src/api/k8s/routes.ts | 5 ++- 3 files changed, 30 insertions(+), 47 deletions(-) diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts index 4951c30894..04fc2e8b27 100644 --- a/backend/src/routes/api/notebooks/utils.ts +++ b/backend/src/routes/api/notebooks/utils.ts @@ -10,7 +10,7 @@ import { getNamespaces, getNotebook, getRoute, - getGatewayRoute, + getServiceMeshGwHost, updateNotebook, } from '../../../utils/notebookUtils'; import { FastifyRequest } from 'fastify'; @@ -30,25 +30,23 @@ export const getNotebookStatus = async ( let newNotebook: Notebook; if (isRunning && !notebook?.metadata.annotations?.['opendatahub.io/link']) { const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; - let route: Route; + let host: string; if (!disableServiceMesh) { - route = await getGatewayRoute(fastify, 'istio-system', 'odh-gateway').catch((e) => { + host = await getServiceMeshGwHost(fastify, namespace).catch((e) => { fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); return undefined; }); } else { - route = await getRoute(fastify, namespace, notebookName).catch((e) => { + const route = await getRoute(fastify, namespace, notebookName).catch((e) => { fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); return undefined; }); + if (route) { + host = route.spec.host; + } } - if (route) { - newNotebook = await patchNotebookRoute( - fastify, - route.spec.host, - namespace, - notebookName, - ).catch((e) => { + if (host) { + newNotebook = await patchNotebookRoute(fastify, host, namespace, notebookName).catch((e) => { fastify.log.warn(`Failed patching route to notebook ${notebookName}: ${e.message}`); return notebook; }); diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index c1fe866181..8649352f51 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -21,6 +21,7 @@ import { getUserName, usernameTranslate } from './userUtils'; import { createCustomError } from './requestUtils'; import { PatchUtils, + V1Namespace, V1PersistentVolumeClaim, V1Role, V1RoleBinding, @@ -68,52 +69,33 @@ export const getRoute = async ( return kubeResponse.body as Route; }; -interface RouteListResponse { - apiVersion: string; - kind: string; - metadata: { - resourceVersion: string; - }; - items: Route[]; -} - -export const getGatewayRoute = async ( +export const getServiceMeshGwHost = async ( fastify: KubeFastifyInstance, namespace: string, - gatewayName: string, -): Promise => { - const selector = `maistra.io/gateway-name=${gatewayName}`; - const kubeResponse = await fastify.kube.customObjectsApi - .listNamespacedCustomObject( - 'route.openshift.io', - 'v1', - namespace, - 'routes', - undefined, - undefined, - undefined, - selector, - ) - .catch((res) => { - const e = res.response.body; - const error = createCustomError('Error getting Gateway Route', e.message, e.code); - fastify.log.error(error); - throw error; - }); +): Promise => { + const kubeResponse = await fastify.kube.coreV1Api.readNamespace(namespace).catch((res) => { + const e = res.response.body; + const error = createCustomError('Error getting Namespace', e.message, e.code); + fastify.log.error(error); + throw error; + }); const body = kubeResponse.body as unknown; - const typedResponse = body as RouteListResponse; + const typedResponse = body as V1Namespace; - if (typedResponse.items.length === 0) { + const annotations = typedResponse.metadata?.annotations; + + if (!annotations || !annotations['service-mesh.opendatahub.io/public-gateway-host-external']) { const error = createCustomError( - 'Route not found', - `Could not find Route with label: ${selector}`, + 'Annotation not found', + `Could not find annotation 'service-mesh.opendatahub.io/public-gateway-host-external' for namespace: ${namespace}`, 404, ); fastify.log.error(error); throw error; } - return typedResponse.items[0]; + + return annotations['service-mesh.opendatahub.io/public-gateway-host-external']; }; export const createRBAC = async ( diff --git a/frontend/src/api/k8s/routes.ts b/frontend/src/api/k8s/routes.ts index 69dfc44a66..7ce435125e 100644 --- a/frontend/src/api/k8s/routes.ts +++ b/frontend/src/api/k8s/routes.ts @@ -20,5 +20,8 @@ export const getServiceMeshGwHost = async (namespace: string): Promise({ model: NamespaceModel, queryOptions }); - return project?.metadata?.annotations?.['opendatahub.io/service-mesh-gw-host'] || null; + return ( + project?.metadata?.annotations?.['service-mesh.opendatahub.io/public-gateway-host-external'] || + null + ); }; From dd22cd76e6aa2e2158829365ca209949c0549264 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Wed, 28 Jun 2023 01:09:42 -0400 Subject: [PATCH 05/18] fix: patch notebook annotations (#6) --- backend/src/utils/notebookUtils.ts | 1 + frontend/src/api/k8s/notebooks.ts | 20 +++++++++++++++++-- .../notebook/NotebookStatusToggle.tsx | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index 8649352f51..e68c0fe8b7 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -298,6 +298,7 @@ export const assembleNotebook = async ( 'notebooks.opendatahub.io/oauth-logout-url': `${url}/notebookController/${translatedUsername}/home`, 'notebooks.opendatahub.io/last-size-selection': notebookSize.name, 'notebooks.opendatahub.io/last-image-selection': imageSelection, + 'notebooks.opendatahub.io/inject-oauth': String(!serviceMeshEnabled), 'opendatahub.io/username': username, 'opendatahub.io/service-mesh': serviceMeshEnabled, 'kubeflow-resource-stopped': null, diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index ffa5d20a55..142e991732 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -181,6 +181,18 @@ const getStopPatch = (): Patch => ({ value: getStopPatchDataString(), }); +const getInjectOAuthPatch = (disableServiceMesh: boolean): Patch => ({ + op: 'add', + path: '/metadata/annotations/notebooks.opendatahub.io~1inject-oauth', + value: String(disableServiceMesh), +}); + +const getServiceMeshPatch = (disableServiceMesh: boolean): Patch => ({ + op: 'add', + path: '/metadata/annotations/opendatahub.io~1service-mesh', + value: String(!disableServiceMesh), +}); + export const getNotebooks = (namespace: string): Promise => k8sListResource({ model: NotebookModel, @@ -204,10 +216,14 @@ export const startNotebook = async ( name: string, namespace: string, tolerationChanges: TolerationChanges, + dashboardConfig: DashboardConfig, enablePipelines?: boolean, ): Promise => { - const patches: Patch[] = []; - patches.push(startPatch); + const patches: Patch[] = [ + startPatch, + getInjectOAuthPatch(dashboardConfig.spec.dashboardConfig.disableServiceMesh), + getServiceMeshPatch(dashboardConfig.spec.dashboardConfig.disableServiceMesh), + ]; const tolerationPatch = getTolerationPatch(tolerationChanges); if (tolerationPatch) { diff --git a/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx b/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx index 83b54db070..87ffeaa19c 100644 --- a/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx +++ b/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx @@ -102,6 +102,7 @@ const NotebookStatusToggle: React.FC = ({ notebookName, notebookNamespace, tolerationSettings, + dashboardConfig, enablePipelines && !currentlyHasPipelines(notebook), ).then(() => { fireNotebookTrackingEvent('started'); From c917d76b667f74d4f74dc90294580372c5f192fc Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Thu, 29 Jun 2023 10:34:27 -0400 Subject: [PATCH 06/18] Linting fixes --- backend/src/routes/api/notebooks/utils.ts | 2 +- frontend/src/__mocks__/mockDashboardConfig.ts | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts index 04fc2e8b27..9b71226a99 100644 --- a/backend/src/routes/api/notebooks/utils.ts +++ b/backend/src/routes/api/notebooks/utils.ts @@ -1,4 +1,4 @@ -import { KubeFastifyInstance, Notebook, NotebookData, Route } from '../../../types'; +import { KubeFastifyInstance, Notebook, NotebookData } from '../../../types'; import { PatchUtils, V1ContainerStatus, V1Pod, V1PodList } from '@kubernetes/client-node'; import { createCustomError } from '../../../utils/requestUtils'; import { getUserName } from '../../../utils/userUtils'; diff --git a/frontend/src/__mocks__/mockDashboardConfig.ts b/frontend/src/__mocks__/mockDashboardConfig.ts index 488d2ec583..2920fb8713 100644 --- a/frontend/src/__mocks__/mockDashboardConfig.ts +++ b/frontend/src/__mocks__/mockDashboardConfig.ts @@ -40,17 +40,17 @@ export const mockDashboardConfig = ({ spec: { dashboardConfig: { enablement: true, - disableInfo: false, - disableSupport: false, - disableClusterManager: false, - disableTracking: true, - disableBYONImageStream: false, - disableISVBadges: false, - disableAppLauncher: false, - disableUserManagement: false, - disableProjects: false, - disableModelServing: false, - disableCustomServingRuntimes: false, + disableInfo, + disableSupport, + disableClusterManager, + disableTracking, + disableBYONImageStream, + disableISVBadges, + disableAppLauncher, + disableUserManagement, + disableProjects, + disableModelServing, + disableCustomServingRuntimes, disableServiceMesh: true, modelMetricsNamespace: 'test-project', disablePipelines: false, From 93aa3aefa20b2ee3ac289a40a56d7256cd682672 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Wed, 2 Aug 2023 10:28:23 -0400 Subject: [PATCH 07/18] add featureFlagEnabled() to backend, use for SM --- .../routes/api/namespaces/namespaceUtils.ts | 9 ++++-- backend/src/routes/api/notebooks/utils.ts | 12 ++++---- backend/src/utils/notebookUtils.ts | 12 ++++---- backend/src/utils/resourceUtils.ts | 3 ++ frontend/src/api/k8s/notebooks.ts | 29 +++++++++++-------- .../projects/notebook/useRouteForNotebook.ts | 6 +++- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/backend/src/routes/api/namespaces/namespaceUtils.ts b/backend/src/routes/api/namespaces/namespaceUtils.ts index 96543fe309..f2a99507e5 100644 --- a/backend/src/routes/api/namespaces/namespaceUtils.ts +++ b/backend/src/routes/api/namespaces/namespaceUtils.ts @@ -2,7 +2,7 @@ import { PatchUtils, V1SelfSubjectAccessReview } from '@kubernetes/client-node'; import { NamespaceApplicationCase } from './const'; import { K8sStatus, KubeFastifyInstance, OauthFastifyRequest } from '../../../types'; import { createCustomError } from '../../../utils/requestUtils'; -import { getDashboardConfig } from '../../../utils/resourceUtils'; +import { featureFlagEnabled, getDashboardConfig } from '../../../utils/resourceUtils'; import { isK8sStatus, safeURLPassThrough } from '../k8s/pass-through'; const checkNamespacePermission = ( @@ -61,7 +61,10 @@ export const applyNamespaceChange = async ( throw createCustomError('Forbidden', "You don't have the access to update the namespace", 403); } - const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; + // calling featureFlagEnabled to set the bool to false if it's set to anything but false ('true', undefined, etc) + const enableServiceMesh = featureFlagEnabled( + getDashboardConfig().spec.dashboardConfig.disableServiceMesh, + ); let labels = {}; let annotations = {}; @@ -72,7 +75,7 @@ export const applyNamespaceChange = async ( 'modelmesh-enabled': 'true', }; annotations = { - 'opendatahub.io/service-mesh': String(!disableServiceMesh), + 'opendatahub.io/service-mesh': String(enableServiceMesh), }; break; case NamespaceApplicationCase.MODEL_SERVING_PROMOTION: diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts index 9b71226a99..58402891b3 100644 --- a/backend/src/routes/api/notebooks/utils.ts +++ b/backend/src/routes/api/notebooks/utils.ts @@ -3,7 +3,7 @@ import { PatchUtils, V1ContainerStatus, V1Pod, V1PodList } from '@kubernetes/cli import { createCustomError } from '../../../utils/requestUtils'; import { getUserName } from '../../../utils/userUtils'; import { RecursivePartial } from '../../../typeHelpers'; -import { getDashboardConfig } from '../../../utils/resourceUtils'; +import { featureFlagEnabled, getDashboardConfig } from '../../../utils/resourceUtils'; import { createNotebook, generateNotebookNameFromUsername, @@ -29,9 +29,11 @@ export const getNotebookStatus = async ( const notebookName = notebook?.metadata.name; let newNotebook: Notebook; if (isRunning && !notebook?.metadata.annotations?.['opendatahub.io/link']) { - const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh; + const enableServiceMesh = featureFlagEnabled( + getDashboardConfig().spec.dashboardConfig.disableServiceMesh, + ); let host: string; - if (!disableServiceMesh) { + if (enableServiceMesh) { host = await getServiceMeshGwHost(fastify, namespace).catch((e) => { fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); return undefined; @@ -41,9 +43,7 @@ export const getNotebookStatus = async ( fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`); return undefined; }); - if (route) { - host = route.spec.host; - } + host = route?.spec.host; } if (host) { newNotebook = await patchNotebookRoute(fastify, host, namespace, notebookName).catch((e) => { diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index e68c0fe8b7..bd764f7474 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -1,4 +1,4 @@ -import { getDashboardConfig } from './resourceUtils'; +import { featureFlagEnabled, getDashboardConfig } from './resourceUtils'; import { EnvironmentVariable, ImageInfo, @@ -281,7 +281,7 @@ export const assembleNotebook = async ( })); const serviceMeshEnabled = String( - !getDashboardConfig().spec?.dashboardConfig?.disableServiceMesh, + !featureFlagEnabled(getDashboardConfig().spec?.dashboardConfig?.disableServiceMesh), ); return { @@ -476,12 +476,12 @@ export const createNotebook = async ( notebookAssembled.metadata.annotations = {}; } + const enableServiceMesh = featureFlagEnabled(config.spec.dashboardConfig.disableServiceMesh); + notebookAssembled.metadata.annotations['notebooks.opendatahub.io/inject-oauth'] = String( - config.spec.dashboardConfig.disableServiceMesh, - ); - notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String( - !config.spec.dashboardConfig.disableServiceMesh, + !enableServiceMesh, ); + notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String(enableServiceMesh); const notebookContainers = notebookAssembled.spec.template.spec.containers; diff --git a/backend/src/utils/resourceUtils.ts b/backend/src/utils/resourceUtils.ts index ee59a38b2e..29726bfeaa 100644 --- a/backend/src/utils/resourceUtils.ts +++ b/backend/src/utils/resourceUtils.ts @@ -565,6 +565,9 @@ export const getDashboardConfig = (): DashboardConfig => { return dashboardConfigWatcher.getResources()?.[0]; }; +export const featureFlagEnabled = (disabledSettingState?: boolean): boolean => + disabledSettingState === false; + export const updateDashboardConfig = (): Promise => { return dashboardConfigWatcher.updateResults(); }; diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 142e991732..e0315c0349 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -26,6 +26,7 @@ import { import { createRoleBinding } from '~/api'; import { Volume, VolumeMount } from '~/types'; import { DashboardConfig } from '~/types'; +import { featureFlagEnabled } from '~/utilities/utils'; import { assemblePodSpecOptions } from './utils'; const assembleNotebook = ( @@ -72,6 +73,10 @@ const assembleNotebook = ( volumeMounts.push(getElyraVolumeMount()); } + const enableServiceMesh = featureFlagEnabled( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ); + return { apiVersion: 'kubeflow.org/v1', kind: 'Notebook', @@ -88,12 +93,8 @@ const assembleNotebook = ( 'notebooks.opendatahub.io/oauth-logout-url': `${origin}/projects/${projectName}?notebookLogout=${notebookId}`, 'notebooks.opendatahub.io/last-size-selection': notebookSize.name, 'notebooks.opendatahub.io/last-image-selection': imageSelection, - 'notebooks.opendatahub.io/inject-oauth': String( - dashboardConfig.spec.dashboardConfig.disableServiceMesh, - ), - 'opendatahub.io/service-mesh': String( - !dashboardConfig.spec.dashboardConfig.disableServiceMesh, - ), + 'notebooks.opendatahub.io/inject-oauth': String(!enableServiceMesh), + 'opendatahub.io/service-mesh': String(enableServiceMesh), 'opendatahub.io/username': username, }, name: notebookId, @@ -181,16 +182,16 @@ const getStopPatch = (): Patch => ({ value: getStopPatchDataString(), }); -const getInjectOAuthPatch = (disableServiceMesh: boolean): Patch => ({ +const getInjectOAuthPatch = (enableServiceMesh: boolean): Patch => ({ op: 'add', path: '/metadata/annotations/notebooks.opendatahub.io~1inject-oauth', - value: String(disableServiceMesh), + value: String(!enableServiceMesh), }); -const getServiceMeshPatch = (disableServiceMesh: boolean): Patch => ({ +const getServiceMeshPatch = (enableServiceMesh: boolean): Patch => ({ op: 'add', path: '/metadata/annotations/opendatahub.io~1service-mesh', - value: String(!disableServiceMesh), + value: String(enableServiceMesh), }); export const getNotebooks = (namespace: string): Promise => @@ -219,10 +220,14 @@ export const startNotebook = async ( dashboardConfig: DashboardConfig, enablePipelines?: boolean, ): Promise => { + const enableServiceMesh = featureFlagEnabled( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ); + const patches: Patch[] = [ startPatch, - getInjectOAuthPatch(dashboardConfig.spec.dashboardConfig.disableServiceMesh), - getServiceMeshPatch(dashboardConfig.spec.dashboardConfig.disableServiceMesh), + getInjectOAuthPatch(enableServiceMesh), + getServiceMeshPatch(enableServiceMesh), ]; const tolerationPatch = getTolerationPatch(tolerationChanges); diff --git a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts index ed74373096..431de0c87c 100644 --- a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts +++ b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts @@ -2,6 +2,7 @@ import * as React from 'react'; import { getServiceMeshGwHost, getRoute } from '~/api'; import { FAST_POLL_INTERVAL } from '~/utilities/const'; import { useAppContext } from '~/app/AppContext'; +import { featureFlagEnabled } from '~/utilities/utils'; const useRouteForNotebook = ( notebookName?: string, @@ -21,8 +22,11 @@ const useRouteForNotebook = ( return; } if (notebookName && projectName) { + const enableServiceMesh = featureFlagEnabled( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ); // if not using service mesh fetch openshift route, otherwise get Istio Ingress Gateway route - const getRoutePromise = dashboardConfig.spec.dashboardConfig.disableServiceMesh + const getRoutePromise = !enableServiceMesh ? getRoute(notebookName, projectName).then((route) => route?.spec.host) : getServiceMeshGwHost(projectName); From abf520dbb2a664e00947d6effd2d225c2c4480b8 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Thu, 24 Aug 2023 14:35:06 -0400 Subject: [PATCH 08/18] explicitly set sidecar injection annotation --- backend/src/utils/notebookUtils.ts | 1 + frontend/src/api/k8s/notebooks.ts | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index cf26ef3933..2e5233465f 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -488,6 +488,7 @@ export const createNotebook = async ( !enableServiceMesh, ); notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String(enableServiceMesh); + notebookAssembled.metadata.annotations['sidecar.istio.io/inject'] = String(enableServiceMesh); const notebookContainers = notebookAssembled.spec.template.spec.containers; diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index ee295a729d..5f0b966d58 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -118,6 +118,7 @@ const assembleNotebook = ( 'notebooks.opendatahub.io/last-image-selection': imageSelection, 'notebooks.opendatahub.io/inject-oauth': String(!enableServiceMesh), 'opendatahub.io/service-mesh': String(enableServiceMesh), + 'sidecar.istio.io/inject': String(enableServiceMesh), 'opendatahub.io/username': username, }, name: notebookId, @@ -211,6 +212,12 @@ const getInjectOAuthPatch = (enableServiceMesh: boolean): Patch => ({ value: String(!enableServiceMesh), }); +const getProxyInjectPatch = (enableServiceMesh: boolean): Patch => ({ + op: 'add', + path: '/metadata/annotations/sidecar.istio.io~1inject', + value: String(enableServiceMesh), +}); + const getServiceMeshPatch = (enableServiceMesh: boolean): Patch => ({ op: 'add', path: '/metadata/annotations/opendatahub.io~1service-mesh', @@ -251,6 +258,7 @@ export const startNotebook = async ( startPatch, getInjectOAuthPatch(enableServiceMesh), getServiceMeshPatch(enableServiceMesh), + getProxyInjectPatch(enableServiceMesh), ]; const tolerationPatch = getTolerationPatch(tolerationChanges); From 5b26e6445f8aa0bbce0254fac965bac8a29d7491 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Mon, 28 Aug 2023 16:19:29 -0400 Subject: [PATCH 09/18] change to istio inject label from anno --- backend/src/utils/notebookUtils.ts | 3 ++- frontend/src/api/k8s/notebooks.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index 2e5233465f..bf7a949cee 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -299,6 +299,7 @@ export const assembleNotebook = async ( 'opendatahub.io/odh-managed': 'true', 'opendatahub.io/user': translatedUsername, 'opendatahub.io/dashboard': 'true', + 'sidecar.istio.io/inject': String(serviceMeshEnabled), }, annotations: { 'notebooks.opendatahub.io/oauth-logout-url': `${url}/notebookController/${translatedUsername}/home`, @@ -488,7 +489,7 @@ export const createNotebook = async ( !enableServiceMesh, ); notebookAssembled.metadata.annotations['opendatahub.io/service-mesh'] = String(enableServiceMesh); - notebookAssembled.metadata.annotations['sidecar.istio.io/inject'] = String(enableServiceMesh); + notebookAssembled.metadata.labels['sidecar.istio.io/inject'] = String(enableServiceMesh); const notebookContainers = notebookAssembled.spec.template.spec.containers; diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 5f0b966d58..84bf3ff32c 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -109,6 +109,7 @@ const assembleNotebook = ( 'opendatahub.io/odh-managed': 'true', 'opendatahub.io/user': translatedUsername, [KnownLabels.DASHBOARD_RESOURCE]: 'true', + 'sidecar.istio.io/inject': String(enableServiceMesh), }, annotations: { 'openshift.io/display-name': notebookName.trim(), @@ -118,7 +119,6 @@ const assembleNotebook = ( 'notebooks.opendatahub.io/last-image-selection': imageSelection, 'notebooks.opendatahub.io/inject-oauth': String(!enableServiceMesh), 'opendatahub.io/service-mesh': String(enableServiceMesh), - 'sidecar.istio.io/inject': String(enableServiceMesh), 'opendatahub.io/username': username, }, name: notebookId, @@ -214,7 +214,7 @@ const getInjectOAuthPatch = (enableServiceMesh: boolean): Patch => ({ const getProxyInjectPatch = (enableServiceMesh: boolean): Patch => ({ op: 'add', - path: '/metadata/annotations/sidecar.istio.io~1inject', + path: '/metadata/labels/sidecar.istio.io~1inject', value: String(enableServiceMesh), }); From 8376c2818e44bd39b6f995b25aaa4df9bdacea5a Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Thu, 21 Sep 2023 16:05:51 -0400 Subject: [PATCH 10/18] remove namespacekind, use projectkind --- frontend/src/api/k8s/routes.ts | 16 +++++++++------- frontend/src/k8sTypes.ts | 9 --------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/frontend/src/api/k8s/routes.ts b/frontend/src/api/k8s/routes.ts index 7ce435125e..21c78bb733 100644 --- a/frontend/src/api/k8s/routes.ts +++ b/frontend/src/api/k8s/routes.ts @@ -1,6 +1,6 @@ import { k8sGetResource } from '@openshift/dynamic-plugin-sdk-utils'; -import { RouteModel, NamespaceModel } from '~/api/models'; -import { K8sAPIOptions, RouteKind, NamespaceKind } from '~/k8sTypes'; +import { RouteModel, ProjectModel } from '~/api/models'; +import { K8sAPIOptions, RouteKind, ProjectKind } from '~/k8sTypes'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; export const getRoute = ( @@ -19,9 +19,11 @@ export const getServiceMeshGwHost = async (namespace: string): Promise({ model: NamespaceModel, queryOptions }); - return ( - project?.metadata?.annotations?.['service-mesh.opendatahub.io/public-gateway-host-external'] || - null - ); + return k8sGetResource({ model: ProjectModel, queryOptions }) + .then(project => { + return project?.metadata?.annotations?.['service-mesh.opendatahub.io/public-gateway-host-external'] || null; + }) + .catch(error => { + return error; + }); }; diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index 93e7115faa..8022ae0d7a 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -300,15 +300,6 @@ export type ProjectKind = K8sResourceCommon & { }; }; -export type NamespaceKind = K8sResourceCommon & { - metadata: { - annotations?: DisplayNameAnnotations; - }; - status?: { - phase: 'Active' | 'Terminating'; - }; -}; - export type ServiceAccountKind = K8sResourceCommon & { metadata: { annotations?: DisplayNameAnnotations; From 48bfe4eb86089d8e486517cd9d34b594c91a62dc Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Thu, 21 Sep 2023 16:12:31 -0400 Subject: [PATCH 11/18] lint getSMGwHost() changes --- frontend/src/api/k8s/routes.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/src/api/k8s/routes.ts b/frontend/src/api/k8s/routes.ts index 21c78bb733..19adaa849f 100644 --- a/frontend/src/api/k8s/routes.ts +++ b/frontend/src/api/k8s/routes.ts @@ -20,10 +20,11 @@ export const getServiceMeshGwHost = async (namespace: string): Promise({ model: ProjectModel, queryOptions }) - .then(project => { - return project?.metadata?.annotations?.['service-mesh.opendatahub.io/public-gateway-host-external'] || null; - }) - .catch(error => { - return error; - }); + .then( + (project) => + project?.metadata?.annotations?.[ + 'service-mesh.opendatahub.io/public-gateway-host-external' + ] || null, + ) + .catch((error) => error); }; From 6b12f82bb750d4604ab41ab616b5ce133bd854f1 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Thu, 21 Sep 2023 16:20:48 -0400 Subject: [PATCH 12/18] move getDashboardConfig call, remove unnec constant --- backend/src/utils/notebookUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index bf7a949cee..1ed2e99483 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -456,8 +456,6 @@ export const createNotebook = async ( url: string, notebookData?: NotebookData, ): Promise => { - const config = getDashboardConfig(); - if (!notebookData) { const error = createCustomError( 'Missing Notebook', @@ -483,7 +481,9 @@ export const createNotebook = async ( notebookAssembled.metadata.annotations = {}; } - const enableServiceMesh = featureFlagEnabled(config.spec.dashboardConfig.disableServiceMesh); + const enableServiceMesh = featureFlagEnabled( + getDashboardConfig().spec.dashboardConfig.disableServiceMesh, + ); notebookAssembled.metadata.annotations['notebooks.opendatahub.io/inject-oauth'] = String( !enableServiceMesh, From ca51ef37fc25d64af1321ebfa40478a10b4fafc7 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Fri, 22 Sep 2023 09:54:09 -0400 Subject: [PATCH 13/18] simplify annotations fetch --- backend/src/utils/notebookUtils.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts index 1ed2e99483..867f68e3bc 100644 --- a/backend/src/utils/notebookUtils.ts +++ b/backend/src/utils/notebookUtils.ts @@ -21,7 +21,6 @@ import { getUserName, usernameTranslate } from './userUtils'; import { createCustomError } from './requestUtils'; import { PatchUtils, - V1Namespace, V1PersistentVolumeClaim, V1Role, V1RoleBinding, @@ -80,10 +79,7 @@ export const getServiceMeshGwHost = async ( throw error; }); - const body = kubeResponse.body as unknown; - const typedResponse = body as V1Namespace; - - const annotations = typedResponse.metadata?.annotations; + const annotations = kubeResponse.body.metadata?.annotations; if (!annotations || !annotations['service-mesh.opendatahub.io/public-gateway-host-external']) { const error = createCustomError( From 74b4c79c8c2e8b11c5e759bd158a7dc4462de224 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Fri, 22 Sep 2023 10:49:14 -0400 Subject: [PATCH 14/18] pass parsed SM flag instead ofdash config --- frontend/src/api/k8s/notebooks.ts | 18 +++++++----------- .../projects/screens/spawner/SpawnerFooter.tsx | 8 ++++++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 9730086836..601a6db204 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -33,7 +33,7 @@ import { assemblePodSpecOptions, getshmVolume, getshmVolumeMount } from './utils const assembleNotebook = ( data: StartNotebookData, username: string, - dashboardConfig: DashboardConfig, + enableServiceMesh: boolean, canEnablePipelines?: boolean, ): NotebookKind => { const { @@ -86,10 +86,6 @@ const assembleNotebook = ( volumeMounts.push(getshmVolumeMount()); } - const enableServiceMesh = featureFlagEnabled( - dashboardConfig.spec.dashboardConfig.disableServiceMesh, - ); - return { apiVersion: 'kubeflow.org/v1', kind: 'Notebook', @@ -278,10 +274,10 @@ export const startNotebook = async ( export const createNotebook = ( data: StartNotebookData, username: string, - dashboardConfig: DashboardConfig, + enableServiceMesh: boolean, canEnablePipelines?: boolean, ): Promise => { - const notebook = assembleNotebook(data, username, dashboardConfig, canEnablePipelines); + const notebook = assembleNotebook(data, username, enableServiceMesh, canEnablePipelines); const notebookPromise = k8sCreateResource({ model: NotebookModel, @@ -301,10 +297,10 @@ export const updateNotebook = ( existingNotebook: NotebookKind, data: StartNotebookData, username: string, - dashboardConfig: DashboardConfig, + enableServiceMesh: boolean, ): Promise => { data.notebookId = existingNotebook.metadata.name; - const notebook = assembleNotebook(data, username, dashboardConfig); + const notebook = assembleNotebook(data, username, enableServiceMesh); const oldNotebook = structuredClone(existingNotebook); const container = oldNotebook.spec.template.spec.containers[0]; @@ -325,10 +321,10 @@ export const updateNotebook = ( export const createNotebookWithoutStarting = ( data: StartNotebookData, username: string, - dashboardConfig: DashboardConfig, + enableServiceMesh: boolean, ): Promise => new Promise((resolve, reject) => - createNotebook(data, username, dashboardConfig).then((notebook) => + createNotebook(data, username, enableServiceMesh).then((notebook) => setTimeout( () => stopNotebook(notebook.metadata.name, notebook.metadata.namespace) diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index 50178173c9..719c522a6a 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -19,6 +19,7 @@ import { useUser } from '~/redux/selectors'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { AppContext, useAppContext } from '~/app/AppContext'; import { fireTrackingEvent } from '~/utilities/segmentIOUtils'; +import { featureFlagEnabled } from '~/utilities/utils'; import { createPvcDataForNotebook, createConfigMapsAndSecretsForNotebook, @@ -77,6 +78,9 @@ const SpawnerFooter: React.FC = ({ existingDataConnections, ); const { dashboardConfig } = useAppContext(); + const enableServiceMesh = featureFlagEnabled( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ); const afterStart = (name: string, type: 'created' | 'updated') => { const { gpus, notebookSize, image } = startNotebookData; @@ -150,7 +154,7 @@ const SpawnerFooter: React.FC = ({ envFrom, tolerationSettings, }; - updateNotebook(editNotebook, newStartNotebookData, username, dashboardConfig) + updateNotebook(editNotebook, newStartNotebookData, username, enableServiceMesh) .then((notebook) => afterStart(notebook.metadata.name, 'updated')) .catch(handleError); } @@ -208,7 +212,7 @@ const SpawnerFooter: React.FC = ({ tolerationSettings, }; - createNotebook(newStartData, username, dashboardConfig, canEnablePipelines) + createNotebook(newStartData, username, enableServiceMesh, canEnablePipelines) .then((notebook) => afterStart(notebook.metadata.name, 'created')) .catch(handleError); }; From b55490f7e2eb43dbdd6db2032cae2551047832a8 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Mon, 25 Sep 2023 15:10:54 -0400 Subject: [PATCH 15/18] add sm manifests changes --- ...esh_overlay-delete-oauth-config-patch.yaml | 6 +++ ...esh_overlay_delete-oauth-client-patch.yaml | 6 +++ manifests/overlays/minimal/README.md | 4 ++ .../minimal/deployment-resources-patch.yaml | 12 +++++ manifests/overlays/minimal/kustomization.yaml | 13 +++++ manifests/overlays/service-mesh/OWNERS | 10 ++++ .../overlays/service-mesh/kustomization.yaml | 47 +++++++++++++++++++ ...esh_overlay-delete-oauth-config-patch.yaml | 6 +++ ...esh_overlay_delete-oauth-client-patch.yaml | 5 ++ .../patches/deployment-patch.yaml | 13 +++++ .../patches/oauth-client-patch.yaml | 5 ++ .../patches/oauth-config-patch.yaml | 5 ++ .../service-mesh/patches/route-patch.yaml | 9 ++++ .../patches/service-account-patch.yaml | 2 + .../service-mesh/patches/service-patch.yaml | 9 ++++ 15 files changed, 152 insertions(+) create mode 100644 manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml create mode 100644 manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml create mode 100644 manifests/overlays/minimal/README.md create mode 100644 manifests/overlays/minimal/deployment-resources-patch.yaml create mode 100644 manifests/overlays/minimal/kustomization.yaml create mode 100644 manifests/overlays/service-mesh/OWNERS create mode 100644 manifests/overlays/service-mesh/kustomization.yaml create mode 100644 manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/deployment-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/oauth-client-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/oauth-config-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/route-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/service-account-patch.yaml create mode 100644 manifests/overlays/service-mesh/patches/service-patch.yaml diff --git a/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml b/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml new file mode 100644 index 0000000000..bb8cdf6360 --- /dev/null +++ b/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml @@ -0,0 +1,6 @@ +# DO NOT DELETE. Check kustomization.yaml comments in overlays/service-mesh +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-config diff --git a/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml b/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml new file mode 100644 index 0000000000..3fe30a1475 --- /dev/null +++ b/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml @@ -0,0 +1,6 @@ +# DO NOT DELETE. Check kustomization.yaml comments in overlays/service-mesh +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-client diff --git a/manifests/overlays/minimal/README.md b/manifests/overlays/minimal/README.md new file mode 100644 index 0000000000..d43bde3167 --- /dev/null +++ b/manifests/overlays/minimal/README.md @@ -0,0 +1,4 @@ +## Dashboard Minimal overlay + +This overlay is specifically used to trim deployment options such as replica count and probes times. +Might be useful for resource-constrained environments such as [Openshift Local](https://developers.redhat.com/products/openshift-local/overview) (aka `crc`). \ No newline at end of file diff --git a/manifests/overlays/minimal/deployment-resources-patch.yaml b/manifests/overlays/minimal/deployment-resources-patch.yaml new file mode 100644 index 0000000000..447c4dd4da --- /dev/null +++ b/manifests/overlays/minimal/deployment-resources-patch.yaml @@ -0,0 +1,12 @@ +# these three replaces are for testing on less powerful clusters (crc) +- op: replace + path: /spec/replicas + value: 1 + +- op: replace + path: /spec/template/spec/containers/0/livenessProbe/periodSeconds + value: 300 + +- op: replace + path: /spec/template/spec/containers/0/readinessProbe/periodSeconds + value: 300 diff --git a/manifests/overlays/minimal/kustomization.yaml b/manifests/overlays/minimal/kustomization.yaml new file mode 100644 index 0000000000..0f22fe3732 --- /dev/null +++ b/manifests/overlays/minimal/kustomization.yaml @@ -0,0 +1,13 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +bases: +- ../../base + +patchesJson6902: +- path: deployment-resources-patch.yaml + target: + group: apps + version: v1 + kind: Deployment + name: odh-dashboard diff --git a/manifests/overlays/service-mesh/OWNERS b/manifests/overlays/service-mesh/OWNERS new file mode 100644 index 0000000000..899fa5820f --- /dev/null +++ b/manifests/overlays/service-mesh/OWNERS @@ -0,0 +1,10 @@ +# Each list is sorted alphabetically, additions should maintain that order +approvers: +- aslakknutsen +- bartoszmajsak +- cam-garrison + +reviewers: +- aslakknutsen +- bartoszmajsak +- cam-garrison diff --git a/manifests/overlays/service-mesh/kustomization.yaml b/manifests/overlays/service-mesh/kustomization.yaml new file mode 100644 index 0000000000..7e9e20ce61 --- /dev/null +++ b/manifests/overlays/service-mesh/kustomization.yaml @@ -0,0 +1,47 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +bases: +- ../../base + +## FIXME: Temporary until odh-dashboard PR is merged upstream +images: +# this needs to match newName from base, not the name used in the deployment +- name: quay.io/opendatahub/odh-dashboard + newName: quay.io/maistra-dev/odh-dashboard + newTag: service-mesh-integration + +patchesStrategicMerge: +# when using bases: +# - ../../base +# the entire directory is truncated and the lookup is performed in base/ +# for this reason the files must be there as well. with the same content, as they will be in fact used +- patches/_mesh_overlay_delete-oauth-client-patch.yaml +- patches/_mesh_overlay-delete-oauth-config-patch.yaml + +# # needs to be patchesJson6902, patches seems to be ignored +# # also: inline patches do not work, operator complains about overlays/service-mesh being +# # a directory and not a kustomization.yaml file +patchesJson6902: +- path: patches/route-patch.yaml + target: + group: route.openshift.io + version: v1 + kind: Route + name: odh-dashboard +- path: patches/service-patch.yaml + target: + version: v1 + kind: Service + name: odh-dashboard +- path: patches/service-account-patch.yaml + target: + version: v1 + kind: ServiceAccount + name: odh-dashboard +- path: patches/deployment-patch.yaml + target: + group: apps + version: v1 + kind: Deployment + name: odh-dashboard diff --git a/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml b/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml new file mode 100644 index 0000000000..856b726ed7 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml @@ -0,0 +1,6 @@ +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-config + diff --git a/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml b/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml new file mode 100644 index 0000000000..80d7896300 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml @@ -0,0 +1,5 @@ +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-client diff --git a/manifests/overlays/service-mesh/patches/deployment-patch.yaml b/manifests/overlays/service-mesh/patches/deployment-patch.yaml new file mode 100644 index 0000000000..5d57c09c3d --- /dev/null +++ b/manifests/overlays/service-mesh/patches/deployment-patch.yaml @@ -0,0 +1,13 @@ +# remove the oauth proxy container. NOTE: this will break if another container is added above. +- op: remove + path: /spec/template/spec/containers/1 + +# remove the volumes, no longer needed and secrets that are mounted are no longer created. +- op: remove + path: /spec/template/spec/volumes + +# add istio inject label +- op: add + path: /spec/template/metadata/annotations + value: + sidecar.istio.io/inject: "true" diff --git a/manifests/overlays/service-mesh/patches/oauth-client-patch.yaml b/manifests/overlays/service-mesh/patches/oauth-client-patch.yaml new file mode 100644 index 0000000000..80d7896300 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/oauth-client-patch.yaml @@ -0,0 +1,5 @@ +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-client diff --git a/manifests/overlays/service-mesh/patches/oauth-config-patch.yaml b/manifests/overlays/service-mesh/patches/oauth-config-patch.yaml new file mode 100644 index 0000000000..9e94d0a655 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/oauth-config-patch.yaml @@ -0,0 +1,5 @@ +$patch: delete +apiVersion: v1 +kind: Secret +metadata: + name: dashboard-oauth-config diff --git a/manifests/overlays/service-mesh/patches/route-patch.yaml b/manifests/overlays/service-mesh/patches/route-patch.yaml new file mode 100644 index 0000000000..095c567051 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/route-patch.yaml @@ -0,0 +1,9 @@ +- op: remove + path: /metadata/annotations + +- op: replace + path: /spec/port/targetPort + value: 8080 + +- op: remove + path: /spec/tls diff --git a/manifests/overlays/service-mesh/patches/service-account-patch.yaml b/manifests/overlays/service-mesh/patches/service-account-patch.yaml new file mode 100644 index 0000000000..7d76e98337 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/service-account-patch.yaml @@ -0,0 +1,2 @@ +- op: remove + path: /metadata/annotations diff --git a/manifests/overlays/service-mesh/patches/service-patch.yaml b/manifests/overlays/service-mesh/patches/service-patch.yaml new file mode 100644 index 0000000000..7c4f062e85 --- /dev/null +++ b/manifests/overlays/service-mesh/patches/service-patch.yaml @@ -0,0 +1,9 @@ +- op: remove + path: /metadata/annotations + +- op: replace + path: /spec/ports/0 + value: + protocol: TCP + targetPort: 8080 + port: 80 From 2d2244cfb67a566504530a66b767aa540de9aca4 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Wed, 4 Oct 2023 10:06:43 -0400 Subject: [PATCH 16/18] remove image patch --- manifests/overlays/service-mesh/kustomization.yaml | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/manifests/overlays/service-mesh/kustomization.yaml b/manifests/overlays/service-mesh/kustomization.yaml index 7e9e20ce61..24a2ca69f9 100644 --- a/manifests/overlays/service-mesh/kustomization.yaml +++ b/manifests/overlays/service-mesh/kustomization.yaml @@ -4,13 +4,6 @@ kind: Kustomization bases: - ../../base -## FIXME: Temporary until odh-dashboard PR is merged upstream -images: -# this needs to match newName from base, not the name used in the deployment -- name: quay.io/opendatahub/odh-dashboard - newName: quay.io/maistra-dev/odh-dashboard - newTag: service-mesh-integration - patchesStrategicMerge: # when using bases: # - ../../base @@ -20,8 +13,8 @@ patchesStrategicMerge: - patches/_mesh_overlay-delete-oauth-config-patch.yaml # # needs to be patchesJson6902, patches seems to be ignored -# # also: inline patches do not work, operator complains about overlays/service-mesh being -# # a directory and not a kustomization.yaml file +# # also: inline patches do not work, operator complains about overlays/service-mesh being +# # a directory and not a kustomization.yaml file patchesJson6902: - path: patches/route-patch.yaml target: From 104c88b2ab2728b610ffa4fd7a03f043112747a8 Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Wed, 4 Oct 2023 15:15:21 -0400 Subject: [PATCH 17/18] no longer poll on dashboardconfig, remove trailing slash --- .../src/pages/projects/notebook/useRouteForNotebook.ts | 8 ++++---- frontend/src/utilities/notebookControllerUtils.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts index ccd87136b5..e0f9ff3730 100644 --- a/frontend/src/pages/projects/notebook/useRouteForNotebook.ts +++ b/frontend/src/pages/projects/notebook/useRouteForNotebook.ts @@ -13,6 +13,9 @@ const useRouteForNotebook = ( const [loaded, setLoaded] = React.useState(false); const [loadError, setLoadError] = React.useState(null); const { dashboardConfig } = useAppContext(); + const enableServiceMesh = featureFlagEnabled( + dashboardConfig.spec.dashboardConfig.disableServiceMesh, + ); React.useEffect(() => { let watchHandle: ReturnType; @@ -22,9 +25,6 @@ const useRouteForNotebook = ( return; } if (notebookName && projectName) { - const enableServiceMesh = featureFlagEnabled( - dashboardConfig.spec.dashboardConfig.disableServiceMesh, - ); // if not using service mesh fetch openshift route, otherwise get Istio Ingress Gateway route const getRoutePromise = !enableServiceMesh ? getRoute(notebookName, projectName).then((route) => route?.spec.host) @@ -57,7 +57,7 @@ const useRouteForNotebook = ( cancelled = true; clearTimeout(watchHandle); }; - }, [notebookName, projectName, isRunning, dashboardConfig]); + }, [notebookName, projectName, isRunning, enableServiceMesh]); return [route, loaded, loadError]; }; diff --git a/frontend/src/utilities/notebookControllerUtils.ts b/frontend/src/utilities/notebookControllerUtils.ts index 9b5a9e1580..7a83212df4 100644 --- a/frontend/src/utilities/notebookControllerUtils.ts +++ b/frontend/src/utilities/notebookControllerUtils.ts @@ -213,7 +213,7 @@ export const useNotebookRedirectLink = (): (() => Promise) => { const call = (resolve: typeof presolve, reject: typeof preject) => { getRoute(notebookNamespace, routeName) .then((route) => { - resolve(`https://${route.spec.host}/notebook/${notebookNamespace}/${routeName}/`); + resolve(`https://${route.spec.host}/notebook/${notebookNamespace}/${routeName}`); }) .catch((e) => { if (backupRoute) { From df9f285d92f72e52ebb833c9b2787837a255fc0c Mon Sep 17 00:00:00 2001 From: Cameron Garrison Date: Wed, 4 Oct 2023 16:08:45 -0400 Subject: [PATCH 18/18] remove unnecessary patches and rename patches in kustomization --- manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml | 6 ------ manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml | 6 ------ manifests/overlays/service-mesh/kustomization.yaml | 4 ++-- .../patches/_mesh_overlay-delete-oauth-config-patch.yaml | 6 ------ .../patches/_mesh_overlay_delete-oauth-client-patch.yaml | 5 ----- 5 files changed, 2 insertions(+), 25 deletions(-) delete mode 100644 manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml delete mode 100644 manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml delete mode 100644 manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml delete mode 100644 manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml diff --git a/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml b/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml deleted file mode 100644 index bb8cdf6360..0000000000 --- a/manifests/base/_mesh_overlay-delete-oauth-config-patch.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# DO NOT DELETE. Check kustomization.yaml comments in overlays/service-mesh -$patch: delete -apiVersion: v1 -kind: Secret -metadata: - name: dashboard-oauth-config diff --git a/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml b/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml deleted file mode 100644 index 3fe30a1475..0000000000 --- a/manifests/base/_mesh_overlay_delete-oauth-client-patch.yaml +++ /dev/null @@ -1,6 +0,0 @@ -# DO NOT DELETE. Check kustomization.yaml comments in overlays/service-mesh -$patch: delete -apiVersion: v1 -kind: Secret -metadata: - name: dashboard-oauth-client diff --git a/manifests/overlays/service-mesh/kustomization.yaml b/manifests/overlays/service-mesh/kustomization.yaml index 24a2ca69f9..f1b6062baf 100644 --- a/manifests/overlays/service-mesh/kustomization.yaml +++ b/manifests/overlays/service-mesh/kustomization.yaml @@ -9,8 +9,8 @@ patchesStrategicMerge: # - ../../base # the entire directory is truncated and the lookup is performed in base/ # for this reason the files must be there as well. with the same content, as they will be in fact used -- patches/_mesh_overlay_delete-oauth-client-patch.yaml -- patches/_mesh_overlay-delete-oauth-config-patch.yaml +- patches/oauth-client-patch.yaml +- patches/oauth-config-patch.yaml # # needs to be patchesJson6902, patches seems to be ignored # # also: inline patches do not work, operator complains about overlays/service-mesh being diff --git a/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml b/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml deleted file mode 100644 index 856b726ed7..0000000000 --- a/manifests/overlays/service-mesh/patches/_mesh_overlay-delete-oauth-config-patch.yaml +++ /dev/null @@ -1,6 +0,0 @@ -$patch: delete -apiVersion: v1 -kind: Secret -metadata: - name: dashboard-oauth-config - diff --git a/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml b/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml deleted file mode 100644 index 80d7896300..0000000000 --- a/manifests/overlays/service-mesh/patches/_mesh_overlay_delete-oauth-client-patch.yaml +++ /dev/null @@ -1,5 +0,0 @@ -$patch: delete -apiVersion: v1 -kind: Secret -metadata: - name: dashboard-oauth-client