Bug 2000935: add volume mode selection in storage creation #9971

shdn-ibm · 2021-09-02T08:57:10Z

add a volume mode selection, to provide an option to use thin or thick volume
default setting is thick mode

openshift-ci · 2021-09-02T08:57:37Z

Hi @shdn-ibm. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

afreen23 · 2021-09-02T10:22:13Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -89,6 +103,18 @@ export const FlashSystemConnectionDetails: React.FC<ExternalComponentProps<Flash
          isRequired
        />
      </FormGroup>
+      <FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volmode-input">


Suggested change

<FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volmode-input">

<FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volume-mode-input">

afreen23 · 2021-09-02T10:22:38Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

+  const volModeOptions = volModeList.map((mode) => {
+    return <SelectOption key={mode} value={mode} />;
+  });


Suggested change

const volModeOptions = volModeList.map((mode) => {

return <SelectOption key={mode} value={mode} />;

});

const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />;

afreen23 · 2021-09-02T10:23:09Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -31,6 +33,18 @@ export const FlashSystemConnectionDetails: React.FC<ExternalComponentProps<Flash
      : setEndpointValid(ValidatedOptions.error);
  };

+  const volModeList = ['thick', 'thin'];


Suggested change

const volModeList = ['thick', 'thin'];

const VOLUME_MODES = ['thick', 'thin'];

Define it outside the component

afreen23 · 2021-09-02T10:24:08Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

+  const volModeOptions = volModeList.map((mode) => {
+    return <SelectOption key={mode} value={mode} />;
+  });
+  const [isOpen, setIsOpen] = React.useState(false);


Move it above with rest of the hooks.
Hooks should always be at the top

afreen23 · 2021-09-02T10:24:37Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

+  const onToggle = () => setIsOpen(!isOpen);
+  const onModeSelect = (event, value) => {


Add a space

Suggested change

const onToggle = () => setIsOpen(!isOpen);

const onModeSelect = (event, value) => {

const onToggle = () => setIsOpen(!isOpen);

const onModeSelect = (event, value) => {

afreen23 · 2021-09-02T10:25:51Z

...storage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/type.ts

@@ -37,4 +37,5 @@ export type FlashSystemState = {
  password: string;
  endpoint: string;
  poolname: string;
+  volmode?: string;


Suggested change

volmode?: string;

volmode: string;

state should not have optional types

afreen23 · 2021-09-02T10:27:18Z

/ok-to-test

afreen23

Just some minors

afreen23 · 2021-09-03T06:21:46Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -31,6 +34,16 @@ export const FlashSystemConnectionDetails: React.FC<ExternalComponentProps<Flash
      : setEndpointValid(ValidatedOptions.error);
  };

+  const VOLUME_MODES = ['thick', 'thin'];


define this outside the component

afreen23 · 2021-09-03T06:21:51Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

+          isDisabled={false}
+          placeholderText={VOLUME_MODES[0]}
+        >
+          {volModeOptions}


Suggested change

{volModeOptions}

{VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />)}

afreen23 · 2021-09-03T06:22:06Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -31,6 +34,16 @@ export const FlashSystemConnectionDetails: React.FC<ExternalComponentProps<Flash
      : setEndpointValid(ValidatedOptions.error);
  };

+  const VOLUME_MODES = ['thick', 'thin'];
+  const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);


Suggested change

const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);

const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);

Suggested change

const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);

afreen23

/lgtm

SanjalKatiyar · 2021-09-03T10:27:12Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -89,6 +105,21 @@ export const FlashSystemConnectionDetails: React.FC<ExternalComponentProps<Flash
          isRequired
        />
      </FormGroup>
+      <FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volume-mode-input">


Suggested change

<FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volume-mode-input">

<FormGroup label={t('ceph-storage-plugin~Volume mode')} fieldId="volume-mode-input">

SanjalKatiyar · 2021-09-03T10:32:54Z

/retitle Bug 2000935: add volume mode selection in storage creation

openshift-ci · 2021-09-03T10:33:04Z

@shdn-ibm: This pull request references Bugzilla bug 2000935, 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.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (ratamir@redhat.com), skipping review request.

In response to this:

Bug 2000935: add volume mode selection in storage creation

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.

fix: add volume mode selection Update index.tsx

update string Update ceph-storage-plugin.json

SanjalKatiyar · 2021-09-03T11:16:20Z

