Skip to content

Commit 42f6f3f

Browse files
bipuladhbipuladh
committed
Fixes storage resource distribution
Removes VolumeGroupSnapshot from UI as well Signed-off-by: Bipul Adhikari <[email protected]>
1 parent 8ba9268 commit 42f6f3f

File tree

4 files changed

+197
-138
lines changed

4 files changed

+197
-138
lines changed

‎packages/odf/components/ResourceDistribution/util.spec.tsx

+108
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { StorageConsumerKind, StorageConsumerState } from '@odf/shared';
2+
import '@testing-library/jest-dom';
3+
import { generatePatchToModifyStorageClasses } from './utils';
4+
5+
const storageConsumer: StorageConsumerKind = {
6+
apiVersion: 'ocs.openshift.io/v1alpha1',
7+
kind: 'StorageConsumer',
8+
metadata: {
9+
name: 'internal',
10+
namespace: 'openshift-storage',
11+
},
12+
spec: {
13+
enable: true,
14+
storageQuotaInGiB: 1000,
15+
storageClasses: [
16+
{
17+
name: 'ocs-storagecluster-ceph-rbd',
18+
},
19+
{
20+
name: 'ocs-storagecluster-cephfs',
21+
},
22+
],
23+
volumeGroupSnapshotClasses: [
24+
{
25+
name: 'ocs-storagecluster-cephfs-groupsnapclass',
26+
},
27+
{
28+
name: 'ocs-storagecluster-ceph-rbd-groupsnapclass',
29+
},
30+
],
31+
volumeSnapshotClasses: [
32+
{
33+
name: 'ocs-storagecluster-rbdplugin-snapclass',
34+
},
35+
{
36+
name: 'ocs-storagecluster-cephfsplugin-snapclass',
37+
},
38+
],
39+
},
40+
status: {
41+
client: {
42+
name: 'ocs-storagecluster',
43+
clusterId: '428c1c58-5a9f-475d-86a1-87ca83fd4fb7',
44+
clusterName: 'dr1-may-2-25.devcluster.openshift.com',
45+
operatorVersion: '4.19.0-52.stable',
46+
platformVersion: '4.19.0-0.nightly-2025-05-01-165245',
47+
storageQuotaUtilizationRatio: 0.5,
48+
},
49+
lastHeartbeat: '2025-05-02T08:14:00Z',
50+
onboardingTicketSecret: {
51+
name: '',
52+
},
53+
resourceNameMappingConfigMap: {
54+
name: 'storageconsumer-internal',
55+
},
56+
state: StorageConsumerState.Ready,
57+
},
58+
};
59+
60+
describe('Storage class distribution patch works as expected', () => {
61+
it('should return empty patch array when no resources are added(preselected present)', () => {
62+
const patches = generatePatchToModifyStorageClasses(storageConsumer, [
63+
'ocs-storagecluster-ceph-rbd',
64+
'ocs-storagecluster-cephfs',
65+
]);
66+
expect(patches).toEqual([]);
67+
});
68+
it('should return replace patch command when resources are removed', () => {
69+
const patches = generatePatchToModifyStorageClasses(storageConsumer, [
70+
'ocs-storagecluster-ceph-rbd',
71+
]);
72+
expect(patches).toEqual([
73+
{
74+
op: 'replace',
75+
path: '/spec/storageClasses',
76+
value: [
77+
{
78+
name: 'ocs-storagecluster-ceph-rbd',
79+
},
80+
],
81+
},
82+
]);
83+
});
84+
it('should return add patch command when resources are removed', () => {
85+
const patches = generatePatchToModifyStorageClasses(storageConsumer, [
86+
'ocs-storagecluster-ceph-rbd',
87+
'ocs-storagecluster-cephfs',
88+
'added-sc',
89+
]);
90+
expect(patches).toEqual([
91+
{
92+
op: 'add',
93+
path: '/spec/storageClasses',
94+
value: [
95+
{
96+
name: 'ocs-storagecluster-ceph-rbd',
97+
},
98+
{
99+
name: 'ocs-storagecluster-cephfs',
100+
},
101+
{
102+
name: 'added-sc',
103+
},
104+
],
105+
},
106+
]);
107+
});
108+
});

‎packages/odf/components/ResourceDistribution/utils.ts

+74-72
Original file line numberDiff line numberDiff line change
@@ -2,91 +2,93 @@ import { StorageConsumerKind } from '@odf/shared';
22
import { Patch } from '@openshift-console/dynamic-plugin-sdk';
33
import * as _ from 'lodash-es';
44

