Bug 1870331: Not allow to change size for OCS based resource #6683

a2batic · 2020-09-18T17:59:48Z

Not allow the user to enter smaller size to create a resource

Signed-off-by: Kanika Murarka kmurarka@redhat.com

openshift-ci-robot · 2020-09-18T17:59:51Z

@a2batic: This pull request references Bugzilla bug 1870331, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1870331: Not allow to change size for OCS based resource

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-robot · 2020-09-18T18:00:25Z

@a2batic: This pull request references Bugzilla bug 1870331, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1870331: Not allow to change size for OCS based resource

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.

a2batic · 2020-09-18T18:04:58Z

/bugzilla refresh

openshift-ci-robot · 2020-09-18T18:05:01Z

@a2batic: This pull request references Bugzilla bug 1870331, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

a2batic · 2020-09-18T18:05:29Z

/bugzilla refresh

openshift-ci-robot · 2020-09-18T18:05:31Z

@a2batic: This pull request references Bugzilla bug 1870331, which is invalid:

expected the bug to target the "4.6.0" release, but it targets "OCS 4.6.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

a2batic · 2020-09-18T18:06:44Z

/bugzilla refresh

openshift-ci-robot · 2020-09-18T18:06:51Z

@a2batic: This pull request references Bugzilla bug 1870331, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

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-robot · 2020-09-18T18:55:23Z

@a2batic: This pull request references Bugzilla bug 1870331, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1870331: Not allow to change size for OCS based resource

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.

afreen23 · 2020-09-18T19:07:54Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

+  const filterUnits = () => {
+    let filteredDropdown = {};
+    _.each(dropdownUnits, () => {
+      if (defaultSize[1] === 'Mi') {
+        filteredDropdown = { Mi: dropdownUnits.Mi };
+      }
+      if (defaultSize[1] === 'Gi' || defaultSize[1] === 'Mi') {
+        filteredDropdown = { ...filteredDropdown, Gi: dropdownUnits.Gi };
+      }
+    });
+    filteredDropdown = { ...filteredDropdown, Ti: dropdownUnits.Ti };
+    return filteredDropdown;
+  };


May be you would like to define this at one place instead of recreating the function twice.

Do we need to iterate over dropdownUnits when the check is on defaultSize[1] ?
May be I am missing something :)

afreen23 · 2020-09-18T19:10:11Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

+            <span className="help-block co-clone-pvc-modal__helpblock">
+              {!validSize ? 'Size should be equal or higher than default size' : <br />}
+            </span>


Suggested change



{!validSize ? 'Size should be equal or higher than default size' : }



{validSize && 

'Size should be equal or higher than default size'

}

Did you consider using FormGroup properties for validation ?
helperText and validated

rawagner · 2020-09-24T09:05:13Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

            <RequestSizeInput
              name="requestSize"
              testID="input-request-size"
              onChange={requestedSizeInputChange}
              defaultRequestSizeUnit={requestedUnit}
              defaultRequestSizeValue={requestedSize}
              dropdownUnits={dropdownUnits}
+              isInputDisabled={isCephProvisioner(scResource?.provisioner)}


You should wait for scResource to load, otherwise on slower connections you may end up with Size input enabled for some time.

afreen23 · 2020-09-24T09:00:20Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

@@ -62,6 +77,9 @@ const ClonePVCModal = withHandlePromise((props: ClonePVCModalProps) => {
  const requestedSizeInputChange = ({ value, unit }) => {
    setRequestedSize(value);
    setRequestedUnit(unit);
+    const sizeInBytes = convertToBaseValue(value + unit);
+    const defaultInBytes = convertToBaseValue(getRequestedPVCSize(resource));
+    sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);


Suggested change

sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);

const isValid = sizeInBytes < defaultInBytes;

setValidSize(isValid);

OR

Suggested change

sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);

setValidSize(sizeInBytes < defaultInBytes);

afreen23 · 2020-09-24T09:01:27Z

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

