New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(feat): Add OCS kebab actions in the LSO disk inventory related to the day2 operations #6124
(feat): Add OCS kebab actions in the LSO disk inventory related to the day2 operations #6124
Conversation
frontend/packages/local-storage-operator-plugin/src/components/disks-list/disks-list-page.tsx
Show resolved
Hide resolved
de46cc3
to
f6f2a40
Compare
For the time being I have removed troubleshoot action as kebab action. It will be right to bring it back as popover in a followup PR. |
f6f2a40
to
59020ee
Compare
...ge-plugin/src/components/attached-devices-mode/lso-disk-inventory/disk-replacement-modal.tsx
Show resolved
Hide resolved
59020ee
to
2134508
Compare
@@ -131,13 +155,21 @@ const DisksListPage: React.FC<{ obj: NodeKind }> = (props) => { | |||
selector: { [LABEL_SELECTOR]: nodeName }, | |||
}, | |||
]} | |||
customData={{ | |||
diskOsdMap: new Map() /* TBD(Afreen) Will be changed to actual state with this https://issues.redhat.com/browse/RHSTOR-1194 */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question - where are you exactly adding values inside diskOsdMap
as you are getting the disk name from this Map?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add the local state to disk list page which will poll data and the pass it down the children.
Will do as part of mentioned issue, its part of another PR.
* TODO:(Afreen) Add validations based on ocs status (part of followup PR) | ||
*/ | ||
let template: TemplateKind; | ||
k8sGet(TemplateModel, OSD_REMOVAL_TEMPLATE, CEPH_STORAGE_NAMESPACE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd prefer async/await syntax. Makes the flow easier to read.
osdId: string; | ||
} & ModalComponentProps; | ||
|
||
type TemplateSecret = K8sResourceCommon & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use SecretKind
type which we already have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That requires to use data
and type
, instead I am using stringData
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type check is enforcing to add those fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export type SecretKind = {
data: { [key: string]: string };
stringData?: { [key: string]: string };
type: string;
} & K8sResourceCommon;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the data
and type
are actually not required ? I would just make them optional in SecretKind
}), | ||
}); | ||
|
||
export const OCSKebabOptions: React.FC<{ diskName: string; diskOsdMap: Map<string, string> }> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create a separate type definition from { diskName: string; diskOsdMap: Map<string, string> }
ada1b9e
to
0c63c1c
Compare
/assign @rawagner |
/* | ||
* TODO:(Afreen) Add validations based on ocs status (part of followup PR) | ||
*/ | ||
instantiatetemplate(osdId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets not mix async
with then
syntax
instantiatetemplate(osdId) | |
try { | |
instantiatetemplate(osdId); | |
close(); | |
} catch (err) { | |
setError(err.message); | |
} finally { | |
setProgress(false); | |
} |
osdId: string; | ||
} & ModalComponentProps; | ||
|
||
type TemplateSecret = K8sResourceCommon & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the data
and type
are actually not required ? I would just make them optional in SecretKind
…e day2 operations - Adds the disk replacement action - Utilizes flag for attached devices cluster (`OCS_ATTACHED_DEVICES`) to enable the OCS kebab actions Implements https://issues.redhat.com/browse/RHSTOR-1197 Signed-off-by: Afreen Rahman <afrahman@redhat.com>
0c63c1c
to
864b145
Compare
@rawagner addressed the reviews. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afreen23, gnehapk, rawagner 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 |
OCS_ATTACHED_DEVICES
) to enable the OCS kebab actionsImplements https://issues.redhat.com/browse/RHSTOR-1197
View when
OCS_ATTACHED_DEVICES
is enabled with OCS based Kebab Actions: