-
Notifications
You must be signed in to change notification settings - Fork 30
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
CNV-36065: Disk modal with persistent reservation #1630
CNV-36065: Disk modal with persistent reservation #1630
Conversation
@upalatucci: This pull request references CNV-36065 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.15.0" version, but no target version was set. In response to this:
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. |
b27af6c
to
b263175
Compare
src/utils/components/DiskModal/DiskFormFields/AdvancedSettings.tsx
Outdated
Show resolved
Hide resolved
...tingsTab/ClusterTab/components/PersistentReservationSection/PersistentReservationSection.tsx
Outdated
Show resolved
Hide resolved
...tingsTab/ClusterTab/components/PersistentReservationSection/PersistentReservationSection.tsx
Outdated
Show resolved
Hide resolved
...tingsTab/ClusterTab/components/PersistentReservationSection/PersistentReservationSection.tsx
Outdated
Show resolved
Hide resolved
b263175
to
3acfa17
Compare
the pr lgtm but , i see u are adding the new init values to state and adding actions, but maybe I'm missing something, but I don't see any addition to create disk function to use those new fields? again, maybe logic of current create disk function is ok and can handle it, but I just want to make sure :) |
3acfa17
to
65b9c15
Compare
Now i pushed the logic to actually add the options to the disk |
} | ||
|
||
if (diskState.sharable) { | ||
disk.shareable = true; |
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.
can this be added to line 101?
sharable: diskState.shareable ? It should be null/true... null will work np as k8s will drop it
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.
okay fixed
65b9c15
to
7277003
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: metalice, upalatucci 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 |
📝 Description
Add the 'Share disk between VMs' and 'Persistent Reservation' flags under 'Advanced settings' in the disk modal
design: https://docs.google.com/document/d/1_fbw2BUZFRmArjhdKc32lTEqU9MRHsj2KYBxbR6ZL_0
If the disk is LUN, enable the persistent reservation flag but disable the 'Share disk' flag .
Share disk for LUN does not make sense as LUN already is a shared disk
Make sure the
FEATURE_HCO_PERSISTENT_RESERVATION
cluster flag is enabled, otherwise is not possible to create a disk like that.🎥 Demo