Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ossm): adds OSSM annotations to the relevant cluster resources #1088

Merged
merged 20 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
bfa5075
feat(ossm): adds OSSM annotations to the relevant cluster resources
cam-garrison Apr 4, 2023
1408c29
fix: adds mesh annotation to the ns
bartoszmajsak Apr 18, 2023
166e409
fix: use project controller created route for ds project routing (#4)
cam-garrison May 23, 2023
47e43ac
feat: use annotation to fetch host for notebook routing (#5)
cam-garrison Jun 26, 2023
dd22cd7
fix: patch notebook annotations (#6)
cam-garrison Jun 28, 2023
c917d76
Linting fixes
cam-garrison Jun 29, 2023
93aa3ae
add featureFlagEnabled() to backend, use for SM
cam-garrison Aug 2, 2023
a0ea817
Merge branch 'main' into ossm_annotations
bartoszmajsak Aug 23, 2023
abf520d
explicitly set sidecar injection annotation
cam-garrison Aug 24, 2023
5b26e64
change to istio inject label from anno
cam-garrison Aug 28, 2023
c106663
Merge branch 'main' into ossm_annotations
cam-garrison Sep 20, 2023
8376c28
remove namespacekind, use projectkind
cam-garrison Sep 21, 2023
48bfe4e
lint getSMGwHost() changes
cam-garrison Sep 21, 2023
6b12f82
move getDashboardConfig call, remove unnec constant
cam-garrison Sep 21, 2023
ca51ef3
simplify annotations fetch
cam-garrison Sep 22, 2023
74b4c79
pass parsed SM flag instead ofdash config
cam-garrison Sep 22, 2023
b55490f
add sm manifests changes
cam-garrison Sep 25, 2023
2d2244c
remove image patch
cam-garrison Oct 4, 2023
104c88b
no longer poll on dashboardconfig, remove trailing slash
cam-garrison Oct 4, 2023
df9f285
remove unnecessary patches and rename patches in kustomization
cam-garrison Oct 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 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,13 +61,19 @@ 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 = {};
let annotations = {};
switch (context) {
case NamespaceApplicationCase.DSG_CREATION:
labels = {
'opendatahub.io/dashboard': 'true',
'modelmesh-enabled': 'true',
};
annotations = {
'opendatahub.io/service-mesh': String(!disableServiceMesh),
lucferbux marked this conversation as resolved.
Show resolved Hide resolved
};
break;
case NamespaceApplicationCase.MODEL_SERVING_PROMOTION:
labels = {
Expand All @@ -78,9 +85,17 @@ export const applyNamespaceChange = async (
}

return fastify.kube.coreV1Api
.patchNamespace(name, { metadata: { labels } }, 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(
Expand Down
30 changes: 22 additions & 8 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,
getServiceMeshGwHost,
updateNotebook,
} from '../../../utils/notebookUtils';
import { FastifyRequest } from 'fastify';
Expand All @@ -27,12 +29,24 @@ 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;
});
if (route) {
newNotebook = await patchNotebookRoute(fastify, route, namespace, notebookName).catch((e) => {
const disableServiceMesh = getDashboardConfig().spec.dashboardConfig.disableServiceMesh;
let host: string;
if (!disableServiceMesh) {
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved
host = await getServiceMeshGwHost(fastify, namespace).catch((e) => {
fastify.log.warn(`Failed getting route ${notebookName}: ${e.message}`);
return undefined;
});
} else {
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 (host) {
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved
newNotebook = await patchNotebookRoute(fastify, host, namespace, notebookName).catch((e) => {
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved
fastify.log.warn(`Failed patching route to notebook ${notebookName}: ${e.message}`);
return notebook;
});
Expand Down Expand Up @@ -71,14 +85,14 @@ export const checkPodContainersReady = (pod: V1Pod): boolean => {

export const patchNotebookRoute = async (
fastify: KubeFastifyInstance,
route: Route,
host: string,
namespace: string,
name: string,
): Promise<Notebook> => {
const patch: RecursivePartial<Notebook> = {
metadata: {
annotations: {
'opendatahub.io/link': `https://${route.spec.host}/notebook/${namespace}/${name}`,
'opendatahub.io/link': `https://${host}/notebook/${namespace}/${name}/`,
},
},
};
Expand Down
4 changes: 4 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type DashboardConfig = K8sResourceCommon & {
disableModelServing: boolean;
disableProjectSharing: boolean;
disableCustomServingRuntimes: boolean;
disableServiceMesh: boolean;
modelMetricsNamespace: string;
disablePipelines: boolean;
};
Expand Down Expand Up @@ -403,6 +404,9 @@ 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;

// 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 @@ -51,6 +51,7 @@ export const blankDashboardCR: DashboardConfig = {
disableModelServing: false,
disableProjectSharing: false,
disableCustomServingRuntimes: false,
disableServiceMesh: true,
modelMetricsNamespace: '',
disablePipelines: false,
},
Expand Down
46 changes: 45 additions & 1 deletion backend/src/utils/notebookUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { getUserName, usernameTranslate } from './userUtils';
import { createCustomError } from './requestUtils';
import {
PatchUtils,
V1Namespace,
V1PersistentVolumeClaim,
V1Role,
V1RoleBinding,
Expand Down Expand Up @@ -68,6 +69,35 @@ export const getRoute = async (
return kubeResponse.body as Route;
};

export const getServiceMeshGwHost = async (
fastify: KubeFastifyInstance,
namespace: string,
): Promise<string> => {
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 V1Namespace;

const annotations = typedResponse.metadata?.annotations;
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved

if (!annotations || !annotations['service-mesh.opendatahub.io/public-gateway-host-external']) {
const error = createCustomError(
'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 annotations['service-mesh.opendatahub.io/public-gateway-host-external'];
};

export const createRBAC = async (
fastify: KubeFastifyInstance,
namespace: string,
Expand Down Expand Up @@ -250,6 +280,10 @@ export const assembleNotebook = async (
},
}));

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

return {
apiVersion: 'kubeflow.org/v1',
kind: 'Notebook',
Expand All @@ -264,7 +298,9 @@ 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,
},
name: name,
Expand Down Expand Up @@ -413,6 +449,8 @@ export const createNotebook = async (
url: string,
notebookData?: NotebookData,
): Promise<Notebook> => {
const config = getDashboardConfig();
lucferbux marked this conversation as resolved.
Show resolved Hide resolved

if (!notebookData) {
const error = createCustomError(
'Missing Notebook',
Expand All @@ -438,7 +476,13 @@ 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,
);

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 @@ -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

Expand Down
23 changes: 12 additions & 11 deletions frontend/src/__mocks__/mockDashboardConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 32 additions & 7 deletions frontend/src/api/k8s/notebooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -86,7 +88,12 @@ 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/username': username,
},
name: notebookId,
Expand Down Expand Up @@ -174,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<NotebookKind[]> =>
k8sListResource<NotebookKind>({
model: NotebookModel,
Expand All @@ -197,10 +216,14 @@ export const startNotebook = async (
name: string,
namespace: string,
tolerationChanges: TolerationChanges,
dashboardConfig: DashboardConfig,
enablePipelines?: boolean,
): Promise<NotebookKind> => {
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) {
Expand Down Expand Up @@ -229,9 +252,10 @@ export const startNotebook = async (
export const createNotebook = (
data: StartNotebookData,
username: string,
dashboardConfig: DashboardConfig,
canEnablePipelines?: boolean,
): Promise<NotebookKind> => {
const notebook = assembleNotebook(data, username, canEnablePipelines);
const notebook = assembleNotebook(data, username, dashboardConfig, canEnablePipelines);
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved

const notebookPromise = k8sCreateResource<NotebookKind>({
model: NotebookModel,
Expand All @@ -251,9 +275,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 @@ -274,9 +299,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) =>
cam-garrison marked this conversation as resolved.
Show resolved Hide resolved
setTimeout(
() =>
stopNotebook(notebook.metadata.name, notebook.metadata.namespace)
Expand All @@ -286,7 +312,6 @@ export const createNotebookWithoutStarting = (
),
),
);

export const deleteNotebook = (notebookName: string, namespace: string): Promise<K8sStatus> =>
k8sDeleteResource<NotebookKind, K8sStatus>({
model: NotebookModel,
Expand Down
15 changes: 13 additions & 2 deletions 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 { RouteModel, NamespaceModel } from '~/api/models';
import { K8sAPIOptions, RouteKind, NamespaceKind } from '~/k8sTypes';
import { applyK8sAPIOptions } from '~/api/apiMergeUtils';

export const getRoute = (
Expand All @@ -14,3 +14,14 @@ export const getRoute = (
queryOptions: { name, ns: namespace },
}),
);

export const getServiceMeshGwHost = async (namespace: string): Promise<string | null> => {
const queryOptions = {
name: namespace,
};
const project = await k8sGetResource<NamespaceKind>({ model: NamespaceModel, queryOptions });
lucferbux marked this conversation as resolved.
Show resolved Hide resolved
return (
project?.metadata?.annotations?.['service-mesh.opendatahub.io/public-gateway-host-external'] ||
null
);
};
Loading