Fixes bug attaching block storage to pods, deployments in attach-stor…

…age page

Using  block storage provisioners with RWX access mode incorrectly attaches the path as volumeMount in PodSpec instead of volumeDevice. This caused the pods to be in a state of ContainerCreation indefinitely. This  commit fixes this by  sending the object with the right configuration for block storage.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1853599

Signed-off-by: Vineet Badrinath <vbadrina@redhat.com>

frontend/public/components/storage/attach-storage.tsx

@@ -19,6 +19,7 @@ import { CreatePVCForm } from './create-pvc';
 import { PersistentVolumeClaimModel } from '../../models';
 import { ContainerSelector } from '../container-selector';
 import { PVCDropdown } from '../utils/pvc-dropdown';
+import { PodTemplate, PersistentVolumeClaimKind, Patch } from '../../../public/module/k8s/types';
 
 export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
   const [obj, setObj] = React.useState(null);
@@ -27,13 +28,16 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
   const [claimName, setClaimName] = React.useState('');
   const [volumeName, setVolumeName] = React.useState('');
   const [mountPath, setMountPath] = React.useState('');
+  const [devicePath, setDevicePath] = React.useState('');
   const [subPath, setSubPath] = React.useState('');
   const [mountAsReadOnly, setMountAsReadOnly] = React.useState(false);
   const [selectedContainers, setSelectedContainers] = React.useState([]);
   const [volumeAlreadyMounted, setVolumeAlreadyMounted] = React.useState(false);
   const [error, setError] = React.useState('');
   const [showCreatePVC, setShowCreatePVC] = React.useState('existing');
+  const [claimVolumeMode, setClaimVolumeMode] = React.useState('');
   const [newPVCObj, setNewPVCObj] = React.useState(null);
+  const [selectedPVC, setSelectedPVC] = React.useState<PersistentVolumeClaimKind>(null);
 
   const { kindObj, resourceName, namespace } = props;
   const supportedKinds = [
@@ -60,12 +64,14 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
         claimName: newClaimName,
       },
     }) as any;
-
     const newVolumeName = volume ? volume.name : newClaimName;
     const newVolumeAlreadyMounted = !!volume;
     setVolumeName(newVolumeName);
     setVolumeAlreadyMounted(newVolumeAlreadyMounted);
-  }, [newPVCObj, obj, claimName, showCreatePVC]);
+    showCreatePVC === 'existing' && newClaimName.trim().length > 0
+      ? setClaimVolumeMode(selectedPVC.spec.volumeMode)
+      : setClaimVolumeMode(newPVCObj?.spec?.volumeMode);
+  }, [newPVCObj, obj, claimName, showCreatePVC, claimVolumeMode, selectedPVC, namespace]);
 
   if (!kindObj || !_.includes(supportedKinds, kindObj.kind)) {
     setError('Unsupported kind.');
@@ -117,12 +123,39 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
     validateMountPaths(event.currentTarget.value);
   };
 
+  const getDevicePaths = (podTemplate: PodTemplate): string[] => {
+    const containers: ContainerSpec[] = podTemplate?.spec?.containers;
+    return containers.reduce((acc: string[], container: ContainerSpec) => {
+      if (!isContainerSelected(container)) {
+        return acc;
+      }
+      const devicePaths: string[] = _.map(container.volumeDevices, 'devicePath');
+      return acc.concat(devicePaths);
+    }, []);
+  };
+
+  const validateDevicePath = (path: string) => {
+    const existingDevicePaths = getDevicePaths(obj.spec.template);
+    if (existingDevicePaths.includes(path)) {
+      setError('Device path is already in use.');
+    }
+  };
+
+  const handleDevicePathChange: React.ReactEventHandler<HTMLInputElement> = (event) => {
+    setDevicePath(event.currentTarget.value);
+    validateDevicePath(event.currentTarget.value);
+  };
   const handleSubPathChange: React.ReactEventHandler<HTMLInputElement> = (event) => {
     setSubPath(event.currentTarget.value);
   };
 