/lgtm

openshift-ci · 2021-09-03T11:16:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afreen23, SanjalKatiyar, shdn-ibm

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

shdn-ibm · 2021-09-03T13:19:29Z

/retest

openshift-ci · 2021-09-03T18:23:16Z

@shdn-ibm: All pull requests linked via external trackers have merged:

openshift/console#9971

Bugzilla bug 2000935 has been moved to the MODIFIED state.

In response to this:

Bug 2000935: add volume mode selection in storage creation

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 requested review from bipuladh and shyRozen September 2, 2021 08:57

openshift-ci bot added the component/ceph Related to ceph-storage-plugin label Sep 2, 2021

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 2, 2021

afreen23 suggested changes Sep 2, 2021

View reviewed changes

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 2, 2021

shdn-ibm force-pushed the fix/add-volmode-selection branch from 9869b1b to 36c800d Compare September 3, 2021 02:08

afreen23 suggested changes Sep 3, 2021

View reviewed changes

shdn-ibm force-pushed the fix/add-volmode-selection branch from 36c800d to c0a07ca Compare September 3, 2021 08:15

openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Sep 3, 2021

afreen23 approved these changes Sep 3, 2021

View reviewed changes

openshift-ci bot assigned afreen23 Sep 3, 2021

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 3, 2021

SanjalKatiyar reviewed Sep 3, 2021

View reviewed changes

openshift-ci bot changed the title ~~fix: add volume mode selection in storage creation~~ Bug 2000935: add volume mode selection in storage creation Sep 3, 2021

openshift-ci bot added the bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. label Sep 3, 2021

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 3, 2021

shdn-ibm force-pushed the fix/add-volmode-selection branch from ff21c58 to f55aeeb Compare September 3, 2021 10:54

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Sep 3, 2021

shdn-ibm added 2 commits September 3, 2021 18:56

fix: add volume mode selection

4ab19c9

fix: add volume mode selection Update index.tsx

fix: use ip address to access flashsystem

ed620d7

update string Update ceph-storage-plugin.json

shdn-ibm force-pushed the fix/add-volmode-selection branch from f55aeeb to ed620d7 Compare September 3, 2021 10:58

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

openshift-ci bot assigned SanjalKatiyar Sep 3, 2021

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

openshift-merge-robot merged commit 7a4de50 into openshift:master Sep 3, 2021

spadgett added this to the v4.9 milestone Sep 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 2000935: add volume mode selection in storage creation #9971

Bug 2000935: add volume mode selection in storage creation #9971

shdn-ibm commented Sep 2, 2021 •

edited

openshift-ci bot commented Sep 2, 2021

afreen23 Sep 2, 2021

afreen23 Sep 2, 2021

afreen23 Sep 2, 2021

afreen23 Sep 2, 2021 •

edited

afreen23 Sep 2, 2021

afreen23 Sep 2, 2021

afreen23 commented Sep 2, 2021

afreen23 left a comment

afreen23 Sep 3, 2021

afreen23 Sep 3, 2021

afreen23 Sep 3, 2021

afreen23 left a comment

SanjalKatiyar Sep 3, 2021

SanjalKatiyar commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

SanjalKatiyar commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

shdn-ibm commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

	<FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volmode-input">
	<FormGroup label={t('ceph-storage-plugin~Volume Mode')} fieldId="volume-mode-input">

	const volModeList = ['thick', 'thin'];
	const VOLUME_MODES = ['thick', 'thin'];

		const onToggle = () => setIsOpen(!isOpen);
		const onModeSelect = (event, value) => {

	{volModeOptions}
	{VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />)}

	const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);
	const volModeOptions = VOLUME_MODES.map((mode) => <SelectOption key={mode} value={mode} />);

Bug 2000935: add volume mode selection in storage creation #9971

Bug 2000935: add volume mode selection in storage creation #9971

Conversation

shdn-ibm commented Sep 2, 2021 • edited

openshift-ci bot commented Sep 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afreen23 Sep 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afreen23 commented Sep 2, 2021

afreen23 left a 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

afreen23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SanjalKatiyar commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

SanjalKatiyar commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

shdn-ibm commented Sep 3, 2021

openshift-ci bot commented Sep 3, 2021

shdn-ibm commented Sep 2, 2021 •

edited

afreen23 Sep 2, 2021 •

edited