Skip to content

Commit 16c6457

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

File tree

7 files changed

+273
-283
lines changed

7 files changed

+273
-283
lines changed

‎locales/en/plugin__odf-console.json

+6-7
Original file line numberDiff line numberDiff line change
@@ -1650,7 +1650,6 @@
16501650
"Deletion policy": "Deletion policy",
16511651
"VolumeSnapshot classes": "VolumeSnapshot classes",
16521652
"Driver": "Driver",
1653-
"VolumeGroupSnapshot classes": "VolumeGroupSnapshot classes",
16541653
"Cancel upload": "Cancel upload",
16551654
"Cancel all ongoing uploads?": "Cancel all ongoing uploads?",
16561655
"Yes, cancel": "Yes, cancel",
@@ -1748,7 +1747,6 @@
17481747
"Only lowercase letters, numbers, non-consecutive periods, or hyphens": "Only lowercase letters, numbers, non-consecutive periods, or hyphens",
17491748
"Cannot be used before": "Cannot be used before",
17501749
"Cannot be used before within the same namespace": "Cannot be used before within the same namespace",
1751-
"Cannot be empty": "Cannot be empty",
17521750
"Not enough usage data": "Not enough usage data",
17531751
"Total requests: ": "Total requests: ",
17541752
"used": "used",
@@ -1797,9 +1795,6 @@
17971795
"No resources available": "No resources available",
17981796
"Select {{resourceLabel}}": "Select {{resourceLabel}}",
17991797
"Error Loading": "Error Loading",
1800-
"no results": "no results",
1801-
"No results found for {{ filterValue }}": "No results found for {{ filterValue }}",
1802-
"Clear selected value": "Clear selected value",
18031798
"Loading empty page": "Loading empty page",
18041799
"You are not authorized to complete this action. See your cluster administrator for role-based access control information.": "You are not authorized to complete this action. See your cluster administrator for role-based access control information.",
18051800
"Not Authorized": "Not Authorized",
@@ -1863,7 +1858,6 @@
18631858
"Infrastructures": "Infrastructures",
18641859
"Subscriptions": "Subscriptions",
18651860
"Project": "Project",
1866-
"Suspended": "Suspended",
18671861
"Composable table": "Composable table",
18681862
"Selectable table": "Selectable table",
18691863
"Select all": "Select all",
@@ -1918,5 +1912,10 @@
19181912
"Cannot change resource name (original: \"{{name}}\", updated: \"{{newName}}\").": "Cannot change resource name (original: \"{{name}}\", updated: \"{{newName}}\").",
19191913
"Cannot change resource namespace (original: \"{{namespace}}\", updated: \"{{newNamespace}}\").": "Cannot change resource namespace (original: \"{{namespace}}\", updated: \"{{newNamespace}}\").",
19201914
"Cannot change resource kind (original: \"{{original}}\", updated: \"{{updated}}\").": "Cannot change resource kind (original: \"{{original}}\", updated: \"{{updated}}\").",
1921-
"Cannot change API group (original: \"{{apiGroup}}\", updated: \"{{newAPIGroup}}\").": "Cannot change API group (original: \"{{apiGroup}}\", updated: \"{{newAPIGroup}}\")."
1915+
"Cannot change API group (original: \"{{apiGroup}}\", updated: \"{{newAPIGroup}}\").": "Cannot change API group (original: \"{{apiGroup}}\", updated: \"{{newAPIGroup}}\").",
1916+
"Cannot be empty": "Cannot be empty",
1917+
"no results": "no results",
1918+
"No results found for {{ filterValue }}": "No results found for {{ filterValue }}",
1919+
"Clear selected value": "Clear selected value",
1920+
"Suspended": "Suspended"
19221921
}

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

+12-18
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,11 @@ const resources = [
3030
];
3131