-  const handlePVCChange = (newClaimName: string) => {
+  const handlePVCChange = (
+    newClaimName: string,
+    kindLabel?: string,
+    resource?: PersistentVolumeClaimKind,
+  ) => {
     setClaimName(newClaimName);
+    setSelectedPVC(resource);
   };
 
   const onMountAsReadOnlyChanged: React.ReactEventHandler<HTMLInputElement> = () => {
@@ -135,16 +168,15 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
       : Promise.resolve(claimName);
   };
 
-  const getVolumePatches = (pvClaimName: string) => {
+  const getVolumeMountPatches = (): Patch[] => {
     const mount = {
       name: volumeName,
       mountPath,
       subPath,
       readOnly: mountAsReadOnly,
     };
-
     const containers: ContainerSpec[] = _.get(obj, 'spec.template.spec.containers', []);
-    const patches = containers.reduce((patch, container, i) => {
+    return containers.reduce((patch, container, i) => {
       // Only add to selected containers
       if (isContainerSelected(container)) {
         if (_.isEmpty(container.volumeMounts)) {
@@ -163,10 +195,41 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
       }
       return patch;
     }, []);
+  };
+
+  const getVolumeDevicePatches = (): Patch[] => {
+    const device = {
+      name: volumeName,
+      devicePath,
+    };
+    const containers: ContainerSpec[] = obj?.spec?.template?.spec?.containers;
+    return containers.reduce((patch, container, i) => {
+      // Only add to selected containers
+      if (isContainerSelected(container)) {
+        if (_.isEmpty(container.volumeMounts)) {
+          patch.push({
+            op: 'add',
+            path: `/spec/template/spec/containers/${i}/volumeDevices`,
+            value: [device],
+          });
+        } else {
+          patch.push({
+            op: 'add',
+            path: `/spec/template/spec/containers/${i}/volumeDevices/-`,
+            value: device,
+          });
+        }
+      }
+      return patch;
+    }, []);
+  };
+  const getVolumePatches = (pvcName: string) => {
+    const patches =
+      claimVolumeMode === 'Block' ? getVolumeDevicePatches() : getVolumeMountPatches();
     const volume = {
       name: volumeName,
       persistentVolumeClaim: {
-        claimName: pvClaimName,
+        claimName: pvcName,
       },
     };
 
@@ -188,8 +251,8 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
     }
     setInProgress(true);
     createPVCIfNecessary().then(
-      (pvClaimName: string) => {
-        return k8sPatch(kindObj, obj, getVolumePatches(pvClaimName)).then((resource) => {
+      (pvcName: string) => {
+        return k8sPatch(kindObj, obj, getVolumePatches(pvcName)).then((resource) => {
           setInProgress(false);
           history.push(resourceObjPath(resource, referenceFor(resource)));
         });
@@ -254,52 +317,76 @@ export const AttachStorageForm: React.FC<AttachStorageFormProps> = (props) => {
           </div>
         )}
 
-        <div className="form-group">
-          <label className="control-label co-required" htmlFor="mount-path">
-            Mount Path
-          </label>
-          <div>
-            <input
-              className="pf-c-form-control"
-              type="text"
-              onChange={handleMountPathChange}
-              aria-describedby="mount-path-help"
-              name="mountPath"
-              id="mount-path"
-              value={mountPath}
-              required
-            />
-            <p className="help-block" id="mount-path-help">
-              Mount path for the volume inside the container.
-            </p>
+        {claimVolumeMode === 'Block' ? (
+          <div className="form-group">
+            <label className="control-label co-required" htmlFor="device-path">
+              Device Path
+            </label>
+            <div>
+              <input
+                className="pf-c-form-control"
+                type="text"
+                onChange={handleDevicePathChange}
+                aria-describedby="volume-device-help"
+                name="devicePath"
+                id="device-path"
+                value={devicePath}
+                required
+              />
+              <p className="help-block" id="volume-device-help">
+                Device path for the block volume inside the container.
+              </p>
+            </div>
           </div>
-        </div>
-        <Checkbox
-          label="Mount as read-only"
-          onChange={onMountAsReadOnlyChanged}
-          checked={mountAsReadOnly}
-          name="mountAsReadOnly"
-        />
-        <div className="form-group">
-          <label className="control-label" htmlFor="subpath">
-            Subpath
-          </label>
-          <div>
-            <input
-              className="pf-c-form-control"
-              type="text"
-              onChange={handleSubPathChange}
-              aria-describedby="subpath-help"
-              id="subpath"
-              name="subPath"
-              value={subPath}
+        ) : (
+          <div className="form-group">
+            <label className="control-label co-required" htmlFor="mount-path">
+              Mount Path
+            </label>
+            <div>
+              <input
+                className="pf-c-form-control"
+                type="text"
+                onChange={handleMountPathChange}
+                aria-describedby="mount-path-help"
+                name="mountPath"
+                id="mount-path"
+                value={mountPath}
+                required
+              />
+              <p className="help-block" id="mount-path-help">
+                Mount path for the volume inside the container.
+              </p>
+            </div>
+            <Checkbox
+              label="Mount as read-only"
+              onChange={onMountAsReadOnlyChanged}
+              checked={mountAsReadOnly}
+              name="mountAsReadOnly"
             />
-            <p className="help-block" id="subpath-help">
-              Optional path within the volume from which it will be mounted into the container.
-              Defaults to the root of volume.
-            </p>
+            <div className="form-group">
+              <label className="control-label" htmlFor="subpath">
+                Subpath
+              </label>
+              <div>
+                <input
+                  className="pf-c-form-control"
+                  type="text"
+                  onChange={handleSubPathChange}
+                  aria-describedby="subpath-help"
+                  id="subpath"
+                  name="subPath"
+                  value={subPath}
+                />
+                <p className="help-block" id="subpath-help">
+                  Optional path within the volume from which it will be mounted into the container.
+                  Defaults to the root of volume.
+                </p>
+              </div>
+            </div>
           </div>
-        </div>
+        )}
+
         {!useContainerSelector && (
           <p>
             The volume will be mounted into all containers. You can{' '}

frontend/public/components/utils/list-dropdown.jsx

@@ -29,9 +29,9 @@ class ListDropdown_ extends React.Component {
     this.autocompleteFilter = (text, item) => fuzzy(text, item.props.name);
     // Pass both the resource name and the resource kind to onChange()
     this.onChange = (key) => {
-      const { name, kindLabel } = _.get(this.state, ['items', key], {});
+      const { name, kindLabel, resource } = _.get(this.state, ['items', key], {});
       this.setState({ selectedKey: key, title: <ResourceName kind={kindLabel} name={name} /> });
-      this.props.onChange(name, kindLabel);
+      this.props.onChange(name, kindLabel, resource);
     };
   }
 
@@ -70,6 +70,7 @@ class ListDropdown_ extends React.Component {
               acc[`${resource.metadata.name}-${kindLabel}`] = {
                 kindLabel,
                 name: resource.metadata.name,
+                resource,
               };
             }
             return acc;

frontend/public/components/utils/pvc-dropdown.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { ListDropdown } from './list-dropdown';
 import { PersistentVolumeClaimModel } from '../../models';
+import { PersistentVolumeClaimKind } from '../../../public/module/k8s/types';
 
 export const PVCDropdown: React.FC<PVCDropdownProps> = (props) => {
   const kind = PersistentVolumeClaimModel.kind;
@@ -21,7 +22,7 @@ export const PVCDropdown: React.FC<PVCDropdownProps> = (props) => {
 export type PVCDropdownProps = {
   namespace: string;
   selectedKey: string;
-  onChange: (string) => void;
+  onChange: (claimName: string, kindLabel?: string, pvc?: PersistentVolumeClaimKind) => void;
   id?: string;
   desc?: string;
   dataTest?: string;

frontend/public/module/k8s/types.ts

@@ -122,6 +122,11 @@ export type VolumeMount = {
   subPathExpr?: string;
 };
 
+export type VolumeDevice = {
+  devicePath: string;
+  name: string;
+};
+
 type ProbePort = string | number;
 
 export type ExecProbe = {
@@ -247,6 +252,7 @@ export type PodAffinity = {
 export type ContainerSpec = {
   name: string;
   volumeMounts?: VolumeMount[];
+  volumeDevices?: VolumeDevice[];
   env?: EnvVar[];
   livenessProbe?: ContainerProbe;
   readinessProbe?: ContainerProbe;