Adds fallback layer for multi cluster code in External mode #1212
Conversation
|
@bipuladh: This pull request references Bugzilla bug 2259961, which is valid. No validations were run on this bugRequesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: nehaberry. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
packages/ocs/dashboards/persistent-external/utilization-card.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const useFallbackHook = (storageClusterName) => { | ||
| const [storageClasses, storageClassesLoaded, storageClassLoadError] = | ||
| useK8sWatchResource<StorageClassResourceKind[]>(storageClusterResource); |
There was a problem hiding this comment.
don't u wanna pass StorageClass resource instead ??
| useK8sWatchResource<StorageClassResourceKind[]>(storageClusterResource); | |
| useK8sWatchResource<StorageClassResourceKind[]>(storageClassResource); |
packages/ocs/dashboards/persistent-external/utilization-card.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const useFallbackHook = (storageClusterName) => { | ||
| const [storageClasses, storageClassesLoaded, storageClassLoadError] = | ||
| useK8sWatchResource<StorageClassResourceKind[]>(storageClusterResource); |
There was a problem hiding this comment.
this way it won't poll for resource unless needed...
| useK8sWatchResource<StorageClassResourceKind[]>(storageClusterResource); | |
| useK8sWatchResource<StorageClassResourceKind[]>(getValidWatchK8sResourceObj(storageClassResource, !storageClusterName)); |
There was a problem hiding this comment.
We already poll for storageClass in the same page. so we can live with this poll. Essentially the connection is going to be shared.
bipuladh
force-pushed
the
fix-multi-client
branch
from
d2b0108
to
db34a09
Compare
| */ | ||
| const useFallbackHook = (storageClusterName) => { | ||
| const [storageClasses, storageClassesLoaded, storageClassLoadError] = | ||
| useK8sWatchResource<StorageClassResourceKind[]>(scResource); |
There was a problem hiding this comment.
this way it won't even poll when we already have a value for storageClusterName...
| useK8sWatchResource<StorageClassResourceKind[]>(scResource); | |
| useK8sWatchResource<StorageClassResourceKind[]>(getValidWatchK8sResourceObj(scResource, !storageClusterName)); |
There was a problem hiding this comment.
this resource is already called in the same page for inventory card. so we can live with this.
There was a problem hiding this comment.
make sense then !! (I did not check the other active components)
Signed-off-by: Bipul Adhikari <[email protected]>
bipuladh
force-pushed
the
fix-multi-client
branch
from
db34a09
to
1d21c8a
Compare
| kind: referenceForModel(StorageClassModel), | ||
| }; | ||
|
|
||
| const fsProvisionerPostFix = '.cephfs.csi.ceph.com'; |
There was a problem hiding this comment.
under common.ts let's create an enum for each ceph provisioner type and reuse the same here, in this way we can replace the cephStorageProvisioners list under common also can be replaced with enums.
There was a problem hiding this comment.
We need to backport this to 4.15. I will create a seperate PR on top of this for 4.16
There was a problem hiding this comment.
and here declare the fsProvisionerPostFix like: '.' + CEPH_FILESYSTEM_PROVISIONER
|
LGTM |
| const fileSystemProvisioner = provisioners.find((item) => | ||
| item.includes(fsProvisionerPostFix) |
There was a problem hiding this comment.
shall we replace this with a util function like isCephFSProvisioner?
There was a problem hiding this comment.
we can do that but I don't think we would have a very specific use case. We could probably go with isOCSProvisioner with more args. Adding it to #1215
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bipuladh, GowthamShanmugam The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retitle Adds fallback layer for multi cluster code in External mode |
openshift-ci
bot
changed the title
|
/cherry-pick release-4.15 |
|
@SanjalKatiyar: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@bipuladh: No Bugzilla bug is referenced in the title of this pull request. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.15-compatibility |
|
@SanjalKatiyar: once the present PR merges, I will cherry-pick it on top of release-4.15-compatibility in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-merge-bot
bot
merged commit a11fb5a
into
red-hat-storage:master
|
@SanjalKatiyar: new pull request created: #1216 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@SanjalKatiyar: new pull request created: #1217 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
No description provided.