Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Cluster storage that was disconnected from the workbench doesn't update it's size info #3481

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ class ClusterStorageRow extends TableRow {
findStorageClassResourceKindText() {
return cy.findByTestId('resource-kind-text');
}

findStorageSizeWarning() {
return cy.findByTestId('size-warning-popover').click();
}

findStorageSizeWarningText() {
return cy
.findByTestId('size-warning-popover-text')
.should(
'have.text',
'To complete the storage size update, you must connect and run a workbench.',
);
}
}

class ClusterStorageModal extends Modal {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,28 @@ const initInterceptors = ({ isEmpty = false, storageClassName }: HandlersProps)
storageClassName,
status: { phase: 'Pending' },
}),
mockPVCK8sResource({
displayName: 'Updated storage with no workbench',
storageClassName,
storage: '13Gi',
status: {
phase: 'Bound',
accessModes: ['ReadWriteOnce'],
capacity: {
storage: '12Gi',
},
conditions: [
{
type: 'FileSystemResizePending',
status: 'True',
lastProbeTime: null,
lastTransitionTime: '2024-11-15T14:04:04Z',
message:
'Waiting for user to (re-)start a pod to finish file system resize of volume on node.',
},
],
},
}),
],
),
);
Expand Down Expand Up @@ -322,6 +344,18 @@ describe('ClusterStorage', () => {
clusterStorage.findClusterStorageTableHeaderButton('Name').should(be.sortDescending);
});

it('should show warning when cluster storage size is updated but no workbench is connected', () => {
initInterceptors({});
clusterStorage.visit('test-project');
const clusterStorageRow = clusterStorage.getClusterStorageRow(
'Updated storage with no workbench',
);
clusterStorageRow.toggleExpandableContent();
clusterStorageRow.shouldHaveStorageSize('Max 13Gi');
clusterStorageRow.findStorageSizeWarning();
clusterStorageRow.findStorageSizeWarning().should('exist');
});

