From a58257522dd637b70dcdc696c3706b93cf01a459 Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Thu, 7 Sep 2023 15:06:43 -0400 Subject: [PATCH] Add owner references to Elyra role binding when creating notebooks --- frontend/src/api/k8s/notebooks.ts | 22 ++---- frontend/src/api/k8s/roleBindings.ts | 18 +++++ .../src/concepts/pipelines/elyra/utils.ts | 68 ++++++++++++++++++- .../notebook/NotebookStatusToggle.tsx | 3 +- 4 files changed, 93 insertions(+), 18 deletions(-) diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index d7a86ba07e..c3cb94f31d 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -17,14 +17,13 @@ import { translateDisplayNameForK8s } from '~/pages/projects/utils'; import { getTolerationPatch, TolerationChanges } from '~/utilities/tolerations'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; import { + createElyraServiceAccountRoleBinding, ELYRA_VOLUME_NAME, - generateElyraServiceAccountRoleBinding, getElyraVolume, getElyraVolumeMount, getPipelineVolumeMountPatch, getPipelineVolumePatch, } from '~/concepts/pipelines/elyra/utils'; -import { createRoleBinding } from '~/api'; import { Volume, VolumeMount } from '~/types'; import { assemblePodSpecOptions, getshmVolume, getshmVolumeMount } from './utils'; @@ -207,8 +206,7 @@ export const stopNotebook = (name: string, namespace: string): Promise => { @@ -223,18 +221,12 @@ export const startNotebook = async ( if (enablePipelines) { patches.push(getPipelineVolumePatch()); patches.push(getPipelineVolumeMountPatch()); - await createRoleBinding(generateElyraServiceAccountRoleBinding(name, namespace)).catch((e) => { - // This is not ideal, but it shouldn't impact the starting of the notebook. Let us log it, and mute the error - // eslint-disable-next-line no-console - console.error( - `Could not patch rolebinding to service account for notebook, ${name}; Reason ${e.message}`, - ); - }); + await createElyraServiceAccountRoleBinding(notebook); } return k8sPatchResource({ model: NotebookModel, - queryOptions: { name, ns: namespace }, + queryOptions: { name: notebook.metadata.name, ns: notebook.metadata.namespace }, patches, }); }; @@ -252,9 +244,9 @@ export const createNotebook = ( }); if (canEnablePipelines) { - return createRoleBinding( - generateElyraServiceAccountRoleBinding(notebook.metadata.name, notebook.metadata.namespace), - ).then(() => notebookPromise); + return notebookPromise.then((notebook) => + createElyraServiceAccountRoleBinding(notebook).then(() => notebook), + ); } return notebookPromise; diff --git a/frontend/src/api/k8s/roleBindings.ts b/frontend/src/api/k8s/roleBindings.ts index bdb129193f..16c83997a5 100644 --- a/frontend/src/api/k8s/roleBindings.ts +++ b/frontend/src/api/k8s/roleBindings.ts @@ -1,4 +1,5 @@ import { + OwnerReference, k8sCreateResource, k8sDeleteResource, k8sGetResource, @@ -162,3 +163,20 @@ export const patchRoleBindingName = ( }, ], }); + +export const patchRoleBindingOwnerRef = ( + rbName: string, + namespace: string, + ownerReferences: OwnerReference[], +): Promise => + k8sPatchResource({ + model: RoleBindingModel, + queryOptions: { name: rbName, ns: namespace }, + patches: [ + { + op: 'replace', + path: '/metadata/ownerReferences', + value: ownerReferences, + }, + ], + }); diff --git a/frontend/src/concepts/pipelines/elyra/utils.ts b/frontend/src/concepts/pipelines/elyra/utils.ts index 028819fbde..bcd430d0cb 100644 --- a/frontend/src/concepts/pipelines/elyra/utils.ts +++ b/frontend/src/concepts/pipelines/elyra/utils.ts @@ -10,9 +10,13 @@ import { import { AWS_KEYS } from '~/pages/projects/dataConnections/const'; import { Volume, VolumeMount } from '~/types'; import { RUNTIME_MOUNT_PATH } from '~/pages/projects/pvc/const'; +import { createRoleBinding, getRoleBinding, patchRoleBindingOwnerRef } from '~/api'; export const ELYRA_VOLUME_NAME = 'elyra-dsp-details'; +export const getElyraServiceAccountRoleBindingName = (notebookName: string) => + `elyra-pipelines-${notebookName}`; + export const getElyraVolumeMount = (): VolumeMount => ({ name: ELYRA_VOLUME_NAME, mountPath: RUNTIME_MOUNT_PATH, @@ -25,6 +29,13 @@ export const getElyraVolume = (): Volume => ({ }, }); +export const getElyraRoleBindingOwnerRef = (notebookName: string, ownerUid: string) => ({ + apiVersion: 'kubeflow.org/v1beta1', + kind: 'Notebook', + name: notebookName, + uid: ownerUid, +}); + export const getPipelineVolumePatch = (): Patch => ({ path: '/spec/template/spec/volumes/-', op: 'add', @@ -83,15 +94,17 @@ export const generateElyraSecret = ( export const generateElyraServiceAccountRoleBinding = ( notebookName: string, namespace: string, + ownerUid: string, ): RoleBindingKind => ({ apiVersion: 'rbac.authorization.k8s.io/v1', kind: 'RoleBinding', metadata: { - name: `elyra-pipelines-${notebookName}`, + name: getElyraServiceAccountRoleBindingName(notebookName), namespace, labels: { [KnownLabels.DASHBOARD_RESOURCE]: 'true', }, + ownerReferences: [getElyraRoleBindingOwnerRef(notebookName, ownerUid)], }, roleRef: { apiGroup: 'rbac.authorization.k8s.io', @@ -105,3 +118,56 @@ export const generateElyraServiceAccountRoleBinding = ( }, ], }); + +export const createElyraServiceAccountRoleBinding = async ( + notebook: NotebookKind, +): Promise => { + const notebookName = notebook.metadata.name; + const namespace = notebook.metadata.namespace; + const notebookUid = notebook.metadata.uid; + + // Check if rolebinding is already exists for backward compatibility + const roleBinding = await getRoleBinding( + namespace, + getElyraServiceAccountRoleBindingName(notebookName), + ).catch((e) => { + // 404 is not an error + if (e.statusObject?.code !== 404) { + // eslint-disable-next-line no-console + console.error( + `Could not get rolebinding to service account for notebook, ${notebookName}; Reason ${e.message}`, + ); + } + return undefined; + }); + + if (notebookUid) { + if (roleBinding) { + const ownerReferences = roleBinding.metadata.ownerReferences || []; + if (!ownerReferences.find((ownerReference) => ownerReference.uid === notebookUid)) { + ownerReferences.push(getElyraRoleBindingOwnerRef(notebookName, notebookUid)); + } + return patchRoleBindingOwnerRef( + roleBinding.metadata.name, + roleBinding.metadata.namespace, + ownerReferences, + ).catch((e) => { + // This is not ideal, but it shouldn't impact the starting of the notebook. Let us log it, and mute the error + // eslint-disable-next-line no-console + console.error( + `Could not patch rolebinding to service account for notebook, ${notebookName}; Reason ${e.message}`, + ); + }); + } + return createRoleBinding( + generateElyraServiceAccountRoleBinding(notebookName, namespace, notebookUid), + ).catch((e) => { + // eslint-disable-next-line no-console + console.error( + `Could not create rolebinding to service account for notebook, ${notebookName}; Reason ${e.message}`, + ); + }); + } + + return undefined; +}; diff --git a/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx b/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx index cc63f9c3a6..c134229d58 100644 --- a/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx +++ b/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx @@ -99,8 +99,7 @@ const NotebookStatusToggle: React.FC = ({ notebookState.notebook, ); startNotebook( - notebookName, - notebookNamespace, + notebook, tolerationSettings, enablePipelines && !currentlyHasPipelines(notebook), ).then(() => {