From 30440ecf4ae609b907cdaed43c8a7dfea49e9f7c Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Wed, 6 Nov 2024 07:25:06 -0600 Subject: [PATCH 1/8] websocket for notebooks scheduling (#3412) * use websockets for events * remove both useWatchNotebookEvents --- frontend/src/api/k8s/events.ts | 20 ++++++ .../notebook/useWatchNotebookEvents.ts | 61 ------------------ frontend/src/pages/projects/notebook/utils.ts | 8 ++- .../src/utilities/notebookControllerUtils.ts | 19 +++--- .../src/utilities/useWatchNotebookEvents.tsx | 64 ------------------- 5 files changed, 35 insertions(+), 137 deletions(-) delete mode 100644 frontend/src/pages/projects/notebook/useWatchNotebookEvents.ts delete mode 100644 frontend/src/utilities/useWatchNotebookEvents.tsx diff --git a/frontend/src/api/k8s/events.ts b/frontend/src/api/k8s/events.ts index 4ca93854af..2d4a5fdabe 100644 --- a/frontend/src/api/k8s/events.ts +++ b/frontend/src/api/k8s/events.ts @@ -1,6 +1,9 @@ import { k8sListResourceItems } from '@openshift/dynamic-plugin-sdk-utils'; import { EventKind } from '~/k8sTypes'; import { EventModel } from '~/api/models'; +import useK8sWatchResourceList from '~/utilities/useK8sWatchResourceList'; +import { CustomWatchK8sResult } from '~/types'; +import { groupVersionKind } from '..'; export const getNotebookEvents = async ( namespace: string, @@ -18,3 +21,20 @@ export const getNotebookEvents = async ( }, }, }); + +export const useWatchNotebookEvents = ( + namespace: string, + name: string, + podUid?: string, +): CustomWatchK8sResult => + useK8sWatchResourceList( + { + isList: true, + groupVersionKind: groupVersionKind(EventModel), + namespace, + fieldSelector: podUid + ? `involvedObject.kind=Pod,involvedObject.uid=${podUid}` + : `involvedObject.kind=StatefulSet,involvedObject.name=${name}`, + }, + EventModel, + ); diff --git a/frontend/src/pages/projects/notebook/useWatchNotebookEvents.ts b/frontend/src/pages/projects/notebook/useWatchNotebookEvents.ts deleted file mode 100644 index c247e22e05..0000000000 --- a/frontend/src/pages/projects/notebook/useWatchNotebookEvents.ts +++ /dev/null @@ -1,61 +0,0 @@ -import * as React from 'react'; -import { EventKind, NotebookKind } from '~/k8sTypes'; -import { getNotebookEvents } from '~/api'; -import { FAST_POLL_INTERVAL } from '~/utilities/const'; - -export const useWatchNotebookEvents = ( - notebook: NotebookKind, - podUid: string, - activeFetch: boolean, -): EventKind[] => { - const notebookName = notebook.metadata.name; - const { namespace } = notebook.metadata; - const [notebookEvents, setNotebookEvents] = React.useState([]); - - // Cached events are returned when activeFetch is false. - // This allows us to reset notebookEvents state when activeFetch becomes - // false to prevent UI blips when activeFetch goes true again. - const notebookEventsCache = React.useRef(notebookEvents); - - // when activeFetch switches to false, reset events state - React.useEffect(() => { - if (!activeFetch) { - setNotebookEvents([]); - } - }, [activeFetch]); - - React.useEffect(() => { - let watchHandle: ReturnType; - let cancelled = false; - - if (activeFetch && namespace && notebookName) { - const watchNotebookEvents = () => { - getNotebookEvents(namespace, notebookName, podUid) - .then((data: EventKind[]) => { - if (!cancelled) { - notebookEventsCache.current = data; - setNotebookEvents(data); - } - }) - .catch((e) => { - /* eslint-disable-next-line no-console */ - console.error('Error fetching notebook events', e); - }); - watchHandle = setTimeout(watchNotebookEvents, FAST_POLL_INTERVAL); - }; - - if (!podUid) { - // delay the initial fetch to avoid older StatefulSet event errors from blipping on screen during notebook startup - watchHandle = setTimeout(watchNotebookEvents, Math.max(FAST_POLL_INTERVAL, 3000)); - } else { - watchNotebookEvents(); - } - } - return () => { - cancelled = true; - clearTimeout(watchHandle); - }; - }, [namespace, notebookName, podUid, activeFetch]); - - return activeFetch ? notebookEvents : notebookEventsCache.current; -}; diff --git a/frontend/src/pages/projects/notebook/utils.ts b/frontend/src/pages/projects/notebook/utils.ts index 2e5a59c9cb..022b6fb45a 100644 --- a/frontend/src/pages/projects/notebook/utils.ts +++ b/frontend/src/pages/projects/notebook/utils.ts @@ -4,7 +4,7 @@ import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const'; import { fireFormTrackingEvent } from '~/concepts/analyticsTracking/segmentIOUtils'; import { TrackingOutcome } from '~/concepts/analyticsTracking/trackingProperties'; import { AcceleratorProfileState } from '~/utilities/useReadAcceleratorState'; -import { useWatchNotebookEvents } from './useWatchNotebookEvents'; +import { useWatchNotebookEvents } from '~/api'; export const hasStopAnnotation = (notebook: NotebookKind): boolean => !!( @@ -124,7 +124,11 @@ export const useNotebookStatus = ( podUid: string, spawnInProgress: boolean, ): [status: NotebookStatus | null, events: EventKind[]] => { - const events = useWatchNotebookEvents(notebook, podUid, spawnInProgress); + const [events] = useWatchNotebookEvents( + notebook.metadata.namespace, + notebook.metadata.name, + podUid, + ); const annotationTime = notebook.metadata.annotations?.['notebooks.kubeflow.org/last-activity']; const lastActivity = annotationTime diff --git a/frontend/src/utilities/notebookControllerUtils.ts b/frontend/src/utilities/notebookControllerUtils.ts index d22f9852aa..240a4011c5 100644 --- a/frontend/src/utilities/notebookControllerUtils.ts +++ b/frontend/src/utilities/notebookControllerUtils.ts @@ -5,7 +5,6 @@ import { createRoleBinding, getRoleBinding } from '~/services/roleBindingService import { EnvVarReducedTypeKeyValues, EventStatus, - K8sEvent, Notebook, NotebookControllerUserState, NotebookStatus, @@ -19,8 +18,8 @@ import { EMPTY_USER_STATE } from '~/pages/notebookController/const'; import useNamespaces from '~/pages/notebookController/useNamespaces'; import { useAppContext } from '~/app/AppContext'; import { getRoute } from '~/services/routeService'; -import { RoleBindingKind } from '~/k8sTypes'; -import { useWatchNotebookEvents } from './useWatchNotebookEvents'; +import { EventKind, RoleBindingKind } from '~/k8sTypes'; +import { useWatchNotebookEvents } from '~/api'; import { useDeepCompareMemoize } from './useDeepCompareMemoize'; export const usernameTranslate = (username: string): string => { @@ -236,13 +235,13 @@ export const useNotebookRedirectLink = (): (() => Promise) => { }, [notebookNamespace, routeName, currentUserNotebookLink]); }; -export const getEventTimestamp = (event: K8sEvent): string => +export const getEventTimestamp = (event: EventKind): string => event.lastTimestamp || event.eventTime; const filterEvents = ( - allEvents: K8sEvent[], + allEvents: EventKind[], lastActivity: Date, -): [filterEvents: K8sEvent[], thisInstanceEvents: K8sEvent[], gracePeroid: boolean] => { +): [filterEvents: EventKind[], thisInstanceEvents: EventKind[], gracePeroid: boolean] => { const thisInstanceEvents = allEvents .filter((event) => new Date(getEventTimestamp(event)) >= lastActivity) .toSorted((a, b) => getEventTimestamp(a).localeCompare(getEventTimestamp(b))); @@ -301,17 +300,17 @@ const useLastActivity = (annotationValue?: string): Date | null => { export const useNotebookStatus = ( spawnInProgress: boolean, -): [status: NotebookStatus | null, events: K8sEvent[]] => { +): [status: NotebookStatus | null, events: EventKind[]] => { const { currentUserNotebook: notebook, currentUserNotebookIsRunning: isNotebookRunning, currentUserNotebookPodUID, } = React.useContext(NotebookControllerContext); - const events = useWatchNotebookEvents( - notebook, + const [events] = useWatchNotebookEvents( + notebook?.metadata.namespace ?? '', + notebook?.metadata.name ?? '', currentUserNotebookPodUID, - spawnInProgress && !isNotebookRunning, ); const lastActivity = diff --git a/frontend/src/utilities/useWatchNotebookEvents.tsx b/frontend/src/utilities/useWatchNotebookEvents.tsx deleted file mode 100644 index 6a0b2d13df..0000000000 --- a/frontend/src/utilities/useWatchNotebookEvents.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import * as React from 'react'; -import { getNotebookEvents } from '~/services/notebookEventsService'; -import { K8sEvent, Notebook } from '~/types'; -import useNotification from './useNotification'; -import { FAST_POLL_INTERVAL } from './const'; - -export const useWatchNotebookEvents = ( - notebook: Notebook | null, - podUid: string, - activeFetch: boolean, -): K8sEvent[] => { - const notebookName = notebook?.metadata.name; - const namespace = notebook?.metadata.namespace; - const [notebookEvents, setNotebookEvents] = React.useState([]); - const notification = useNotification(); - - // Cached events are returned when activeFetch is false. - // This allows us to reset notebookEvents state when activeFetch becomes - // false to prevent UI blips when activeFetch goes true again. - const notebookEventsCache = React.useRef(notebookEvents); - - // when activeFetch switches to false, reset events state - React.useEffect(() => { - if (!activeFetch) { - setNotebookEvents([]); - } - }, [activeFetch]); - - React.useEffect(() => { - let watchHandle: ReturnType; - let cancelled = false; - - if (activeFetch && namespace && notebookName) { - const watchNotebookEvents = () => { - getNotebookEvents(namespace, notebookName, podUid) - .then((data: K8sEvent[]) => { - if (!cancelled) { - notebookEventsCache.current = data; - setNotebookEvents(data); - } - }) - .catch((e) => { - notification.error('Error fetching notebook events', e.response.data.message); - /* eslint-disable-next-line no-console */ - console.error('Error fetching notebook events', e); - }); - watchHandle = setTimeout(watchNotebookEvents, FAST_POLL_INTERVAL); - }; - - if (!podUid) { - // delay the initial fetch to avoid older StatefulSet event errors from blipping on screen during notebook startup - watchHandle = setTimeout(watchNotebookEvents, Math.max(FAST_POLL_INTERVAL, 3000)); - } else { - watchNotebookEvents(); - } - } - return () => { - cancelled = true; - clearTimeout(watchHandle); - }; - }, [namespace, notebookName, podUid, activeFetch, notification]); - - return activeFetch ? notebookEvents : notebookEventsCache.current; -}; From b897d9d9938a8eb4add83bfbad27b7ca55fd336d Mon Sep 17 00:00:00 2001 From: Christian Vogt Date: Wed, 6 Nov 2024 10:29:12 -0500 Subject: [PATCH 2/8] guard against corrupt connection types and support previous categories (#3440) * provide previously used categories as options when creating a connection type * guard against corrupt data in connection types --- .../cypress/cypress/pages/connectionTypes.ts | 2 +- .../connectionTypes/connectionTypes.cy.ts | 10 ++++++ .../createConnectionType.cy.ts | 10 +++--- .../connectionTypes/ConnectionTypeForm.tsx | 2 +- .../src/concepts/connectionTypes/utils.ts | 24 +++++++++----- .../connectionTypes/ConnectionTypesPage.tsx | 1 + .../manage/DuplicateConnectionTypePage.tsx | 9 ++++- .../manage/EditConnectionTypePage.tsx | 9 ++++- .../manage/ManageConnectionTypePage.tsx | 17 +++++++--- .../src/services/connectionTypesService.ts | 33 +++++++++++-------- 10 files changed, 82 insertions(+), 35 deletions(-) diff --git a/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts index 8004fa97c5..e1f63f399f 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts @@ -139,7 +139,7 @@ class CategorySection extends Contextual { } findMultiGroupSelectButton(name: string) { - return cy.findByTestId(`select-multi-typeahead-${name}`).click(); + return cy.findByTestId(`select-multi-typeahead-${name}`); } } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts index 029d535ee7..a98bd7c7ee 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts @@ -34,7 +34,9 @@ it('Connection types should be hidden by feature flag', () => { }), ); + cy.interceptOdh('GET /api/connection-types', []); connectionTypesPage.visit(); + connectionTypesPage.shouldBeEmpty(); }); describe('Connection types', () => { @@ -48,6 +50,12 @@ describe('Connection types', () => { }), ); cy.interceptOdh('GET /api/connection-types', [ + { + ...mockConnectionTypeConfigMap({ + name: 'corrupt', + }), + data: { category: '[[', fields: '{{' }, + }, mockConnectionTypeConfigMap({}), mockConnectionTypeConfigMap({ name: 'no-display-name', @@ -74,6 +82,8 @@ describe('Connection types', () => { it('should show the correct column values', () => { connectionTypesPage.visit(); + connectionTypesPage.findTable().find('tbody tr').should('have.length', 3); + const row = connectionTypesPage.getConnectionTypeRow('Test display name'); row.shouldHaveDescription('Test description'); row.shouldHaveCreator('dashboard-admin'); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts index cca087dc4c..266ce99db9 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts @@ -22,6 +22,7 @@ describe('create', () => { mockConnectionTypeConfigMap({ displayName: 'URI - v1', name: 'uri-v1', + category: ['existing-category'], fields: [ { type: 'uri', @@ -49,15 +50,16 @@ describe('create', () => { createConnectionTypePage.findConnectionTypeName().type('hello'); categorySection.findCategoryTable(); - categorySection.findMultiGroupSelectButton('Object-storage'); + categorySection.findMultiGroupSelectButton('existing-category').should('exist'); + categorySection.findMultiGroupSelectButton('Object-storage').click(); createConnectionTypePage.findSubmitButton().should('be.enabled'); categorySection.findMultiGroupInput().type('Database'); - categorySection.findMultiGroupSelectButton('Database'); + categorySection.findMultiGroupSelectButton('Database').click(); categorySection.findMultiGroupInput().type('New category'); - categorySection.findMultiGroupSelectButton('Option'); + categorySection.findMultiGroupSelectButton('Option').click(); categorySection.findChipItem('New category').should('exist'); categorySection.findMultiGroupInput().type('{esc}'); @@ -88,7 +90,7 @@ describe('create', () => { createConnectionTypePage.findConnectionTypeName().type('hello'); categorySection.findCategoryTable(); - categorySection.findMultiGroupSelectButton('Object-storage'); + categorySection.findMultiGroupSelectButton('Object-storage').click(); createConnectionTypePage.findSubmitButton().should('be.enabled').click(); createConnectionTypePage.findFooterError().should('contain.text', 'returned error message'); diff --git a/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx b/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx index 696799aefc..20f520b755 100644 --- a/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx +++ b/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx @@ -40,7 +40,7 @@ const createSelectOption = ( )} - {connectionType.data?.category?.length && ( + {!!connectionType.data?.category?.length && ( diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index d32ddd5046..372310ba37 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -37,15 +37,21 @@ export const getConnectionTypeRef = (connection: Connection | undefined): string export const toConnectionTypeConfigMapObj = ( configMap: ConnectionTypeConfigMap, -): ConnectionTypeConfigMapObj => ({ - ...configMap, - data: configMap.data - ? { - category: configMap.data.category ? JSON.parse(configMap.data.category) : undefined, - fields: configMap.data.fields ? JSON.parse(configMap.data.fields) : undefined, - } - : undefined, -}); +): ConnectionTypeConfigMapObj => { + try { + return { + ...configMap, + data: configMap.data + ? { + category: configMap.data.category ? JSON.parse(configMap.data.category) : undefined, + fields: configMap.data.fields ? JSON.parse(configMap.data.fields) : undefined, + } + : undefined, + }; + } catch (e) { + throw new Error('Failed to parse connection type data.'); + } +}; export const toConnectionTypeConfigMap = ( obj: ConnectionTypeConfigMapObj, diff --git a/frontend/src/pages/connectionTypes/ConnectionTypesPage.tsx b/frontend/src/pages/connectionTypes/ConnectionTypesPage.tsx index 606835e95d..24f867b4da 100644 --- a/frontend/src/pages/connectionTypes/ConnectionTypesPage.tsx +++ b/frontend/src/pages/connectionTypes/ConnectionTypesPage.tsx @@ -16,6 +16,7 @@ const ConnectionTypesPage: React.FC = () => { emptyStatePage={} title="Connection types" description="Create and manage connection types for users in your organization. Connection types include customizable fields and optional default values to decrease the time required to add connections to data sources and sinks." + errorMessage="Unable to load connection types" > diff --git a/frontend/src/pages/connectionTypes/manage/DuplicateConnectionTypePage.tsx b/frontend/src/pages/connectionTypes/manage/DuplicateConnectionTypePage.tsx index 77cb298f54..f5cae40d4b 100644 --- a/frontend/src/pages/connectionTypes/manage/DuplicateConnectionTypePage.tsx +++ b/frontend/src/pages/connectionTypes/manage/DuplicateConnectionTypePage.tsx @@ -12,7 +12,14 @@ const DuplicateConnectionTypePage: React.FC = () => { const [existingConnectionType, isLoaded, error] = useConnectionType(name); if (!isLoaded || error) { - return ; + return ( + + ); } const connectionType = stateConnectionType || existingConnectionType; diff --git a/frontend/src/pages/connectionTypes/manage/EditConnectionTypePage.tsx b/frontend/src/pages/connectionTypes/manage/EditConnectionTypePage.tsx index fd71e4bd99..eeaa0512db 100644 --- a/frontend/src/pages/connectionTypes/manage/EditConnectionTypePage.tsx +++ b/frontend/src/pages/connectionTypes/manage/EditConnectionTypePage.tsx @@ -18,7 +18,14 @@ const EditConnectionTypePage: React.FC = () => { } if (!isLoaded || error) { - return ; + return ( + + ); } return ( diff --git a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx index a973b7cd4d..2e52e587b0 100644 --- a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx +++ b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx @@ -69,6 +69,12 @@ const ManageConnectionTypePage: React.FC = ({ prefill, isEdit, onSave }) [connectionTypes], ); + const allCategories = React.useMemo( + () => + new Set([...categoryOptions, ...connectionTypes.map((ct) => ct.data?.category ?? []).flat()]), + [connectionTypes], + ); + const [isDrawerExpanded, setIsDrawerExpanded] = React.useState(false); const initialValues = React.useMemo(() => { @@ -100,11 +106,12 @@ const ManageConnectionTypePage: React.FC = ({ prefill, isEdit, onSave }) const categoryItems = React.useMemo( () => - category - .filter((c) => !categoryOptions.includes(c)) - .concat(categoryOptions) - .map((c) => ({ id: c, name: c, selected: category.includes(c) })), - [category], + [...new Set([...allCategories, ...category])].toSorted().map((c) => ({ + id: c, + name: c, + selected: category.includes(c), + })), + [category, allCategories], ); const connectionTypeObj = React.useMemo( diff --git a/frontend/src/services/connectionTypesService.ts b/frontend/src/services/connectionTypesService.ts index 5ea98d9e1b..1dce39d38f 100644 --- a/frontend/src/services/connectionTypesService.ts +++ b/frontend/src/services/connectionTypesService.ts @@ -12,22 +12,29 @@ import { export const fetchConnectionTypes = (): Promise => { const url = `/api/connection-types`; return axios - .get(url) + .get(url) .then((response) => - response.data.map((cm: ConnectionTypeConfigMap) => toConnectionTypeConfigMapObj(cm)), + response.data.reduce((acc, cm) => { + try { + acc.push(toConnectionTypeConfigMapObj(cm)); + } catch (e) { + // ignore those which fail to parse + } + return acc; + }, []), ) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; export const fetchConnectionType = (name: string): Promise => { const url = `/api/connection-types/${name}`; return axios - .get(url) + .get(url) .then((response) => toConnectionTypeConfigMapObj(response.data)) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; @@ -36,10 +43,10 @@ export const createConnectionType = ( ): Promise => { const url = `/api/connection-types`; return axios - .post(url, toConnectionTypeConfigMap(connectionType)) + .post(url, toConnectionTypeConfigMap(connectionType)) .then((response) => response.data) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; @@ -48,10 +55,10 @@ export const updateConnectionType = ( ): Promise => { const url = `/api/connection-types/${connectionType.metadata.name}`; return axios - .put(url, toConnectionTypeConfigMap(connectionType)) + .put(url, toConnectionTypeConfigMap(connectionType)) .then((response) => response.data) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; @@ -76,19 +83,19 @@ export const updateConnectionTypeEnabled = ( }); return axios - .patch(url, patch) + .patch(url, patch) .then((response) => response.data) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; export const deleteConnectionType = (name: string): Promise => { const url = `/api/connection-types/${name}`; return axios - .delete(url) + .delete(url) .then((response) => response.data) .catch((e) => { - throw new Error(e.response.data.message); + throw new Error(e?.response?.data?.message ?? e.message); }); }; From 50e92c76bd423a9a11c8dd10b7cde2e56065e528 Mon Sep 17 00:00:00 2001 From: Dipanshu Gupta <97534722+dpanshug@users.noreply.github.com> Date: Wed, 6 Nov 2024 21:59:43 +0530 Subject: [PATCH 3/8] Cluster storage modal (#3378) --- .../cypress/cypress/pages/workbench.ts | 2 +- .../detail/storage/BaseStorageModal.tsx | 118 +++++++++++ .../detail/storage/ClusterStorageModal.tsx | 182 +++++++++++++++++ .../screens/detail/storage/StorageList.tsx | 8 +- .../screens/detail/storage/StorageTable.tsx | 4 +- .../storage/AddExistingStorageField.tsx | 14 +- .../storage/AttachExistingStorageModal.tsx | 78 +++++++ .../spawner/storage/SpawnerMountPathField.tsx | 184 +++++++++++++++++ .../spawner/storage/WorkbenchStorageModal.tsx | 51 +++++ .../spawner/storage/__tests__/utils.spec.ts | 192 ++++++++++++++++++ .../projects/screens/spawner/storage/const.ts | 1 + .../projects/screens/spawner/storage/types.ts | 4 + .../projects/screens/spawner/storage/utils.ts | 95 +++++++++ 13 files changed, 925 insertions(+), 8 deletions(-) create mode 100644 frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx create mode 100644 frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts create mode 100644 frontend/src/pages/projects/screens/spawner/storage/const.ts create mode 100644 frontend/src/pages/projects/screens/spawner/storage/types.ts diff --git a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts index 7b03d5b6ef..9824aea5fe 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts @@ -234,7 +234,7 @@ class CreateSpawnerPage { cy.findByTestId('persistent-storage-group') .findByRole('button', { name: 'Typeahead menu toggle' }) .click(); - cy.get('[id="dashboard-page-main"]').findByRole('option', { name }).click(); + cy.get('[id="dashboard-page-main"]').contains('button.pf-v5-c-menu__item', name).click(); } selectPVSize(name: string) { diff --git a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx new file mode 100644 index 0000000000..ffa5ca397a --- /dev/null +++ b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx @@ -0,0 +1,118 @@ +import * as React from 'react'; +import { Form, Modal, Stack, StackItem } from '@patternfly/react-core'; +import { PersistentVolumeClaimKind } from '~/k8sTypes'; +import CreateNewStorageSection from '~/pages/projects/screens/spawner/storage/CreateNewStorageSection'; +import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; +import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; +import usePreferredStorageClass from '~/pages/projects/screens/spawner/storage/usePreferredStorageClass'; +import useDefaultStorageClass from '~/pages/projects/screens/spawner/storage/useDefaultStorageClass'; +import { useCreateStorageObject } from '~/pages/projects/screens/spawner/storage/utils'; +import { CreatingStorageObject } from '~/pages/projects/types'; + +export type BaseStorageModalProps = { + submitLabel?: string; + title?: string; + description?: string; + children: React.ReactNode; + isValid: boolean; + onSubmit: (data: CreatingStorageObject) => Promise; + existingData?: PersistentVolumeClaimKind; + onClose: (submitted: boolean) => void; +}; + +const BaseStorageModal: React.FC = ({ + existingData, + onSubmit, + submitLabel = 'Add storage', + title = 'Add cluster storage', + description = 'Add storage and optionally connect it with an existing workbench.', + children, + isValid, + onClose, +}) => { + const [createData, setCreateData, resetData] = useCreateStorageObject(existingData); + const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; + const preferredStorageClass = usePreferredStorageClass(); + const [defaultStorageClass] = useDefaultStorageClass(); + const [error, setError] = React.useState(); + const [actionInProgress, setActionInProgress] = React.useState(false); + React.useEffect(() => { + if (!existingData) { + if (isStorageClassesAvailable) { + setCreateData('storageClassName', defaultStorageClass?.metadata.name); + } else { + setCreateData('storageClassName', preferredStorageClass?.metadata.name); + } + } + }, [ + isStorageClassesAvailable, + defaultStorageClass, + preferredStorageClass, + existingData, + setCreateData, + ]); + + const onBeforeClose = (submitted: boolean) => { + onClose(submitted); + setError(undefined); + setActionInProgress(false); + resetData(); + }; + + const canCreate = !actionInProgress && createData.nameDesc.name.trim().length > 0 && isValid; + + const submit = () => { + setError(undefined); + setActionInProgress(true); + + onSubmit(createData) + .then(() => onBeforeClose(true)) + .catch((err) => { + setError(err); + setActionInProgress(false); + }); + }; + + return ( + onBeforeClose(false)} + showClose + footer={ + onBeforeClose(false)} + isSubmitDisabled={!canCreate} + error={error} + alertTitle="Error creating storage" + /> + } + > +
{ + e.preventDefault(); + submit(); + }} + > + + + + + {children} + +
+ + ); +}; + +export default BaseStorageModal; diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx new file mode 100644 index 0000000000..cae3f2ff28 --- /dev/null +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx @@ -0,0 +1,182 @@ +import * as React from 'react'; +import { StackItem } from '@patternfly/react-core'; +import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; +import { CreatingStorageObject, ForNotebookSelection, StorageData } from '~/pages/projects/types'; +import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; +import { attachNotebookPVC, createPvc, removeNotebookPVC, restartNotebook, updatePvc } from '~/api'; +import { + ConnectedNotebookContext, + useRelatedNotebooks, +} from '~/pages/projects/notebook/useRelatedNotebooks'; +import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; +import StorageNotebookConnections from '~/pages/projects/notebook/StorageNotebookConnections'; +import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart'; +import BaseStorageModal from './BaseStorageModal'; +import ExistingConnectedNotebooks from './ExistingConnectedNotebooks'; + +type ClusterStorageModalProps = { + existingData?: PersistentVolumeClaimKind; + onClose: (submit: boolean) => void; +}; + +const ClusterStorageModal: React.FC = (props) => { + const { currentProject } = React.useContext(ProjectDetailsContext); + const namespace = currentProject.metadata.name; + const [removedNotebooks, setRemovedNotebooks] = React.useState([]); + const [notebookData, setNotebookData] = React.useState({ + name: '', + mountPath: { value: '', error: '' }, + }); + const { notebooks: connectedNotebooks } = useRelatedNotebooks( + ConnectedNotebookContext.EXISTING_PVC, + props.existingData?.metadata.name, + ); + const { notebooks: relatedNotebooks } = useRelatedNotebooks( + ConnectedNotebookContext.REMOVABLE_PVC, + props.existingData?.metadata.name, + ); + + const hasExistingNotebookConnections = relatedNotebooks.length > 0; + + const { + notebooks: removableNotebooks, + loaded: removableNotebookLoaded, + error: removableNotebookError, + } = useRelatedNotebooks( + ConnectedNotebookContext.REMOVABLE_PVC, + props.existingData?.metadata.name, + ); + + const restartNotebooks = useWillNotebooksRestart([...removedNotebooks, notebookData.name]); + + const handleSubmit = async ( + storageData: StorageData['creating'], + attachNotebookData?: ForNotebookSelection, + dryRun?: boolean, + ) => { + const promises: Promise[] = []; + const { existingData } = props; + + if (existingData) { + const pvcName = existingData.metadata.name; + + // Check if PVC needs to be updated (name, description, size, storageClass) + if ( + getDisplayNameFromK8sResource(existingData) !== storageData.nameDesc.name || + getDescriptionFromK8sResource(existingData) !== storageData.nameDesc.description || + existingData.spec.resources.requests.storage !== storageData.size || + existingData.spec.storageClassName !== storageData.storageClassName + ) { + promises.push(updatePvc(storageData, existingData, namespace, { dryRun })); + } + + // Restart notebooks if the PVC size has changed + if (existingData.spec.resources.requests.storage !== storageData.size) { + connectedNotebooks.map((connectedNotebook) => + promises.push(restartNotebook(connectedNotebook.metadata.name, namespace, { dryRun })), + ); + } + + // Remove connections to notebooks that were removed + if (removedNotebooks.length > 0) { + promises.push( + ...removedNotebooks.map((nM) => removeNotebookPVC(nM, namespace, pvcName, { dryRun })), + ); + } + + await Promise.all(promises); + + if (attachNotebookData?.name) { + await attachNotebookPVC( + attachNotebookData.name, + namespace, + pvcName, + attachNotebookData.mountPath.value, + { + dryRun, + }, + ); + } + return; + } + // Create new PVC if it doesn't exist + const createdPvc = await createPvc(storageData, namespace, { dryRun }); + + // Attach the new PVC to a notebook if specified + if (attachNotebookData?.name) { + await attachNotebookPVC( + attachNotebookData.name, + namespace, + createdPvc.metadata.name, + attachNotebookData.mountPath.value, + { dryRun }, + ); + } + }; + + const submit = async (data: CreatingStorageObject) => { + const storageData: CreatingStorageObject = { + nameDesc: data.nameDesc, + size: data.size, + storageClassName: data.storageClassName, + }; + + return handleSubmit(storageData, notebookData, true).then(() => + handleSubmit(storageData, notebookData, false), + ); + }; + + const hasValidNotebookRelationship = notebookData.name + ? !!notebookData.mountPath.value && !notebookData.mountPath.error + : true; + + const isValid = hasValidNotebookRelationship; + + return ( + submit(data)} + title={props.existingData ? 'Update cluster storage' : 'Add cluster storage'} + description={ + props.existingData + ? 'Make changes to cluster storage, or connect it to additional workspaces.' + : 'Add storage and optionally connect it with an existing workbench.' + } + submitLabel={props.existingData ? 'Update' : 'Add storage'} + isValid={isValid} + onClose={(submitted) => props.onClose(submitted)} + > + <> + {hasExistingNotebookConnections && ( + + + setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) + } + loaded={removableNotebookLoaded} + error={removableNotebookError} + /> + + )} + + { + setNotebookData(forNotebookData); + }} + forNotebookData={notebookData} + connectedNotebooks={connectedNotebooks} + /> + + {restartNotebooks.length !== 0 && ( + + + + )} + + + ); +}; + +export default ClusterStorageModal; diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx index 612b34f358..9a9e36cade 100644 --- a/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx @@ -9,7 +9,7 @@ import DetailsSection from '~/pages/projects/screens/detail/DetailsSection'; import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; import { ProjectObjectType, typedEmptyImage } from '~/concepts/design/utils'; import StorageTable from './StorageTable'; -import ManageStorageModal from './ManageStorageModal'; +import ClusterStorageModal from './ClusterStorageModal'; const StorageList: React.FC = () => { const [isOpen, setOpen] = React.useState(false); @@ -73,10 +73,10 @@ const StorageList: React.FC = () => { ) : null} {isOpen ? ( - { + { setOpen(false); - if (submit) { + if (submitted) { refresh(); } }} diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx index 7921662be5..c1e888ced0 100644 --- a/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx @@ -8,8 +8,8 @@ import { getStorageClassConfig } from '~/pages/storageClasses/utils'; import useStorageClasses from '~/concepts/k8s/useStorageClasses'; import StorageTableRow from './StorageTableRow'; import { columns } from './data'; -import ManageStorageModal from './ManageStorageModal'; import { StorageTableData } from './types'; +import ClusterStorageModal from './ClusterStorageModal'; type StorageTableProps = { pvcs: PersistentVolumeClaimKind[]; @@ -86,7 +86,7 @@ const StorageTable: React.FC = ({ pvcs, refresh, onAddPVC }) )} /> {editPVC ? ( - { if (updated) { diff --git a/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx b/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx index ee12ed072d..16235a5697 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx @@ -22,12 +22,23 @@ const AddExistingStorageField: React.FC = ({ const { currentProject } = React.useContext(ProjectDetailsContext); const [storages, loaded, loadError] = useProjectPvcs(currentProject.metadata.name); + const selectDescription = (size: string, description?: string) => ( +
+
Size: {size}
+ {description &&
Description: {description}
} +
+ ); + const selectOptions = React.useMemo( () => loaded ? storages.map((pvc) => ({ value: pvc.metadata.name, content: getDisplayNameFromK8sResource(pvc), + description: selectDescription( + pvc.spec.resources.requests.storage, + pvc.metadata.annotations?.['openshift.io/description'], + ), })) : [], [loaded, storages], @@ -44,7 +55,7 @@ const AddExistingStorageField: React.FC = ({ let placeholderText: string; if (!loaded) { - placeholderText = 'Loading storages...'; + placeholderText = 'Loading storages'; } else if (storages.length === 0) { placeholderText = 'No existing storages available'; } else { @@ -66,6 +77,7 @@ const AddExistingStorageField: React.FC = ({ noOptionsFoundMessage={(filter) => `No persistent storage was found for "${filter}"`} popperProps={{ direction: selectDirection, appendTo: menuAppendTo }} isDisabled={!loaded || storages.length === 0} + data-testid="persistent-storage-typeahead" /> ); diff --git a/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx new file mode 100644 index 0000000000..56db9041ed --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx @@ -0,0 +1,78 @@ +import { Form, Modal, Stack, StackItem } from '@patternfly/react-core'; +import React from 'react'; +import { ExistingStorageObject, MountPath } from '~/pages/projects/types'; +import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; +import SpawnerMountPathField from './SpawnerMountPathField'; +import AddExistingStorageField from './AddExistingStorageField'; + +type AttachExistingStorageModalData = ExistingStorageObject & { + mountPath: MountPath; +}; + +type AttachExistingStorageModalProps = { + onClose: (submit: boolean, storageData?: AttachExistingStorageModalData) => void; +}; + +const initialState: AttachExistingStorageModalData = { + storage: '', + mountPath: { value: '', error: '' }, +}; + +const AttachExistingStorageModal: React.FC = ({ onClose }) => { + const [data, setData] = React.useState(initialState); + + const onBeforeClose = (submitted: boolean, storageData?: AttachExistingStorageModalData) => { + onClose(submitted, storageData); + setData(initialState); + }; + + const isValid = + data.mountPath.value.length > 0 && + data.mountPath.value !== '/' && + !data.mountPath.error && + data.storage.trim() !== ''; + + return ( + onBeforeClose(false)} + showClose + isOpen + footer={ + onBeforeClose(true, data)} + onCancel={() => onBeforeClose(false)} + isSubmitDisabled={!isValid} + alertTitle="Error creating storage" + /> + } + > +
{ + e.preventDefault(); + onBeforeClose(true, data); + }} + > + + + setData({ ...data, storage: storageName.storage })} + /> + + + setData({ ...data, mountPath: path })} + /> + + +
+ + ); +}; + +export default AttachExistingStorageModal; diff --git a/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx b/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx new file mode 100644 index 0000000000..d3f076ac8a --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx @@ -0,0 +1,184 @@ +import { + Flex, + FlexItem, + FormGroup, + HelperText, + HelperTextItem, + InputGroup, + InputGroupItem, + InputGroupText, + Radio, + Stack, + StackItem, + TextInput, +} from '@patternfly/react-core'; +import React from 'react'; +import FieldGroupHelpLabelIcon from '~/components/FieldGroupHelpLabelIcon'; +import { useMountPathFormat, validateMountPath } from './utils'; +import { MountPathFormat } from './types'; +import { MOUNT_PATH_PREFIX } from './const'; + +interface MountPath { + value: string; + error: string; +} + +interface SpawnerMountPathFieldProps { + isCreate: boolean; + mountPath: MountPath; + inUseMountPaths?: string[]; + onChange: (path: MountPath) => void; +} + +const SpawnerMountPathField: React.FC = ({ + isCreate, + mountPath, + inUseMountPaths, + onChange, +}) => { + const [format, setFormat] = useMountPathFormat(isCreate, mountPath.value); + const [shouldShowValidation, setShouldShowValidation] = React.useState(false); + + const pathSuffix = React.useMemo(() => { + const prefix = format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; + return mountPath.value.startsWith(prefix) + ? mountPath.value.slice(prefix.length) + : mountPath.value; + }, [mountPath.value, format]); + + React.useEffect(() => { + const initialValue = format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; + onChange({ value: initialValue, error: '' }); + // Only run on initial mount + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const validateAndUpdate = React.useCallback( + (suffix: string, newFormat: MountPathFormat = format) => { + const prefix = newFormat === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; + const newValue = `${prefix}${suffix}`; + + // Only validate after the field has been touched + if (!shouldShowValidation && !suffix.trim()) { + onChange({ value: newValue, error: '' }); + return; + } + + const error = validateMountPath(suffix, inUseMountPaths || [], newFormat); + onChange({ value: newValue, error }); + }, + [format, inUseMountPaths, onChange, shouldShowValidation], + ); + + const handleFormatChange = (newFormat: MountPathFormat) => { + setFormat(newFormat); + validateAndUpdate(pathSuffix, newFormat); + }; + + const handleSuffixChange = (suffix: string) => { + if (!shouldShowValidation) { + setShouldShowValidation(true); + } + validateAndUpdate(suffix); + }; + + return ( + + The directory within a container where a volume is mounted and accessible. Only + standard paths that begin with {MOUNT_PATH_PREFIX} are visible in the + JupyterLab file browser. + + } + /> + } + > + + {isCreate ? ( + <> + + + + handleFormatChange(MountPathFormat.STANDARD)} + /> + + + handleFormatChange(MountPathFormat.CUSTOM)} + /> + + + + + + + {format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'} + + + handleSuffixChange(value)} + isRequired + validated={ + mountPath.error ? 'error' : pathSuffix.length > 0 ? 'success' : 'default' + } + /> + + + + + {mountPath.error || + 'Must only consist of lowercase letters, dashes, and slashes.'} + + {format === MountPathFormat.CUSTOM && ( + + Depending on the workbench type, this location may not be visible or accessible. + For example, the JupyterLab file browser only displays folders and files under + /opt/app-root/src + + )} + + + + ) : ( + + onChange({ value, error: mountPath.error })} + id="mount-path" + data-testid="mount-path-input" + /> + + )} + + + ); +}; + +export default SpawnerMountPathField; diff --git a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx new file mode 100644 index 0000000000..ebdf1bed66 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx @@ -0,0 +1,51 @@ +import * as React from 'react'; +import { StackItem } from '@patternfly/react-core'; +import { PersistentVolumeClaimKind } from '~/k8sTypes'; +import { CreatingStorageObject, MountPath } from '~/pages/projects/types'; +import BaseStorageModal from '~/pages/projects/screens/detail/storage/BaseStorageModal'; +import SpawnerMountPathField from './SpawnerMountPathField'; + +type WorkbenchStorageModalProps = { + existingData?: PersistentVolumeClaimKind; + onClose: (submit: boolean) => void; +}; + +const WorkbenchStorageModal: React.FC = (props) => { + const [mountPath, setMountPath] = React.useState({ + value: '', + error: '', + }); + const [actionInProgress, setActionInProgress] = React.useState(false); + + //TODO: handleSubmit function to be completed in RHOAIENG-1101 + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const handleSubmit = async (createData: CreatingStorageObject) => { + setActionInProgress(true); + }; + + const isValid = + !actionInProgress && + mountPath.error === '' && + mountPath.value.length > 0 && + mountPath.value !== '/'; + + return ( + handleSubmit(createData)} + title={props.existingData ? 'Edit storage' : 'Create storage'} + submitLabel={props.existingData ? 'Save' : 'Create'} + isValid={isValid} + > + + setMountPath(path)} + /> + + + ); +}; + +export default WorkbenchStorageModal; diff --git a/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts new file mode 100644 index 0000000000..f7d319bd26 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts @@ -0,0 +1,192 @@ +import { renderHook, act } from '@testing-library/react'; +import { + useCreateStorageObject, + useMountPathFormat, + validateMountPath, +} from '~/pages/projects/screens/spawner/storage/utils'; +import { MountPathFormat } from '~/pages/projects/screens/spawner/storage/types'; +import { MOUNT_PATH_PREFIX } from '~/pages/projects/screens/spawner/storage/const'; +import { PersistentVolumeClaimKind } from '~/k8sTypes'; + +jest.mock('~/pages/projects/screens/spawner/storage/useDefaultPvcSize.ts', () => ({ + __esModule: true, + default: jest.fn().mockReturnValue('1Gi'), // Set the default PVC size to 1Gi +})); + +jest.mock('~/concepts/k8s/utils', () => ({ + getDisplayNameFromK8sResource: jest.fn( + (data) => data?.metadata.annotations?.['openshift.io/display-name'] || '', + ), + getDescriptionFromK8sResource: jest.fn( + (data) => data?.metadata.annotations?.['openshift.io/description'] || '', + ), +})); + +describe('useCreateStorageObject', () => { + const existingData: PersistentVolumeClaimKind = { + apiVersion: 'v1', + kind: 'PersistentVolumeClaim', + metadata: { + annotations: { + 'openshift.io/description': 'Test PVC Description', + 'openshift.io/display-name': 'test-pvc', + }, + labels: { 'opendatahub.io/dashboard': 'true' }, + name: 'test-pvc', + namespace: 'namespace', + }, + spec: { + accessModes: ['ReadWriteOnce'], + resources: { requests: { storage: '2Gi' } }, + volumeMode: 'Filesystem', + storageClassName: 'test-storage-class', + }, + status: { phase: 'Pending' }, + }; + it('should initialize with default values when no existingData is provided', () => { + const { result } = renderHook(() => useCreateStorageObject()); + + const [data] = result.current; + expect(data).toEqual({ + nameDesc: { name: '', k8sName: undefined, description: '' }, + size: '1Gi', + }); + }); + + it('should initialize with existingData when provided', () => { + const { result } = renderHook(() => useCreateStorageObject(existingData)); + + const [data] = result.current; + expect(data.nameDesc.name).toBe('test-pvc'); + expect(data.nameDesc.description).toBe('Test PVC Description'); + expect(data.size).toBe('2Gi'); + expect(data.storageClassName).toBe('test-storage-class'); + }); + + it('should reset to default values when resetDefaults is called', () => { + const { result } = renderHook(() => useCreateStorageObject(existingData)); + const [, , resetDefaults] = result.current; + + act(() => { + resetDefaults(); + }); + + const [data] = result.current; + expect(data.nameDesc.name).toBe(''); + expect(data.nameDesc.description).toBe(''); + expect(data.size).toBe('1Gi'); // Default size from mock + expect(data.storageClassName).toBeUndefined(); + }); +}); + +describe('validateMountPath', () => { + const inUseMountPaths = ['/existing-folder', '/another-folder']; + + it('should return error message for empty value in CUSTOM format', () => { + const result = validateMountPath('', inUseMountPaths, MountPathFormat.CUSTOM); + expect(result).toBe( + 'Enter a path to a model or folder. This path cannot point to a root folder.', + ); + }); + + it('should not return an error for empty value in STANDARD format', () => { + const result = validateMountPath('', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe(''); + }); + + it('should return error message for invalid characters in the path', () => { + const result = validateMountPath('Invalid/Path', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); + }); + + it('should return error message for already in-use mount path', () => { + const result = validateMountPath('existing-folder', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe('Mount folder is already in use for this workbench.'); + }); + + it('should return an empty string for valid and unused mount path', () => { + const result = validateMountPath('new-folder', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe(''); + }); + + it('should allow valid folder name with a trailing slash', () => { + const result = validateMountPath('valid-folder/', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe(''); + }); + + it('should return error for an invalid folder name with numbers or uppercase letters', () => { + const result = validateMountPath('Invalid123', inUseMountPaths, MountPathFormat.STANDARD); + expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); + }); + + it('should return an empty string for valid mount path in CUSTOM format', () => { + const result = validateMountPath('custom-folder', inUseMountPaths, MountPathFormat.CUSTOM); + expect(result).toBe(''); + }); + + it('should return error for an invalid folder name with uppercase letters in CUSTOM format', () => { + const result = validateMountPath('InvalidFolder', inUseMountPaths, MountPathFormat.CUSTOM); + expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); + }); +}); + +describe('useMountPathFormat', () => { + it('return MountPathFormat.STANDARD if isCreate is true', () => { + const { result } = renderHook(() => useMountPathFormat(true, 'some-path')); + + const [format] = result.current; + expect(format).toBe(MountPathFormat.STANDARD); + }); + + it('return MountPathFormat.STANDARD if mountPath starts with /opt/app-root/src/', () => { + const { result } = renderHook(() => + useMountPathFormat(false, `${MOUNT_PATH_PREFIX}/some-path`), + ); + + const [format] = result.current; + expect(format).toBe(MountPathFormat.STANDARD); + }); + + it('return MountPathFormat.CUSTOM if mountPath does not start with /opt/app-root/src/', () => { + const { result } = renderHook(() => useMountPathFormat(false, '/custom-path')); + + const [format] = result.current; + expect(format).toBe(MountPathFormat.CUSTOM); + }); + + it('should update format based on the mountPath change', () => { + const { result, rerender } = renderHook( + ({ isCreate, mountPath }) => useMountPathFormat(isCreate, mountPath), + { + initialProps: { isCreate: false, mountPath: '/custom-path' }, + }, + ); + + // Initial format + expect(result.current[0]).toBe(MountPathFormat.CUSTOM); + + // Change the mountPath to a path with MOUNT_PATH_PREFIX + rerender({ isCreate: false, mountPath: `${MOUNT_PATH_PREFIX}/new-path` }); + + // Format should update to STANDARD + expect(result.current[0]).toBe(MountPathFormat.STANDARD); + }); + + it('should not update format if isCreate is true, regardless of mountPath', () => { + const { result, rerender } = renderHook( + ({ isCreate, mountPath }) => useMountPathFormat(isCreate, mountPath), + { + initialProps: { isCreate: true, mountPath: '/custom-path' }, + }, + ); + + // Initial format + expect(result.current[0]).toBe(MountPathFormat.STANDARD); + + // Change the mountPath but keep isCreate true + rerender({ isCreate: true, mountPath: `${MOUNT_PATH_PREFIX}/new-path` }); + + // Format should remain STANDARD + expect(result.current[0]).toBe(MountPathFormat.STANDARD); + }); +}); diff --git a/frontend/src/pages/projects/screens/spawner/storage/const.ts b/frontend/src/pages/projects/screens/spawner/storage/const.ts new file mode 100644 index 0000000000..3aecd3325c --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/const.ts @@ -0,0 +1 @@ +export const MOUNT_PATH_PREFIX = '/opt/app-root/src/'; diff --git a/frontend/src/pages/projects/screens/spawner/storage/types.ts b/frontend/src/pages/projects/screens/spawner/storage/types.ts new file mode 100644 index 0000000000..aed06196d8 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/types.ts @@ -0,0 +1,4 @@ +export enum MountPathFormat { + STANDARD = 'standard', + CUSTOM = 'custom', +} diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 871d80338c..41e461216d 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -1,5 +1,6 @@ import * as React from 'react'; import { + CreatingStorageObject, CreatingStorageObjectForNotebook, ExistingStorageObjectForNotebook, StorageData, @@ -15,6 +16,46 @@ import useGenericObjectState from '~/utilities/useGenericObjectState'; import { getRootVolumeName } from '~/pages/projects/screens/spawner/spawnerUtils'; import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import useDefaultPvcSize from './useDefaultPvcSize'; +import { MountPathFormat } from './types'; +import { MOUNT_PATH_PREFIX } from './const'; + +export const useCreateStorageObject = ( + existingData?: PersistentVolumeClaimKind, +): [ + data: CreatingStorageObject, + setData: UpdateObjectAtPropAndValue, + resetDefaults: () => void, +] => { + const size = useDefaultPvcSize(); + const createDataState = useGenericObjectState({ + nameDesc: { + name: '', + k8sName: undefined, + description: '', + }, + size, + }); + + const [, setCreateData] = createDataState; + + const existingName = existingData ? getDisplayNameFromK8sResource(existingData) : ''; + const existingDescription = existingData ? getDescriptionFromK8sResource(existingData) : ''; + const existingSize = existingData ? existingData.spec.resources.requests.storage : size; + const existingStorageClassName = existingData?.spec.storageClassName; + + React.useEffect(() => { + if (existingName) { + setCreateData('nameDesc', { + name: existingName, + description: existingDescription, + }); + setCreateData('size', existingSize); + setCreateData('storageClassName', existingStorageClassName); + } + }, [existingName, existingDescription, setCreateData, existingSize, existingStorageClassName]); + + return createDataState; +}; export const useCreateStorageObjectForNotebook = ( existingData?: PersistentVolumeClaimKind, @@ -113,3 +154,57 @@ export const useStorageDataObject = ( }, }); }; + +// Returns the initial mount path format based on the isCreate and mountPath props. +export const useMountPathFormat = ( + isCreate: boolean, + mountPath: string, +): [MountPathFormat, React.Dispatch>] => { + const getInitialFormat = React.useCallback(() => { + if (isCreate) { + return MountPathFormat.STANDARD; + } + return mountPath.startsWith(MOUNT_PATH_PREFIX) + ? MountPathFormat.STANDARD + : MountPathFormat.CUSTOM; + }, [isCreate, mountPath]); + + const [format, setFormat] = React.useState(getInitialFormat); + + React.useEffect(() => { + if (!isCreate) { + const newFormat = mountPath.startsWith(MOUNT_PATH_PREFIX) + ? MountPathFormat.STANDARD + : MountPathFormat.CUSTOM; + setFormat(newFormat); + } + }, [isCreate, mountPath]); + + return [format, setFormat] as const; +}; + +// Validates the mount path for a storage object. +export const validateMountPath = ( + value: string, + inUseMountPaths: string[], + format: MountPathFormat, +): string => { + if (value.length === 0 && format === MountPathFormat.CUSTOM) { + return 'Enter a path to a model or folder. This path cannot point to a root folder.'; + } + + // Regex to allow empty string for Standard format + const regex = + format === MountPathFormat.STANDARD + ? /^(\/?[a-z-]+(\/[a-z-]+)*\/?|)$/ + : /^(\/?[a-z-]+(\/[a-z-]+)*\/?)$/; + + if (!regex.test(value)) { + return 'Must only consist of lowercase letters, dashes, and slashes.'; + } + + if (inUseMountPaths.includes(`/${value}`)) { + return 'Mount folder is already in use for this workbench.'; + } + return ''; +}; From f24a1c8cfcbb7321ef9efb8f5411c8836380c717 Mon Sep 17 00:00:00 2001 From: Mike Turley Date: Wed, 6 Nov 2024 11:48:40 -0500 Subject: [PATCH 4/8] Model Registry Selector: don't navigate on selection unless necessary (#3439) Signed-off-by: Mike Turley --- .../screens/ModelRegistrySelectorNavigator.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx index 7c606fd31f..262bef4eb5 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx @@ -11,15 +11,17 @@ const ModelRegistrySelectorNavigator: React.FC { const navigate = useNavigate(); - const { modelRegistry } = useParams<{ modelRegistry: string }>(); + const { modelRegistry: currentModelRegistry } = useParams<{ modelRegistry: string }>(); return ( { - navigate(getRedirectPath(modelRegistryName)); + if (modelRegistryName !== currentModelRegistry) { + navigate(getRedirectPath(modelRegistryName)); + } }} - modelRegistry={modelRegistry ?? ''} + modelRegistry={currentModelRegistry ?? ''} /> ); }; From 9242b81e7acdffbc5739df849d44a3f16ed39adf Mon Sep 17 00:00:00 2001 From: Jeff Phillips Date: Wed, 6 Nov 2024 12:24:37 -0500 Subject: [PATCH 5/8] Update 'Resource name' fields to meet UX guidelines: Accelerator Profiles (#3425) --- .../acceleratorProfilesUtils.ts | 5 +- .../cypress/pages/acceleratorProfile.ts | 9 +- .../manageAcceleratorProfiles.cy.ts | 38 +++++- .../ResourceNameField.tsx | 1 + .../k8s/K8sNameDescriptionField/types.ts | 8 +- .../k8s/K8sNameDescriptionField/utils.ts | 13 +- .../manage/ManageAcceleratorProfile.tsx | 64 +++++++++- ...ManageAcceleratorProfileDetailsSection.tsx | 112 ++++++------------ .../manage/ManageAcceleratorProfileFooter.tsx | 9 +- .../screens/manage/types.ts | 4 + .../src/services/acceleratorProfileService.ts | 2 +- 11 files changed, 162 insertions(+), 103 deletions(-) diff --git a/backend/src/routes/api/accelerator-profiles/acceleratorProfilesUtils.ts b/backend/src/routes/api/accelerator-profiles/acceleratorProfilesUtils.ts index 23557a6fdc..3cb1e8d76e 100644 --- a/backend/src/routes/api/accelerator-profiles/acceleratorProfilesUtils.ts +++ b/backend/src/routes/api/accelerator-profiles/acceleratorProfilesUtils.ts @@ -10,13 +10,14 @@ export const postAcceleratorProfile = async ( ): Promise<{ success: boolean; error: string }> => { const { customObjectsApi } = fastify.kube; const { namespace } = fastify.kube; - const body = request.body as AcceleratorProfileKind['spec']; + const requestBody = request.body as { name?: string } & AcceleratorProfileKind['spec']; + const { name, ...body } = requestBody; const payload: AcceleratorProfileKind = { apiVersion: 'dashboard.opendatahub.io/v1', kind: 'AcceleratorProfile', metadata: { - name: translateDisplayNameForK8s(body.displayName), + name: name || translateDisplayNameForK8s(body.displayName), namespace, annotations: { 'opendatahub.io/modified-date': new Date().toISOString(), diff --git a/frontend/src/__tests__/cypress/cypress/pages/acceleratorProfile.ts b/frontend/src/__tests__/cypress/cypress/pages/acceleratorProfile.ts index ba103e5b35..ee50c0a55c 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/acceleratorProfile.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/acceleratorProfile.ts @@ -1,3 +1,4 @@ +import { K8sNameDescriptionField } from '~/__tests__/cypress/cypress/pages/components/subComponents/K8sNameDescriptionField'; import { Modal } from './components/Modal'; import { TableToolbar } from './components/TableToolbar'; import { TableRow } from './components/table'; @@ -86,9 +87,7 @@ class AcceleratorProfile { } class ManageAcceleratorProfile { - findAcceleratorNameInput() { - return cy.findByTestId('accelerator-name-input'); - } + k8sNameDescription = new K8sNameDescriptionField('accelerator-profile'); findIdentifierInput() { return cy.findByTestId('accelerator-identifier-input'); @@ -103,10 +102,6 @@ class ManageAcceleratorProfile { return cy.findByTestId('add-toleration-button'); } - findDescriptionInput() { - return cy.findByTestId('accelerator-description-input'); - } - findSubmitButton() { return cy.findByTestId('accelerator-profile-create-button'); } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts index 9f9f525387..1b11742620 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/acceleratorProfiles/manageAcceleratorProfiles.cy.ts @@ -50,11 +50,34 @@ describe('Manage Accelerator Profile', () => { createAcceleratorProfile.findSubmitButton().should('be.disabled'); // test required fields - createAcceleratorProfile.findAcceleratorNameInput().fill('test-accelerator'); + createAcceleratorProfile.k8sNameDescription.findDisplayNameInput().fill('test-accelerator'); createAcceleratorProfile.findSubmitButton().should('be.disabled'); createAcceleratorProfile.findIdentifierInput().fill('nvidia.com/gpu'); createAcceleratorProfile.findSubmitButton().should('be.enabled'); + // test resource name validation + createAcceleratorProfile.k8sNameDescription.findResourceEditLink().click(); + createAcceleratorProfile.k8sNameDescription + .findResourceNameInput() + .should('have.attr', 'aria-invalid', 'false'); + createAcceleratorProfile.k8sNameDescription + .findResourceNameInput() + .should('have.value', 'test-accelerator'); + // Invalid character k8s names fail + createAcceleratorProfile.k8sNameDescription + .findResourceNameInput() + .clear() + .type('InVaLiD vAlUe!'); + createAcceleratorProfile.k8sNameDescription + .findResourceNameInput() + .should('have.attr', 'aria-invalid', 'true'); + createAcceleratorProfile.findSubmitButton().should('be.disabled'); + createAcceleratorProfile.k8sNameDescription + .findResourceNameInput() + .clear() + .type('test-accelerator-name'); + createAcceleratorProfile.findSubmitButton().should('be.enabled'); + // test tolerations createAcceleratorProfile.shouldHaveModalEmptyState(); @@ -127,7 +150,9 @@ describe('Manage Accelerator Profile', () => { cy.wait('@createAccelerator').then((interception) => { expect(interception.request.body).to.be.eql({ + name: 'test-accelerator-name', displayName: 'test-accelerator', + description: '', identifier: 'nvidia.com/gpu', enabled: true, tolerations: [], @@ -138,9 +163,13 @@ describe('Manage Accelerator Profile', () => { it('edit page has expected values', () => { initIntercepts({}); editAcceleratorProfile.visit('test-accelerator'); - editAcceleratorProfile.findAcceleratorNameInput().should('have.value', 'Test Accelerator'); + editAcceleratorProfile.k8sNameDescription + .findDisplayNameInput() + .should('have.value', 'Test Accelerator'); editAcceleratorProfile.findIdentifierInput().should('have.value', 'nvidia.com/gpu'); - editAcceleratorProfile.findDescriptionInput().should('have.value', 'Test description'); + editAcceleratorProfile.k8sNameDescription + .findDescriptionInput() + .should('have.value', 'Test description'); editAcceleratorProfile .getRow('nvidia.com/gpu') .shouldHaveEffect('NoSchedule') @@ -153,10 +182,11 @@ describe('Manage Accelerator Profile', () => { { path: { name: 'test-accelerator' } }, { success: true }, ).as('updatedAccelerator'); - editAcceleratorProfile.findDescriptionInput().fill('Updated description'); + editAcceleratorProfile.k8sNameDescription.findDescriptionInput().fill('Updated description'); editAcceleratorProfile.findSubmitButton().click(); cy.wait('@updatedAccelerator').then((interception) => { expect(interception.request.body).to.eql({ + name: 'test-accelerator', displayName: 'Test Accelerator', identifier: 'nvidia.com/gpu', enabled: true, diff --git a/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx b/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx index b254a82e11..98f9fd4ab2 100644 --- a/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx +++ b/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx @@ -48,6 +48,7 @@ const ResourceNameField: React.FC = ({ return ( !!x && 'k8sName' in x; + export const setupDefaults = ({ initialData, limitNameResourceType, @@ -43,11 +49,16 @@ export const setupDefaults = ({ let initialK8sNameValue = ''; let configuredMaxLength = MAX_RESOURCE_NAME_LENGTH; - if (isK8sDSGResource(initialData)) { + if (isK8sNameDescriptionType(initialData)) { + initialName = initialData.name || ''; + initialDescription = initialData.description || ''; + initialK8sNameValue = initialData.k8sName || ''; + } else if (isK8sDSGResource(initialData)) { initialName = getDisplayNameFromK8sResource(initialData); initialDescription = getDescriptionFromK8sResource(initialData); initialK8sNameValue = initialData.metadata.name; } + if (limitNameResourceType != null) { configuredMaxLength = ROUTE_BASED_NAME_LENGTH; } diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfile.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfile.tsx index 8c5d6f6b09..8d781e4c38 100644 --- a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfile.tsx +++ b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfile.tsx @@ -1,14 +1,26 @@ import * as React from 'react'; import { Link } from 'react-router-dom'; -import { Breadcrumb, BreadcrumbItem, Form, PageSection } from '@patternfly/react-core'; +import { + Breadcrumb, + BreadcrumbItem, + Form, + FormSection, + PageSection, + Stack, + StackItem, +} from '@patternfly/react-core'; import ApplicationsPage from '~/pages/ApplicationsPage'; import useGenericObjectState from '~/utilities/useGenericObjectState'; import GenericSidebar from '~/components/GenericSidebar'; import { AcceleratorProfileKind } from '~/k8sTypes'; +import K8sNameDescriptionField, { + useK8sNameDescriptionFieldData, +} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField'; +import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils'; import { ManageAcceleratorProfileFooter } from './ManageAcceleratorProfileFooter'; import { ManageAcceleratorProfileTolerationsSection } from './ManageAcceleratorProfileTolerationsSection'; -import { ManageAcceleratorProfileSectionID } from './types'; +import { AcceleratorProfileFormData, ManageAcceleratorProfileSectionID } from './types'; import { ManageAcceleratorProfileSectionTitles, ScrollableSelectorID } from './const'; import { ManageAcceleratorProfileDetailsSection } from './ManageAcceleratorProfileDetailsSection'; @@ -25,19 +37,39 @@ const ManageAcceleratorProfile: React.FC = ({ enabled: true, tolerations: [], }); + const { data: profileNameDesc, onDataChange: setProfileNameDesc } = + useK8sNameDescriptionFieldData({ + initialData: existingAcceleratorProfile + ? { + name: existingAcceleratorProfile.spec.displayName, + k8sName: existingAcceleratorProfile.metadata.name, + description: existingAcceleratorProfile.spec.description, + } + : undefined, + }); React.useEffect(() => { if (existingAcceleratorProfile) { - setState('displayName', existingAcceleratorProfile.spec.displayName); setState('identifier', existingAcceleratorProfile.spec.identifier); - setState('description', existingAcceleratorProfile.spec.description); setState('enabled', existingAcceleratorProfile.spec.enabled); setState('tolerations', existingAcceleratorProfile.spec.tolerations); } }, [existingAcceleratorProfile, setState]); + const formState: AcceleratorProfileFormData = React.useMemo( + () => ({ + ...state, + name: profileNameDesc.k8sName.value, + displayName: profileNameDesc.name, + description: profileNameDesc.description, + }), + [state, profileNameDesc], + ); + const sectionIDs = Object.values(ManageAcceleratorProfileSectionID); + const validFormData = isK8sNameDescriptionDataValid(profileNameDesc) && !!state.identifier; + return ( = ({ >
- + + + + + + + + setState('tolerations', tolerations)} @@ -76,8 +127,9 @@ const ManageAcceleratorProfile: React.FC = ({ diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileDetailsSection.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileDetailsSection.tsx index 667d3829c8..f1b907fb22 100644 --- a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileDetailsSection.tsx +++ b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileDetailsSection.tsx @@ -1,25 +1,15 @@ -import { - FormSection, - Stack, - StackItem, - FormGroup, - TextInput, - TextArea, - Switch, - Popover, -} from '@patternfly/react-core'; +import { StackItem, FormGroup, Switch, Popover } from '@patternfly/react-core'; import React from 'react'; import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import { useSearchParams } from 'react-router-dom'; import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import { AcceleratorProfileKind } from '~/k8sTypes'; import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; -import { ManageAcceleratorProfileSectionTitles } from './const'; -import { ManageAcceleratorProfileSectionID } from './types'; +import { AcceleratorProfileFormData } from './types'; import { IdentifierSelectField } from './IdentifierSelectField'; type ManageAcceleratorProfileDetailsSectionProps = { - state: AcceleratorProfileKind['spec']; + state: AcceleratorProfileFormData; setState: UpdateObjectAtPropAndValue; }; @@ -34,69 +24,37 @@ export const ManageAcceleratorProfileDetailsSection: React.FC< ); return ( - - - - - setState('displayName', name)} - aria-label="Name" - data-testid="accelerator-name-input" - /> - - - - - } - aria-label="More info for identifier field" - /> - - } - > - setState('identifier', identifier)} - identifierOptions={acceleratorIdentifiers} - /> - - - - -