5-
export const generatePatchForDistributionOfResources = (
5+
const getInitiallySelectedStorageClasses = (
6+
storageConsumer: StorageConsumerKind
7+
): string[] => {
8+
const storageClasses = storageConsumer?.spec?.storageClasses || [];
9+
return storageClasses.map((sc) => sc.name);
10+
};
11+
12+
const getInitiallySelectedVolumeSnapshotClasses = (
13+
storageConsumer: StorageConsumerKind
14+
): string[] => {
15+
const volumeSnapshotClasses =
16+
storageConsumer?.spec?.volumeSnapshotClasses || [];
17+
return volumeSnapshotClasses.map((vsc) => vsc.name);
18+
};
19+
20+
const generatePatchToModifyResourcesForDistribution = (
621
storageConsumer: StorageConsumerKind,
7-
storageClassNames: string[],
8-
volumeSnapshotClassNames: string[],
9-
volumeGroupSnapshotClassNames: string[]
22+
resourceType: 'SC' | 'VSC',
23+
resourceNames: string[]
1024
): Patch[] => {
1125
const patches: Patch[] = [];
12-
const isStorageClassesDistributed = !_.isEmpty(
13-
storageConsumer?.spec?.storageClasses
14-
);
15-
const isVolumeSnapshotClassesDistributed = !_.isEmpty(
16-
storageConsumer?.spec?.volumeSnapshotClasses
26+
const path =
27+
resourceType === 'SC'
28+
? '/spec/storageClasses'
29+
: '/spec/volumeSnapshotClasses';
30+
const currentlySelectedResources =
31+
resourceType === 'SC'
32+
? getInitiallySelectedStorageClasses(storageConsumer)
33+
: getInitiallySelectedVolumeSnapshotClasses(storageConsumer);
34+
const removedResources = currentlySelectedResources.filter(
35+
(sc) => !resourceNames.includes(sc)
1736
);
18-
const isVolumeGroupSnapshotClassesDistributed = !_.isEmpty(
19-
storageConsumer?.spec?.volumeGroupSnapshotClasses
37+
const addedResources = resourceNames.filter(
38+
(sc) => !currentlySelectedResources.includes(sc)
2039
);
21-
const currentlySelectedStorageClasses =
22-
storageConsumer.spec?.storageClasses?.map((sc) => sc.name) || [];
23-
const currentlySelectedVolumeSnapshotClasses =
24-
storageConsumer.spec?.volumeSnapshotClasses?.map((vsc) => vsc.name) || [];
25-
const currentlySelectedVolumeGroupSnapshotClasses =
26-
storageConsumer.spec?.volumeGroupSnapshotClasses?.map(
27-
(vgsc) => vgsc.name
28-
) || [];
29-
const isStorageClassesChanged =
30-
storageClassNames.length !== currentlySelectedStorageClasses.length ||
31-
!storageClassNames.every((res) =>
32-
currentlySelectedStorageClasses.includes(res)
33-
);
34-
const isVolumeSnapshotClassesChanged =
35-
volumeSnapshotClassNames.length !==
36-
currentlySelectedVolumeSnapshotClasses.length ||
37-
!volumeSnapshotClassNames?.every((res) =>
38-
currentlySelectedVolumeSnapshotClasses?.includes(res)
39-
);
40-
const isVolumeGroupSnapshotClassesChanged =
41-
volumeGroupSnapshotClassNames.length !==
42-
currentlySelectedVolumeGroupSnapshotClasses.length ||
43-
!volumeGroupSnapshotClassNames?.every((res) =>
44-
currentlySelectedVolumeGroupSnapshotClasses?.includes(res)
45-
);
46-
47-
if (!isStorageClassesDistributed && !_.isEmpty(storageClassNames)) {
48-
patches.push({
49-
op: 'add',
50-
path: '/spec/storageClasses',
51-
value: storageClassNames.map((name) => ({ name })),
52-
});
53-
} else if (isStorageClassesChanged) {
40+
if (removedResources.length > 0) {
5441
patches.push({
5542
op: 'replace',
56-
path: '/spec/storageClasses',
57-
value: storageClassNames.map((name) => ({ name })),
43+
path,
44+
value: resourceNames.map((name) => ({ name })),
5845
});
5946
}
60-
61-
if (
62-
!isVolumeSnapshotClassesDistributed &&
63-
!_.isEmpty(volumeSnapshotClassNames)
64-
) {
47+
if (addedResources.length > 0) {
6548
patches.push({
6649
op: 'add',
67-
path: '/spec/volumeSnapshotClasses',
68-
value: volumeSnapshotClassNames.map((name) => ({ name })),
69-
});
70-
} else if (isVolumeSnapshotClassesChanged) {
71-
patches.push({
72-
op: 'replace',
73-
path: '/spec/volumeSnapshotClasses',
74-
value: volumeSnapshotClassNames.map((name) => ({ name })),
75-
});
76-
}
77-
if (!isVolumeGroupSnapshotClassesDistributed) {
78-
patches.push({
79-
op: 'add',
80-
path: '/spec/volumeGroupSnapshotClasses',
81-
value: volumeGroupSnapshotClassNames.map((name) => ({ name })),
82-
});
83-
} else if (isVolumeGroupSnapshotClassesChanged) {
84-
patches.push({
85-
op: 'replace',
86-
path: '/spec/volumeGroupSnapshotClasses',
87-
value: volumeGroupSnapshotClassNames.map((name) => ({ name })),
50+
path,
51+
value: resourceNames.map((name) => ({ name })),
8852
});
8953
}
54+
return patches;
55+
};
56+
57+
export const generatePatchToModifyStorageClasses = (
58+
storageConsumer: StorageConsumerKind,
59+
storageClassNames: string[]
60+
): Patch[] =>
61+
generatePatchToModifyResourcesForDistribution(
62+
storageConsumer,
63+
'SC',
64+
storageClassNames
65+
);
66+
67+
export const generatePatchToModifyVolumeSnapshotClasses = (
68+
storageConsumer: StorageConsumerKind,
69+
volumeSnapshotClassNames: string[]
70+
): Patch[] =>
71+
generatePatchToModifyResourcesForDistribution(
72+
storageConsumer,
73+
'VSC',
74+
volumeSnapshotClassNames
75+
);
9076

77+
export const generatePatchForDistributionOfResources = (
78+
storageConsumer: StorageConsumerKind,
79+
storageClassNames: string[],
80+
volumeSnapshotClassNames: string[]
81+
): Patch[] => {
82+
const patches: Patch[] = [];
83+
const storageClassPatches = generatePatchToModifyStorageClasses(
84+
storageConsumer,
85+
storageClassNames
86+
);
87+
const volumeSnapshotClassPatches = generatePatchToModifyVolumeSnapshotClasses(
88+
storageConsumer,
89+
volumeSnapshotClassNames
90+
);
91+
patches.push(...storageClassPatches);
92+
patches.push(...volumeSnapshotClassPatches);
9193
return patches;
9294
};

‎packages/odf/modals/ResourceDistributionModal/ResourceDistributionModal.tsx

+1-37
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,6 @@ export const DistributeResourceModal: React.FC<
141141
(res) => isCephProvisioner(res?.driver)
142142
);
143143

144-
const filteredVolumeGroupSnapshotClasses =
145-
data.volumeGroupSnapshotClasses.data.filter((res) =>
146-
isCephProvisioner(res?.driver)
147-
);
148-
149144
const isStorageClassDataLoaded = data?.storageClasses?.loaded;
150145
const isVolumeSnapshotClassDataLoaded = data?.volumeSnapshotClasses?.loaded;
151146
const isVolumeGroupSnapshotClassDataLoaded =
@@ -163,11 +158,6 @@ export const DistributeResourceModal: React.FC<
163158
).filter(
164159
(res) => res.resourceType === 'volumeSnapshotClass' && res.selected
165160
).length;
166-
const selectedVolumeGroupSnapshotClassesCount = Object.values(
167-
selectedResources
168-
).filter(
169-
(res) => res.resourceType === 'volumeGroupSnapshotClass' && res.selected
170-
).length;
171161

172162
React.useEffect(() => {
173163
if (
@@ -247,17 +237,10 @@ export const DistributeResourceModal: React.FC<
247237
value.selected && value.resourceType === 'volumeSnapshotClass'
248238
)
249239
.map(([key]) => key);
250-
const selectedVolumeGroupSnapshotClasses = Object.entries(selectedResources)
251-
.filter(
252-
([, value]) =>
253-
value.selected && value.resourceType === 'volumeGroupSnapshotClass'
254-
)
255-
.map(([key]) => key);
256240
const patch = generatePatchForDistributionOfResources(
257241
resource,
258242
selectedStorageClasses,
259-
selectedVolumeSnapshotClasses,
260-
selectedVolumeGroupSnapshotClasses
243+
selectedVolumeSnapshotClasses
261244
);
262245
setProgress(true);
263246
k8sPatch({ model: StorageConsumerModel, resource, data: patch })
@@ -317,25 +300,6 @@ export const DistributeResourceModal: React.FC<
317300
resourceType="volumeSnapshotClass"
318301
/>
319302
</Tab>
320-
<Tab
321-
eventKey={3}
322-
title={
323-
<TabTitleText>
324-
{t('VolumeGroupSnapshot classes')}{' '}
325-
<Label>{selectedVolumeGroupSnapshotClassesCount}</Label>
326-
</TabTitleText>
327-
}
328-
>
329-
<ResourceDistributionTable
330-
resources={filteredVolumeGroupSnapshotClasses}
331-
selectedResources={selectedResources}
332-
setSelectedResources={setSelectedResources}
333-
RowGenerator={VolumeSnapshotClassRowGenerator}
334-
loaded={isVolumeGroupSnapshotClassDataLoaded}
335-
columns={[t('Name'), t('Driver'), t('Deletion policy')]}
336-
resourceType="volumeGroupSnapshotClass"
337-
/>
338-
</Tab>
339303
</Tabs>
340304
<ModalFooter inProgress={inProgress} errorMessage={error}>
341305
<Flex direction={{ default: 'row' }}>

0 commit comments

Comments
 (0)