3232
const selectedResources: SelectedResources = {
33-
'test-storage-class': {
34-
selected: false,
35-
resourceType: 'storageClass',
33+
storageClass: {
34+
'test-storage-class': false,
3635
},
37-
'test-snapshot-class': {
38-
selected: false,
39-
resourceType: 'volumeSnapshotClass',
36+
volumeSnapshotClass: {
37+
'test-snapshot-class': false,
4038
},
4139
};
4240

@@ -101,27 +99,23 @@ describe('Resource distribution table component renders correctly for a storage
10199
const user = userEvent.setup();
102100
await user.click(checkbox);
103101
expect(setSelectedResourcesMock).toHaveBeenCalledWith({
104-
'test-snapshot-class': {
105-
resourceType: 'volumeSnapshotClass',
106-
selected: false,
102+
volumeSnapshotClass: {
103+
'test-snapshot-class': false,
107104
},
108-
'test-storage-class': {
109-
resourceType: 'storageClass',
110-
selected: true,
105+
storageClass: {
106+
'test-storage-class': true,
111107
},
112108
});
113109
// Test select all
114110
const selectAll = screen.getByLabelText('Select all rows');
115111
expect(selectAll).toBeInTheDocument();
116112
await userEvent.click(selectAll);
117113
expect(setSelectedResourcesMock).toHaveBeenCalledWith({
118-
'test-snapshot-class': {
119-
resourceType: 'volumeSnapshotClass',
120-
selected: false,
114+
volumeSnapshotClass: {
115+
'test-snapshot-class': false,
121116
},
122-
'test-storage-class': {
123-
resourceType: 'storageClass',
124-
selected: true,
117+
storageClass: {
118+
'test-storage-class': true,
125119
},
126120
});
127121
});

‎packages/odf/components/ResourceDistribution/ResourceDistributionTable.tsx

+21-40
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,22 @@ import * as _ from 'lodash-es';
1010
import { Thead, Tr, Th, Tbody, Table } from '@patternfly/react-table';
1111

1212
export type SelectedResources = {
13-
[name: string]: {
14-
selected: boolean;
15-
resourceType:
16-
| 'storageClass'
17-
| 'volumeSnapshotClass'
18-
| 'volumeGroupSnapshotClass';
13+
storageClass: {
14+
[name: string]: boolean;
15+
};
16+
volumeSnapshotClass: {
17+
[name: string]: boolean;
1918
};
2019
};
2120

22-
type ResourceDistributionTableProps = {
21+
export type ResourceDistributionTableProps = {
2322
resources: K8sResourceCommon[];
24-
selectedResources: {
25-
[name: string]: {
26-
selected: boolean;
27-
resourceType:
28-
| 'storageClass'
29-
| 'volumeSnapshotClass'
30-
| 'volumeGroupSnapshotClass';
31-
};
32-
};
23+
selectedResources: SelectedResources;
3324
setSelectedResources: React.Dispatch<React.SetStateAction<SelectedResources>>;
3425
RowGenerator: React.FC<RowGeneratorProps<K8sResourceCommon>>;
3526
columns: string[];
3627
loaded: boolean;
37-
resourceType:
38-
| 'storageClass'
39-
| 'volumeSnapshotClass'
40-
| 'volumeGroupSnapshotClass';
28+
resourceType: 'storageClass' | 'volumeSnapshotClass';
4129
};
4230

4331
export type RowGeneratorProps<T extends K8sResourceCommon> = {
@@ -66,10 +54,7 @@ export const ResourceDistributionTable: React.FC<
6654
? false
6755
: resources.every((resource) => {
6856
const name = getName(resource);
69-
return (
70-
selectedResources[name]?.selected === true &&
71-
selectedResources[name]?.resourceType === resourceType
72-
);
57+
return !!selectedResources?.[resourceType]?.[name];
7358
});
7459
if (allSelected) {
7560
setAllResourcesSelected(true);
@@ -79,14 +64,11 @@ export const ResourceDistributionTable: React.FC<
7964
(resource: K8sResourceCommon) => (selected: boolean) => {
8065
const updatedResources = {
8166
...selectedResources,
82-
[getName(resource)]: {
83-
selected,
84-
resourceType,
85-
},
8667
};
87-
const isAllSelected = Object.values(updatedResources)
88-
.filter((res) => res.resourceType === resourceType)
89-
.every((res) => res.selected);
68+
updatedResources[resourceType][getName(resource)] = selected;
69+
const isAllSelected = Object.entries(
70+
updatedResources?.[resourceType]
71+
).every(([, v]) => v);
9072
if (isAllSelected) {
9173
setAllResourcesSelected(true);
9274
} else {
@@ -99,22 +81,21 @@ export const ResourceDistributionTable: React.FC<
9981

10082
const isRowSelected = React.useCallback(
10183
(resource: K8sResourceCommon) =>
102-
selectedResources?.[getName(resource)]?.selected,
103-
[selectedResources]
84+
selectedResources[resourceType]?.[getName(resource)],
85+
[resourceType, selectedResources]
10486
);
10587

10688
const [unfilteredData, filteredData, onFilterChange] =
10789
useListPageFilter(resources);
10890

10991
const selectAllResources = (select: boolean) => {
11092
setAllResourcesSelected(select);
111-
const newSelectedResources = filteredData.reduce((acc, storageClass) => {
112-
acc[storageClass.metadata.name] = {
113-
resourceType,
114-
selected: select,
115-
};
116-
return acc;
117-
}, {} as SelectedResources);
93+
const newSelectedResources = {
94+
[resourceType]: filteredData.reduce((acc, storageClass) => {
95+
acc[storageClass.metadata.name] = select;
96+
return acc;
97+
}, {}),
98+
};
11899
setSelectedResources((prev) => ({ ...prev, ...newSelectedResources }));
119100
};
120101
return !loaded ? (

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

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

0 commit comments

Comments
 (0)