it('Edit cluster storage', () => {
initInterceptors({});
clusterStorage.visit('test-project');
Expand Down
15 changes: 13 additions & 2 deletions frontend/src/concepts/dashboard/DashboardPopupIconButton.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
import React from 'react';
import { Button, ButtonProps, Icon } from '@patternfly/react-core';
import { Button, ButtonProps, Icon, IconComponentProps } from '@patternfly/react-core';

type DashboardPopupIconButtonProps = Omit<ButtonProps, 'variant' | 'isInline'> & {
icon: React.ReactNode;
iconProps?: Omit<IconComponentProps, 'isInline'>;
};

/**
* Overriding PF's button styles to allow for a11y in opening tooltips or popovers on a single item
*/
const DashboardPopupIconButton = ({
icon,
iconProps,
...props
}: DashboardPopupIconButtonProps): React.JSX.Element => (
<Button icon={<Icon isInline>{icon}</Icon>} variant="plain" isInline {...props} />
<Button
icon={
<Icon isInline {...iconProps}>
{icon}
</Icon>
}
variant="plain"
isInline
{...props}
/>
);

export default DashboardPopupIconButton;
1 change: 1 addition & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ export type PersistentVolumeClaimKind = K8sResourceCommon & {
capacity?: {
storage: string;
};
conditions?: K8sCondition[];
} & Record<string, unknown>;
};

Expand Down
18 changes: 16 additions & 2 deletions frontend/src/pages/projects/components/PVSizeField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { ExclamationTriangleIcon } from '@patternfly/react-icons';
import ValueUnitField from '~/components/ValueUnitField';
import { MEMORY_UNITS_FOR_SELECTION, UnitOption } from '~/utilities/valueUnits';
import { PersistentVolumeClaimKind } from '~/k8sTypes';
import {
ConnectedNotebookContext,
useRelatedNotebooks,
} from '~/pages/projects/notebook/useRelatedNotebooks';

type PVSizeFieldProps = {
fieldID: string;
Expand All @@ -13,6 +17,7 @@ type PVSizeFieldProps = {
currentStatus?: PersistentVolumeClaimKind['status'];
label?: string;
options?: UnitOption[];
existingPvcName?: string;
};

const PVSizeField: React.FC<PVSizeFieldProps> = ({
Expand All @@ -23,10 +28,18 @@ const PVSizeField: React.FC<PVSizeFieldProps> = ({
currentStatus,
label = 'Persistent storage size',
options = MEMORY_UNITS_FOR_SELECTION,
existingPvcName,
}) => {
const currentSize = currentStatus?.capacity?.storage;
const isUnbound = currentStatus && currentStatus.phase !== 'Bound';

const { notebooks } = useRelatedNotebooks(
ConnectedNotebookContext.REMOVABLE_PVC,
existingPvcName,
);

const hasExistingNotebookConnections = notebooks.length > 0;

return (
<FormGroup label={label} fieldId={fieldID} data-testid={fieldID}>
<ValueUnitField
Expand Down Expand Up @@ -60,8 +73,9 @@ const PVSizeField: React.FC<PVSizeFieldProps> = ({
variant="warning"
icon={<ExclamationTriangleIcon />}
>
Storage size can only be increased. If you do so, the workbench will restart and be
unavailable for a period of time that is usually proportional to the size change.
{!hasExistingNotebookConnections
? 'Storage size can only be increased. To complete the storage size update, you must connect to and run a workbench.'
: 'Storage size can only be increased. If you do so, the workbench will restart and be unavailable for a period of time that is usually proportional to the size change.'}
</HelperTextItem>
)}
</HelperText>
Expand Down
27 changes: 25 additions & 2 deletions frontend/src/pages/projects/components/StorageSizeBars.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import * as React from 'react';
import {
Bullseye,
Popover,
Progress,
ProgressMeasureLocation,
Spinner,
Split,
SplitItem,
Content,
Tooltip,
Flex,
} from '@patternfly/react-core';
import { ExclamationCircleIcon } from '@patternfly/react-icons';
import { ExclamationCircleIcon, ExclamationTriangleIcon } from '@patternfly/react-icons';
import { PersistentVolumeClaimKind } from '~/k8sTypes';
import { getPvcTotalSize } from '~/pages/projects/utils';
import { getPvcRequestSize, getPvcTotalSize } from '~/pages/projects/utils';
import { usePVCFreeAmount } from '~/api';
import { bytesAsRoundedGiB } from '~/utilities/number';
import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton';

type StorageSizeBarProps = {
pvc: PersistentVolumeClaimKind;
Expand All @@ -22,6 +25,26 @@ type StorageSizeBarProps = {
const StorageSizeBar: React.FC<StorageSizeBarProps> = ({ pvc }) => {
const [inUseInBytes, loaded, error] = usePVCFreeAmount(pvc);
const maxValue = getPvcTotalSize(pvc);
const requestedValue = getPvcRequestSize(pvc);

if (pvc.status?.conditions?.find((c) => c.type === 'FileSystemResizePending')) {
return (
<Flex>
<Content component="small">Max {requestedValue}</Content>
<Popover
bodyContent="To complete the storage size update, you must connect and run a workbench."
data-testid="size-warning-popover-text"
>
<DashboardPopupIconButton
icon={<ExclamationTriangleIcon />}
aria-label="Size warning"
data-testid="size-warning-popover"
iconProps={{ status: 'warning' }}
/>
</Popover>
</Flex>
);
}

if (!error && Number.isNaN(inUseInBytes)) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const CreateNewStorageSection = <D extends StorageData>({
currentStatus={currentStatus}
size={String(data.size)}
setSize={(size) => setData('size', size)}
existingPvcName={data.existingPvc?.metadata.name}
/>
</FormSection>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const useCreateStorageObject = (
formData?.description || (existingData ? getDescriptionFromK8sResource(existingData) : ''),
size: formData?.size || (existingData ? existingData.spec.resources.requests.storage : size),
storageClassName: formData?.storageClassName || existingData?.spec.storageClassName,
existingPvc: existingData,
};

const [data, setData] = useGenericObjectState<StorageData>(createStorageData);
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/pages/projects/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ export const getNotebookStatusPriority = (notebookState: NotebookState): number
export const getPvcTotalSize = (pvc: PersistentVolumeClaimKind): string =>
formatMemory(pvc.status?.capacity?.storage || pvc.spec.resources.requests.storage);

export const getPvcRequestSize = (pvc: PersistentVolumeClaimKind): string =>
formatMemory(pvc.spec.resources.requests.storage);

export const getCustomNotebookSize = (
existingNotebook: NotebookKind | undefined,
): NotebookSize => ({
Expand Down
Loading