@@ -74,6 +76,9 @@ const RestorePVCModal = withHandlePromise<RestorePVCModalProps>(
    const requestedSizeInputChange = ({ value, unit }) => {
      setRequestedSize(value);
      setRequestedUnit(unit);
+      const sizeInBytes = convertToBaseValue(value + unit);
+      const defaultInBytes = convertToBaseValue(resource?.status?.restoreSize);
+      sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);


same as above

cloudbehl · 2020-09-24T09:05:31Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

+            isRequired
+            fieldId="clone-pvc-modal__size"
+            className="co-clone-pvc-modal__ocs-size"
+            helperTextInvalid="Size should be equal or higher than default size"


Suggested change

helperTextInvalid="Size should be equal or higher than default size"

helperTextInvalid="Size should be equal or higher than the requested size of PVC "

cloudbehl · 2020-09-24T09:11:21Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

@@ -62,6 +77,9 @@ const ClonePVCModal = withHandlePromise((props: ClonePVCModalProps) => {
  const requestedSizeInputChange = ({ value, unit }) => {
    setRequestedSize(value);
    setRequestedUnit(unit);
+    const sizeInBytes = convertToBaseValue(value + unit);


Suggested change

const sizeInBytes = convertToBaseValue(value + unit);

const cloneSizeInBytes = convertToBaseValue(value + unit);

cloudbehl · 2020-09-24T09:11:56Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

@@ -62,6 +77,9 @@ const ClonePVCModal = withHandlePromise((props: ClonePVCModalProps) => {
  const requestedSizeInputChange = ({ value, unit }) => {
    setRequestedSize(value);
    setRequestedUnit(unit);
+    const sizeInBytes = convertToBaseValue(value + unit);
+    const defaultInBytes = convertToBaseValue(getRequestedPVCSize(resource));


Suggested change

const defaultInBytes = convertToBaseValue(getRequestedPVCSize(resource));

const PVCSizeInBytes = convertToBaseValue(getRequestedPVCSize(resource));

cloudbehl · 2020-09-24T09:12:36Z

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

@@ -74,6 +76,9 @@ const RestorePVCModal = withHandlePromise<RestorePVCModalProps>(
    const requestedSizeInputChange = ({ value, unit }) => {
      setRequestedSize(value);
      setRequestedUnit(unit);
+      const sizeInBytes = convertToBaseValue(value + unit);


Suggested change

const sizeInBytes = convertToBaseValue(value + unit);

const RestoreSizeInBytes = convertToBaseValue(value + unit);

cloudbehl · 2020-09-24T09:13:05Z

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

@@ -74,6 +76,9 @@ const RestorePVCModal = withHandlePromise<RestorePVCModalProps>(
    const requestedSizeInputChange = ({ value, unit }) => {
      setRequestedSize(value);
      setRequestedUnit(unit);
+      const sizeInBytes = convertToBaseValue(value + unit);
+      const defaultInBytes = convertToBaseValue(resource?.status?.restoreSize);


Suggested change

const defaultInBytes = convertToBaseValue(resource?.status?.restoreSize);

const snapshotSizeInBytes = convertToBaseValue(resource?.status?.restoreSize);

openshift-ci-robot · 2020-09-24T09:34:18Z

@a2batic: This pull request references Bugzilla bug 1870331, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1870331: Not allow to change size for OCS based resource

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.

rawagner · 2020-09-24T09:38:15Z

/approve

/assign @cloudbehl

afreen23 · 2020-09-24T09:49:07Z

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

+                  required
+                />
+              ) : (
+                <div className="skeleton-text" />


What if you have error ? It will be always showing loading state ?

Good point, its better to disable size input in that case.

afreen23 · 2020-09-24T09:49:14Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

+                required
+              />
+            ) : (
+              <div className="skeleton-text" />


same as above

afreen23 · 2020-09-24T10:15:21Z

/approve

cloudbehl

nit:

cloudbehl · 2020-09-24T10:35:09Z

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx

+            isRequired
+            fieldId="clone-pvc-modal__size"
+            className="co-clone-pvc-modal__ocs-size"
+            helperTextInvalid="Size should be equal or higher than the requested size of PVC"


Suggested change

helperTextInvalid="Size should be equal or higher than the requested size of PVC"

helperTextInvalid="Size should be equal or greater than the requested size of PVC"

cloudbehl · 2020-09-24T10:35:32Z

frontend/packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx

@@ -165,16 +170,26 @@ const RestorePVCModal = withHandlePromise<RestorePVCModalProps>(
              label="Size"
              isRequired
              fieldId="pvc-size"
-              className="co-restore-pvc-modal__input"
+              className="co-restore-pvc-modal__input co-restore-pvc-modal__ocs-size"
+              helperTextInvalid="Size should be equal or higher than the restore size of snapshot"


Suggested change

helperTextInvalid="Size should be equal or higher than the restore size of snapshot"

helperTextInvalid="Size should be equal or greater than the restore size of snapshot"

Signed-off-by: Kanika Murarka <kmurarka@redhat.com>

cloudbehl · 2020-09-24T12:25:08Z

/lgtm

openshift-ci-robot · 2020-09-24T12:25:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a2batic, afreen23, cloudbehl, 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:

~~frontend/OWNERS~~ [rawagner]

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

openshift-ci-robot · 2020-09-24T14:28:44Z

@a2batic: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-console	`225f043`	link	`/test e2e-gcp-console`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

a2batic · 2020-09-24T14:29:24Z

/test e2e-gcp-console

openshift-ci-robot · 2020-09-24T15:44:08Z

@a2batic: All pull requests linked via external trackers have merged:

openshift/console#6683

Bugzilla bug 1870331 has been moved to the MODIFIED state.

In response to this:

Bug 1870331: Not allow to change size for OCS based resource

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-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Sep 18, 2020

openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. component/core Related to console core functionality labels Sep 18, 2020

openshift-ci-robot requested review from dtaylor113 and spadgett September 18, 2020 18:00

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 18, 2020

a2batic force-pushed the size branch from 297ed7b to d1c57ea Compare September 18, 2020 18:54

a2batic force-pushed the size branch from d1c57ea to 2262851 Compare September 18, 2020 18:57

afreen23 suggested changes Sep 18, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2020

a2batic force-pushed the size branch from 2262851 to ddbf6be Compare September 24, 2020 08:57

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2020

a2batic requested a review from afreen23 September 24, 2020 08:57

rawagner suggested changes Sep 24, 2020

View reviewed changes

afreen23 suggested changes Sep 24, 2020

View reviewed changes

cloudbehl reviewed Sep 24, 2020

View reviewed changes

a2batic force-pushed the size branch from ddbf6be to 3bb14f2 Compare September 24, 2020 09:31

openshift-ci-robot assigned cloudbehl Sep 24, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2020

afreen23 reviewed Sep 24, 2020

View reviewed changes

a2batic force-pushed the size branch from 3bb14f2 to 2fb3b14 Compare September 24, 2020 10:04

cloudbehl reviewed Sep 24, 2020

View reviewed changes

Bug 1870331: Not allow to change size for OCS based resource

225f043

Signed-off-by: Kanika Murarka <kmurarka@redhat.com>

a2batic force-pushed the size branch from 2fb3b14 to 225f043 Compare September 24, 2020 11:12

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2020

openshift-merge-robot merged commit 2db0996 into openshift:master Sep 24, 2020

spadgett added this to the v4.6 milestone Sep 30, 2020

	sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);
	const isValid = sizeInBytes < defaultInBytes;
	setValidSize(isValid);

	sizeInBytes < defaultInBytes ? setValidSize(false) : setValidSize(true);
	setValidSize(sizeInBytes < defaultInBytes);

	helperTextInvalid="Size should be equal or higher than default size"
	helperTextInvalid="Size should be equal or higher than the requested size of PVC "

	const sizeInBytes = convertToBaseValue(value + unit);
	const cloneSizeInBytes = convertToBaseValue(value + unit);

	const defaultInBytes = convertToBaseValue(getRequestedPVCSize(resource));
	const PVCSizeInBytes = convertToBaseValue(getRequestedPVCSize(resource));

	const sizeInBytes = convertToBaseValue(value + unit);
	const RestoreSizeInBytes = convertToBaseValue(value + unit);

	const defaultInBytes = convertToBaseValue(resource?.status?.restoreSize);
	const snapshotSizeInBytes = convertToBaseValue(resource?.status?.restoreSize);

Bug 1870331: Not allow to change size for OCS based resource #6683

Bug 1870331: Not allow to change size for OCS based resource #6683

Conversation

a2batic commented Sep 18, 2020 • edited

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

a2batic commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

a2batic commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

a2batic commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

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

openshift-ci-robot commented Sep 24, 2020

rawagner commented Sep 24, 2020

afreen23 Sep 24, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afreen23 Sep 24, 2020 • edited

Choose a reason for hiding this comment

afreen23 commented Sep 24, 2020

cloudbehl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudbehl commented Sep 24, 2020

openshift-ci-robot commented Sep 24, 2020

openshift-ci-robot commented Sep 24, 2020

a2batic commented Sep 24, 2020

openshift-ci-robot commented Sep 24, 2020

a2batic commented Sep 18, 2020 •

edited

afreen23 Sep 24, 2020 •

edited

afreen23 Sep 24, 2020 •

edited