Show provisioner based storage classes #3473
Conversation
openshift-ci-robot
added
component/ceph
|
/kind bug |
openshift-ci-robot
added
kind/bug
cloudbehl
force-pushed
the
add-capacity-dropdown
branch
from
6f387c6
to
032679e
Compare
|
/test images |
| @@ -1,10 +1,11 @@ | |||
| import * as React from 'react'; | |||
| import * as _ from 'lodash'; | |||
| import { Firehose } from '@console/internal/components/utils'; | |||
| import { InfrastructureModel } from '@console/internal/models'; | |||
| import { K8sResourceKind, StorageClassResourceKind, k8sGet } from '@console/internal/module/k8s'; | |||
There was a problem hiding this comment.
Already alpha. capital K and small k diff.
| setInfrastructureError(error); | ||
| } | ||
| }; | ||
| fetchInfrastructure(); |
There was a problem hiding this comment.
IMO we can directly execute the code instead of defining in separate function and the calling it immediately. Also I don't see that fetchInfrastructure is getting used anywhere.
There was a problem hiding this comment.
Await needs async function.
| if (scLoaded && !infrastructureError && !!infrastructurePlatform) { | ||
| // find infra supported provisioner | ||
| const provisioner = infraProvisionerMap[_.toLower(infrastructurePlatform)]; | ||
| scConfig.resources.StorageClass.data = _.filter(scData, (sc) => sc.provisioner === provisioner); |
There was a problem hiding this comment.
Please add types for both provisioner and scConfig
There was a problem hiding this comment.
Ack on provisioner. scConfig type is inherited.
| }); | ||
| makeOCSRequest(); | ||
| // take the first provisioner based storageclass | ||
| const firstSC = _.get(scList, '0'); |
There was a problem hiding this comment.
Does the 0th index of the list will always be picked up or it will be picked when default storage class is not present
There was a problem hiding this comment.
Correct if sclist is having the default storage class then we should use it otherwise the index 0.
- OCS install flow will use first provisioner based storage class - OCS add capacity lists only provisioner based storage class Signed-off-by: cloudbehl <cloudbehl@gmail.com>
cloudbehl
force-pushed
the
add-capacity-dropdown
branch
from
032679e
to
11fb898
Compare
| // take the first storageclass if default not set | ||
| if (!storageClass && storageClass.length > 0) { | ||
| storageClass = getName(_.get(scList, '0')); | ||
| } |
There was a problem hiding this comment.
@cloudbehl Also we need to handle the scenario, when no storage class is present. UI should throw error.
There was a problem hiding this comment.
@gnehapk I want the backend to throw an error on this one.
There was a problem hiding this comment.
I don't want to write logic to throw error messages from UI. What you think?
|
/test e2e-gcp-console |
|
/approve |
|
/test e2e-gcp-console |
|
Can you please add bugzilla to this PR? |
|
This is 4.4 PR doesn't needs bugzilla right now |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cloudbehl, gnehapk 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 |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Signed-off-by: cloudbehl cloudbehl@gmail.com