Merge pull request #1714 from DaoDaoNoCode/upstream-issue-1480

Support storage size unit and show error message correctly

frontend/src/api/k8s/notebooks.ts

@@ -384,6 +384,7 @@ export const attachNotebookPVC = (
   namespace: string,
   pvcName: string,
   mountSuffix: string,
+  opts?: K8sAPIOptions,
 ): Promise<NotebookKind> => {
   const patches: Patch[] = [
     {
@@ -399,17 +400,20 @@ export const attachNotebookPVC = (
     },
   ];
 
-  return k8sPatchResource<NotebookKind>({
-    model: NotebookModel,
-    queryOptions: { name: notebookName, ns: namespace },
-    patches,
-  });
+  return k8sPatchResource<NotebookKind>(
+    applyK8sAPIOptions(opts, {
+      model: NotebookModel,
+      queryOptions: { name: notebookName, ns: namespace },
+      patches,
+    }),
+  );
 };
 
 export const removeNotebookPVC = (
   notebookName: string,
   namespace: string,
   pvcName: string,
+  opts?: K8sAPIOptions,
 ): Promise<NotebookKind> =>
   new Promise((resolve, reject) => {
     getNotebook(notebookName, namespace)
@@ -438,11 +442,13 @@ export const removeNotebookPVC = (
           },
         ];
 
-        k8sPatchResource<NotebookKind>({
-          model: NotebookModel,
-          queryOptions: { name: notebookName, ns: namespace },
-          patches,
-        })
+        k8sPatchResource<NotebookKind>(
+          applyK8sAPIOptions(opts, {
+            model: NotebookModel,
+            queryOptions: { name: notebookName, ns: namespace },
+            patches,
+          }),
+        )
           .then(resolve)
           .catch(reject);
       })

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<PersistentVolumeClaimKind> =>
   k8sGetResource<PersistentVolumeClaimKind>({
@@ -68,42 +80,30 @@ export const getAvailableMultiUsePvcs = (
     }),
   );
 
-export const createPvc = (data: PersistentVolumeClaimKind): Promise<PersistentVolumeClaimKind> =>
-  k8sCreateResource<PersistentVolumeClaimKind>({ model: PVCModel, resource: data });
-
-export const updatePvcDisplayName = (
-  pvcName: string,
+export const createPvc = (
+  data: CreatingStorageObject,
   namespace: string,
-  displayName: string,
-): Promise<PersistentVolumeClaimKind> =>
-  k8sPatchResource({
-    model: PVCModel,
-    queryOptions: { name: pvcName, ns: namespace },
-    patches: [
-      {
-        op: 'replace',
-        path: '/metadata/annotations/openshift.io~1display-name',
-        value: displayName,
-      },
-    ],
-  });
+  opts?: K8sAPIOptions,
+): Promise<PersistentVolumeClaimKind> => {
+  const pvc = assemblePvc(data, namespace);
 
-export const updatePvcDescription = (
-  pvcName: string,
+  return k8sCreateResource<PersistentVolumeClaimKind>(
+    applyK8sAPIOptions(opts, { model: PVCModel, resource: pvc }),
+  );
+};
+
+export const updatePvc = (
+  data: CreatingStorageObject,
+  existingData: PersistentVolumeClaimKind,
   namespace: string,
-  description: string,
-): Promise<PersistentVolumeClaimKind> =>
-  k8sPatchResource({
-    model: PVCModel,
-    queryOptions: { name: pvcName, ns: namespace },
-    patches: [
-      {
-        op: 'replace',
-        path: '/metadata/annotations/openshift.io~1description',
-        value: description,
-      },
-    ],
-  });
+  opts?: K8sAPIOptions,
+): Promise<PersistentVolumeClaimKind> => {
+  const pvc = assemblePvc(data, namespace, existingData.metadata.name);
+
+  return k8sUpdateResource<PersistentVolumeClaimKind>(
+    applyK8sAPIOptions(opts, { model: PVCModel, resource: _.merge({}, existingData, pvc) }),
+  );
+};
 
 export const deletePvc = (pvcName: string, namespace: string): Promise<K8sStatus> =>
   k8sDeleteResource<PersistentVolumeClaimKind, K8sStatus>({
@@ -115,17 +115,20 @@ export const updatePvcSize = (
   pvcName: string,
   namespace: string,
   size: string,
+  opts?: K8sAPIOptions,
 ): Promise<PersistentVolumeClaimKind> =>
-  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,
+          },
         },
-      },
-    ],
-  });
+      ],
+    }),
+  );

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%;
+  }
+}

frontend/src/components/ValueUnitField.tsx

@@ -37,6 +37,7 @@ const ValueUnitField: React.FC<ValueUnitFieldProps> = ({
       </SplitItem>
       <SplitItem>
         <Dropdown
+          menuAppendTo="parent"
           toggle={
             <DropdownToggle id="toggle-basic" onToggle={() => setOpen(!open)}>
               {currentUnitOption.name}

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<Error>();
   const [clusterSettings, setClusterSettings] = React.useState(DEFAULT_CONFIG);
-  const [pvcSize, setPvcSize] = React.useState<number | string>(DEFAULT_PVC_SIZE);
+  const [pvcSize, setPvcSize] = React.useState<number>(DEFAULT_PVC_SIZE);
   const [userTrackingEnabled, setUserTrackingEnabled] = React.useState(false);
   const [cullerTimeout, setCullerTimeout] = React.useState(DEFAULT_CULLER_TIMEOUT);
   const { dashboardConfig } = useAppContext();

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<PVCSizeSettingsProps> = ({ initialValue, pvcSize, setPvcSize }) => {
@@ -57,7 +57,7 @@ all users."
                       : newValue;
                   setPvcSize(newValue);
                 } else {
-                  setPvcSize(modifiedValue);
+                  setPvcSize(0);
                 }
               }}
             />
@@ -79,8 +79,8 @@ all users."
           <HelperText>
             <HelperTextItem
               data-id="pvc-size-helper-text"
-              variant={pvcSize === '' ? 'error' : 'indeterminate'}
-              hasIcon={pvcSize === ''}
+              variant={!pvcSize ? 'error' : 'indeterminate'}
+              hasIcon={!pvcSize}
             >
               Note: PVC size must be between 1 GiB and 16384 GiB.
             </HelperTextItem>

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';
 

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 = {