(feat): Add capacity and nodes step and review step to ODF wizard #9581

afreen23 · 2021-07-22T07:33:00Z

implements https://issues.redhat.com/browse/ODFE-86
implements https://issues.redhat.com/browse/ODFE-87
merged the steps of attached-devices and internal mode previously in OCS
incorporated new UI changes for raw available capacity

Signed-off-by: Afreen Rahman afrahman@redhat.com

afreen23 · 2021-07-22T07:34:46Z

...components/ocs-install/internal-mode/install-wizard-steps/select-capacity-and-nodes-step.tsx

+            <EnableTaintNodes
+              enableTaint={state.enableTaint}
+              setEnableTaint={() =>
+                dispatch({ type: ActionType.SET_ENABLE_TAINT, payload: !state.enableTaint })
+              }


refactored taint component , so it can be utilized by ODF UI when required.

afreen23 · 2021-07-22T09:56:16Z

Tests are failing due to i18n . Will fix them with next round of reviews.

vbnrh · 2021-07-22T08:23:39Z

...ackages/ceph-storage-plugin/src/components/ocs-install/install-wizard/capacity-and-nodes.tsx

+  );
+};
+
+export const EnableTaintNodes: React.FC<EnableTaintNodesProps> = ({


Same here as previous PR. It is a boolean, We could make use of Switch instead of a checkbox. Could take a look into this later

ack. its design related so I can only note it for now.
Thanks

vbnrh · 2021-07-22T10:30:06Z

...orage-system/create-storage-system-steps/capacity-and-nodes-step/capacity-and-nodes-step.tsx

+          nodeCount: nodes.length,
+          count: nodes.length,
+        })}{' '}
+        {t('ceph-storage-plugin~selected ({{cpu}} CPU and {{memory}} on ', {


I can't see the totalcpu on the capacity and nodes page. Please check

Oh yes, did not update the names.

vbnrh · 2021-07-22T10:54:28Z

...orage-system/create-storage-system-steps/capacity-and-nodes-step/capacity-and-nodes-step.tsx

+  nodes: WizardState['capacityAndNodes']['nodes'];
+};
+
+const SelectedCapacityAndNodes: React.FC<SelectedCapacityAndNodesProps> = ({


Suggested change

const SelectedCapacityAndNodes: React.FC<SelectedCapacityAndNodesProps> = ({

const SelectedCapacityAndNodesArbiter: React.FC<SelectedCapacityAndNodesProps> = ({

or

Suggested change

const SelectedCapacityAndNodes: React.FC<SelectedCapacityAndNodesProps> = ({

const SelectedCapacityAndNodesNoProvisioner: React.FC<SelectedCapacityAndNodesProps> = ({

Both the components have very similiar names. We can name one something different

One is Select other is Selected as per designs:

vbnrh · 2021-07-22T10:57:56Z

...orage-system/create-storage-system-steps/capacity-and-nodes-step/capacity-and-nodes-step.tsx

+            <TextContent>
+              <Text component={TextVariants.small}>
+                <Trans ns="ceph-storage-plugin">
+                  The available capacity is based on all attached disks associated with the selected


does it need to be externalised or does the Trans do the same thing ?

yes, Trans is the translation component

ok.. i don't see this line in ceph-storage-plugin file which is why i asked

#9581 (comment)
Check now.

vbnrh · 2021-07-22T11:12:50Z

...storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx

+import { WizardNodeState, WizardState } from '../../reducer';
+import './review-and-create-step.scss';
+
+const NodesCard: React.FC<NodeCardProps> = ({ nodes }) =>


We can reuse this card from frontend/packages/ceph-storage-plugin/src/components/ocs-install/install-wizard/review-and-create.tsx , pass the classnames and improvise the state

I am using this here since soon ocs one will be deprecated. We should not waste effort in maintaining older code.

vbnrh · 2021-07-22T11:17:28Z

...storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx

+  [AlertVariant.danger]: RedExclamationCircleIcon,
+};
+
+export const ReviewListTitle: React.FC<ReviewListTitleProps> = ({ text }) => (


reuse from frontend/packages/ceph-storage-plugin/src/components/ocs-install/install-wizard/review-and-create.tsx

The css is different and also we will remove storage cluster related code post 4.10

vbnrh · 2021-07-22T11:18:12Z

...storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx

+
+type ReviewListTitleProps = { text: string };
+
+export const ReviewListBody: React.FC<ReviewListBodyProps> = ({


reuse from frontend/packages/ceph-storage-plugin/src/components/ocs-install/install-wizard/review-and-create.tsx

same as above

vbnrh · 2021-07-22T11:25:41Z

...storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx

+  );
+};
+
+type ReviewListBodyProps = {


Many of these variables can be reused from the other page

vbnrh · 2021-07-22T11:28:43Z

We need to revisit this later for refactoring the code structure. It has become very confusing to navigate. Lot of these components can be organised and repurposed for our case

afreen23 · 2021-07-22T12:16:55Z

We need to revisit this later for refactoring the code structure. It has become very confusing to navigate. Lot of these components can be organised and repurposed for our case

I think its simple navigation. You start from create step -> CapacityAndNodes => SelectedCapacityAndNodes and SelectCapacityAndNodes.
Each component has its own SelecCapacity/SelectedCapacity and SelecetdNodes/SelectNodes.

Pleas specify which part is confusing ? I will make changes if it sounds valid.

vbnrh · 2021-07-22T12:30:41Z

We need to revisit this later for refactoring the code structure. It has become very confusing to navigate. Lot of these components can be organised and repurposed for our case

I think its simple navigation. You start from create step -> CapacityAndNodes => SelectedCapacityAndNodes and SelectCapacityAndNodes.
Each component has its own SelecCapacity/SelectedCapacity and SelecetdNodes/SelectNodes.

Pleas specify which part is confusing ? I will make changes if it sounds valid.

Ok. thanks for clearing it now . I was confused with why we have so many variants of same component. Basically n-1 issue.

- implements https://issues.redhat.com/browse/ODFE-86 - implements https://issues.redhat.com/browse/ODFE-87 - merged the steps of attached-devices and internal mode previously in OCS - incorporated new UI changes for raw capacity Signed-off-by: Afreen Rahman <afrahman@redhat.com>

vbnrh · 2021-07-22T12:35:45Z

/lgtm

openshift-ci · 2021-07-22T12:35:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afreen23, vbnrh

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:

~~frontend/packages/ceph-storage-plugin/OWNERS~~ [afreen23]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from cloudbehl and rawagner July 22, 2021 07:33

openshift-ci bot added component/ceph Related to ceph-storage-plugin kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 22, 2021

afreen23 commented Jul 22, 2021

View reviewed changes

vbnrh suggested changes Jul 22, 2021

View reviewed changes

afreen23 force-pushed the capacity-and-nodes branch from 0f85677 to 461f64e Compare July 22, 2021 12:09

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2021

afreen23 force-pushed the capacity-and-nodes branch from 461f64e to 73a63a3 Compare July 22, 2021 12:32

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2021

openshift-ci bot assigned vbnrh Jul 22, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021

openshift-merge-robot merged commit c3a8424 into openshift:master Jul 22, 2021

spadgett added this to the v4.9 milestone Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat): Add capacity and nodes step and review step to ODF wizard #9581

(feat): Add capacity and nodes step and review step to ODF wizard #9581

afreen23 commented Jul 22, 2021 •

edited

afreen23 Jul 22, 2021

afreen23 commented Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021 •

edited

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

vbnrh Jul 22, 2021

afreen23 Jul 22, 2021

vbnrh Jul 22, 2021

vbnrh Jul 22, 2021

vbnrh commented Jul 22, 2021

afreen23 commented Jul 22, 2021

vbnrh commented Jul 22, 2021

vbnrh commented Jul 22, 2021

openshift-ci bot commented Jul 22, 2021

	const SelectedCapacityAndNodes: React.FC<SelectedCapacityAndNodesProps> = ({
	const SelectedCapacityAndNodesArbiter: React.FC<SelectedCapacityAndNodesProps> = ({

	const SelectedCapacityAndNodes: React.FC<SelectedCapacityAndNodesProps> = ({
	const SelectedCapacityAndNodesNoProvisioner: React.FC<SelectedCapacityAndNodesProps> = ({


		type ReviewListTitleProps = { text: string };

		export const ReviewListBody: React.FC<ReviewListBodyProps> = ({

(feat): Add capacity and nodes step and review step to ODF wizard #9581

(feat): Add capacity and nodes step and review step to ODF wizard #9581

Conversation

afreen23 commented Jul 22, 2021 • edited

Choose a reason for hiding this comment

afreen23 commented Jul 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbnrh Jul 22, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbnrh commented Jul 22, 2021

afreen23 commented Jul 22, 2021

vbnrh commented Jul 22, 2021

vbnrh commented Jul 22, 2021

openshift-ci bot commented Jul 22, 2021

afreen23 commented Jul 22, 2021 •

edited

vbnrh Jul 22, 2021 •

edited