Skip to content
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

Expose access and volume modes in the create/edit disk modal #4729

Conversation

pcbailey
Copy link
Contributor

@pcbailey pcbailey commented Mar 13, 2020

This PR adds the access and volume mode options in an advanced drawer to the create/edit disk modal.

OKD - Google Chrome_649

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2020
@openshift-ci-robot openshift-ci-robot added the component/kubevirt Related to kubevirt-plugin label Mar 13, 2020
@pcbailey pcbailey force-pushed the expose-access-and-volume-modes-in-disk-ui branch 2 times, most recently from 358becb to 08b3a1d Compare March 20, 2020 00:31
@pcbailey pcbailey force-pushed the expose-access-and-volume-modes-in-disk-ui branch from 08b3a1d to 43661bd Compare March 20, 2020 13:28
@pcbailey pcbailey changed the title WIP: Expose access and volume modes in the create/edit disk modal Expose access and volume modes in the create/edit disk modal Mar 20, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 20, 2020
@pcbailey
Copy link
Contributor Author

/assign @yaacov

export * from './data-volume-source-type';
export * from './disk-bus';
export * from './disk-type';
export * from './volume-type';
export * from './volume-mode';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetically volume-mode is before volume-type :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct! I'll fix it in a follow-up. I want to fix a few misspelled function names that I found, but didn't want to do it in this PR. I'll fix it with those.

@yaacov
Copy link
Member

yaacov commented Mar 22, 2020

/test e2e-gcp-console

@yaacov
Copy link
Member

yaacov commented Mar 22, 2020

/lgtm
/approve

@yaacov
Copy link
Member

yaacov commented Mar 22, 2020

/test e2e-gcp-console

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pcbailey, yaacov

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2020
return volumeMode;
}

return storageClassName === 'local-sc' ? VolumeMode.FILESYSTEM : VolumeMode.BLOCK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we should default to block on anything not 'local-sc', ( not a blocker ) , do we have a reason ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That comes from the design doc. We try to pull the defaults from the config map first. If nothing is found (the config map doesn't exist or there isn't an entry for the selected storage class), then these values are used as the fallbacks. Just to make sure, I'll check with the storage folks and make sure this logic works. If it doesn't, I'll fix it in a follow-up.

@yaacov
Copy link
Member

yaacov commented Mar 22, 2020

/test e2e-gcp-console

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

7 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 9fd5e82 into openshift:master Mar 23, 2020
@spadgett spadgett added this to the v4.5 milestone Mar 23, 2020
@pcbailey pcbailey deleted the expose-access-and-volume-modes-in-disk-ui branch October 26, 2022 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/kubevirt Related to kubevirt-plugin lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants