Add VolumeHealthCard - #2986
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SanjalKatiyar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
efb184f to
f5e3079
Compare
| import { ExternalLinkAltIcon } from '@patternfly/react-icons'; | ||
| import { SortByDirection } from '@patternfly/react-table'; | ||
|
|
||
| const VOLUME_HEALTH_ANNOTATION_PREFIX = 'csiaddons.openshift.io/volumehealth.'; |
There was a problem hiding this comment.
This can go to constants folder.
There was a problem hiding this comment.
it's not something that will be useful outside the context of this card... it's very specific constant...
| scData.filter((sc) => (sc?.provisioner).includes(CEPH_NFS_PROVISIONER)); | ||
|
|
||
| // All CephNFS based StorageClasses from a particular Ceph cluster (multiple StorageSystem/StorageCluster scenario) | ||
| export const filterCephNFSSCByCluster: ( |
There was a problem hiding this comment.
Does external mode support NFS?
There was a problem hiding this comment.
nope...
but these utils are generic, it doesn't care about internal/external or their combinations, all it knows that there should be one cluster per namespace...
| type VolumeHealthAnnotation = { | ||
| state: VolumeHealthState; | ||
| lastChecked?: string; | ||
| since?: string; | ||
| node?: string; | ||
| }; |
There was a problem hiding this comment.
Move this type to shared.
There was a problem hiding this comment.
it's not something that will be useful outside the context of this card... it's very specific type...
| UNHEALTHY = 'unhealthy', | ||
| } | ||
|
|
||
| type VolumeHealthAnnotation = { |
There was a problem hiding this comment.
The better thing to do would be make PVC kind an interface and extend upon that here instead of having these orphaned types not relating to actual things to which these relate. This pattern is not used in our repo so far but that is the correct way.
There was a problem hiding this comment.
I didn't get it... it's just a type for an object that's been read from annotation...
It has nothing to do with PVC kind/interface...
| return direction === SortByDirection.asc ? sortValue : -sortValue; | ||
| }; | ||
|
|
||
| const getUnhealthyRows = ( |
There was a problem hiding this comment.
| const getUnhealthyRows = ( | |
| const getUnhealthyRow = ( |
Isn't this related to only one row?
There was a problem hiding this comment.
it returns a list VolumeHealthRow[], hence plural :)
| const pvcCount = new Set( | ||
| rows.map((row) => `${row.namespace}/${row.pvcName}`) | ||
| ).size; |
There was a problem hiding this comment.
I am confused is there a chance of duplicates? Isn't rows.length enough?
There was a problem hiding this comment.
PVCs can we RWX (same volume attached to multiple nodes), each entry is shown as separate row rn (we could change the UX later)...
Example:
Row1: PVC1 Node1
Row2: PVC1 Node2
But, pvcCount should still be 1 in this case.
|
/hold cancel |
|
/test odf-console-e2e-aws |
1 similar comment
|
/test odf-console-e2e-aws |
|
/hold |
f5e3079 to
8e9a704
Compare
8e9a704 to
e323cb6
Compare
|
/hold cancel |
2 participants
Description
https://redhat.atlassian.net/browse/RHSTOR-6220
Change Type
Please select all applicable options:
Component / Area Impacted
Please select all applicable options:
Screenshots / Recordings
Screenshots
TBA
Recordings / Demo Videos
N/A
Testing
Please select the type of tests included:
Manually testing.
Additional Notes
N/A