Add StorageClass input to 'Add bootable Volume' modal #1016
Conversation
avivtur
force-pushed
the
add-sc-to-bootable-volume
branch
from
9cd846f
to
d51d59e
Compare
|
Looks good to me, but I'd like to have someone else review it as well since it's used in multiple places and I don't have that much direct experience with this area of the code. |
avivtur
force-pushed
the
add-sc-to-bootable-volume
branch
from
d51d59e
to
91f0162
Compare
| @@ -16,12 +16,14 @@ import CapacityInput from '@kubevirt-utils/components/CapacityInput/CapacityInpu | |||
| import { removeByteSuffix } from '@kubevirt-utils/components/CapacityInput/utils'; | |||
There was a problem hiding this comment.
i think that some functions can be extracted to utiil? what do u think?
There was a problem hiding this comment.
I agree with that, I think this PR is already quite big so maybe can you name which functions other than this one you had in mind and I'll create a separate PR for moving them to utils, wdyt?
avivtur
force-pushed
the
add-sc-to-bootable-volume
branch
from
91f0162
to
d19aaa7
Compare
Signed-off-by: Aviv Turgeman <aturgema@redhat.com>
avivtur
force-pushed
the
add-sc-to-bootable-volume
branch
from
d19aaa7
to
c0f80b5
Compare
|
/retest |
| <FilterSelect | ||
| selected={labels?.[DEFAULT_PREFERENCE_LABEL]} | ||
| setSelected={setBootableVolumeField('labels', DEFAULT_PREFERENCE_LABEL)} | ||
| options={preferencesNames} | ||
| optionLabelText={t('Preference')} | ||
| /> | ||
| </FormGroup> | ||
| <FormGroup label={t('Preferred Instancetype')}> | ||
| <FormGroup label={t('Instancetypes')} isRequired> |
There was a problem hiding this comment.
Shouldn't this be 'InstanceTypes'?
There was a problem hiding this comment.
well the kind of the resource is Instancetype so I just made it plural, I wouldn't waste any time about naming for now as the names of everything can easily be changed (it's currently under discussion as Ronen and Yifat told me in the last sync meeting :) )
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: avivtur, pcbailey 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 |
Signed-off-by: Aviv Turgeman aturgema@redhat.com
📝 Description
Main goal:
Adding Storage class dropdown to
Add bootable Volumemodal under upload image form onlyAfter:
Making the form submit to create a DS on point at existing PVC or at the cloned PVC or uploaded disk
Side goals:
Add disk modal was not working
Before:
Storage class dropdown taking the default SC as value automatically on the first load, either if we already have a previous value or not, which is incorrect (should simply take an existing value from YAML file if exists or stay empty if not selected)
Before:
https://user-images.githubusercontent.com/67270715/211790070-f496c6f4-c339-48a3-9f05-b31d90fdcffa.mp4
🎥 Demo
Disk modal is now able to create a disk without enforcing any value from the UI and still get to proper default SC by the backend. (not included in video/screenshot but hotplug disk creation was having same issues which now are resolved)
default-already-selected-after.mp4