From 9ee0ffe44745a84093a31075f1cae3797647c8ca Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Mon, 7 Aug 2023 15:35:47 -0400 Subject: [PATCH 1/3] Show non-DS projects in the project table --- .../routes/api/namespaces/namespaceUtils.ts | 1 - frontend/src/api/k8s/projects.ts | 3 -- .../src/concepts/projects/ProjectsContext.tsx | 48 +++++++++++++------ .../ManageServingRuntimeModal.tsx | 2 +- .../screens/projects/ProjectListView.tsx | 12 +++-- .../screens/projects/ProjectScopeSelect.tsx | 44 +++++++++++++++++ .../screens/projects/ProjectTableRow.tsx | 19 ++++++-- .../projects/screens/projects/ProjectView.tsx | 18 +++++-- frontend/src/pages/projects/types.ts | 4 ++ frontend/src/typeHelpers.ts | 5 ++ 10 files changed, 124 insertions(+), 32 deletions(-) create mode 100644 frontend/src/pages/projects/screens/projects/ProjectScopeSelect.tsx diff --git a/backend/src/routes/api/namespaces/namespaceUtils.ts b/backend/src/routes/api/namespaces/namespaceUtils.ts index 05b1a7dc7f..c55387debb 100644 --- a/backend/src/routes/api/namespaces/namespaceUtils.ts +++ b/backend/src/routes/api/namespaces/namespaceUtils.ts @@ -65,7 +65,6 @@ export const applyNamespaceChange = async ( case NamespaceApplicationCase.DSG_CREATION: labels = { 'opendatahub.io/dashboard': 'true', - 'modelmesh-enabled': 'true', }; break; case NamespaceApplicationCase.MODEL_SERVING_PROMOTION: diff --git a/frontend/src/api/k8s/projects.ts b/frontend/src/api/k8s/projects.ts index 02224aa101..6426c79578 100644 --- a/frontend/src/api/k8s/projects.ts +++ b/frontend/src/api/k8s/projects.ts @@ -27,9 +27,6 @@ export const getProjects = (withLabel?: string): Promise => queryOptions: withLabel ? { queryParams: { labelSelector: withLabel } } : undefined, }).then((listResource) => listResource.items); -export const getDSGProjects = (): Promise => - getProjects(LABEL_SELECTOR_DASHBOARD_RESOURCE); - export const createProject = ( username: string, displayName: string, diff --git a/frontend/src/concepts/projects/ProjectsContext.tsx b/frontend/src/concepts/projects/ProjectsContext.tsx index b9e25bca1e..7d3342965d 100644 --- a/frontend/src/concepts/projects/ProjectsContext.tsx +++ b/frontend/src/concepts/projects/ProjectsContext.tsx @@ -1,11 +1,13 @@ import * as React from 'react'; import useFetchState, { FetchState } from '~/utilities/useFetchState'; -import { getDSGProjects } from '~/api'; +import { getProjects } from '~/api'; import { KnownLabels, ProjectKind } from '~/k8sTypes'; +import { useDashboardNamespace } from '~/redux/selectors'; type ProjectFetchState = FetchState; type ProjectsContext = { projects: ProjectKind[]; + dataScienceProjects: ProjectKind[]; modelServingProjects: ProjectKind[]; /** eg. Terminating state, etc */ nonActiveProjects: ProjectKind[]; @@ -26,6 +28,7 @@ type ProjectsContext = { export const ProjectsContext = React.createContext({ projects: [], + dataScienceProjects: [], modelServingProjects: [], nonActiveProjects: [], preferredProject: null, @@ -44,39 +47,55 @@ type ProjectsProviderProps = { }; const ProjectsContextProvider: React.FC = ({ children }) => { - const fetchProjects = React.useCallback(() => getDSGProjects(), []); + const fetchProjects = React.useCallback(() => getProjects(), []); const [preferredProject, setPreferredProject] = React.useState(null); const [projectData, loaded, loadError, refreshProjects] = useFetchState( fetchProjects, [], ); + const { dashboardNamespace } = useDashboardNamespace(); - const { projects, modelServingProjects, nonActiveProjects } = React.useMemo( + const { projects, dataScienceProjects, modelServingProjects, nonActiveProjects } = React.useMemo( () => projectData.reduce<{ projects: ProjectKind[]; + dataScienceProjects: ProjectKind[]; modelServingProjects: ProjectKind[]; nonActiveProjects: ProjectKind[]; }>( (states, project) => { - if (project.status?.phase === 'Active') { - // Project that is active - states.projects.push(project); - if (project.metadata.labels?.[KnownLabels.MODEL_SERVING_PROJECT]) { - // Model Serving active projects - states.modelServingProjects.push(project); + if ( + !( + project.metadata.name.startsWith('openshift-') || + project.metadata.name.startsWith('kube-') || + project.metadata.name === 'default' || + project.metadata.name === 'system' || + project.metadata.name === 'openshift' || + project.metadata.name === dashboardNamespace + ) + ) { + if (project.status?.phase === 'Active') { + // Project that is active + states.projects.push(project); + if (project.metadata.labels?.[KnownLabels.DASHBOARD_RESOURCE]) { + states.dataScienceProjects.push(project); + } + if (project.metadata.labels?.[KnownLabels.MODEL_SERVING_PROJECT]) { + // Model Serving active projects + states.modelServingProjects.push(project); + } + } else { + // Non 'Active' -- aka terminating + states.nonActiveProjects.push(project); } - } else { - // Non 'Active' -- aka terminating - states.nonActiveProjects.push(project); } return states; }, - { projects: [], modelServingProjects: [], nonActiveProjects: [] }, + { projects: [], dataScienceProjects: [], modelServingProjects: [], nonActiveProjects: [] }, ), - [projectData], + [projectData, dashboardNamespace], ); const refresh = React.useCallback( @@ -114,6 +133,7 @@ const ProjectsContextProvider: React.FC = ({ children }) = ({ ]) .then(() => Promise.all([ - ...(currentProject.metadata.labels?.['modelmesh-enabled'] && allowCreate + ...(currentProject.metadata.labels?.['modelmesh-enabled'] === undefined && allowCreate ? [addSupportModelMeshProject(currentProject.metadata.name)] : []), ...(editInfo?.servingRuntime diff --git a/frontend/src/pages/projects/screens/projects/ProjectListView.tsx b/frontend/src/pages/projects/screens/projects/ProjectListView.tsx index 757853dba4..cdd3751c6f 100644 --- a/frontend/src/pages/projects/screens/projects/ProjectListView.tsx +++ b/frontend/src/pages/projects/screens/projects/ProjectListView.tsx @@ -2,13 +2,13 @@ import * as React from 'react'; import { Button, ButtonVariant, ToolbarItem } from '@patternfly/react-core'; import { useNavigate } from 'react-router-dom'; import Table from '~/components/table/Table'; -import useTableColumnSort from '~/components/table/useTableColumnSort'; import SearchField, { SearchType } from '~/pages/projects/components/SearchField'; import { ProjectKind } from '~/k8sTypes'; import { getProjectDisplayName, getProjectOwner } from '~/pages/projects/utils'; import { useAppContext } from '~/app/AppContext'; import LaunchJupyterButton from '~/pages/projects/screens/projects/LaunchJupyterButton'; import { ProjectsContext } from '~/concepts/projects/ProjectsContext'; +import { ProjectScope } from '~/pages/projects/types'; import NewProjectButton from './NewProjectButton'; import { columns } from './tableData'; import ProjectTableRow from './ProjectTableRow'; @@ -17,16 +17,18 @@ import ManageProjectModal from './ManageProjectModal'; type ProjectListViewProps = { allowCreate: boolean; + scope: ProjectScope; }; -const ProjectListView: React.FC = ({ allowCreate }) => { +const ProjectListView: React.FC = ({ allowCreate, scope }) => { const { dashboardConfig } = useAppContext(); - const { projects: unfilteredProjects, refresh } = React.useContext(ProjectsContext); + const { projects, dataScienceProjects, refresh } = React.useContext(ProjectsContext); const navigate = useNavigate(); const [searchType, setSearchType] = React.useState(SearchType.NAME); const [search, setSearch] = React.useState(''); - const sort = useTableColumnSort(columns, 0); - const filteredProjects = sort.transformData(unfilteredProjects).filter((project) => { + const filteredProjects = ( + scope === ProjectScope.ALL_PROJECTS ? projects : dataScienceProjects + ).filter((project) => { if (!search) { return true; } diff --git a/frontend/src/pages/projects/screens/projects/ProjectScopeSelect.tsx b/frontend/src/pages/projects/screens/projects/ProjectScopeSelect.tsx new file mode 100644 index 0000000000..b5512f73db --- /dev/null +++ b/frontend/src/pages/projects/screens/projects/ProjectScopeSelect.tsx @@ -0,0 +1,44 @@ +import * as React from 'react'; +import { Select, SelectOption } from '@patternfly/react-core'; +import { isInEnum } from '~/typeHelpers'; +import { ProjectScope } from '~/pages/projects/types'; + +type ProjectScopeSelectProps = { + selection: ProjectScope; + setSelection: (selection: ProjectScope) => void; +}; + +const isProjectScope = isInEnum(ProjectScope); + +const ProjectScopeSelect: React.FC = ({ selection, setSelection }) => { + const [isOpen, setOpen] = React.useState(false); + return ( + + ); +}; + +export default ProjectScopeSelect; diff --git a/frontend/src/pages/projects/screens/projects/ProjectTableRow.tsx b/frontend/src/pages/projects/screens/projects/ProjectTableRow.tsx index a54934e7dd..cba59d1fd9 100644 --- a/frontend/src/pages/projects/screens/projects/ProjectTableRow.tsx +++ b/frontend/src/pages/projects/screens/projects/ProjectTableRow.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; -import { Text, TextVariants, Timestamp } from '@patternfly/react-core'; +import { Flex, Label, Text, TextVariants, Timestamp, Tooltip } from '@patternfly/react-core'; import { ActionsColumn, Td, Tr } from '@patternfly/react-table'; -import { ProjectKind } from '~/k8sTypes'; +import { KnownLabels, ProjectKind } from '~/k8sTypes'; import useProjectTableRowItems from '~/pages/projects/screens/projects/useProjectTableRowItems'; import useProjectNotebookStates from '~/pages/projects/notebook/useProjectNotebookStates'; import ListNotebookState from '~/pages/projects/notebook/ListNotebookState'; @@ -28,9 +28,18 @@ const ProjectTableRow: React.FC = ({ return ( - - - + + {project.metadata.labels?.[KnownLabels.DASHBOARD_RESOURCE] && ( + + + + )} + + + + {owner && {owner}} diff --git a/frontend/src/pages/projects/screens/projects/ProjectView.tsx b/frontend/src/pages/projects/screens/projects/ProjectView.tsx index a5417e8169..b97ea06781 100644 --- a/frontend/src/pages/projects/screens/projects/ProjectView.tsx +++ b/frontend/src/pages/projects/screens/projects/ProjectView.tsx @@ -4,8 +4,11 @@ import { useAccessReview } from '~/api'; import { AccessReviewResourceAttributes } from '~/k8sTypes'; import { ProjectsContext } from '~/concepts/projects/ProjectsContext'; import useMountProjectRefresh from '~/concepts/projects/useMountProjectRefresh'; +import { useBrowserStorage } from '~/components/browserStorage'; +import { ProjectScope } from '~/pages/projects/types'; import EmptyProjects from './EmptyProjects'; import ProjectListView from './ProjectListView'; +import ProjectScopeSelect from './ProjectScopeSelect'; const accessReviewResource: AccessReviewResourceAttributes = { group: 'project.openshift.io', @@ -14,7 +17,11 @@ const accessReviewResource: AccessReviewResourceAttributes = { }; const ProjectView: React.FC = () => { - const { projects } = React.useContext(ProjectsContext); + const [scope, setScope] = useBrowserStorage( + 'odh.dashboard.project.scope', + ProjectScope.DS_PROJECTS, + ); + const { projects, dataScienceProjects } = React.useContext(ProjectsContext); useMountProjectRefresh(); const [allowCreate, rbacLoaded] = useAccessReview(accessReviewResource); @@ -26,12 +33,17 @@ const ProjectView: React.FC = () => { ? `View your existing projects${allowCreate ? ' or create new projects' : ''}.` : undefined } + headerContent={} loaded={rbacLoaded} - empty={projects.length === 0} + empty={ + scope === ProjectScope.ALL_PROJECTS + ? projects.length === 0 + : dataScienceProjects.length === 0 + } emptyStatePage={} provideChildrenPadding > - + ); }; diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index 89de7934e3..74645f665d 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -136,3 +136,7 @@ export enum ConfigMapCategory { GENERIC = 'configmap key-value', UPLOAD = 'configmap upload', } +export enum ProjectScope { + DS_PROJECTS = 'Data science projects', + ALL_PROJECTS = 'All available projects', +} diff --git a/frontend/src/typeHelpers.ts b/frontend/src/typeHelpers.ts index 65026cdb5f..48cb3f1e57 100644 --- a/frontend/src/typeHelpers.ts +++ b/frontend/src/typeHelpers.ts @@ -93,3 +93,8 @@ export type EitherNotBoth = (TypeA & Never) | (TypeB & Neve export type EitherOrNone = | EitherNotBoth | (Never & Never); + +export const isInEnum = + (e: T) => + (token: unknown): token is T[keyof T] => + Object.values(e).includes(token as T[keyof T]); From 6dc30ef8a6d6a39e69b702cb6c72123a636e4100 Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Mon, 28 Aug 2023 12:43:56 -0400 Subject: [PATCH 2/3] support unit for notebook storage dry run and show error message on storage modal --- frontend/src/api/k8s/notebooks.ts | 26 +-- frontend/src/api/k8s/pvcs.ts | 151 ++++++++-------- frontend/src/app/App.scss | 22 ++- frontend/src/components/ValueUnitField.tsx | 1 + .../pages/clusterSettings/ClusterSettings.tsx | 2 +- .../pages/clusterSettings/PVCSizeSettings.tsx | 12 +- .../DataConnectionSection.tsx | 1 - .../src/pages/notebookController/const.ts | 1 - .../pages/projects/components/PVSizeField.tsx | 74 +++----- .../detail/storage/ManageStorageModal.scss | 12 -- .../detail/storage/ManageStorageModal.tsx | 169 ++++++++---------- .../pages/projects/screens/spawner/service.ts | 19 +- .../screens/spawner/storage/StorageField.tsx | 2 - .../spawner/storage/useAvailablePvcSize.ts | 24 --- .../spawner/storage/useDefaultPvcSize.ts | 14 ++ .../projects/screens/spawner/storage/utils.ts | 31 ++-- frontend/src/pages/projects/types.ts | 2 +- frontend/src/types.ts | 2 +- 18 files changed, 256 insertions(+), 309 deletions(-) delete mode 100644 frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.scss delete mode 100644 frontend/src/pages/projects/screens/spawner/storage/useAvailablePvcSize.ts create mode 100644 frontend/src/pages/projects/screens/spawner/storage/useDefaultPvcSize.ts diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 8c1ddb8d80..d9ce73acd2 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -384,6 +384,7 @@ export const attachNotebookPVC = ( namespace: string, pvcName: string, mountSuffix: string, + opts?: K8sAPIOptions, ): Promise => { const patches: Patch[] = [ { @@ -399,17 +400,20 @@ export const attachNotebookPVC = ( }, ]; - return k8sPatchResource({ - model: NotebookModel, - queryOptions: { name: notebookName, ns: namespace }, - patches, - }); + return k8sPatchResource( + applyK8sAPIOptions(opts, { + model: NotebookModel, + queryOptions: { name: notebookName, ns: namespace }, + patches, + }), + ); }; export const removeNotebookPVC = ( notebookName: string, namespace: string, pvcName: string, + opts?: K8sAPIOptions, ): Promise => new Promise((resolve, reject) => { getNotebook(notebookName, namespace) @@ -438,11 +442,13 @@ export const removeNotebookPVC = ( }, ]; - k8sPatchResource({ - model: NotebookModel, - queryOptions: { name: notebookName, ns: namespace }, - patches, - }) + k8sPatchResource( + applyK8sAPIOptions(opts, { + model: NotebookModel, + queryOptions: { name: notebookName, ns: namespace }, + patches, + }), + ) .then(resolve) .catch(reject); }) diff --git a/frontend/src/api/k8s/pvcs.ts b/frontend/src/api/k8s/pvcs.ts index 9fe9a16f96..62614c0f9f 100644 --- a/frontend/src/api/k8s/pvcs.ts +++ b/frontend/src/api/k8s/pvcs.ts @@ -1,47 +1,59 @@ +import * as _ from 'lodash'; import { k8sCreateResource, k8sDeleteResource, k8sGetResource, k8sListResourceItems, k8sPatchResource, + k8sUpdateResource, } from '@openshift/dynamic-plugin-sdk-utils'; -import { K8sStatus, KnownLabels, PersistentVolumeClaimKind } from '~/k8sTypes'; +import { K8sAPIOptions, K8sStatus, KnownLabels, PersistentVolumeClaimKind } from '~/k8sTypes'; import { PVCModel } from '~/api/models'; import { translateDisplayNameForK8s } from '~/pages/projects/utils'; import { LABEL_SELECTOR_DASHBOARD_RESOURCE } from '~/const'; +import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; +import { CreatingStorageObject } from '~/pages/projects/types'; export const assemblePvc = ( - pvcName: string, - projectName: string, - description: string, - pvcSize: number, -): PersistentVolumeClaimKind => ({ - apiVersion: 'v1', - kind: 'PersistentVolumeClaim', - metadata: { - name: translateDisplayNameForK8s(pvcName), - namespace: projectName, - labels: { - [KnownLabels.DASHBOARD_RESOURCE]: 'true', - }, - annotations: { - 'openshift.io/display-name': pvcName.trim(), - 'openshift.io/description': description, + data: CreatingStorageObject, + namespace: string, + editName?: string, +): PersistentVolumeClaimKind => { + const { + nameDesc: { name: pvcName, description }, + size, + } = data; + + const name = editName || translateDisplayNameForK8s(pvcName); + + return { + apiVersion: 'v1', + kind: 'PersistentVolumeClaim', + metadata: { + name, + namespace, + labels: { + [KnownLabels.DASHBOARD_RESOURCE]: 'true', + }, + annotations: { + 'openshift.io/display-name': pvcName.trim(), + 'openshift.io/description': description, + }, }, - }, - spec: { - accessModes: ['ReadWriteOnce'], - resources: { - requests: { - storage: `${pvcSize}Gi`, + spec: { + accessModes: ['ReadWriteOnce'], + resources: { + requests: { + storage: size, + }, }, + volumeMode: 'Filesystem', + }, + status: { + phase: 'Pending', }, - volumeMode: 'Filesystem', - }, - status: { - phase: 'Pending', - }, -}); + }; +}; export const getPvc = (projectName: string, pvcName: string): Promise => k8sGetResource({ @@ -68,42 +80,30 @@ export const getAvailableMultiUsePvcs = ( }), ); -export const createPvc = (data: PersistentVolumeClaimKind): Promise => - k8sCreateResource({ model: PVCModel, resource: data }); - -export const updatePvcDisplayName = ( - pvcName: string, +export const createPvc = ( + data: CreatingStorageObject, namespace: string, - displayName: string, -): Promise => - k8sPatchResource({ - model: PVCModel, - queryOptions: { name: pvcName, ns: namespace }, - patches: [ - { - op: 'replace', - path: '/metadata/annotations/openshift.io~1display-name', - value: displayName, - }, - ], - }); + opts?: K8sAPIOptions, +): Promise => { + const pvc = assemblePvc(data, namespace); -export const updatePvcDescription = ( - pvcName: string, + return k8sCreateResource( + applyK8sAPIOptions(opts, { model: PVCModel, resource: pvc }), + ); +}; + +export const updatePvc = ( + data: CreatingStorageObject, + existingData: PersistentVolumeClaimKind, namespace: string, - description: string, -): Promise => - k8sPatchResource({ - model: PVCModel, - queryOptions: { name: pvcName, ns: namespace }, - patches: [ - { - op: 'replace', - path: '/metadata/annotations/openshift.io~1description', - value: description, - }, - ], - }); + opts?: K8sAPIOptions, +): Promise => { + const pvc = assemblePvc(data, namespace, existingData.metadata.name); + + return k8sUpdateResource( + applyK8sAPIOptions(opts, { model: PVCModel, resource: _.merge({}, existingData, pvc) }), + ); +}; export const deletePvc = (pvcName: string, namespace: string): Promise => k8sDeleteResource({ @@ -115,17 +115,20 @@ export const updatePvcSize = ( pvcName: string, namespace: string, size: string, + opts?: K8sAPIOptions, ): Promise => - k8sPatchResource({ - model: PVCModel, - queryOptions: { name: pvcName, ns: namespace }, - patches: [ - { - op: 'replace', - path: '/spec/resources/requests', - value: { - storage: size, + k8sPatchResource( + applyK8sAPIOptions(opts, { + model: PVCModel, + queryOptions: { name: pvcName, ns: namespace }, + patches: [ + { + op: 'replace', + path: '/spec/resources/requests', + value: { + storage: size, + }, }, - }, - ], - }); + ], + }), + ); diff --git a/frontend/src/app/App.scss b/frontend/src/app/App.scss index 5167531596..a0cdb25bfc 100644 --- a/frontend/src/app/App.scss +++ b/frontend/src/app/App.scss @@ -1,4 +1,6 @@ -html, body, #root { +html, +body, +#root { height: 100%; } @@ -14,19 +16,25 @@ html, body, #root { flex: 1; margin: var(--pf-global--spacer--md); } + .pf-c-app-launcher__group-title { font-size: 13px; } + .pf-c-app-launcher__menu-item-external-icon { opacity: 1; color: var(--pf-global--icon--Color--light); } + &__brand { height: 36px; } } + // specificity targeting form elements to override --pf-global--FontSize--md -.pf-c-page, .pf-c-modal-box { +.pf-c-page, +.pf-c-modal-box { + .pf-c-app-launcher, .pf-c-button, .pf-c-dropdown, @@ -41,3 +49,13 @@ html, body, #root { height: auto; } } + +// temp fix until https://github.com/patternfly/patternfly-react/issues/8028 is resolved +// Remove this file and its uses after bumping to PF5 +.checkbox-radio-fix-body-width { + + .pf-c-check__body, + .pf-c-radio__body { + width: 100%; + } +} \ No newline at end of file diff --git a/frontend/src/components/ValueUnitField.tsx b/frontend/src/components/ValueUnitField.tsx index f4de70994e..0a639fe7a6 100644 --- a/frontend/src/components/ValueUnitField.tsx +++ b/frontend/src/components/ValueUnitField.tsx @@ -37,6 +37,7 @@ const ValueUnitField: React.FC = ({ setOpen(!open)}> {currentUnitOption.name} diff --git a/frontend/src/pages/clusterSettings/ClusterSettings.tsx b/frontend/src/pages/clusterSettings/ClusterSettings.tsx index f8a20e6be3..7f17fef4a5 100644 --- a/frontend/src/pages/clusterSettings/ClusterSettings.tsx +++ b/frontend/src/pages/clusterSettings/ClusterSettings.tsx @@ -25,7 +25,7 @@ const ClusterSettings: React.FC = () => { const [saving, setSaving] = React.useState(false); const [loadError, setLoadError] = React.useState(); const [clusterSettings, setClusterSettings] = React.useState(DEFAULT_CONFIG); - const [pvcSize, setPvcSize] = React.useState(DEFAULT_PVC_SIZE); + const [pvcSize, setPvcSize] = React.useState(DEFAULT_PVC_SIZE); const [userTrackingEnabled, setUserTrackingEnabled] = React.useState(false); const [cullerTimeout, setCullerTimeout] = React.useState(DEFAULT_CULLER_TIMEOUT); const { dashboardConfig } = useAppContext(); diff --git a/frontend/src/pages/clusterSettings/PVCSizeSettings.tsx b/frontend/src/pages/clusterSettings/PVCSizeSettings.tsx index 0195977829..d10fd1bd43 100644 --- a/frontend/src/pages/clusterSettings/PVCSizeSettings.tsx +++ b/frontend/src/pages/clusterSettings/PVCSizeSettings.tsx @@ -15,9 +15,9 @@ import SettingSection from '~/components/SettingSection'; import { DEFAULT_PVC_SIZE, MAX_PVC_SIZE, MIN_PVC_SIZE } from './const'; type PVCSizeSettingsProps = { - initialValue: number | string; - pvcSize: number | string; - setPvcSize: (size: number | string) => void; + initialValue: number; + pvcSize: number; + setPvcSize: (size: number) => void; }; const PVCSizeSettings: React.FC = ({ initialValue, pvcSize, setPvcSize }) => { @@ -57,7 +57,7 @@ all users." : newValue; setPvcSize(newValue); } else { - setPvcSize(modifiedValue); + setPvcSize(0); } }} /> @@ -79,8 +79,8 @@ all users." Note: PVC size must be between 1 GiB and 16384 GiB. diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionSection.tsx index 86f5ebd55b..39800aaae3 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/DataConnectionSection.tsx @@ -7,7 +7,6 @@ import { } from '~/pages/modelServing/screens/types'; import AWSField from '~/pages/projects/dataConnections/AWSField'; import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; -import '~/pages/projects/screens/detail/storage/ManageStorageModal.scss'; import DataConnectionExistingField from './DataConnectionExistingField'; import DataConnectionFolderPathField from './DataConnectionFolderPathField'; diff --git a/frontend/src/pages/notebookController/const.ts b/frontend/src/pages/notebookController/const.ts index d28a998ba1..40f2b4042f 100644 --- a/frontend/src/pages/notebookController/const.ts +++ b/frontend/src/pages/notebookController/const.ts @@ -2,7 +2,6 @@ import { NotebookControllerUserState } from '~/types'; export const CUSTOM_VARIABLE = 'Custom variable'; export const EMPTY_KEY = '---NO KEY---'; -export const DEFAULT_PVC_SIZE = '20Gi'; export const CURRENT_BROWSER_TAB_PREFERENCE = 'odh.dashboard.kfnbc.tab.preference'; export const ENV_VAR_NAME_REGEX = new RegExp('^[-._a-zA-Z][-._a-zA-Z0-9]*$'); export const EMPTY_USER_STATE: NotebookControllerUserState = { diff --git a/frontend/src/pages/projects/components/PVSizeField.tsx b/frontend/src/pages/projects/components/PVSizeField.tsx index 4e8366515b..b30aa31ac0 100644 --- a/frontend/src/pages/projects/components/PVSizeField.tsx +++ b/frontend/src/pages/projects/components/PVSizeField.tsx @@ -1,59 +1,35 @@ import * as React from 'react'; -import { FormGroup, InputGroup, InputGroupText, NumberInput } from '@patternfly/react-core'; +import { FormGroup } from '@patternfly/react-core'; import { ExclamationTriangleIcon } from '@patternfly/react-icons'; -import { isHTMLInputElement } from '~/utilities/utils'; +import ValueUnitField from '~/components/ValueUnitField'; +import { MEMORY_UNITS } from '~/utilities/valueUnits'; type PVSizeFieldProps = { fieldID: string; - size: number; - setSize: (size: number) => void; + size: string; + setSize: (size: string) => void; currentSize?: string; }; -const PVSizeField: React.FC = ({ fieldID, size, setSize, currentSize }) => { - const minSize = parseInt(currentSize || '') || 1; - - const onStep = (step: number) => { - setSize(Math.max(size + step, minSize)); - }; - return ( - } - validated={currentSize ? 'warning' : 'default'} - fieldId={fieldID} - > - - onStep(1)} - onMinus={() => onStep(-1)} - onChange={(event) => { - if (isHTMLInputElement(event.target)) { - const newSize = Number(event.target.value); - setSize(isNaN(newSize) ? size : newSize); - } - }} - onBlur={(event) => { - if (isHTMLInputElement(event.target)) { - const blurSize = Number(event.target.value); - setSize(Math.max(blurSize, minSize)); - } - }} - /> - GiB - - - ); -}; +const PVSizeField: React.FC = ({ fieldID, size, setSize, currentSize }) => ( + } + validated={currentSize ? 'warning' : 'default'} + fieldId={fieldID} + > + setSize(value)} + options={MEMORY_UNITS} + value={size} + /> + +); export default PVSizeField; diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.scss b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.scss deleted file mode 100644 index 31610106aa..0000000000 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.scss +++ /dev/null @@ -1,12 +0,0 @@ -// temp fix until https://github.com/patternfly/patternfly-react/issues/8028 is resolved -.checkbox-radio-fix-body-width-50 { - .pf-c-check__body, .pf-c-radio__body { - width: 50%; - } -} - -.checkbox-radio-fix-body-width { - .pf-c-check__body, .pf-c-radio__body { - width: 100%; - } -} diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx index d007c90736..59334b0eaa 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx @@ -1,14 +1,6 @@ import * as React from 'react'; -import { Alert, Button, Form, Modal, Stack, StackItem } from '@patternfly/react-core'; -import { - assemblePvc, - attachNotebookPVC, - createPvc, - removeNotebookPVC, - updatePvcDescription, - updatePvcDisplayName, - updatePvcSize, -} from '~/api'; +import { Form, Modal, Stack, StackItem } from '@patternfly/react-core'; +import { attachNotebookPVC, createPvc, removeNotebookPVC, updatePvc } from '~/api'; import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { useCreateStorageObjectForNotebook } from '~/pages/projects/screens/spawner/storage/utils'; @@ -17,13 +9,12 @@ import StorageNotebookConnections from '~/pages/projects/notebook/StorageNoteboo import useRelatedNotebooks, { ConnectedNotebookContext, } from '~/pages/projects/notebook/useRelatedNotebooks'; -import { getPvcDescription, getPvcDisplayName, getPvcTotalSize } from '~/pages/projects/utils'; import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart'; +import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; +import { getPvcDescription, getPvcDisplayName } from '~/pages/projects/utils'; import ExistingConnectedNotebooks from './ExistingConnectedNotebooks'; -import './ManageStorageModal.scss'; - type AddStorageModalProps = { existingData?: PersistentVolumeClaimKind; isOpen: boolean; @@ -52,6 +43,7 @@ const ManageStorageModal: React.FC = ({ existingData, isOp onClose(submitted); setActionInProgress(false); setRemovedNotebooks([]); + setError(undefined); resetData(); }; @@ -61,72 +53,60 @@ const ManageStorageModal: React.FC = ({ existingData, isOp const canCreate = !actionInProgress && createData.nameDesc.name.trim() && hasValidNotebookRelationship; - const submit = async () => { - setError(undefined); - setActionInProgress(true); - + const runPromiseActions = async (dryRun: boolean) => { const { - nameDesc: { name, description }, - size, forNotebook: { name: notebookName, mountPath }, } = createData; - - const pvc = assemblePvc(name, namespace, description, size); - - const handleError = (e: Error) => { - setError(e); - setActionInProgress(false); - }; - const handleNotebookNameConnection = (pvcName: string) => { - if (notebookName) { - attachNotebookPVC(notebookName, namespace, pvcName, mountPath.value) - .then(() => { - setActionInProgress(false); - onBeforeClose(true); - }) - .catch((e) => { - setError(e); - setActionInProgress(false); - }); - } else { - setActionInProgress(false); - onBeforeClose(true); - } - }; - + const pvcPromises: Promise[] = []; if (existingData) { const pvcName = existingData.metadata.name; - if (getPvcDisplayName(existingData) !== createData.nameDesc.name) { - await updatePvcDisplayName(pvcName, namespace, createData.nameDesc.name); - } - if (getPvcDescription(existingData) !== createData.nameDesc.description) { - await updatePvcDescription(pvcName, namespace, createData.nameDesc.description); + if ( + getPvcDisplayName(existingData) !== createData.nameDesc.name || + getPvcDescription(existingData) !== createData.nameDesc.description || + existingData.spec.resources.requests.storage !== createData.size + ) { + pvcPromises.push(updatePvc(createData, existingData, namespace, { dryRun })); } if (removedNotebooks.length > 0) { // Remove connected pvcs - Promise.all( - removedNotebooks.map((notebookName) => - removeNotebookPVC(notebookName, namespace, pvcName), + pvcPromises.push( + ...removedNotebooks.map((notebookName) => + removeNotebookPVC(notebookName, namespace, pvcName, { dryRun }), ), - ) - .then(() => handleNotebookNameConnection(pvcName)) - .catch(handleError); - return; + ); } - handleNotebookNameConnection(pvcName); - if (parseInt(getPvcTotalSize(existingData)) !== createData.size) { - await updatePvcSize(pvcName, namespace, `${createData.size}Gi`); + + await Promise.all(pvcPromises); + if (notebookName) { + await attachNotebookPVC(notebookName, namespace, pvcName, mountPath.value, { + dryRun, + }); } - } else { - createPvc(pvc) - .then((createdPvc) => handleNotebookNameConnection(createdPvc.metadata.name)) - .catch(handleError); + return; } + const createdPvc = await createPvc(createData, namespace, { dryRun }); + if (notebookName) { + await attachNotebookPVC(notebookName, namespace, createdPvc.metadata.name, mountPath.value, { + dryRun, + }); + } + }; + + const submit = () => { + setError(undefined); + setActionInProgress(true); + + runPromiseActions(true) + .then(() => runPromiseActions(false).then(() => onBeforeClose(true))) + .catch((e) => { + setError(e); + setActionInProgress(false); + }); }; return ( = ({ existingData, isOp isOpen={isOpen} onClose={() => onBeforeClose(false)} showClose - actions={[ - , - , - ]} + footer={ + onBeforeClose(false)} + isSubmitDisabled={!canCreate} + error={error} + alertTitle="Error creating storage" + /> + } > - - -
{ - e.preventDefault(); - submit(); - }} - > + { + e.preventDefault(); + submit(); + }} + > + + setCreateData(key, value)} currentSize={existingData?.status?.capacity?.storage} autoFocusName /> - {createData.hasExistingNotebookConnections && ( + + {createData.hasExistingNotebookConnections && ( + @@ -168,7 +152,9 @@ const ManageStorageModal: React.FC = ({ existingData, isOp loaded={notebookLoaded} error={notebookError} /> - )} + + )} + { setCreateData('forNotebook', forNotebookData); @@ -176,21 +162,14 @@ const ManageStorageModal: React.FC = ({ existingData, isOp forNotebookData={createData.forNotebook} isDisabled={connectedNotebooks.length !== 0 && removedNotebooks.length === 0} /> - - - {restartNotebooks.length !== 0 && ( - - - - )} - {error && ( - - - {error.message} - - )} -
+ {restartNotebooks.length !== 0 && ( + + + + )} + + ); }; diff --git a/frontend/src/pages/projects/screens/spawner/service.ts b/frontend/src/pages/projects/screens/spawner/service.ts index cd3fa8a273..7deda08b56 100644 --- a/frontend/src/pages/projects/screens/spawner/service.ts +++ b/frontend/src/pages/projects/screens/spawner/service.ts @@ -1,7 +1,6 @@ import * as _ from 'lodash'; import { assembleConfigMap, - assemblePvc, assembleSecret, createConfigMap, createPvc, @@ -31,19 +30,12 @@ export const createPvcDataForNotebook = async ( projectName: string, storageData: StorageData, ): Promise<{ volumes: Volume[]; volumeMounts: VolumeMount[] }> => { - const { - storageType, - creating: { - nameDesc: { name: pvcName, description: pvcDescription }, - size, - }, - } = storageData; + const { storageType } = storageData; const { volumes, volumeMounts } = getVolumesByStorageData(storageData); if (storageType === StorageType.NEW_PVC) { - const pvcData = assemblePvc(pvcName, projectName, pvcDescription, size); - const pvc = await createPvc(pvcData); + const pvc = await createPvc(storageData.creating, projectName); const newPvcName = pvc.metadata.name; volumes.push({ name: newPvcName, persistentVolumeClaim: { claimName: newPvcName } }); volumeMounts.push({ mountPath: ROOT_MOUNT_PATH, name: newPvcName }); @@ -58,10 +50,6 @@ export const replaceRootVolumesForNotebook = async ( ): Promise<{ volumes: Volume[]; volumeMounts: VolumeMount[] }> => { const { storageType, - creating: { - nameDesc: { name: creatingName, description }, - size, - }, existing: { storage: existingName }, } = storageData; @@ -78,8 +66,7 @@ export const replaceRootVolumesForNotebook = async ( }; replacedVolumeMount = { name: existingName, mountPath: ROOT_MOUNT_PATH }; } else { - const pvcData = assemblePvc(creatingName, projectName, description, size); - const pvc = await createPvc(pvcData); + const pvc = await createPvc(storageData.creating, projectName); const newPvcName = pvc.metadata.name; replacedVolume = { name: newPvcName, persistentVolumeClaim: { claimName: newPvcName } }; replacedVolumeMount = { mountPath: ROOT_MOUNT_PATH, name: newPvcName }; diff --git a/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx b/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx index 8f74f6a47f..b6ce58c6bd 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx @@ -5,8 +5,6 @@ import { getDashboardMainContainer } from '~/utilities/utils'; import CreateNewStorageSection from './CreateNewStorageSection'; import AddExistingStorageField from './AddExistingStorageField'; -import '~/pages/projects/screens/detail/storage/ManageStorageModal.scss'; - type StorageFieldType = { storageData: StorageData; setStorageData: UpdateObjectAtPropAndValue; diff --git a/frontend/src/pages/projects/screens/spawner/storage/useAvailablePvcSize.ts b/frontend/src/pages/projects/screens/spawner/storage/useAvailablePvcSize.ts deleted file mode 100644 index 893074d886..0000000000 --- a/frontend/src/pages/projects/screens/spawner/storage/useAvailablePvcSize.ts +++ /dev/null @@ -1,24 +0,0 @@ -import * as React from 'react'; -import { AppContext } from '~/app/AppContext'; - -const DEFAULT_PVC_SIZE = 20; - -const useDefaultPvcSize = (): number => { - const { - dashboardConfig: { - spec: { notebookController }, - }, - } = React.useContext(AppContext); - - let defaultPvcSize = DEFAULT_PVC_SIZE; - if (notebookController?.pvcSize) { - const parsedConfigSize = parseInt(notebookController?.pvcSize); - if (!isNaN(parsedConfigSize)) { - defaultPvcSize = parsedConfigSize; - } - } - - return defaultPvcSize; -}; - -export default useDefaultPvcSize; diff --git a/frontend/src/pages/projects/screens/spawner/storage/useDefaultPvcSize.ts b/frontend/src/pages/projects/screens/spawner/storage/useDefaultPvcSize.ts new file mode 100644 index 0000000000..48ff049a79 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/useDefaultPvcSize.ts @@ -0,0 +1,14 @@ +import * as React from 'react'; +import { AppContext } from '~/app/AppContext'; + +const useDefaultPvcSize = (): string => { + const { + dashboardConfig: { + spec: { notebookController }, + }, + } = React.useContext(AppContext); + + return notebookController?.pvcSize || '20Gi'; +}; + +export default useDefaultPvcSize; diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 5a2282c438..2ba0deff6e 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -13,7 +13,7 @@ import useRelatedNotebooks, { } from '~/pages/projects/notebook/useRelatedNotebooks'; import useGenericObjectState from '~/utilities/useGenericObjectState'; import { getRootVolumeName } from '~/pages/projects/screens/spawner/spawnerUtils'; -import useDefaultPvcSize from './useAvailablePvcSize'; +import useDefaultPvcSize from './useDefaultPvcSize'; export const useCreateStorageObjectForNotebook = ( existingData?: PersistentVolumeClaimKind, @@ -22,14 +22,14 @@ export const useCreateStorageObjectForNotebook = ( setData: UpdateObjectAtPropAndValue, resetDefaults: () => void, ] => { - const defaultPvcSize = useDefaultPvcSize(); + const size = useDefaultPvcSize(); const createDataState = useGenericObjectState({ nameDesc: { name: '', k8sName: undefined, description: '', }, - size: defaultPvcSize, + size, forNotebook: { name: '', mountPath: { @@ -44,11 +44,13 @@ export const useCreateStorageObjectForNotebook = ( const existingName = existingData ? getPvcDisplayName(existingData) : ''; const existingDescription = existingData ? getPvcDescription(existingData) : ''; - const existingSize = existingData ? existingData.spec.resources.requests.storage : ''; + const existingSize = existingData ? existingData.spec.resources.requests.storage : size; const { notebooks: relatedNotebooks } = useRelatedNotebooks( ConnectedNotebookContext.REMOVABLE_PVC, existingData ? existingData.metadata.name : undefined, ); + const hasExistingNotebookConnections = relatedNotebooks.length > 0; + React.useEffect(() => { if (existingName) { setCreateData('nameDesc', { @@ -56,16 +58,17 @@ export const useCreateStorageObjectForNotebook = ( description: existingDescription, }); - if (relatedNotebooks.length > 0) { - setCreateData('hasExistingNotebookConnections', true); - } + setCreateData('hasExistingNotebookConnections', hasExistingNotebookConnections); - const newSize = parseInt(existingSize); - if (newSize) { - setCreateData('size', newSize); - } + setCreateData('size', existingSize); } - }, [existingName, existingDescription, setCreateData, relatedNotebooks, existingSize]); + }, [ + existingName, + existingDescription, + setCreateData, + hasExistingNotebookConnections, + existingSize, + ]); return createDataState; }; @@ -90,7 +93,7 @@ export const useStorageDataObject = ( setData: UpdateObjectAtPropAndValue, resetDefaults: () => void, ] => { - const defaultPvcSize = useDefaultPvcSize(); + const size = useDefaultPvcSize(); return useGenericObjectState({ storageType: notebook ? StorageType.EXISTING_PVC : StorageType.NEW_PVC, creating: { @@ -98,7 +101,7 @@ export const useStorageDataObject = ( name: '', description: '', }, - size: defaultPvcSize, + size, }, existing: { storage: getRootVolumeName(notebook), diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index 74645f665d..2524ecc2cc 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -19,7 +19,7 @@ export type NameDescType = { export type CreatingStorageObject = { nameDesc: NameDescType; - size: number; + size: string; }; export type MountPath = { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 4781bc9072..dac57bdefd 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -153,7 +153,7 @@ export type NotebookTolerationFormSettings = TolerationSettings & { export type ClusterSettingsType = { userTrackingEnabled: boolean; - pvcSize: number | string; + pvcSize: number; cullerTimeout: number; notebookTolerationSettings: TolerationSettings | null; }; From f9404529218c14d0ab9120fca7d2b3cdea03e590 Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Tue, 31 Oct 2023 16:30:07 -0400 Subject: [PATCH 3/3] Add tests for project view --- .../src/__mocks__/mockProjectK8sResource.ts | 42 ++++++++++++++++++- .../pages/projects/ProjectView.spec.ts | 29 ++++++++++++- .../pages/projects/ProjectView.stories.tsx | 22 ++++++---- .../src/concepts/projects/ProjectsContext.tsx | 12 +----- .../__tests__/isAvailableProject.spec.ts | 33 +++++++++++++++ frontend/src/concepts/projects/utils.ts | 9 ++++ frontend/src/k8sTypes.ts | 6 ++- .../screens/projects/ProjectListView.tsx | 1 + .../screens/projects/ProjectScopeSelect.tsx | 1 + 9 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 frontend/src/concepts/projects/__tests__/isAvailableProject.spec.ts create mode 100644 frontend/src/concepts/projects/utils.ts diff --git a/frontend/src/__mocks__/mockProjectK8sResource.ts b/frontend/src/__mocks__/mockProjectK8sResource.ts index 01373f114d..14033257d4 100644 --- a/frontend/src/__mocks__/mockProjectK8sResource.ts +++ b/frontend/src/__mocks__/mockProjectK8sResource.ts @@ -1,3 +1,4 @@ +import { K8sResourceListResult } from '@openshift/dynamic-plugin-sdk-utils'; import { KnownLabels, ProjectKind } from '~/k8sTypes'; type MockResourceConfigType = { @@ -6,6 +7,7 @@ type MockResourceConfigType = { description?: string; k8sName?: string; enableModelMesh?: boolean; + isDSProject?: boolean; }; export const mockProjectK8sResource = ({ @@ -14,6 +16,7 @@ export const mockProjectK8sResource = ({ k8sName = 'test-project', enableModelMesh = true, description = '', + isDSProject = true, }: MockResourceConfigType): ProjectKind => ({ kind: 'Project', apiVersion: 'project.openshift.io/v1', @@ -23,7 +26,7 @@ export const mockProjectK8sResource = ({ labels: { 'kubernetes.io/metadata.name': k8sName, [KnownLabels.MODEL_SERVING_PROJECT]: enableModelMesh ? 'true' : 'false', - [KnownLabels.DASHBOARD_RESOURCE]: 'true', + ...(isDSProject && { [KnownLabels.DASHBOARD_RESOURCE]: 'true' }), }, annotations: { 'openshift.io/description': description, @@ -35,3 +38,40 @@ export const mockProjectK8sResource = ({ phase: 'Active', }, }); + +export const mockProjectsK8sList = (): K8sResourceListResult => ({ + apiVersion: 'project.openshift.io/v1', + metadata: { continue: '', resourceVersion: '1462210' }, + items: [ + mockProjectK8sResource({ + k8sName: 'ds-project-1', + displayName: 'DS Project 1', + isDSProject: true, + }), + mockProjectK8sResource({ + k8sName: 'ds-project-2', + displayName: 'DS Project 2', + isDSProject: true, + }), + mockProjectK8sResource({ + k8sName: 'ds-project-3', + displayName: 'DS Project 3', + isDSProject: true, + }), + mockProjectK8sResource({ + k8sName: 'non-ds-project-1', + displayName: 'Non-DS Project 1', + isDSProject: false, + }), + mockProjectK8sResource({ + k8sName: 'non-ds-project-2', + displayName: 'Non-DS Project 2', + isDSProject: false, + }), + mockProjectK8sResource({ + k8sName: 'non-ds-project-3', + displayName: 'Non-DS Project 3', + isDSProject: false, + }), + ], +}); diff --git a/frontend/src/__tests__/integration/pages/projects/ProjectView.spec.ts b/frontend/src/__tests__/integration/pages/projects/ProjectView.spec.ts index 7f4800427b..2fb71a1974 100644 --- a/frontend/src/__tests__/integration/pages/projects/ProjectView.spec.ts +++ b/frontend/src/__tests__/integration/pages/projects/ProjectView.spec.ts @@ -1,5 +1,32 @@ import { test, expect } from '@playwright/test'; +test('Project view page', async ({ page }) => { + await page.goto( + './iframe.html?id=tests-integration-pages-projects-projectview--default&viewMode=story', + ); + + // wait for page to load + await page.waitForSelector('text=Data science projects'); + + // Test that it only shows DS Projects at first + await expect(page.getByText('DS Project 1', { exact: true })).toBeVisible(); + await expect(page.getByText('DS Project 2', { exact: true })).toBeVisible(); + await expect(page.getByText('DS Project 3', { exact: true })).toBeVisible(); + await expect(page.getByText('Non-DS Project 1', { exact: true })).toBeHidden(); + await expect(page.getByText('Non-DS Project 2', { exact: true })).toBeHidden(); + await expect(page.getByText('Non-DS Project 3', { exact: true })).toBeHidden(); + + // Change the selection and make sure it shows all projects + await page.locator('#project-scope-selection').click(); + await page.getByText('All available projects', { exact: true }).click(); + await expect(page.getByText('DS Project 1', { exact: true })).toBeVisible(); + await expect(page.getByText('DS Project 2', { exact: true })).toBeVisible(); + await expect(page.getByText('DS Project 3', { exact: true })).toBeVisible(); + await expect(page.getByText('Non-DS Project 1', { exact: true })).toBeVisible(); + await expect(page.getByText('Non-DS Project 2', { exact: true })).toBeVisible(); + await expect(page.getByText('Non-DS Project 3', { exact: true })).toBeVisible(); +}); + test('Create project', async ({ page }) => { await page.goto( './iframe.html?id=tests-integration-pages-projects-projectview--create-project&viewMode=story', @@ -81,7 +108,7 @@ test('Delete project', async ({ page }) => { // Test that can submit on valid form await expect(page.getByRole('button', { name: 'Delete project' })).toBeDisabled(); - await page.getByRole('textbox', { name: 'Delete modal input' }).fill('Test Project'); + await page.getByRole('textbox', { name: 'Delete modal input' }).fill('DS Project 1'); await expect(page.getByRole('button', { name: 'Delete project' })).toBeEnabled(); // Test if error alert will pop up diff --git a/frontend/src/__tests__/integration/pages/projects/ProjectView.stories.tsx b/frontend/src/__tests__/integration/pages/projects/ProjectView.stories.tsx index 3015663db7..62f6939098 100644 --- a/frontend/src/__tests__/integration/pages/projects/ProjectView.stories.tsx +++ b/frontend/src/__tests__/integration/pages/projects/ProjectView.stories.tsx @@ -1,7 +1,7 @@ import { Meta, StoryObj } from '@storybook/react'; import { rest } from 'msw'; import { within, userEvent } from '@storybook/testing-library'; -import { mockProjectK8sResource } from '~/__mocks__/mockProjectK8sResource'; +import { mockProjectsK8sList } from '~/__mocks__/mockProjectK8sResource'; import { mockNotebookK8sResource } from '~/__mocks__/mockNotebookK8sResource'; import { mockK8sResourceList } from '~/__mocks__/mockK8sResourceList'; import { mockPodK8sResource } from '~/__mocks__/mockPodK8sResource'; @@ -26,7 +26,7 @@ export default { (req, res, ctx) => res(ctx.json(mockK8sResourceList([mockNotebookK8sResource({})]))), ), rest.get('/api/k8s/apis/project.openshift.io/v1/projects', (req, res, ctx) => - res(ctx.json(mockK8sResourceList([mockProjectK8sResource({})]))), + res(ctx.json(mockProjectsK8sList())), ), rest.delete( '/api/k8s/apis/project.openshift.io/v1/projects/test-project', @@ -45,6 +45,14 @@ export default { }, } as Meta; +export const Default: StoryObj = { + play: async ({ canvasElement }) => { + // load page and wait until settled + const canvas = within(canvasElement); + await canvas.findByText('DS Project 1', undefined, { timeout: 5000 }); + }, +}; + export const EditProject: StoryObj = { parameters: { a11y: { @@ -56,10 +64,10 @@ export const EditProject: StoryObj = { play: async ({ canvasElement }) => { // load page and wait until settled const canvas = within(canvasElement); - await canvas.findByText('Test Project', undefined, { timeout: 5000 }); + await canvas.findByText('DS Project 1', undefined, { timeout: 5000 }); // user flow for editing a project - await userEvent.click(canvas.getByLabelText('Actions', { selector: 'button' })); + await userEvent.click(canvas.getAllByLabelText('Actions', { selector: 'button' })[0]); await userEvent.click(canvas.getByText('Edit project', { selector: 'button' })); }, }; @@ -74,10 +82,10 @@ export const DeleteProject: StoryObj = { play: async ({ canvasElement }) => { // load page and wait until settled const canvas = within(canvasElement); - await canvas.findByText('Test Project', undefined, { timeout: 5000 }); + await canvas.findByText('DS Project 1', undefined, { timeout: 5000 }); // user flow for deleting a project - await userEvent.click(canvas.getByLabelText('Actions', { selector: 'button' })); + await userEvent.click(canvas.getAllByLabelText('Actions', { selector: 'button' })[0]); await userEvent.click(canvas.getByText('Delete project', { selector: 'button' })); }, }; @@ -92,7 +100,7 @@ export const CreateProject: StoryObj = { play: async ({ canvasElement }) => { // load page and wait until settled const canvas = within(canvasElement); - await canvas.findByText('Test Project', undefined, { timeout: 5000 }); + await canvas.findByText('DS Project 1', undefined, { timeout: 5000 }); // user flow for deleting a project await userEvent.click(canvas.getByText('Create data science project', { selector: 'button' })); diff --git a/frontend/src/concepts/projects/ProjectsContext.tsx b/frontend/src/concepts/projects/ProjectsContext.tsx index 7d3342965d..94202b1cc6 100644 --- a/frontend/src/concepts/projects/ProjectsContext.tsx +++ b/frontend/src/concepts/projects/ProjectsContext.tsx @@ -3,6 +3,7 @@ import useFetchState, { FetchState } from '~/utilities/useFetchState'; import { getProjects } from '~/api'; import { KnownLabels, ProjectKind } from '~/k8sTypes'; import { useDashboardNamespace } from '~/redux/selectors'; +import { isAvailableProject } from '~/concepts/projects/utils'; type ProjectFetchState = FetchState; type ProjectsContext = { @@ -65,16 +66,7 @@ const ProjectsContextProvider: React.FC = ({ children }) nonActiveProjects: ProjectKind[]; }>( (states, project) => { - if ( - !( - project.metadata.name.startsWith('openshift-') || - project.metadata.name.startsWith('kube-') || - project.metadata.name === 'default' || - project.metadata.name === 'system' || - project.metadata.name === 'openshift' || - project.metadata.name === dashboardNamespace - ) - ) { + if (isAvailableProject(project.metadata.name, dashboardNamespace)) { if (project.status?.phase === 'Active') { // Project that is active states.projects.push(project); diff --git a/frontend/src/concepts/projects/__tests__/isAvailableProject.spec.ts b/frontend/src/concepts/projects/__tests__/isAvailableProject.spec.ts new file mode 100644 index 0000000000..73df4f0c8d --- /dev/null +++ b/frontend/src/concepts/projects/__tests__/isAvailableProject.spec.ts @@ -0,0 +1,33 @@ +import { isAvailableProject } from '~/concepts/projects/utils'; + +const mockDashboardNamespace = 'mock-opendatahub'; + +describe('isAvailableProject', () => { + it('should be false when the project starts with "openshift-"', () => { + expect(isAvailableProject('openshift-monitoring', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift-apiserver', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift-authentication', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift-config', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift-infra', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift-node', mockDashboardNamespace)).toBe(false); + }); + it('should be false when the project starts with "kube-"', () => { + expect(isAvailableProject('kube-public', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('kube-system', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('kube-node-lease', mockDashboardNamespace)).toBe(false); + }); + it('should be false when the project is "default", "system", or "openshift"', () => { + expect(isAvailableProject('default', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('system', mockDashboardNamespace)).toBe(false); + expect(isAvailableProject('openshift', mockDashboardNamespace)).toBe(false); + }); + it('should be false when the project is where the dashboard is deployed', () => { + expect(isAvailableProject(mockDashboardNamespace, mockDashboardNamespace)).toBe(false); + }); + it('should be true in all the other situations', () => { + expect(isAvailableProject('notebook-images', mockDashboardNamespace)).toBe(true); + expect(isAvailableProject('openshiftblabla', mockDashboardNamespace)).toBe(true); + expect(isAvailableProject('kubelike', mockDashboardNamespace)).toBe(true); + expect(isAvailableProject('odh-not-dashboard', mockDashboardNamespace)).toBe(true); + }); +}); diff --git a/frontend/src/concepts/projects/utils.ts b/frontend/src/concepts/projects/utils.ts new file mode 100644 index 0000000000..374b55fb92 --- /dev/null +++ b/frontend/src/concepts/projects/utils.ts @@ -0,0 +1,9 @@ +export const isAvailableProject = (projectName: string, dashboardNamespace: string) => + !( + projectName.startsWith('openshift-') || + projectName.startsWith('kube-') || + projectName === 'default' || + projectName === 'system' || + projectName === 'openshift' || + projectName === dashboardNamespace + ); diff --git a/frontend/src/k8sTypes.ts b/frontend/src/k8sTypes.ts index 2795ef2eed..26fb4061a3 100644 --- a/frontend/src/k8sTypes.ts +++ b/frontend/src/k8sTypes.ts @@ -284,14 +284,12 @@ export type PodKind = K8sResourceCommon & { }; }; -/** Assumed Dashboard Project -- if we need more beyond that we should break this type up */ export type ProjectKind = K8sResourceCommon & { metadata: { annotations?: DisplayNameAnnotations & Partial<{ 'openshift.io/requester': string; // the username of the user that requested this project }>; - labels: DashboardLabels & Partial; name: string; }; status?: { @@ -299,6 +297,10 @@ export type ProjectKind = K8sResourceCommon & { }; }; +export type DashboardProjectKind = ProjectKind & { + labels: DashboardLabels & Partial; +}; + export type ServiceAccountKind = K8sResourceCommon & { metadata: { annotations?: DisplayNameAnnotations; diff --git a/frontend/src/pages/projects/screens/projects/ProjectListView.tsx b/frontend/src/pages/projects/screens/projects/ProjectListView.tsx index cdd3751c6f..2a8e3b706b 100644 --- a/frontend/src/pages/projects/screens/projects/ProjectListView.tsx +++ b/frontend/src/pages/projects/screens/projects/ProjectListView.tsx @@ -67,6 +67,7 @@ const ProjectListView: React.FC = ({ allowCreate, scope }) } + data-id="project-view-table" rowRenderer={(project) => ( = ({ selection, setS const [isOpen, setOpen] = React.useState(false); return (