Skip to content

Commit

Permalink
feat(ossm): adds OSSM annotations to the relevant cluster resources
Browse files Browse the repository at this point in the history
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 opendatahub-io/opendatahub-operator#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](opendatahub-io/opendatahub-operator#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 <[email protected]>
  • Loading branch information
cam-garrison and bartoszmajsak committed Apr 12, 2023
1 parent f8ae34f commit a3442eb
Show file tree
Hide file tree
Showing 16 changed files with 160 additions and 21 deletions.
4 changes: 4 additions & 0 deletions backend/src/routes/api/namespaces/namespaceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 16 additions & 5 deletions backend/src/routes/api/notebooks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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}`);
Expand Down Expand Up @@ -78,7 +89,7 @@ export const patchNotebookRoute = async (
const patch: RecursivePartial<Notebook> = {
metadata: {
annotations: {
'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}`,
'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}/`,
},
},
};
Expand Down
5 changes: 5 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type DashboardConfig = K8sResourceCommon & {
disableUserManagement: boolean;
disableProjects: boolean;
disableModelServing: boolean;
disableServiceMesh: boolean;
modelMetricsNamespace: string;
};
groupsConfig?: {
Expand Down Expand Up @@ -386,6 +387,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
Expand Down
1 change: 1 addition & 0 deletions backend/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const blankDashboardCR: DashboardConfig = {
disableUserManagement: false,
disableProjects: false,
disableModelServing: false,
disableServiceMesh: true,
modelMetricsNamespace: '',
},
notebookController: {
Expand Down
65 changes: 64 additions & 1 deletion backend/src/utils/notebookUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Route> => {
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,
Expand Down Expand Up @@ -250,6 +298,10 @@ export const assembleNotebook = async (
},
}));

const serviceMeshEnabled = String(
!getDashboardConfig().spec?.dashboardConfig?.disableServiceMesh,
);

return {
apiVersion: 'kubeflow.org/v1',
kind: 'Notebook',
Expand All @@ -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,
Expand Down Expand Up @@ -413,6 +467,8 @@ export const createNotebook = async (
url: string,
notebookData?: NotebookData,
): Promise<Notebook> => {
const config = getDashboardConfig();

if (!notebookData) {
const error = createCustomError(
'Missing Notebook',
Expand All @@ -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]) {
Expand Down
1 change: 1 addition & 0 deletions docs/dashboard_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ The following are a list of features that are supported, along with there defaul
| disableAppLauncher | false | Removes the application launcher that is used in OKD environments
| disableUserManagement | false | Removes the User Management panel in Settings.
| disableProjects | false | Disables Data Science Projects from the dashboard.
| disableServiceMesh | true | Disables use of service mesh for routing and authorization.
| disableModelServing | false | Disables Model Serving from the dashboard and from Data Science Projects.
| modelMetricsNamespace | false | Enables the namespace in which the Model Serving Metrics' Prometheus Operator is installed.

Expand Down
1 change: 1 addition & 0 deletions frontend/src/__mocks__/mockDashboardConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const mockDashboardConfig: DashboardConfig = {
disableUserManagement: false,
disableProjects: false,
disableModelServing: false,
disableServiceMesh: true,
modelMetricsNamespace: 'test-project',
},
notebookController: {
Expand Down
25 changes: 19 additions & 6 deletions frontend/src/api/k8s/notebooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const';
import { translateDisplayNameForK8s } from '~/pages/projects/utils';
import { getTolerationPatch, TolerationChanges } from '~/utilities/tolerations';
import { applyK8sAPIOptions } from '~/api/apiMergeUtils';
import { DashboardConfig } from '~/types';
import { assemblePodSpecOptions } from './utils';

const assembleNotebook = (data: StartNotebookData, username: string): NotebookKind => {
const assembleNotebook = (
data: StartNotebookData,
username: string,
dashboardConfig: DashboardConfig,
): NotebookKind => {
const {
projectName,
notebookName,
Expand Down Expand Up @@ -63,7 +68,13 @@ const assembleNotebook = (data: StartNotebookData, username: string): NotebookKi
'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,
Expand Down Expand Up @@ -193,8 +204,9 @@ export const startNotebook = (
export const createNotebook = (
data: StartNotebookData,
username: string,
dashboardConfig: DashboardConfig,
): Promise<NotebookKind> => {
const notebook = assembleNotebook(data, username);
const notebook = assembleNotebook(data, username, dashboardConfig);

return k8sCreateResource<NotebookKind>({
model: NotebookModel,
Expand All @@ -206,9 +218,10 @@ export const updateNotebook = (
existingNotebook: NotebookKind,
data: StartNotebookData,
username: string,
dashboardConfig: DashboardConfig,
): Promise<NotebookKind> => {
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];
Expand All @@ -229,9 +242,10 @@ export const updateNotebook = (
export const createNotebookWithoutStarting = (
data: StartNotebookData,
username: string,
dashboardConfig: DashboardConfig,
): Promise<NotebookKind> =>
new Promise((resolve, reject) =>
createNotebook(data, username).then((notebook) =>
createNotebook(data, username, dashboardConfig).then((notebook) =>
setTimeout(
() =>
stopNotebook(notebook.metadata.name, notebook.metadata.namespace)
Expand All @@ -241,7 +255,6 @@ export const createNotebookWithoutStarting = (
),
),
);

export const deleteNotebook = (notebookName: string, namespace: string): Promise<K8sStatus> =>
k8sDeleteResource<NotebookKind, K8sStatus>({
model: NotebookModel,
Expand Down
21 changes: 20 additions & 1 deletion frontend/src/api/k8s/routes.ts
Original file line number Diff line number Diff line change
@@ -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 = (
Expand All @@ -14,3 +14,22 @@ export const getRoute = (
queryOptions: { name, ns: namespace },
}),
);

export const getGatewayRoute = (
namespace: string,
gatewayName: string,
): Promise<RouteKind | null> => {
const labelSelector = `maistra.io/gateway-name=${gatewayName}`;
const queryOptions = {
ns: namespace,
labelSelector,
};
return k8sGetResource<List<RouteKind>>({ 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);
};
9 changes: 9 additions & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,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
}>;
Expand Down Expand Up @@ -358,6 +360,13 @@ export type RouteKind = K8sResourceCommon & {
};
};

export type List<T> = {
apiVersion?: string;
kind?: string;
metadata: Record<string, unknown>;
items: T[];
} & K8sResourceCommon;

export type SecretKind = K8sResourceCommon & {
metadata: {
name: string;
Expand Down
15 changes: 11 additions & 4 deletions frontend/src/pages/projects/notebook/useRouteForNotebook.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -10,6 +11,7 @@ const useRouteForNotebook = (
const [route, setRoute] = React.useState<string | null>(null);
const [loaded, setLoaded] = React.useState(false);
const [loadError, setLoadError] = React.useState<Error | null>(null);
const { dashboardConfig } = useAppContext();

React.useEffect(() => {
let watchHandle;
Expand All @@ -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);
})
Expand All @@ -46,7 +53,7 @@ const useRouteForNotebook = (
cancelled = true;
clearTimeout(watchHandle);
};
}, [notebookName, projectName, isRunning]);
}, [notebookName, projectName, isRunning, dashboardConfig]);

return [route, loaded, loadError];
};
Expand Down
Loading

0 comments on commit a3442eb

Please sign in to comment.