Skip to content

Commit

Permalink
Merge pull request #6221 from vbnrh/pvc-block-volume-fix
Browse files Browse the repository at this point in the history
Bug 1853599: Fixes bug attaching block storage PVC to deployments
  • Loading branch information
openshift-merge-robot committed Sep 8, 2020
2 parents 2cb8b41 + 48f057e commit 9cdb472
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 55 deletions.
191 changes: 139 additions & 52 deletions frontend/public/components/storage/attach-storage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 = [
Expand All @@ -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.');
Expand Down Expand Up @@ -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> = () => {
Expand All @@ -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)) {
Expand All @@ -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,
},
};

Expand All @@ -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)));
});
Expand Down Expand Up @@ -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{' '}
Expand Down
5 changes: 3 additions & 2 deletions frontend/public/components/utils/list-dropdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}

Expand Down Expand Up @@ -70,6 +70,7 @@ class ListDropdown_ extends React.Component {
acc[`${resource.metadata.name}-${kindLabel}`] = {
kindLabel,
name: resource.metadata.name,
resource,
};
}
return acc;
Expand Down
3 changes: 2 additions & 1 deletion frontend/public/components/utils/pvc-dropdown.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions frontend/public/module/k8s/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ export type VolumeMount = {
subPathExpr?: string;
};

export type VolumeDevice = {
devicePath: string;
name: string;
};

type ProbePort = string | number;

export type ExecProbe = {
Expand Down Expand Up @@ -247,6 +252,7 @@ export type PodAffinity = {
export type ContainerSpec = {
name: string;
volumeMounts?: VolumeMount[];
volumeDevices?: VolumeDevice[];
env?: EnvVar[];
livenessProbe?: ContainerProbe;
readinessProbe?: ContainerProbe;
Expand Down

0 comments on commit 9cdb472

Please sign in to comment.